Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Glen Choo <chooglen@google.com>
To: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH 8/8] fetch: introduce machine-parseable "porcelain" output format
Date: Wed, 26 Apr 2023 12:52:46 -0700	[thread overview]
Message-ID: <kl6lzg6umne9.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <301138da039451519c6e60d6126e7756a54d346f.1681906949.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> The output format is quite simple:
>
> ```
> <flag> <old-object-id> <new-object-id> <local-reference>\n
> ```

This format doesn't show the remote name or url that was fetched. That
seems okay when fetching with a single remote, but it seems necessary
with "--all". Perhaps you were planning to add that in a later series?
If so, I think it's okay to call the "porcelain" format experimental,
and forbid porcelain + --all until then.

>
> We assume two conditions which are generally true:
>
>     - The old and new object IDs have fixed known widths and cannot
>       contain spaces.
>
>     - References cannot contain newlines.

This seems like a non-issue if we add a -z CLI option to indicate that
entries should be NUL terminated instead of newline terminated, but that
can be done as a followup.

> With these assumptions, the output format becomes unambiguously
> parseable. Furthermore, given that this output is designed to be
> consumed by scripts, the machine-readable data is printed to stdout
> instead of stderr like the human-readable output is. This is mostly done
> so that other data printed to stderr, like error messages or progress
> meters, don't interfere with the parseable data.

Sending the 'main output' to stdout makes sense to me, but this (and
possibly respecting -z) sounds like a different mode of operation, not
just a matter of formats. It seems different enough that I'd prefer not
to piggyback on "fetch.output" for this (even though this adds more
surface to the interface...).

We could add --porcelain and say that "fetch.output" is ignored if
--porcelain is also given. That also eliminates the need for
--output-format, I think.

The .c changes look good to me.

> +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
> +'

As mentioned upthread, I think this test isn't needed because
"porcelain" wouldn't run into the bug we are checking for anyway.

  reply	other threads:[~2023-04-26 19:52 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-19 12:31 [PATCH 0/8] fetch: introduce machine-parseable output Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 1/8] fetch: split out tests for output format Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 2/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 3/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-04-26 19:20   ` Jacob Keller
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-26 19:21   ` Jacob Keller
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-26 19:25   ` Glen Choo
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 4/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 5/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 6/8] fetch: move option related variables " Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 7/8] fetch: introduce new `--output-format` option Patrick Steinhardt
2023-04-26 19:40   ` Glen Choo
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-19 12:31 ` [PATCH 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-04-26 19:52   ` Glen Choo [this message]
2023-04-27 10:58     ` Patrick Steinhardt
2023-04-27 23:20       ` Glen Choo
2023-04-28  8:51         ` Patrick Steinhardt
2023-04-28 17:20           ` Glen Choo
2023-05-02 20:55       ` Felipe Contreras
2023-04-24 20:17 ` [PATCH 0/8] fetch: introduce machine-parseable output Felipe Contreras
2023-04-25  9:58   ` Patrick Steinhardt
2023-04-26 19:14     ` Jacob Keller
2023-04-26 20:23       ` Junio C Hamano
2023-04-26 20:30         ` Jacob Keller
2023-04-27 10:58         ` Patrick Steinhardt
2023-04-27 19:46           ` Jacob Keller
2023-04-27 22:49         ` Glen Choo
2023-04-26 20:24       ` Junio C Hamano
2023-04-26 18:54 ` Glen Choo
2023-04-26 21:14   ` Glen Choo
2023-04-26 19:17 ` Jacob Keller
2023-04-27 11:13 ` [PATCH v2 " Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 1/8] fetch: split out tests for output format Patrick Steinhardt
2023-04-29 17:34     ` SZEDER Gábor
2023-05-03 11:21       ` Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 2/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 3/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-04-27 17:26     ` Glen Choo
2023-04-27 19:49     ` Jacob Keller
2023-04-27 11:13   ` [PATCH v2 4/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 5/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 6/8] fetch: move option related variables " Patrick Steinhardt
2023-04-27 21:52     ` Junio C Hamano
2023-04-27 11:13   ` [PATCH v2 7/8] fetch: introduce new `--output-format` option Patrick Steinhardt
2023-04-27 22:01     ` Junio C Hamano
2023-04-28 22:03       ` Glen Choo
2023-05-03  9:12         ` Patrick Steinhardt
2023-04-28 22:31     ` Glen Choo
2023-05-03  9:43       ` Patrick Steinhardt
2023-05-03 11:36         ` Patrick Steinhardt
2023-04-27 11:13   ` [PATCH v2 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-04-27 19:52     ` Jacob Keller
2023-04-28 22:42     ` Glen Choo
2023-05-03 11:34 ` [PATCH v3 0/8] fetch: introduce machine-parseable output Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 1/8] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-08 22:51     ` Glen Choo
2023-05-09 12:41       ` Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 2/8] fetch: split out tests for output format Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 3/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 4/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 5/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 6/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 7/8] fetch: move option related variables " Patrick Steinhardt
2023-05-03 11:34   ` [PATCH v3 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-08 23:42     ` Glen Choo
2023-05-09 12:41       ` Patrick Steinhardt
2023-05-09  0:03     ` Glen Choo
2023-05-03 16:48   ` [PATCH v3 0/8] fetch: introduce machine-parseable output Junio C Hamano
2023-05-03 16:53     ` Junio C Hamano
2023-05-04  7:57       ` Patrick Steinhardt
2023-05-09  0:06   ` Glen Choo
2023-05-09 12:42     ` Patrick Steinhardt
2023-05-09 13:01 ` [PATCH v4 " Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 1/8] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-09 17:49     ` Junio C Hamano
2023-05-09 18:27       ` Glen Choo
2023-05-10 12:34         ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 2/8] fetch: split out tests for output format Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 3/8] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-09 17:58     ` Junio C Hamano
2023-05-10 12:34       ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 4/8] fetch: fix missing from-reference when fetching HEAD:foo Patrick Steinhardt
2023-05-09 19:28     ` Junio C Hamano
2023-05-10 12:34       ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 5/8] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-09 20:19     ` Junio C Hamano
2023-05-10 12:35       ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 6/8] fetch: move display format parsing into main function Patrick Steinhardt
2023-05-09 20:35     ` Junio C Hamano
2023-05-09 22:30     ` Glen Choo
2023-05-10 12:35       ` Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 7/8] fetch: move option related variables " Patrick Steinhardt
2023-05-09 13:02   ` [PATCH v4 8/8] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-09 20:43     ` Junio C Hamano
2023-05-10 12:35       ` Patrick Steinhardt
2023-05-10 12:33 ` [PATCH v5 0/9] fetch: introduce machine-parseable output Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 1/9] fetch: fix `--no-recurse-submodules` with multi-remote fetches Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 2/9] fetch: split out tests for output format Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 3/9] fetch: add a test to exercise invalid output formats Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 4/9] fetch: print left-hand side when fetching HEAD:foo Patrick Steinhardt
2023-05-12  0:16     ` Glen Choo
2023-05-13 16:59     ` Jeff King
2023-05-15  5:15       ` Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 5/9] fetch: refactor calculation of the display table width Patrick Steinhardt
2023-05-12  0:49     ` Glen Choo
2023-05-10 12:34   ` [PATCH v5 6/9] fetch: introduce `display_format` enum Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 7/9] fetch: lift up parsing of "fetch.output" config variable Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 8/9] fetch: move option related variables into main function Patrick Steinhardt
2023-05-10 12:34   ` [PATCH v5 9/9] fetch: introduce machine-parseable "porcelain" output format Patrick Steinhardt
2023-05-12  1:02     ` Glen Choo
2023-05-10 18:05   ` [PATCH v5 0/9] fetch: introduce machine-parseable output Junio C Hamano
2023-05-11 11:05     ` Patrick Steinhardt
2023-05-11 16:53       ` Junio C Hamano
2023-05-11 17:24       ` Felipe Contreras
2023-05-12  1:09   ` Glen Choo
2023-05-12  7:16     ` Patrick Steinhardt

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

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

  git send-email \
    --in-reply-to=kl6lzg6umne9.fsf@chooglen-macbookpro.roam.corp.google.com \
    --to=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=ps@pks.im \
    /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.
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).