Re: [PATCH v2 8/9] fetch-pack: support more than one pack lockfile
From: Junio C Hamano <hidden>
Date: 2020-06-11 01:41:26
Jonathan Tan [off-list ref] writes:
quoted hunk
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 4771100072..f66891b010 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c@@ -48,8 +48,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) struct ref **sought = NULL; int nr_sought = 0, alloc_sought = 0; int fd[2]; - char *pack_lockfile = NULL; - char **pack_lockfile_ptr = NULL; + struct string_list pack_lockfiles = STRING_LIST_INIT_DUP; + struct string_list *pack_lockfiles_ptr = NULL; struct child_process *conn; struct fetch_pack_args args; struct oid_array shallow = OID_ARRAY_INIT;@@ -134,7 +134,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) } if (!strcmp("--lock-pack", arg)) { args.lock_pack = 1; + pack_lockfiles_ptr = &pack_lockfiles; continue; } if (!strcmp("--check-self-contained-and-connected", arg)) {@@ -235,10 +235,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) } ref = fetch_pack(&args, fd, ref, sought, nr_sought, + &shallow, pack_lockfiles_ptr, version); + if (pack_lockfiles.nr) {
In other parts of this patch, "is the pack_lockfiles pointer NULL?" is used and "does the pack_lockfiles actually have any element?" is not checked, which was a bit hard to reason about, but the idea here is that the presence of the list tells the callee to use lockfiles and accumulate them in the string list and then this caller checks if we ended up locking any, which makes sense.
quoted hunk
+ int i; + + printf("lock %s\n", pack_lockfiles.items[0].string); fflush(stdout); + for (i = 1; i < pack_lockfiles.nr; i++) + warning(_("Lockfile created but not reported: %s"), + pack_lockfiles.items[i].string); } if (args.check_self_contained_and_connected && args.self_contained_and_connected) {diff --git a/connected.c b/connected.c index 3135b71e19..937b4bae38 100644 --- a/connected.c +++ b/connected.c@@ -43,10 +43,12 @@ int check_connected(oid_iterate_fn fn, void *cb_data, if (transport && transport->smart_options && transport->smart_options->self_contained_and_connected && - transport->pack_lockfile && - strip_suffix(transport->pack_lockfile, ".keep", &base_len)) { + transport->pack_lockfiles.nr == 1 &&
Hmph, I can understand why "== 1" case must behave differently from "== 0" case, but shouldn't the behavior in "> 1" cases be more similar to the "== 1" case than "== 0" case?
+ strip_suffix(transport->pack_lockfiles.items[0].string,
+ ".keep", &base_len)) {
struct strbuf idx_file = STRBUF_INIT;
- strbuf_add(&idx_file, transport->pack_lockfile, base_len);
+ strbuf_add(&idx_file, transport->pack_lockfiles.items[0].string,
+ base_len);
strbuf_addstr(&idx_file, ".idx");
new_pack = add_packed_git(idx_file.buf, idx_file.len, 1);
strbuf_release(&idx_file);