* [PATCH 01/11] packfile: allow prepare_packed_git_mru to handle arbitrary repositories
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
@ 2018-02-28 1:05 ` Stefan Beller
2018-02-28 1:05 ` [PATCH 02/11] packfile: allow rearrange_packed_git " Stefan Beller
` (11 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:05 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
This conversion was done without the #define trick used in the earlier
series refactoring to have better repository access, because this function
is easy to review, as all lines are converted and it has only one caller
Signed-off-by: Stefan Beller <sbeller@google.com>
---
packfile.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/packfile.c b/packfile.c
index ccfe084642b..b844b653265 100644
--- a/packfile.c
+++ b/packfile.c
@@ -873,14 +873,14 @@ static void rearrange_packed_git(void)
set_next_packed_git, sort_pack);
}
-static void prepare_packed_git_mru(void)
+static void prepare_packed_git_mru(struct repository *r)
{
struct packed_git *p;
- INIT_LIST_HEAD(&the_repository->objects.packed_git_mru);
+ INIT_LIST_HEAD(&r->objects.packed_git_mru);
- for (p = the_repository->objects.packed_git; p; p = p->next)
- list_add_tail(&p->mru, &the_repository->objects.packed_git_mru);
+ for (p = r->objects.packed_git; p; p = p->next)
+ list_add_tail(&p->mru, &r->objects.packed_git_mru);
}
void prepare_packed_git(void)
@@ -894,7 +894,7 @@ void prepare_packed_git(void)
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
- prepare_packed_git_mru();
+ prepare_packed_git_mru(the_repository);
the_repository->objects.packed_git_initialized = 1;
}
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 02/11] packfile: allow rearrange_packed_git to handle arbitrary repositories
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
2018-02-28 1:05 ` [PATCH 01/11] packfile: allow prepare_packed_git_mru to handle arbitrary repositories Stefan Beller
@ 2018-02-28 1:05 ` Stefan Beller
2018-02-28 1:06 ` [PATCH 03/11] packfile: allow install_packed_git " Stefan Beller
` (10 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:05 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com>
---
packfile.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/packfile.c b/packfile.c
index b844b653265..5ccce419354 100644
--- a/packfile.c
+++ b/packfile.c
@@ -866,10 +866,10 @@ static int sort_pack(const void *a_, const void *b_)
return -1;
}
-static void rearrange_packed_git(void)
+static void rearrange_packed_git(struct repository *r)
{
- the_repository->objects.packed_git = llist_mergesort(
- the_repository->objects.packed_git, get_next_packed_git,
+ r->objects.packed_git = llist_mergesort(
+ r->objects.packed_git, get_next_packed_git,
set_next_packed_git, sort_pack);
}
@@ -893,7 +893,7 @@ void prepare_packed_git(void)
prepare_alt_odb(the_repository);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
- rearrange_packed_git();
+ rearrange_packed_git(the_repository);
prepare_packed_git_mru(the_repository);
the_repository->objects.packed_git_initialized = 1;
}
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 03/11] packfile: allow install_packed_git to handle arbitrary repositories
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
2018-02-28 1:05 ` [PATCH 01/11] packfile: allow prepare_packed_git_mru to handle arbitrary repositories Stefan Beller
2018-02-28 1:05 ` [PATCH 02/11] packfile: allow rearrange_packed_git " Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 14:38 ` Derrick Stolee
2018-02-28 1:06 ` [PATCH 04/11] packfile: add repository argument to prepare_packed_git_one Stefan Beller
` (9 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
This conversion was done without the #define trick used in the earlier
series refactoring to have better repository access, because this function
is easy to review, as it only has one caller and all lines but the first
two are converted.
We must not convert 'pack_open_fds' to be a repository specific variable,
as it is used to monitor resource usage of the machine that Git executes
on.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
fast-import.c | 2 +-
http.c | 2 +-
packfile.c | 8 ++++----
packfile.h | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 1685fe59a20..67550584da4 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1038,7 +1038,7 @@ static void end_packfile(void)
if (!new_p)
die("core git rejected index %s", idx_name);
all_packs[pack_id] = new_p;
- install_packed_git(new_p);
+ install_packed_git(the_repository, new_p);
free(idx_name);
/* Print the boundary */
diff --git a/http.c b/http.c
index afe2707e86b..f3c2302f84b 100644
--- a/http.c
+++ b/http.c
@@ -2134,7 +2134,7 @@ int finish_http_pack_request(struct http_pack_request *preq)
return -1;
}
- install_packed_git(p);
+ install_packed_git(the_repository, p);
free(tmp_idx);
return 0;
}
diff --git a/packfile.c b/packfile.c
index 5ccce419354..1c24b02cc84 100644
--- a/packfile.c
+++ b/packfile.c
@@ -680,13 +680,13 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
return p;
}
-void install_packed_git(struct packed_git *pack)
+void install_packed_git(struct repository *r, struct packed_git *pack)
{
if (pack->pack_fd != -1)
pack_open_fds++;
- pack->next = the_repository->objects.packed_git;
- the_repository->objects.packed_git = pack;
+ pack->next = r->objects.packed_git;
+ r->objects.packed_git = pack;
}
void (*report_garbage)(unsigned seen_bits, const char *path);
@@ -782,7 +782,7 @@ static void prepare_packed_git_one(char *objdir, int local)
* corresponding .pack file that we can map.
*/
(p = add_packed_git(path.buf, path.len, local)) != NULL)
- install_packed_git(p);
+ install_packed_git(the_repository, p);
}
if (!report_garbage)
diff --git a/packfile.h b/packfile.h
index 6a2c57045ca..cc3bf67ab50 100644
--- a/packfile.h
+++ b/packfile.h
@@ -36,7 +36,7 @@ extern void (*report_garbage)(unsigned seen_bits, const char *path);
extern void prepare_packed_git(void);
extern void reprepare_packed_git(void);
-extern void install_packed_git(struct packed_git *pack);
+extern void install_packed_git(struct repository *r, struct packed_git *pack);
/*
* Give a rough count of objects in the repository. This sacrifices accuracy
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 04/11] packfile: add repository argument to prepare_packed_git_one
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (2 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 03/11] packfile: allow install_packed_git " Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 1:06 ` [PATCH 05/11] packfile: add repository argument to prepare_packed_git Stefan Beller
` (8 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com>
---
packfile.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/packfile.c b/packfile.c
index 1c24b02cc84..8bb158fc84e 100644
--- a/packfile.c
+++ b/packfile.c
@@ -735,7 +735,8 @@ static void report_pack_garbage(struct string_list *list)
report_helper(list, seen_bits, first, list->nr);
}
-static void prepare_packed_git_one(char *objdir, int local)
+#define prepare_packed_git_one(r, o, l) prepare_packed_git_one_##r(o, l)
+static void prepare_packed_git_one_the_repository(char *objdir, int local)
{
struct strbuf path = STRBUF_INIT;
size_t dirnamelen;
@@ -889,10 +890,10 @@ void prepare_packed_git(void)
if (the_repository->objects.packed_git_initialized)
return;
- prepare_packed_git_one(get_object_directory(), 1);
+ prepare_packed_git_one(the_repository, get_object_directory(), 1);
prepare_alt_odb(the_repository);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
- prepare_packed_git_one(alt->path, 0);
+ prepare_packed_git_one(the_repository, alt->path, 0);
rearrange_packed_git(the_repository);
prepare_packed_git_mru(the_repository);
the_repository->objects.packed_git_initialized = 1;
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 05/11] packfile: add repository argument to prepare_packed_git
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (3 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 04/11] packfile: add repository argument to prepare_packed_git_one Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 1:06 ` [PATCH 06/11] packfile: add repository argument to reprepare_packed_git Stefan Beller
` (7 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
Add a repository argument to allow prepare_packed_git callers to
be more specific about which repository to handle. See c28d027a52c
(sha1_file: add repository argument to link_alt_odb_entry, 2018-02-20)
for an explanation of the #define trick.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/count-objects.c | 2 +-
builtin/fsck.c | 2 +-
builtin/gc.c | 2 +-
builtin/pack-objects.c | 2 +-
builtin/pack-redundant.c | 2 +-
fast-import.c | 2 +-
http-backend.c | 2 +-
pack-bitmap.c | 2 +-
packfile.c | 10 +++++-----
packfile.h | 3 ++-
server-info.c | 2 +-
sha1_name.c | 4 ++--
12 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9334648dccf..b262327bf6d 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -123,7 +123,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
if (!the_repository->objects.packed_git)
- prepare_packed_git();
+ prepare_packed_git(the_repository);
for (p = the_repository->objects.packed_git; p; p = p->next) {
if (!p->pack_local)
continue;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 76c94e9b5af..27f580352c5 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -727,7 +727,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
uint32_t total = 0, count = 0;
struct progress *progress = NULL;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
if (show_progress) {
for (p = the_repository->objects.packed_git; p;
diff --git a/builtin/gc.c b/builtin/gc.c
index 96ff7908677..97f34ae9fe0 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -174,7 +174,7 @@ static int too_many_packs(void)
if (gc_auto_pack_limit <= 0)
return 0;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
for (cnt = 0, p = the_repository->objects.packed_git; p; p = p->next) {
if (!p->pack_local)
continue;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7f376c2aefb..84e940a53ba 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3151,7 +3151,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (progress && all_progress_implied)
progress = 2;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
if (ignore_packed_keep) {
struct packed_git *p;
for (p = the_repository->objects.packed_git; p; p = p->next)
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 55462bc826a..83d2395dae9 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -631,7 +631,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
break;
}
- prepare_packed_git();
+ prepare_packed_git(the_repository);
if (load_all_packs)
load_all();
diff --git a/fast-import.c b/fast-import.c
index 67550584da4..39cd0b7540d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3471,7 +3471,7 @@ int cmd_main(int argc, const char **argv)
rc_free[i].next = &rc_free[i + 1];
rc_free[cmd_save - 1].next = NULL;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
start_packfile();
set_die_routine(die_nicely);
set_checkpoint_signal();
diff --git a/http-backend.c b/http-backend.c
index 4d93126c15f..4950078c936 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -519,7 +519,7 @@ static void get_info_packs(struct strbuf *hdr, char *arg)
size_t cnt = 0;
select_getanyfile(hdr);
- prepare_packed_git();
+ prepare_packed_git(the_repository);
for (p = the_repository->objects.packed_git; p; p = p->next) {
if (p->pack_local)
cnt++;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index d51172b1d5b..273bdfb631f 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -336,7 +336,7 @@ static int open_pack_bitmap(void)
assert(!bitmap_git.map && !bitmap_git.loaded);
- prepare_packed_git();
+ prepare_packed_git(the_repository);
for (p = the_repository->objects.packed_git; p; p = p->next) {
if (open_pack_bitmap_1(p) == 0)
ret = 0;
diff --git a/packfile.c b/packfile.c
index 8bb158fc84e..6e8b98ab405 100644
--- a/packfile.c
+++ b/packfile.c
@@ -817,7 +817,7 @@ unsigned long approximate_object_count(void)
unsigned long count;
struct packed_git *p;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
count = 0;
for (p = the_repository->objects.packed_git; p; p = p->next) {
if (open_pack_index(p))
@@ -884,7 +884,7 @@ static void prepare_packed_git_mru(struct repository *r)
list_add_tail(&p->mru, &r->objects.packed_git_mru);
}
-void prepare_packed_git(void)
+void prepare_packed_git_the_repository(void)
{
struct alternate_object_database *alt;
@@ -903,7 +903,7 @@ void reprepare_packed_git(void)
{
the_repository->objects.approximate_object_count_valid = 0;
the_repository->objects.packed_git_initialized = 0;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
}
unsigned long unpack_object_header_buffer(const unsigned char *buf,
@@ -1844,7 +1844,7 @@ int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
{
struct list_head *pos;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
if (!the_repository->objects.packed_git)
return 0;
@@ -1897,7 +1897,7 @@ int for_each_packed_object(each_packed_object_fn cb, void *data, unsigned flags)
int r = 0;
int pack_errors = 0;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
for (p = the_repository->objects.packed_git; p; p = p->next) {
if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)
continue;
diff --git a/packfile.h b/packfile.h
index cc3bf67ab50..39eb590b2ae 100644
--- a/packfile.h
+++ b/packfile.h
@@ -34,7 +34,8 @@ extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_
#define PACKDIR_FILE_GARBAGE 4
extern void (*report_garbage)(unsigned seen_bits, const char *path);
-extern void prepare_packed_git(void);
+#define prepare_packed_git(r) prepare_packed_git_##r()
+extern void prepare_packed_git_the_repository(void);
extern void reprepare_packed_git(void);
extern void install_packed_git(struct repository *r, struct packed_git *pack);
diff --git a/server-info.c b/server-info.c
index 96885c45f64..6cd1506e5e3 100644
--- a/server-info.c
+++ b/server-info.c
@@ -201,7 +201,7 @@ static void init_pack_info(const char *infofile, int force)
objdir = get_object_directory();
objdirlen = strlen(objdir);
- prepare_packed_git();
+ prepare_packed_git(the_repository);
for (p = the_repository->objects.packed_git; p; p = p->next) {
/* we ignore things on alternate path since they are
* not available to the pullers in general.
diff --git a/sha1_name.c b/sha1_name.c
index 3e490ee8f62..d3769713ea0 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -196,7 +196,7 @@ static void find_short_packed_object(struct disambiguate_state *ds)
{
struct packed_git *p;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
for (p = the_repository->objects.packed_git; p && !ds->ambiguous;
p = p->next)
unique_in_pack(p, ds);
@@ -567,7 +567,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)
{
struct packed_git *p;
- prepare_packed_git();
+ prepare_packed_git(the_repository);
for (p = the_repository->objects.packed_git; p; p = p->next)
find_abbrev_len_for_pack(p, mad);
}
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 06/11] packfile: add repository argument to reprepare_packed_git
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (4 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 05/11] packfile: add repository argument to prepare_packed_git Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 1:06 ` [PATCH 07/11] packfile: allow prepare_packed_git_one to handle arbitrary repositories Stefan Beller
` (6 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
See previous patch for explanation.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/gc.c | 2 +-
builtin/receive-pack.c | 3 ++-
bulk-checkin.c | 3 ++-
fetch-pack.c | 3 ++-
packfile.c | 2 +-
packfile.h | 3 ++-
sha1_file.c | 2 +-
7 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 97f34ae9fe0..c16020ef42a 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -478,7 +478,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
return error(FAILED_RUN, rerere.argv[0]);
report_garbage = report_pack_garbage;
- reprepare_packed_git();
+ reprepare_packed_git(the_repository);
if (pack_garbage.nr > 0)
clean_pack_garbage();
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 954fc72c7cb..8b03a6e03dc 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1,4 +1,5 @@
#include "builtin.h"
+#include "repository.h"
#include "config.h"
#include "lockfile.h"
#include "pack.h"
@@ -1778,7 +1779,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)
status = finish_command(&child);
if (status)
return "index-pack abnormal exit";
- reprepare_packed_git();
+ reprepare_packed_git(the_repository);
}
return NULL;
}
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 3310fd210a1..eadc2d51720 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -3,6 +3,7 @@
*/
#include "cache.h"
#include "bulk-checkin.h"
+#include "repository.h"
#include "csum-file.h"
#include "pack.h"
#include "strbuf.h"
@@ -57,7 +58,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
strbuf_release(&packname);
/* Make objects we just wrote available to ourselves */
- reprepare_packed_git();
+ reprepare_packed_git(the_repository);
}
static int already_written(struct bulk_checkin_state *state, unsigned char sha1[])
diff --git a/fetch-pack.c b/fetch-pack.c
index 8253d746e0c..eac5928a27b 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "repository.h"
#include "config.h"
#include "lockfile.h"
#include "refs.h"
@@ -1192,7 +1193,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
prepare_shallow_info(&si, shallow);
ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,
&si, pack_lockfile);
- reprepare_packed_git();
+ reprepare_packed_git(the_repository);
update_shallow(args, sought, nr_sought, &si);
clear_shallow_info(&si);
return ref_cpy;
diff --git a/packfile.c b/packfile.c
index 6e8b98ab405..6ea50230aab 100644
--- a/packfile.c
+++ b/packfile.c
@@ -899,7 +899,7 @@ void prepare_packed_git_the_repository(void)
the_repository->objects.packed_git_initialized = 1;
}
-void reprepare_packed_git(void)
+void reprepare_packed_git_the_repository(void)
{
the_repository->objects.approximate_object_count_valid = 0;
the_repository->objects.packed_git_initialized = 0;
diff --git a/packfile.h b/packfile.h
index 39eb590b2ae..9afbf73657e 100644
--- a/packfile.h
+++ b/packfile.h
@@ -36,7 +36,8 @@ extern void (*report_garbage)(unsigned seen_bits, const char *path);
#define prepare_packed_git(r) prepare_packed_git_##r()
extern void prepare_packed_git_the_repository(void);
-extern void reprepare_packed_git(void);
+#define reprepare_packed_git(r) reprepare_packed_git_##r()
+extern void reprepare_packed_git_the_repository(void);
extern void install_packed_git(struct repository *r, struct packed_git *pack);
/*
diff --git a/sha1_file.c b/sha1_file.c
index 36282acb1a8..0b9fefaaf02 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1281,7 +1281,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,
return 0;
/* Not a loose object; someone else may have just packed it. */
- reprepare_packed_git();
+ reprepare_packed_git(the_repository);
if (find_pack_entry(real, &e))
break;
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 07/11] packfile: allow prepare_packed_git_one to handle arbitrary repositories
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (5 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 06/11] packfile: add repository argument to reprepare_packed_git Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 1:06 ` [PATCH 08/11] packfile: allow prepare_packed_git " Stefan Beller
` (5 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com>
---
packfile.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/packfile.c b/packfile.c
index 6ea50230aab..c45516acd41 100644
--- a/packfile.c
+++ b/packfile.c
@@ -735,8 +735,7 @@ static void report_pack_garbage(struct string_list *list)
report_helper(list, seen_bits, first, list->nr);
}
-#define prepare_packed_git_one(r, o, l) prepare_packed_git_one_##r(o, l)
-static void prepare_packed_git_one_the_repository(char *objdir, int local)
+static void prepare_packed_git_one(struct repository *r, char *objdir, int local)
{
struct strbuf path = STRBUF_INIT;
size_t dirnamelen;
@@ -769,7 +768,7 @@ static void prepare_packed_git_one_the_repository(char *objdir, int local)
base_len = path.len;
if (strip_suffix_mem(path.buf, &base_len, ".idx")) {
/* Don't reopen a pack we already have. */
- for (p = the_repository->objects.packed_git; p;
+ for (p = r->objects.packed_git; p;
p = p->next) {
size_t len;
if (strip_suffix(p->pack_name, ".pack", &len) &&
@@ -783,7 +782,7 @@ static void prepare_packed_git_one_the_repository(char *objdir, int local)
* corresponding .pack file that we can map.
*/
(p = add_packed_git(path.buf, path.len, local)) != NULL)
- install_packed_git(the_repository, p);
+ install_packed_git(r, p);
}
if (!report_garbage)
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 08/11] packfile: allow prepare_packed_git to handle arbitrary repositories
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (6 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 07/11] packfile: allow prepare_packed_git_one to handle arbitrary repositories Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 1:06 ` [PATCH 09/11] packfile: allow reprepare_packed_git " Stefan Beller
` (4 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com>
---
packfile.c | 18 +++++++++---------
packfile.h | 3 +--
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/packfile.c b/packfile.c
index c45516acd41..9a3efc01555 100644
--- a/packfile.c
+++ b/packfile.c
@@ -883,19 +883,19 @@ static void prepare_packed_git_mru(struct repository *r)
list_add_tail(&p->mru, &r->objects.packed_git_mru);
}
-void prepare_packed_git_the_repository(void)
+void prepare_packed_git(struct repository *r)
{
struct alternate_object_database *alt;
- if (the_repository->objects.packed_git_initialized)
+ if (r->objects.packed_git_initialized)
return;
- prepare_packed_git_one(the_repository, get_object_directory(), 1);
- prepare_alt_odb(the_repository);
- for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
- prepare_packed_git_one(the_repository, alt->path, 0);
- rearrange_packed_git(the_repository);
- prepare_packed_git_mru(the_repository);
- the_repository->objects.packed_git_initialized = 1;
+ prepare_packed_git_one(r, get_object_directory(), 1);
+ prepare_alt_odb(r);
+ for (alt = r->objects.alt_odb_list; alt; alt = alt->next)
+ prepare_packed_git_one(r, alt->path, 0);
+ rearrange_packed_git(r);
+ prepare_packed_git_mru(r);
+ r->objects.packed_git_initialized = 1;
}
void reprepare_packed_git_the_repository(void)
diff --git a/packfile.h b/packfile.h
index 9afbf73657e..9142866c8ae 100644
--- a/packfile.h
+++ b/packfile.h
@@ -34,8 +34,7 @@ extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_
#define PACKDIR_FILE_GARBAGE 4
extern void (*report_garbage)(unsigned seen_bits, const char *path);
-#define prepare_packed_git(r) prepare_packed_git_##r()
-extern void prepare_packed_git_the_repository(void);
+extern void prepare_packed_git(struct repository *r);
#define reprepare_packed_git(r) reprepare_packed_git_##r()
extern void reprepare_packed_git_the_repository(void);
extern void install_packed_git(struct repository *r, struct packed_git *pack);
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 09/11] packfile: allow reprepare_packed_git to handle arbitrary repositories
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (7 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 08/11] packfile: allow prepare_packed_git " Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 1:06 ` [PATCH 10/11] packfile: add repository argument to find_pack_entry Stefan Beller
` (3 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com>
---
packfile.c | 8 ++++----
packfile.h | 3 +--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/packfile.c b/packfile.c
index 9a3efc01555..b12fc0789e0 100644
--- a/packfile.c
+++ b/packfile.c
@@ -898,11 +898,11 @@ void prepare_packed_git(struct repository *r)
r->objects.packed_git_initialized = 1;
}
-void reprepare_packed_git_the_repository(void)
+void reprepare_packed_git(struct repository *r)
{
- the_repository->objects.approximate_object_count_valid = 0;
- the_repository->objects.packed_git_initialized = 0;
- prepare_packed_git(the_repository);
+ r->objects.approximate_object_count_valid = 0;
+ r->objects.packed_git_initialized = 0;
+ prepare_packed_git(r);
}
unsigned long unpack_object_header_buffer(const unsigned char *buf,
diff --git a/packfile.h b/packfile.h
index 9142866c8ae..99f4741bd95 100644
--- a/packfile.h
+++ b/packfile.h
@@ -35,8 +35,7 @@ extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_
extern void (*report_garbage)(unsigned seen_bits, const char *path);
extern void prepare_packed_git(struct repository *r);
-#define reprepare_packed_git(r) reprepare_packed_git_##r()
-extern void reprepare_packed_git_the_repository(void);
+extern void reprepare_packed_git(struct repository *r);
extern void install_packed_git(struct repository *r, struct packed_git *pack);
/*
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 10/11] packfile: add repository argument to find_pack_entry
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (8 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 09/11] packfile: allow reprepare_packed_git " Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 1:06 ` [PATCH 11/11] packfile: allow find_pack_entry to handle arbitrary repositories Stefan Beller
` (2 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
While at it move the documentation to the header and mention which pack
files are searched.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
packfile.c | 8 ++------
packfile.h | 7 ++++++-
sha1_file.c | 6 +++---
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/packfile.c b/packfile.c
index b12fc0789e0..e16f8252233 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1835,11 +1835,7 @@ static int fill_pack_entry(const unsigned char *sha1,
return 1;
}
-/*
- * Iff a pack file contains the object named by sha1, return true and
- * store its location to e.
- */
-int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
+int find_pack_entry_the_repository(const unsigned char *sha1, struct pack_entry *e)
{
struct list_head *pos;
@@ -1860,7 +1856,7 @@ int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
int has_sha1_pack(const unsigned char *sha1)
{
struct pack_entry e;
- return find_pack_entry(sha1, &e);
+ return find_pack_entry(the_repository, sha1, &e);
}
int has_pack_index(const unsigned char *sha1)
diff --git a/packfile.h b/packfile.h
index 99f4741bd95..feeabd6f031 100644
--- a/packfile.h
+++ b/packfile.h
@@ -120,7 +120,12 @@ extern int packed_object_info(struct packed_git *pack, off_t offset, struct obje
extern void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1);
extern const struct packed_git *has_packed_and_bad(const unsigned char *sha1);
-extern int find_pack_entry(const unsigned char *sha1, struct pack_entry *e);
+/*
+ * Iff a pack file in the given repository contains the object named by sha1,
+ * return true and store its location to e.
+ */
+#define find_pack_entry(r, s, e) find_pack_entry_##r(s, e)
+extern int find_pack_entry_the_repository(const unsigned char *sha1, struct pack_entry *e);
extern int has_sha1_pack(const unsigned char *sha1);
diff --git a/sha1_file.c b/sha1_file.c
index 0b9fefaaf02..d498b3e90b9 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1273,7 +1273,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,
}
while (1) {
- if (find_pack_entry(real, &e))
+ if (find_pack_entry(the_repository, real, &e))
break;
/* Most likely it's a loose object. */
@@ -1282,7 +1282,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,
/* Not a loose object; someone else may have just packed it. */
reprepare_packed_git(the_repository);
- if (find_pack_entry(real, &e))
+ if (find_pack_entry(the_repository, real, &e))
break;
/* Check if it is a missing object */
@@ -1662,7 +1662,7 @@ static int freshen_loose_object(const unsigned char *sha1)
static int freshen_packed_object(const unsigned char *sha1)
{
struct pack_entry e;
- if (!find_pack_entry(sha1, &e))
+ if (!find_pack_entry(the_repository, sha1, &e))
return 0;
if (e.p->freshened)
return 1;
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 11/11] packfile: allow find_pack_entry to handle arbitrary repositories
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (9 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 10/11] packfile: add repository argument to find_pack_entry Stefan Beller
@ 2018-02-28 1:06 ` Stefan Beller
2018-02-28 2:15 ` [PATCH 00/11] Moving global state into the repository object (part 2) Duy Nguyen
2018-02-28 17:57 ` Junio C Hamano
12 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 1:06 UTC (permalink / raw)
To: git; +Cc: pclouds, Stefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com>
---
packfile.c | 10 +++++-----
packfile.h | 3 +--
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/packfile.c b/packfile.c
index e16f8252233..399c59e640f 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1835,18 +1835,18 @@ static int fill_pack_entry(const unsigned char *sha1,
return 1;
}
-int find_pack_entry_the_repository(const unsigned char *sha1, struct pack_entry *e)
+int find_pack_entry(struct repository *r, const unsigned char *sha1, struct pack_entry *e)
{
struct list_head *pos;
- prepare_packed_git(the_repository);
- if (!the_repository->objects.packed_git)
+ prepare_packed_git(r);
+ if (!r->objects.packed_git)
return 0;
- list_for_each(pos, &the_repository->objects.packed_git_mru) {
+ list_for_each(pos, &r->objects.packed_git_mru) {
struct packed_git *p = list_entry(pos, struct packed_git, mru);
if (fill_pack_entry(sha1, e, p)) {
- list_move(&p->mru, &the_repository->objects.packed_git_mru);
+ list_move(&p->mru, &r->objects.packed_git_mru);
return 1;
}
}
diff --git a/packfile.h b/packfile.h
index feeabd6f031..6f7b9e91d66 100644
--- a/packfile.h
+++ b/packfile.h
@@ -124,8 +124,7 @@ extern const struct packed_git *has_packed_and_bad(const unsigned char *sha1);
* Iff a pack file in the given repository contains the object named by sha1,
* return true and store its location to e.
*/
-#define find_pack_entry(r, s, e) find_pack_entry_##r(s, e)
-extern int find_pack_entry_the_repository(const unsigned char *sha1, struct pack_entry *e);
+extern int find_pack_entry(struct repository *r, const unsigned char *sha1, struct pack_entry *e);
extern int has_sha1_pack(const unsigned char *sha1);
--
2.16.2.395.g2e18187dfd-goog
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (10 preceding siblings ...)
2018-02-28 1:06 ` [PATCH 11/11] packfile: allow find_pack_entry to handle arbitrary repositories Stefan Beller
@ 2018-02-28 2:15 ` Duy Nguyen
2018-02-28 14:47 ` Derrick Stolee
2018-02-28 17:59 ` Junio C Hamano
2018-02-28 17:57 ` Junio C Hamano
12 siblings, 2 replies; 24+ messages in thread
From: Duy Nguyen @ 2018-02-28 2:15 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
On Tue, Feb 27, 2018 at 05:05:57PM -0800, Stefan Beller wrote:
> This applies on top of origin/sb/object-store and is the continuation of
> that series, adding the repository as a context argument to functions.
>
> This series focusses on packfile handling, exposing (re)prepare_packed_git
> and find_pack_entry to a repository argument.
It looks good.
Looking at the full-series diff though, it makes me wonder if we
should keep prepare_packed_git() private (i.e. how we initialize the
object store, packfile included, is a private matter). How about
something like this on top?
-- 8< --
Subject: [PATCH] packfile: keep raw_object_store.packed_git{,_mru} fields private
These fields are initialized lazily via prepare_packed_git(). All
access to these must call that function first but unless you know the
implementation details, you may not see the connection.
Keep that connection internal. These fields should only be accessed
via get_packed_git() and get_packed_git_mru() outside packfile.c.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/count-objects.c | 4 +---
builtin/fsck.c | 6 ++----
builtin/gc.c | 3 +--
builtin/pack-objects.c | 21 +++++++++++----------
builtin/pack-redundant.c | 6 ++----
fast-import.c | 7 ++-----
http-backend.c | 5 ++---
object-store.h | 4 ++--
pack-bitmap.c | 3 +--
packfile.c | 15 ++++++++++++++-
packfile.h | 6 +++++-
server-info.c | 5 ++---
sha1_name.c | 6 ++----
13 files changed, 47 insertions(+), 44 deletions(-)
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index d480301763..088309945b 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -121,9 +121,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
struct strbuf loose_buf = STRBUF_INIT;
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
- if (!the_repository->objects.packed_git)
- prepare_packed_git(the_repository);
- for (p = the_repository->objects.packed_git; p; p = p->next) {
+ for (p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
if (open_pack_index(p))
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0a043a43c2..6d86f2581a 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -732,10 +732,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
uint32_t total = 0, count = 0;
struct progress *progress = NULL;
- prepare_packed_git(the_repository);
-
if (show_progress) {
- for (p = the_repository->objects.packed_git; p;
+ for (p = get_packed_git(the_repository); p;
p = p->next) {
if (open_pack_index(p))
continue;
@@ -744,7 +742,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
progress = start_progress(_("Checking objects"), total);
}
- for (p = the_repository->objects.packed_git; p;
+ for (p = get_packed_git(the_repository); p;
p = p->next) {
/* verify gives error messages itself */
if (verify_pack(p, fsck_obj_buffer,
diff --git a/builtin/gc.c b/builtin/gc.c
index 80d19c54d5..be63bec09c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -173,8 +173,7 @@ static int too_many_packs(void)
if (gc_auto_pack_limit <= 0)
return 0;
- prepare_packed_git(the_repository);
- for (cnt = 0, p = the_repository->objects.packed_git; p; p = p->next) {
+ for (cnt = 0, p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
if (p->pack_keep)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5e2590f882..a305f50100 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1026,7 +1026,7 @@ static int want_object_in_pack(const struct object_id *oid,
if (want != -1)
return want;
}
- list_for_each(pos, &the_repository->objects.packed_git_mru) {
+ list_for_each(pos, get_packed_git_mru(the_repository)) {
struct packed_git *p = list_entry(pos, struct packed_git, mru);
off_t offset;
@@ -1045,7 +1045,7 @@ static int want_object_in_pack(const struct object_id *oid,
want = want_found_object(exclude, p);
if (!exclude && want > 0)
list_move(&p->mru,
- &the_repository->objects.packed_git_mru);
+ get_packed_git_mru(the_repository));
if (want != -1)
return want;
}
@@ -2674,7 +2674,7 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)
memset(&in_pack, 0, sizeof(in_pack));
- for (p = the_repository->objects.packed_git; p; p = p->next) {
+ for (p = get_packed_git(the_repository); p; p = p->next) {
struct object_id oid;
struct object *o;
@@ -2737,8 +2737,10 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)
static struct packed_git *last_found = (void *)1;
struct packed_git *p;
- p = (last_found != (void *)1) ? last_found :
- the_repository->objects.packed_git;
+ if (last_found != (void *)1)
+ p = last_found;
+ else
+ p = get_packed_git(the_repository);
while (p) {
if ((!p->pack_local || p->pack_keep) &&
@@ -2747,7 +2749,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)
return 1;
}
if (p == last_found)
- p = the_repository->objects.packed_git;
+ p = get_packed_git(the_repository);
else
p = p->next;
if (p == last_found)
@@ -2783,7 +2785,7 @@ static void loosen_unused_packed_objects(struct rev_info *revs)
uint32_t i;
struct object_id oid;
- for (p = the_repository->objects.packed_git; p; p = p->next) {
+ for (p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local || p->pack_keep)
continue;
@@ -3151,10 +3153,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (progress && all_progress_implied)
progress = 2;
- prepare_packed_git(the_repository);
if (ignore_packed_keep) {
struct packed_git *p;
- for (p = the_repository->objects.packed_git; p; p = p->next)
+ for (p = get_packed_git(the_repository); p; p = p->next)
if (p->pack_local && p->pack_keep)
break;
if (!p) /* no keep-able packs found */
@@ -3167,7 +3168,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
* also covers non-local objects
*/
struct packed_git *p;
- for (p = the_repository->objects.packed_git; p; p = p->next) {
+ for (p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local) {
have_non_local_packs = 1;
break;
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 3b02f94248..02b5f0becc 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -572,7 +572,7 @@ static struct pack_list * add_pack(struct packed_git *p)
static struct pack_list * add_pack_file(const char *filename)
{
- struct packed_git *p = the_repository->objects.packed_git;
+ struct packed_git *p = get_packed_git(the_repository);
if (strlen(filename) < 40)
die("Bad pack filename: %s", filename);
@@ -587,7 +587,7 @@ static struct pack_list * add_pack_file(const char *filename)
static void load_all(void)
{
- struct packed_git *p = the_repository->objects.packed_git;
+ struct packed_git *p = get_packed_git(the_repository);
while (p) {
add_pack(p);
@@ -630,8 +630,6 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
break;
}
- prepare_packed_git(the_repository);
-
if (load_all_packs)
load_all();
else
diff --git a/fast-import.c b/fast-import.c
index f2e255ed7e..041217eedf 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1111,8 +1111,7 @@ static int store_object(
if (e->idx.offset) {
duplicate_count_by_type[type]++;
return 1;
- } else if (find_sha1_pack(oid.hash,
- the_repository->objects.packed_git)) {
+ } else if (find_sha1_pack(oid.hash, get_packed_git(the_repository))) {
e->type = type;
e->pack_id = MAX_PACK_ID;
e->idx.offset = 1; /* just not zero! */
@@ -1309,8 +1308,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
duplicate_count_by_type[OBJ_BLOB]++;
truncate_pack(&checkpoint);
- } else if (find_sha1_pack(oid.hash,
- the_repository->objects.packed_git)) {
+ } else if (find_sha1_pack(oid.hash, get_packed_git(the_repository))) {
e->type = OBJ_BLOB;
e->pack_id = MAX_PACK_ID;
e->idx.offset = 1; /* just not zero! */
@@ -3474,7 +3472,6 @@ int cmd_main(int argc, const char **argv)
rc_free[i].next = &rc_free[i + 1];
rc_free[cmd_save - 1].next = NULL;
- prepare_packed_git(the_repository);
start_packfile();
set_die_routine(die_nicely);
set_checkpoint_signal();
diff --git a/http-backend.c b/http-backend.c
index defa6ba350..22d2e1668e 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -518,14 +518,13 @@ static void get_info_packs(struct strbuf *hdr, char *arg)
size_t cnt = 0;
select_getanyfile(hdr);
- prepare_packed_git(the_repository);
- for (p = the_repository->objects.packed_git; p; p = p->next) {
+ for (p = get_packed_git(the_repository); p; p = p->next) {
if (p->pack_local)
cnt++;
}
strbuf_grow(&buf, cnt * 53 + 2);
- for (p = the_repository->objects.packed_git; p; p = p->next) {
+ for (p = get_packed_git(the_repository); p; p = p->next) {
if (p->pack_local)
strbuf_addf(&buf, "P %s\n", p->pack_name + objdirlen + 6);
}
diff --git a/object-store.h b/object-store.h
index afe2f93459..636375753f 100644
--- a/object-store.h
+++ b/object-store.h
@@ -87,9 +87,9 @@ struct raw_object_store {
*/
char *objectdir;
- struct packed_git *packed_git;
+ struct packed_git *packed_git; /* private */
/* A most-recently-used ordered version of the packed_git list. */
- struct list_head packed_git_mru;
+ struct list_head packed_git_mru; /* private */
struct alternate_object_database *alt_odb_list;
struct alternate_object_database **alt_odb_tail;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index bcc04bc45e..2a007b5539 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -335,8 +335,7 @@ static int open_pack_bitmap(void)
assert(!bitmap_git.map && !bitmap_git.loaded);
- prepare_packed_git(the_repository);
- for (p = the_repository->objects.packed_git; p; p = p->next) {
+ for (p = get_packed_git(the_repository); p; p = p->next) {
if (open_pack_bitmap_1(p) == 0)
ret = 0;
}
diff --git a/packfile.c b/packfile.c
index d9065ccd32..ed26ab1b3b 100644
--- a/packfile.c
+++ b/packfile.c
@@ -802,6 +802,7 @@ static void prepare_packed_git_one(struct repository *r, char *objdir, int local
strbuf_release(&path);
}
+static void prepare_packed_git(struct repository *r);
/*
* Give a fast, rough count of the number of objects in the repository. This
* ignores loose objects completely. If you have a lot of them, then either
@@ -882,7 +883,7 @@ static void prepare_packed_git_mru(struct repository *r)
list_add_tail(&p->mru, &r->objects.packed_git_mru);
}
-void prepare_packed_git(struct repository *r)
+static void prepare_packed_git(struct repository *r)
{
struct alternate_object_database *alt;
@@ -897,6 +898,18 @@ void prepare_packed_git(struct repository *r)
r->objects.packed_git_initialized = 1;
}
+struct packed_git *get_packed_git(struct repository *r)
+{
+ prepare_packed_git(r);
+ return r->objects.packed_git;
+}
+
+struct list_head *get_packed_git_mru(struct repository *r)
+{
+ prepare_packed_git(r);
+ return &r->objects.packed_git_mru;
+}
+
void reprepare_packed_git(struct repository *r)
{
r->objects.approximate_object_count_valid = 0;
diff --git a/packfile.h b/packfile.h
index 6f7b9e91d6..1903984e22 100644
--- a/packfile.h
+++ b/packfile.h
@@ -3,6 +3,9 @@
#include "oidset.h"
+struct list_head;
+struct packed_git;
+
/*
* Generate the filename to be used for a pack file with checksum "sha1" and
* extension "ext". The result is written into the strbuf "buf", overwriting
@@ -34,7 +37,8 @@ extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_
#define PACKDIR_FILE_GARBAGE 4
extern void (*report_garbage)(unsigned seen_bits, const char *path);
-extern void prepare_packed_git(struct repository *r);
+extern struct packed_git *get_packed_git(struct repository *r);
+extern struct list_head *get_packed_git_mru(struct repository *r);
extern void reprepare_packed_git(struct repository *r);
extern void install_packed_git(struct repository *r, struct packed_git *pack);
diff --git a/server-info.c b/server-info.c
index f5e4d1cc89..af737acd24 100644
--- a/server-info.c
+++ b/server-info.c
@@ -200,8 +200,7 @@ static void init_pack_info(const char *infofile, int force)
objdir = get_object_directory();
objdirlen = strlen(objdir);
- prepare_packed_git(the_repository);
- for (p = the_repository->objects.packed_git; p; p = p->next) {
+ for (p = get_packed_git(the_repository); p; p = p->next) {
/* we ignore things on alternate path since they are
* not available to the pullers in general.
*/
@@ -211,7 +210,7 @@ static void init_pack_info(const char *infofile, int force)
}
num_pack = i;
info = xcalloc(num_pack, sizeof(struct pack_info *));
- for (i = 0, p = the_repository->objects.packed_git; p; p = p->next) {
+ for (i = 0, p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
info[i] = xcalloc(1, sizeof(struct pack_info));
diff --git a/sha1_name.c b/sha1_name.c
index 773322ac81..f25a0970ca 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -195,8 +195,7 @@ static void find_short_packed_object(struct disambiguate_state *ds)
{
struct packed_git *p;
- prepare_packed_git(the_repository);
- for (p = the_repository->objects.packed_git; p && !ds->ambiguous;
+ for (p = get_packed_git(the_repository); p && !ds->ambiguous;
p = p->next)
unique_in_pack(p, ds);
}
@@ -566,8 +565,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)
{
struct packed_git *p;
- prepare_packed_git(the_repository);
- for (p = the_repository->objects.packed_git; p; p = p->next)
+ for (p = get_packed_git(the_repository); p; p = p->next)
find_abbrev_len_for_pack(p, mad);
}
--
2.16.1.399.g632f88eed1
-- 8< --
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 2:15 ` [PATCH 00/11] Moving global state into the repository object (part 2) Duy Nguyen
@ 2018-02-28 14:47 ` Derrick Stolee
2018-02-28 17:59 ` Junio C Hamano
1 sibling, 0 replies; 24+ messages in thread
From: Derrick Stolee @ 2018-02-28 14:47 UTC (permalink / raw)
To: Duy Nguyen, Stefan Beller; +Cc: git
On 2/27/2018 9:15 PM, Duy Nguyen wrote:
> On Tue, Feb 27, 2018 at 05:05:57PM -0800, Stefan Beller wrote:
>> This applies on top of origin/sb/object-store and is the continuation of
>> that series, adding the repository as a context argument to functions.
>>
>> This series focusses on packfile handling, exposing (re)prepare_packed_git
>> and find_pack_entry to a repository argument.
> It looks good.
I agree.
> Looking at the full-series diff though, it makes me wonder if we
> should keep prepare_packed_git() private (i.e. how we initialize the
> object store, packfile included, is a private matter). How about
> something like this on top?
I think the get_packed_git() approach is cleaner than navigating
directly to the_repository->objects.packed_git that you expect to be
initialized by an earlier method call.
Thanks,
-Stolee
> -- 8< --
> Subject: [PATCH] packfile: keep raw_object_store.packed_git{,_mru} fields private
>
> These fields are initialized lazily via prepare_packed_git(). All
> access to these must call that function first but unless you know the
> implementation details, you may not see the connection.
>
> Keep that connection internal. These fields should only be accessed
> via get_packed_git() and get_packed_git_mru() outside packfile.c.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> builtin/count-objects.c | 4 +---
> builtin/fsck.c | 6 ++----
> builtin/gc.c | 3 +--
> builtin/pack-objects.c | 21 +++++++++++----------
> builtin/pack-redundant.c | 6 ++----
> fast-import.c | 7 ++-----
> http-backend.c | 5 ++---
> object-store.h | 4 ++--
> pack-bitmap.c | 3 +--
> packfile.c | 15 ++++++++++++++-
> packfile.h | 6 +++++-
> server-info.c | 5 ++---
> sha1_name.c | 6 ++----
> 13 files changed, 47 insertions(+), 44 deletions(-)
>
> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> index d480301763..088309945b 100644
> --- a/builtin/count-objects.c
> +++ b/builtin/count-objects.c
> @@ -121,9 +121,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
> struct strbuf loose_buf = STRBUF_INIT;
> struct strbuf pack_buf = STRBUF_INIT;
> struct strbuf garbage_buf = STRBUF_INIT;
> - if (!the_repository->objects.packed_git)
> - prepare_packed_git(the_repository);
> - for (p = the_repository->objects.packed_git; p; p = p->next) {
> + for (p = get_packed_git(the_repository); p; p = p->next) {
> if (!p->pack_local)
> continue;
> if (open_pack_index(p))
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 0a043a43c2..6d86f2581a 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -732,10 +732,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
> uint32_t total = 0, count = 0;
> struct progress *progress = NULL;
>
> - prepare_packed_git(the_repository);
> -
> if (show_progress) {
> - for (p = the_repository->objects.packed_git; p;
> + for (p = get_packed_git(the_repository); p;
> p = p->next) {
> if (open_pack_index(p))
> continue;
> @@ -744,7 +742,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
>
> progress = start_progress(_("Checking objects"), total);
> }
> - for (p = the_repository->objects.packed_git; p;
> + for (p = get_packed_git(the_repository); p;
> p = p->next) {
> /* verify gives error messages itself */
> if (verify_pack(p, fsck_obj_buffer,
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 80d19c54d5..be63bec09c 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -173,8 +173,7 @@ static int too_many_packs(void)
> if (gc_auto_pack_limit <= 0)
> return 0;
>
> - prepare_packed_git(the_repository);
> - for (cnt = 0, p = the_repository->objects.packed_git; p; p = p->next) {
> + for (cnt = 0, p = get_packed_git(the_repository); p; p = p->next) {
> if (!p->pack_local)
> continue;
> if (p->pack_keep)
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 5e2590f882..a305f50100 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1026,7 +1026,7 @@ static int want_object_in_pack(const struct object_id *oid,
> if (want != -1)
> return want;
> }
> - list_for_each(pos, &the_repository->objects.packed_git_mru) {
> + list_for_each(pos, get_packed_git_mru(the_repository)) {
> struct packed_git *p = list_entry(pos, struct packed_git, mru);
> off_t offset;
>
> @@ -1045,7 +1045,7 @@ static int want_object_in_pack(const struct object_id *oid,
> want = want_found_object(exclude, p);
> if (!exclude && want > 0)
> list_move(&p->mru,
> - &the_repository->objects.packed_git_mru);
> + get_packed_git_mru(the_repository));
> if (want != -1)
> return want;
> }
> @@ -2674,7 +2674,7 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)
>
> memset(&in_pack, 0, sizeof(in_pack));
>
> - for (p = the_repository->objects.packed_git; p; p = p->next) {
> + for (p = get_packed_git(the_repository); p; p = p->next) {
> struct object_id oid;
> struct object *o;
>
> @@ -2737,8 +2737,10 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)
> static struct packed_git *last_found = (void *)1;
> struct packed_git *p;
>
> - p = (last_found != (void *)1) ? last_found :
> - the_repository->objects.packed_git;
> + if (last_found != (void *)1)
> + p = last_found;
> + else
> + p = get_packed_git(the_repository);
>
> while (p) {
> if ((!p->pack_local || p->pack_keep) &&
> @@ -2747,7 +2749,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)
> return 1;
> }
> if (p == last_found)
> - p = the_repository->objects.packed_git;
> + p = get_packed_git(the_repository);
> else
> p = p->next;
> if (p == last_found)
> @@ -2783,7 +2785,7 @@ static void loosen_unused_packed_objects(struct rev_info *revs)
> uint32_t i;
> struct object_id oid;
>
> - for (p = the_repository->objects.packed_git; p; p = p->next) {
> + for (p = get_packed_git(the_repository); p; p = p->next) {
> if (!p->pack_local || p->pack_keep)
> continue;
>
> @@ -3151,10 +3153,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> if (progress && all_progress_implied)
> progress = 2;
>
> - prepare_packed_git(the_repository);
> if (ignore_packed_keep) {
> struct packed_git *p;
> - for (p = the_repository->objects.packed_git; p; p = p->next)
> + for (p = get_packed_git(the_repository); p; p = p->next)
> if (p->pack_local && p->pack_keep)
> break;
> if (!p) /* no keep-able packs found */
> @@ -3167,7 +3168,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> * also covers non-local objects
> */
> struct packed_git *p;
> - for (p = the_repository->objects.packed_git; p; p = p->next) {
> + for (p = get_packed_git(the_repository); p; p = p->next) {
> if (!p->pack_local) {
> have_non_local_packs = 1;
> break;
> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> index 3b02f94248..02b5f0becc 100644
> --- a/builtin/pack-redundant.c
> +++ b/builtin/pack-redundant.c
> @@ -572,7 +572,7 @@ static struct pack_list * add_pack(struct packed_git *p)
>
> static struct pack_list * add_pack_file(const char *filename)
> {
> - struct packed_git *p = the_repository->objects.packed_git;
> + struct packed_git *p = get_packed_git(the_repository);
>
> if (strlen(filename) < 40)
> die("Bad pack filename: %s", filename);
> @@ -587,7 +587,7 @@ static struct pack_list * add_pack_file(const char *filename)
>
> static void load_all(void)
> {
> - struct packed_git *p = the_repository->objects.packed_git;
> + struct packed_git *p = get_packed_git(the_repository);
>
> while (p) {
> add_pack(p);
> @@ -630,8 +630,6 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
> break;
> }
>
> - prepare_packed_git(the_repository);
> -
> if (load_all_packs)
> load_all();
> else
> diff --git a/fast-import.c b/fast-import.c
> index f2e255ed7e..041217eedf 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1111,8 +1111,7 @@ static int store_object(
> if (e->idx.offset) {
> duplicate_count_by_type[type]++;
> return 1;
> - } else if (find_sha1_pack(oid.hash,
> - the_repository->objects.packed_git)) {
> + } else if (find_sha1_pack(oid.hash, get_packed_git(the_repository))) {
> e->type = type;
> e->pack_id = MAX_PACK_ID;
> e->idx.offset = 1; /* just not zero! */
> @@ -1309,8 +1308,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
> duplicate_count_by_type[OBJ_BLOB]++;
> truncate_pack(&checkpoint);
>
> - } else if (find_sha1_pack(oid.hash,
> - the_repository->objects.packed_git)) {
> + } else if (find_sha1_pack(oid.hash, get_packed_git(the_repository))) {
> e->type = OBJ_BLOB;
> e->pack_id = MAX_PACK_ID;
> e->idx.offset = 1; /* just not zero! */
> @@ -3474,7 +3472,6 @@ int cmd_main(int argc, const char **argv)
> rc_free[i].next = &rc_free[i + 1];
> rc_free[cmd_save - 1].next = NULL;
>
> - prepare_packed_git(the_repository);
> start_packfile();
> set_die_routine(die_nicely);
> set_checkpoint_signal();
> diff --git a/http-backend.c b/http-backend.c
> index defa6ba350..22d2e1668e 100644
> --- a/http-backend.c
> +++ b/http-backend.c
> @@ -518,14 +518,13 @@ static void get_info_packs(struct strbuf *hdr, char *arg)
> size_t cnt = 0;
>
> select_getanyfile(hdr);
> - prepare_packed_git(the_repository);
> - for (p = the_repository->objects.packed_git; p; p = p->next) {
> + for (p = get_packed_git(the_repository); p; p = p->next) {
> if (p->pack_local)
> cnt++;
> }
>
> strbuf_grow(&buf, cnt * 53 + 2);
> - for (p = the_repository->objects.packed_git; p; p = p->next) {
> + for (p = get_packed_git(the_repository); p; p = p->next) {
> if (p->pack_local)
> strbuf_addf(&buf, "P %s\n", p->pack_name + objdirlen + 6);
> }
> diff --git a/object-store.h b/object-store.h
> index afe2f93459..636375753f 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -87,9 +87,9 @@ struct raw_object_store {
> */
> char *objectdir;
>
> - struct packed_git *packed_git;
> + struct packed_git *packed_git; /* private */
> /* A most-recently-used ordered version of the packed_git list. */
> - struct list_head packed_git_mru;
> + struct list_head packed_git_mru; /* private */
>
> struct alternate_object_database *alt_odb_list;
> struct alternate_object_database **alt_odb_tail;
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index bcc04bc45e..2a007b5539 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -335,8 +335,7 @@ static int open_pack_bitmap(void)
>
> assert(!bitmap_git.map && !bitmap_git.loaded);
>
> - prepare_packed_git(the_repository);
> - for (p = the_repository->objects.packed_git; p; p = p->next) {
> + for (p = get_packed_git(the_repository); p; p = p->next) {
> if (open_pack_bitmap_1(p) == 0)
> ret = 0;
> }
> diff --git a/packfile.c b/packfile.c
> index d9065ccd32..ed26ab1b3b 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -802,6 +802,7 @@ static void prepare_packed_git_one(struct repository *r, char *objdir, int local
> strbuf_release(&path);
> }
>
> +static void prepare_packed_git(struct repository *r);
> /*
> * Give a fast, rough count of the number of objects in the repository. This
> * ignores loose objects completely. If you have a lot of them, then either
> @@ -882,7 +883,7 @@ static void prepare_packed_git_mru(struct repository *r)
> list_add_tail(&p->mru, &r->objects.packed_git_mru);
> }
>
> -void prepare_packed_git(struct repository *r)
> +static void prepare_packed_git(struct repository *r)
> {
> struct alternate_object_database *alt;
>
> @@ -897,6 +898,18 @@ void prepare_packed_git(struct repository *r)
> r->objects.packed_git_initialized = 1;
> }
>
> +struct packed_git *get_packed_git(struct repository *r)
> +{
> + prepare_packed_git(r);
> + return r->objects.packed_git;
> +}
> +
> +struct list_head *get_packed_git_mru(struct repository *r)
> +{
> + prepare_packed_git(r);
> + return &r->objects.packed_git_mru;
> +}
> +
> void reprepare_packed_git(struct repository *r)
> {
> r->objects.approximate_object_count_valid = 0;
> diff --git a/packfile.h b/packfile.h
> index 6f7b9e91d6..1903984e22 100644
> --- a/packfile.h
> +++ b/packfile.h
> @@ -3,6 +3,9 @@
>
> #include "oidset.h"
>
> +struct list_head;
> +struct packed_git;
> +
> /*
> * Generate the filename to be used for a pack file with checksum "sha1" and
> * extension "ext". The result is written into the strbuf "buf", overwriting
> @@ -34,7 +37,8 @@ extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_
> #define PACKDIR_FILE_GARBAGE 4
> extern void (*report_garbage)(unsigned seen_bits, const char *path);
>
> -extern void prepare_packed_git(struct repository *r);
> +extern struct packed_git *get_packed_git(struct repository *r);
> +extern struct list_head *get_packed_git_mru(struct repository *r);
> extern void reprepare_packed_git(struct repository *r);
> extern void install_packed_git(struct repository *r, struct packed_git *pack);
>
> diff --git a/server-info.c b/server-info.c
> index f5e4d1cc89..af737acd24 100644
> --- a/server-info.c
> +++ b/server-info.c
> @@ -200,8 +200,7 @@ static void init_pack_info(const char *infofile, int force)
> objdir = get_object_directory();
> objdirlen = strlen(objdir);
>
> - prepare_packed_git(the_repository);
> - for (p = the_repository->objects.packed_git; p; p = p->next) {
> + for (p = get_packed_git(the_repository); p; p = p->next) {
> /* we ignore things on alternate path since they are
> * not available to the pullers in general.
> */
> @@ -211,7 +210,7 @@ static void init_pack_info(const char *infofile, int force)
> }
> num_pack = i;
> info = xcalloc(num_pack, sizeof(struct pack_info *));
> - for (i = 0, p = the_repository->objects.packed_git; p; p = p->next) {
> + for (i = 0, p = get_packed_git(the_repository); p; p = p->next) {
> if (!p->pack_local)
> continue;
> info[i] = xcalloc(1, sizeof(struct pack_info));
> diff --git a/sha1_name.c b/sha1_name.c
> index 773322ac81..f25a0970ca 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -195,8 +195,7 @@ static void find_short_packed_object(struct disambiguate_state *ds)
> {
> struct packed_git *p;
>
> - prepare_packed_git(the_repository);
> - for (p = the_repository->objects.packed_git; p && !ds->ambiguous;
> + for (p = get_packed_git(the_repository); p && !ds->ambiguous;
> p = p->next)
> unique_in_pack(p, ds);
> }
> @@ -566,8 +565,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad)
> {
> struct packed_git *p;
>
> - prepare_packed_git(the_repository);
> - for (p = the_repository->objects.packed_git; p; p = p->next)
> + for (p = get_packed_git(the_repository); p; p = p->next)
> find_abbrev_len_for_pack(p, mad);
> }
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 2:15 ` [PATCH 00/11] Moving global state into the repository object (part 2) Duy Nguyen
2018-02-28 14:47 ` Derrick Stolee
@ 2018-02-28 17:59 ` Junio C Hamano
2018-02-28 18:59 ` Stefan Beller
1 sibling, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2018-02-28 17:59 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Stefan Beller, git
Duy Nguyen <pclouds@gmail.com> writes:
> Looking at the full-series diff though, it makes me wonder if we
> should keep prepare_packed_git() private (i.e. how we initialize the
> object store, packfile included, is a private matter). How about
> something like this on top?
Yup, that looks cleaner.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 17:59 ` Junio C Hamano
@ 2018-02-28 18:59 ` Stefan Beller
2018-02-28 19:09 ` Junio C Hamano
0 siblings, 1 reply; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 18:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, git
On Wed, Feb 28, 2018 at 9:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> Looking at the full-series diff though, it makes me wonder if we
>> should keep prepare_packed_git() private (i.e. how we initialize the
>> object store, packfile included, is a private matter). How about
>> something like this on top?
>
> Yup, that looks cleaner.
I agree that it looks cleaner. So we plan on just putting
it on top of that series?
Thanks for the help on refactoring,
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 18:59 ` Stefan Beller
@ 2018-02-28 19:09 ` Junio C Hamano
0 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2018-02-28 19:09 UTC (permalink / raw)
To: Stefan Beller; +Cc: Duy Nguyen, git
Stefan Beller <sbeller@google.com> writes:
> On Wed, Feb 28, 2018 at 9:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Duy Nguyen <pclouds@gmail.com> writes:
>>
>>> Looking at the full-series diff though, it makes me wonder if we
>>> should keep prepare_packed_git() private (i.e. how we initialize the
>>> object store, packfile included, is a private matter). How about
>>> something like this on top?
>>
>> Yup, that looks cleaner.
>
> I agree that it looks cleaner. So we plan on just putting
> it on top of that series?
We tend to avoid "oops, that was wrong and here is a band aid on
top" for things that are still mushy, so it would be preferrable to
get it fixed inline, especially if there are more changes to the
other parts of the series coming.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 1:05 [PATCH 00/11] Moving global state into the repository object (part 2) Stefan Beller
` (11 preceding siblings ...)
2018-02-28 2:15 ` [PATCH 00/11] Moving global state into the repository object (part 2) Duy Nguyen
@ 2018-02-28 17:57 ` Junio C Hamano
2018-02-28 18:56 ` Stefan Beller
2018-02-28 19:02 ` Junio C Hamano
12 siblings, 2 replies; 24+ messages in thread
From: Junio C Hamano @ 2018-02-28 17:57 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, pclouds
Stefan Beller <sbeller@google.com> writes:
> This applies on top of origin/sb/object-store and is the continuation of
> that series, adding the repository as a context argument to functions.
Wait a minute. Is that topic ever shown to work well together with
other topics in flight and are now ready to be built upon? I had an
impression that it is just starting to get serious reviews.
Sorry, but I am behind ;-)
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 17:57 ` Junio C Hamano
@ 2018-02-28 18:56 ` Stefan Beller
2018-02-28 19:19 ` Jonathan Nieder
2018-02-28 19:02 ` Junio C Hamano
1 sibling, 1 reply; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 18:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Duy Nguyen
On Wed, Feb 28, 2018 at 9:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> This applies on top of origin/sb/object-store and is the continuation of
>> that series, adding the repository as a context argument to functions.
>
> Wait a minute. Is that topic ever shown to work well together with
> other topics in flight and are now ready to be built upon? I had an
> impression that it is just starting to get serious reviews.
And I had the impression the serious reviews were done and fine;
the only issue would be demonstrating its working fine with other
series, where I was also worrying about conflicts with
brians series. And to address that, I'd just send series in small sizes.
> Sorry, but I am behind ;-)
Is there anything that a contributor can help with that eases
refactoring series in flight?
Personally I find these refactorings not as rewarding as new
features or fixing bugs, and additionally the risk of conflict
rises, whereby the maintainer is likely to push back to the contributor
to "please rebase", and rebasing with just textual conflicts is also
not rewarding.
Is there anything process wise that we can do to make refactoring
easier for contributors?
Sorry for the miscommunication, though,
Thanks,
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 18:56 ` Stefan Beller
@ 2018-02-28 19:19 ` Jonathan Nieder
0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Nieder @ 2018-02-28 19:19 UTC (permalink / raw)
To: Stefan Beller; +Cc: Junio C Hamano, git, Duy Nguyen
Hi,
Stefan Beller wrote:
> On Wed, Feb 28, 2018 at 9:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Wait a minute. Is that topic ever shown to work well together with
>> other topics in flight and are now ready to be built upon? I had an
>> impression that it is just starting to get serious reviews.
>
> And I had the impression the serious reviews were done and fine;
> the only issue would be demonstrating its working fine with other
> series, where I was also worrying about conflicts with
> brians series. And to address that, I'd just send series in small sizes.
Some of the patches looked cooked to me and others still do not look
cooked yet. I marked the former with Reviewed-by. In general, a few
things can help to make the process easier for me:
1. Giving a quick reply to a review to say how the comments were
resolved, sometimes even with a resend of that one patch to
illustrate. That way the conversation can continue and the
individual patch can get to a reviewed state faster, without
having to chase between different rerolls of the entire series.
This also has an effect of making the review process more
collaborative: perhaps after seeing how you address their
comments, a reviewer may have another idea that they suggest via a
patch to squash in, etc.
2. In a reroll, summarizing the result of previous reviews by
including acks as appropriate and Reviewed-by if a reviewer
granted it. This helps with reviewing the reroll since it tells
people where to focus their attention.
[...]
> Is there anything that a contributor can help with that eases
> refactoring series in flight?
For helping reviewers, see above.
For helping Junio, what I've seen people occasionally do is to locally
run a "trial merge" against next and pu and see what semantic or
lexical conflicts arise. In the cover letter you can describe these
and give Junio advice to make applying the patch easier for him.
[...]
> Sorry for the miscommunication, though,
FWIW, even though part 1 doesn't look done to me yet, it looks *close*
to done, and I was happy to see the sneak peek at part 2.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 17:57 ` Junio C Hamano
2018-02-28 18:56 ` Stefan Beller
@ 2018-02-28 19:02 ` Junio C Hamano
2018-02-28 19:09 ` Stefan Beller
1 sibling, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2018-02-28 19:02 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, pclouds
Junio C Hamano <gitster@pobox.com> writes:
> Stefan Beller <sbeller@google.com> writes:
>
>> This applies on top of origin/sb/object-store and is the continuation of
>> that series, adding the repository as a context argument to functions.
>
> Wait a minute. Is that topic ever shown to work well together with
> other topics in flight and are now ready to be built upon? I had an
> impression that it is just starting to get serious reviews.
>
> Sorry, but I am behind ;-)
OK, so I finally picked up the last round, which wasn't even in my
private build. I had the previous round but hadn't convinced myself
that my conflict resolution with other topics in flight that were
still mushy was correct, so it was out of 'pu', but at least it was
in my tree, so that is where my impression came from.
I saw that the way a list-head in a repository is initialized has
been revamped in v4, which looked sensible. Will merge it to 'pu'
so that I can pick up the ignore-env removal from Duy.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 00/11] Moving global state into the repository object (part 2)
2018-02-28 19:02 ` Junio C Hamano
@ 2018-02-28 19:09 ` Stefan Beller
0 siblings, 0 replies; 24+ messages in thread
From: Stefan Beller @ 2018-02-28 19:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Duy Nguyen
On Wed, Feb 28, 2018 at 11:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> OK, so I finally picked up the last round, which wasn't even in my
> private build. I had the previous round but hadn't convinced myself
> that my conflict resolution with other topics in flight that were
> still mushy was correct, so it was out of 'pu', but at least it was
> in my tree, so that is where my impression came from.
>
> I saw that the way a list-head in a repository is initialized has
> been revamped in v4, which looked sensible. Will merge it to 'pu'
> so that I can pick up the ignore-env removal from Duy.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread