git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 8/9] fetch-pack: support more than one pack lockfile
Date: Wed, 10 Jun 2020 18:41:19 -0700	[thread overview]
Message-ID: <xmqqk10ecrb4.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <566419ae74348ad3f7b1e5d484cf986fea29af73.1591821067.git.jonathantanmy@google.com> (Jonathan Tan's message of "Wed, 10 Jun 2020 13:57:22 -0700")

Jonathan Tan <jonathantanmy@google.com> writes:

> 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.

> +		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);

  reply	other threads:[~2020-06-11  1:41 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-29 22:30 [PATCH 0/8] CDN offloading update Jonathan Tan
2020-05-29 22:30 ` [PATCH 1/8] http: use --stdin when getting dumb HTTP pack Jonathan Tan
2020-05-29 23:00   ` Junio C Hamano
2020-06-01 20:37     ` Jonathan Tan
2020-05-29 22:30 ` [PATCH 2/8] http: improve documentation of http_pack_request Jonathan Tan
2020-05-29 22:30 ` [PATCH 3/8] http-fetch: support fetching packfiles by URL Jonathan Tan
2020-05-29 23:25   ` Junio C Hamano
2020-06-01 20:54     ` Jonathan Tan
2020-05-29 22:30 ` [PATCH 4/8] Documentation: order protocol v2 sections Jonathan Tan
2020-05-29 23:32   ` Junio C Hamano
2020-06-01 20:57     ` Jonathan Tan
2020-05-29 22:30 ` [PATCH 5/8] Documentation: add Packfile URIs design doc Jonathan Tan
2020-05-30  0:15   ` Junio C Hamano
2020-05-30  0:22     ` Junio C Hamano
2020-06-01 23:10       ` Jonathan Tan
2020-06-01 23:07     ` Jonathan Tan
2020-06-10  1:14     ` Jonathan Tan
2020-06-10 17:16       ` Junio C Hamano
2020-06-10 18:04         ` Jonathan Tan
2020-05-29 22:30 ` [PATCH 6/8] upload-pack: refactor reading of pack-objects out Jonathan Tan
2020-05-29 22:30 ` [PATCH 7/8] fetch-pack: support more than one pack lockfile Jonathan Tan
2020-05-29 22:30 ` [PATCH 8/8] upload-pack: send part of packfile response as uri Jonathan Tan
2020-05-31 16:59   ` Junio C Hamano
2020-05-31 17:35   ` Junio C Hamano
2020-06-01 23:20     ` Jonathan Tan
2020-06-01 20:00   ` Jonathan Nieder
2020-06-10 20:57 ` [PATCH v2 0/9] CDN offloading update Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 1/9] http: use --stdin when indexing dumb HTTP pack Jonathan Tan
2020-06-11  1:10     ` Junio C Hamano
2020-06-10 20:57   ` [PATCH v2 2/9] http: refactor finish_http_pack_request() Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 3/9] http-fetch: refactor into function Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 4/9] http-fetch: support fetching packfiles by URL Jonathan Tan
2020-06-11  1:30     ` Junio C Hamano
2020-06-10 20:57   ` [PATCH v2 5/9] Documentation: order protocol v2 sections Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 6/9] Documentation: add Packfile URIs design doc Jonathan Tan
2020-06-11  1:55     ` Junio C Hamano
2020-11-25  9:15     ` Ævar Arnfjörð Bjarmason
2020-11-25 19:09       ` Jonathan Tan
2020-12-01 12:48         ` Ævar Arnfjörð Bjarmason
2020-06-10 20:57   ` [PATCH v2 7/9] upload-pack: refactor reading of pack-objects out Jonathan Tan
2020-06-10 20:57   ` [PATCH v2 8/9] fetch-pack: support more than one pack lockfile Jonathan Tan
2020-06-11  1:41     ` Junio C Hamano [this message]
2020-06-10 20:57   ` [PATCH v2 9/9] upload-pack: send part of packfile response as uri Jonathan Tan
2020-06-10 23:16   ` [PATCH v2 0/9] CDN offloading update Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqk10ecrb4.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).