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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Patrick Steinhardt <ps@xxxxxx> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux