Re: [PATCH v2 8/8] fetch: introduce machine-parseable "porcelain" output format

From: Glen Choo <chooglen@google.com>
Date: Apr 28, 2023, 22:42 UTC
Message-ID: <kl6l354j39yt.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <d7c1bc1a80406ad320c2de684e0c97ba14827c7a.1682593865.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 8 quoted lines

> A notable ommission here is that the output format does not include the
> remote from which a reference was fetched, which might be important
> information especially in the context of multi-remote fetches. But as
> such a format would require us to print the remote for every single
> reference update due to parallelizable fetches it feels wasteful for the
> most likely usecase, which is when fetching from a single remote. If
> usecases come up for this in the future though it is easy enough to add
> a new "porcelain-v2" format that adds this information.

We discussed this elsewhere in the thread, but if we are just adding information (and not omitting existing information or shuffling it around), I would prefer for us to make the format extensible using flags than to add a whole new format enum. We can't imagine what other information users might want (maybe the remote ref name?), and it would be nice to avoid bumping the 'porcelain version' unnecessarily.

I agree that this format is good enough as a starting point, though.

It's not new to v2, but I've mentioned my reservations on --output-format on:

  https://lore.kernel.org/git/kl6ledo33ovx.fsf@chooglen-macbookpro.roam.corp.google.com/

I will be out of office all of next week. If, in the meantime, everyone else decides that --output-format=porcelain is good enough, I'm happy to accept the result.

Show 22 quoted lines

> @@ -127,6 +193,24 @@ test_expect_success 'fetch output with HEAD and --dry-run' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'fetch porcelain output with HEAD and --dry-run' '
> +	test_when_finished "rm -rf head" &&
> +	git clone . head &&
> +	COMMIT_ID=$(git rev-parse HEAD) &&
> +
> +	git -C head fetch --output-format=porcelain --dry-run origin HEAD >actual &&
> +	cat >expect <<-EOF &&
> +	* $ZERO_OID $COMMIT_ID FETCH_HEAD
> +	EOF
> +	test_cmp expect actual &&
> +
> +	git -C head fetch --output-format=porcelain --dry-run origin HEAD:foo >actual &&
> +	cat >expect <<-EOF &&
> +	* $ZERO_OID $COMMIT_ID refs/heads/foo
> +	EOF
> +	test_cmp expect actual
> +'
> +

Now that the earlier test also tests without --dry-run, shouldn't this one too?