[PATCH v3 0/8] fetch: introduce machine-parseable output
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 3, 2023, 11:34 UTC
- Message-ID
- <cover.1683113177.git.ps@pks.im>
- In-Reply-To
- <cover.1681906948.git.ps@pks.im>
Hi,
this is the third version of my patch series to introduce a machine-parseable output for git-fetch(1).
Changes compared to v2:
- A new commit that fixes `--no-recurse-submodules` not being
honored for multi-remote fetches was added. This fixes issues that
would otherwise be hit later in the patch series. - Tests were adjusted to explicitly verify stdout and stderr as
standalone units instead of redirecting 2>&1. - Instead of introducing `--output-format` and a new "porcelain"
format I've changed this to instead introduce `--porcelain`. What
finally convinced me to go this way is Glen's list of preexisting
commands that have `--porcelain`. So I decided to not be the only
outlier here. - The DISPLAY_FORMAT_MAX macro was dropped in favor of Junio's
proposal that uses designated initializers for the array. - Similar to `--negotiate-only`, the porcelain output is now
incompatible with `--recurse-submodules=[yes|on-demand]`. This is
done to avoid ambiguity when references in both the superproject
and any of its submodules is updated.Thanks again for all the feedback!
Patrick
Patrick Steinhardt (8): fetch: fix `--no-recurse-submodules` with multi-remote fetches fetch: split out tests for output format fetch: add a test to exercise invalid output formats fetch: fix missing from-reference when fetching HEAD:foo fetch: introduce `display_format` enum fetch: move display format parsing into main function fetch: move option related variables into main function fetch: introduce machine-parseable "porcelain" output format
Documentation/fetch-options.txt | 6 + Documentation/git-fetch.txt | 9 + builtin/fetch.c | 437 +++++++++++++++++++------------- t/t5510-fetch.sh | 53 ---- t/t5526-fetch-submodules.sh | 31 +++ t/t5574-fetch-output.sh | 231 +++++++++++++++++ 6 files changed, 543 insertions(+), 224 deletions(-) create mode 100755 t/t5574-fetch-output.sh
Range-diff against v2:
-: ---------- > 1: 4b2b0cfe15 fetch: fix `--no-recurse-submodules` with multi-remote fetches
1: 0d0d50d14c = 2: 6ebc7450ba fetch: split out tests for output format
2: 29d2c58914 ! 3: 78479922ac fetch: add a test to exercise invalid output formats
@@ t/t5574-fetch-output.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+ test_when_finished "rm -rf clone" &&
+ git clone . clone &&
+
-+ test_must_fail git -C clone -c fetch.output= fetch origin >actual 2>&1 &&
++ test_must_fail git -C clone -c fetch.output= fetch origin >actual.out 2>actual.err &&
+ cat >expect <<-EOF &&
+ fatal: invalid value for ${SQ}fetch.output${SQ}: ${SQ}${SQ}
+ EOF
-+ test_cmp expect actual &&
++ test_must_be_empty actual.out &&
++ test_cmp expect actual.err &&
+
-+ test_must_fail git -C clone -c fetch.output=garbage fetch origin >actual 2>&1 &&
++ test_must_fail git -C clone -c fetch.output=garbage fetch origin >actual.out 2>actual.err &&
+ cat >expect <<-EOF &&
+ fatal: invalid value for ${SQ}fetch.output${SQ}: ${SQ}garbage${SQ}
+ EOF
-+ test_cmp expect actual
++ test_must_be_empty actual.out &&
++ test_cmp expect actual.err
+'
+
test_expect_success 'fetch aligned output' '
3: d1fb6eeae7 ! 4: 46e1266ab0 fetch: fix missing from-reference when fetching HEAD:foo
@@ t/t5574-fetch-output.sh: test_expect_success 'fetch compact output' '
test_cmp expect actual
'
-+test_expect_success 'fetch output with HEAD and --dry-run' '
++test_expect_success 'fetch output with HEAD' '
+ test_when_finished "rm -rf head" &&
+ git clone . head &&
+
-+ git -C head fetch --dry-run origin HEAD >actual 2>&1 &&
++ git -C head fetch --dry-run origin HEAD >actual.out 2>actual.err &&
+ cat >expect <<-EOF &&
+ From $(test-tool path-utils real_path .)/.
+ * branch HEAD -> FETCH_HEAD
+ EOF
-+ test_cmp expect actual &&
++ test_must_be_empty actual.out &&
++ test_cmp expect actual.err &&
+
-+ git -C head fetch origin HEAD >actual 2>&1 &&
-+ test_cmp expect actual &&
++ git -C head fetch origin HEAD >actual.out 2>actual.err &&
++ test_must_be_empty actual.out &&
++ test_cmp expect actual.err &&
+
-+ git -C head fetch --dry-run origin HEAD:foo >actual 2>&1 &&
++ git -C head fetch --dry-run origin HEAD:foo >actual.out 2>actual.err &&
+ cat >expect <<-EOF &&
+ From $(test-tool path-utils real_path .)/.
+ * [new ref] HEAD -> foo
+ EOF
-+ test_cmp expect actual &&
++ test_must_be_empty actual.out &&
++ test_cmp expect actual.err &&
+
-+ git -C head fetch origin HEAD:foo >actual 2>&1 &&
-+ test_cmp expect actual
++ git -C head fetch origin HEAD:foo >actual.out 2>actual.err &&
++ test_must_be_empty actual.out &&
++ test_cmp expect actual.err
+'
+
test_expect_success '--no-show-forced-updates' '
4: b545bf8bb9 ! 5: acc0f7f520 fetch: introduce `display_format` enum
@@ builtin/fetch.c: static void display_state_init(struct display_state *display_st
+ break;
+ }
+ default:
-+ BUG("unexpected display foramt %d", display_state->format);
++ BUG("unexpected display format %d", display_state->format);
}
}
5: 4990d35998 = 6: cd23440128 fetch: move display format parsing into main function
6: cfe84129ab = 7: edbc31013f fetch: move option related variables into main function
7: 0335e5eeb4 < -: ---------- fetch: introduce new `--output-format` option
8: d7c1bc1a80 ! 8: e132d9494e fetch: introduce machine-parseable "porcelain" output format
@@ Commit message
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.
+ most likely usecase, which is when fetching from a single remote.
+
+ In a similar spirit, a second restriction is that this cannot be used
+ with `--recurse-submodules`. This is because any reference updates would
+ be ambiguous without also printing the repository in which the update
+ happens.
+
+ Considering that both multi-remote and submodule fetches are rather
+ niche and likely not going to be useful for the majority of usecases
+ these omissions feel acceptable. If usecases for either of these come up
+ in the future though it is easy enough to add a new "porcelain-v2"
+ format that adds this information.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
- ## Documentation/config/fetch.txt ##
-@@ Documentation/config/fetch.txt: fetch.pruneTags::
-
- fetch.output::
- Control how ref update status is printed. Valid values are
-- `full` and `compact`. Default value is `full`. See section
-- OUTPUT in linkgit:git-fetch[1] for detail.
-+ `full`, `compact` and `porcelain`. Default value is `full`.
-+ See section OUTPUT in linkgit:git-fetch[1] for detail.
-
- fetch.negotiationAlgorithm::
- Control how information about the commits in the local repository
-
## Documentation/fetch-options.txt ##
@@ Documentation/fetch-options.txt: linkgit:git-config[1].
+ --dry-run::
+ Show what would be done, without making any changes.
- --output-format::
- Control how ref update status is printed. Valid values are
-- `full` and `compact`. Default value is `full`. See section
-- OUTPUT in linkgit:git-fetch[1] for detail.
-+ `full`, `compact` and `porcelain`. Default value is `full`.
-+ See section OUTPUT in linkgit:git-fetch[1] for detail.
-
++--porcelain::
++ Print the output to standard output in an easy-to-parse format for
++ scripts. See section OUTPUT in linkgit:git-fetch[1] for details.
+++
++This is incompatible with `--recurse-submodules=[yes|on-demand]`.
++
ifndef::git-pull[]
--[no-]write-fetch-head::
+ Write the list of remote refs fetched in the `FETCH_HEAD`
## Documentation/git-fetch.txt ##
-@@ Documentation/git-fetch.txt: The output of "git fetch" depends on the transport method used; this
- section describes the output when fetching over the Git protocol
- (either locally or via ssh) and Smart HTTP protocol.
-
--The status of the fetch is output in tabular form, with each line
--representing the status of a single ref. Each line is of the form:
-+The output format can be chosen either via the `fetch.output` config
-+(see linkgit:git-config[1]), or via the `--output-format` switch.
-+Supported values include:
-+
-+For the `full` and `compact` output formats, the status of the fetch is
-+output in tabular, with each line representing the status of a single
-+ref. Each line is of the form:
-
- -------------------------------
+@@ Documentation/git-fetch.txt: representing the status of a single ref. Each line is of the form:
<flag> <summary> <from> -> <to> [<reason>]
-------------------------------
-+The `porcelain` output format is intended to be machine-parseable. In
-+contrast to the human-readable output formats it thus prints to standard
-+output instead of standard error. Each line is of the form:
++When using `--porcelain`, the output format is intended to be
++machine-parseable. In contrast to the human-readable output formats it
++thus prints to standard output instead of standard error. Each line is
++of the form:
+
+-------------------------------
+<flag> <old-object-id> <new-object-id> <local-reference>
@@ builtin/fetch.c: enum display_format {
DISPLAY_FORMAT_FULL,
DISPLAY_FORMAT_COMPACT,
+ DISPLAY_FORMAT_PORCELAIN,
- DISPLAY_FORMAT_MAX,
- };
-
-@@ builtin/fetch.c: static const char * const display_formats[DISPLAY_FORMAT_MAX] = {
- NULL,
- "full",
- "compact",
-+ "porcelain",
};
struct display_state {
@@ builtin/fetch.c: static void display_state_init(struct display_state *display_st
+ /* We don't need to precompute anything here. */
+ break;
default:
-- BUG("unexpected display foramt %d", display_state->format);
-+ BUG("unexpected display format %d", display_state->format);
+ BUG("unexpected display format %d", display_state->format);
}
- }
-
@@ builtin/fetch.c: static void print_compact(struct display_state *display_state,
static void display_ref_update(struct display_state *display_state, char code,
const char *summary, const char *error,
@@ builtin/fetch.c: static int prune_refs(struct display_state *display_state,
summary_width);
warn_dangling_symref(stderr, dangling_msg, ref->name);
}
+@@ builtin/fetch.c: static int add_remote_or_group(const char *name, struct string_list *list)
+ return 1;
+ }
+
+-static void add_options_to_argv(struct strvec *argv)
++static void add_options_to_argv(struct strvec *argv,
++ enum display_format format)
+ {
+ if (dry_run)
+ strvec_push(argv, "--dry-run");
+@@ builtin/fetch.c: static void add_options_to_argv(struct strvec *argv)
+ strvec_push(argv, "--ipv6");
+ if (!write_fetch_head)
+ strvec_push(argv, "--no-write-fetch-head");
++ if (format == DISPLAY_FORMAT_PORCELAIN)
++ strvec_pushf(argv, "--porcelain");
+ }
+
+ /* Fetch multiple remotes in parallel */
+@@ builtin/fetch.c: struct parallel_fetch_state {
+ const char **argv;
+ struct string_list *remotes;
+ int next, result;
++ enum display_format format;
+ };
+
+ static int fetch_next_remote(struct child_process *cp,
+@@ builtin/fetch.c: static int fetch_next_remote(struct child_process *cp,
+ strvec_push(&cp->args, remote);
+ cp->git_cmd = 1;
+
+- if (verbosity >= 0)
++ if (verbosity >= 0 && state->format != DISPLAY_FORMAT_PORCELAIN)
+ printf(_("Fetching %s\n"), remote);
+
+ return 1;
+@@ builtin/fetch.c: static int fetch_finished(int result, struct strbuf *out,
+ return 0;
+ }
+
+-static int fetch_multiple(struct string_list *list, int max_children)
++static int fetch_multiple(struct string_list *list, int max_children,
++ enum display_format format)
+ {
+ int i, result = 0;
+ struct strvec argv = STRVEC_INIT;
+@@ builtin/fetch.c: static int fetch_multiple(struct string_list *list, int max_children)
+
+ strvec_pushl(&argv, "fetch", "--append", "--no-auto-gc",
+ "--no-write-commit-graph", NULL);
+- add_options_to_argv(&argv);
++ add_options_to_argv(&argv, format);
+
+ if (max_children != 1 && list->nr != 1) {
+- struct parallel_fetch_state state = { argv.v, list, 0, 0 };
++ struct parallel_fetch_state state = { argv.v, list, 0, 0, format };
+ const struct run_process_parallel_opts opts = {
+ .tr2_category = "fetch",
+ .tr2_label = "parallel/fetch",
+@@ builtin/fetch.c: static int fetch_multiple(struct string_list *list, int max_children)
+
+ strvec_pushv(&cmd.args, argv.v);
+ strvec_push(&cmd.args, name);
+- if (verbosity >= 0)
++ if (verbosity >= 0 && format != DISPLAY_FORMAT_PORCELAIN)
+ printf(_("Fetching %s\n"), name);
+ cmd.git_cmd = 1;
+ if (run_command(&cmd)) {
+@@ builtin/fetch.c: static int fetch_one(struct remote *remote, int argc, const char **argv,
+ return exit_code;
+ }
+
++static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
++{
++ enum display_format *format = opt->value;
++ *format = DISPLAY_FORMAT_PORCELAIN;
++ return 0;
++}
++
+ int cmd_fetch(int argc, const char **argv, const char *prefix)
+ {
+ const char *bundle_uri;
+@@ builtin/fetch.c: int cmd_fetch(int argc, const char **argv, const char *prefix)
+ PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
+ OPT_BOOL(0, "dry-run", &dry_run,
+ N_("dry run")),
++ OPT_CALLBACK_F(0, "porcelain", &display_format, NULL, N_("machine-readable output"),
++ PARSE_OPT_NOARG|PARSE_OPT_NONEG, opt_parse_porcelain),
+ OPT_BOOL(0, "write-fetch-head", &write_fetch_head,
+ N_("write fetched references to the FETCH_HEAD file")),
+ OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")),
+@@ builtin/fetch.c: int cmd_fetch(int argc, const char **argv, const char *prefix)
+ fetch_config_from_gitmodules(sfjc, rs);
+ }
+
++ if (display_format == DISPLAY_FORMAT_PORCELAIN) {
++ switch (recurse_submodules_cli) {
++ case RECURSE_SUBMODULES_OFF:
++ case RECURSE_SUBMODULES_DEFAULT:
++ /*
++ * Reference updates in submodules would be ambiguous
++ * in porcelain mode, so we reject this combination.
++ */
++ recurse_submodules = RECURSE_SUBMODULES_OFF;
++ break;
++
++ default:
++ die(_("options '%s' and '%s' cannot be used together"),
++ "--porcelain", "--recurse-submodules");
++ }
++ }
++
+ if (negotiate_only && !negotiation_tip.nr)
+ die(_("--negotiate-only needs one or more --negotiation-tip=*"));
+
+@@ builtin/fetch.c: int cmd_fetch(int argc, const char **argv, const char *prefix)
+ max_children = fetch_parallel_config;
+
+ /* TODO should this also die if we have a previous partial-clone? */
+- result = fetch_multiple(&list, max_children);
++ result = fetch_multiple(&list, max_children, display_format);
+ }
+
+
+@@ builtin/fetch.c: int cmd_fetch(int argc, const char **argv, const char *prefix)
+ if (max_children < 0)
+ max_children = fetch_parallel_config;
+
+- add_options_to_argv(&options);
++ add_options_to_argv(&options, display_format);
+ result = fetch_submodules(the_repository,
+ &options,
+ submodule_prefix,
## t/t5574-fetch-output.sh ##
-@@ t/t5574-fetch-output.sh: test_expect_success 'fetch compact output with multiple remotes' '
+@@ t/t5574-fetch-output.sh: test_expect_success 'fetch compact output' '
test_cmp expect actual
'
+test_expect_success 'fetch porcelain output' '
-+ test_when_finished "rm -rf porcelain-cfg porcelain-cli" &&
++ test_when_finished "rm -rf porcelain" &&
+
+ # Set up a bunch of references that we can use to demonstrate different
+ # kinds of flag symbols in the output format.
@@ t/t5574-fetch-output.sh: test_expect_success 'fetch compact output with multiple
+ # namespaces so that we can test that rejected and force-updated
+ # references are reported properly.
+ refspecs="refs/heads/*:refs/unforced/* +refs/heads/*:refs/forced/*" &&
-+ git clone . porcelain-cli &&
-+ git clone . porcelain-cfg &&
-+ git -C porcelain-cfg fetch origin $refspecs &&
-+ git -C porcelain-cli fetch origin $refspecs &&
++ git clone . porcelain &&
++ git -C porcelain fetch origin $refspecs &&
+
+ # Now that we have set up the client repositories we can change our
+ # local references.
@@ t/t5574-fetch-output.sh: test_expect_success 'fetch compact output with multiple
+ # Execute a dry-run fetch first. We do this to assert that the dry-run
+ # and non-dry-run fetches produces the same output. Execution of the
+ # fetch is expected to fail as we have a rejected reference update.
-+ test_must_fail git -C porcelain-cfg -c fetch.output=porcelain fetch --dry-run --prune origin $refspecs >actual-dry-run-cfg &&
-+ test_must_fail git -C porcelain-cli fetch --output-format=porcelain --dry-run --prune origin $refspecs >actual-dry-run-cli &&
-+ test_cmp actual-dry-run-cfg actual-dry-run-cli &&
-+ test_cmp expect actual-dry-run-cfg &&
++ test_must_fail git -C porcelain fetch \
++ --porcelain --dry-run --prune origin $refspecs >actual &&
++ test_cmp expect actual &&
+
+ # And now we perform a non-dry-run fetch.
-+ test_must_fail git -C porcelain-cfg -c fetch.output=porcelain fetch --prune origin $refspecs >actual-cfg &&
-+ test_must_fail git -C porcelain-cli fetch --output-format=porcelain --prune origin $refspecs >actual-cli &&
-+ test_cmp actual-cfg actual-cli &&
-+ test_cmp expect actual-cfg &&
-+
-+ # Ensure that the dry-run and non-dry-run output matches.
-+ test_cmp actual-dry-run-cfg actual-cfg
++ test_must_fail git -C porcelain fetch \
++ --porcelain --prune origin $refspecs >actual 2>stderr &&
++ test_cmp expect actual &&
++ test_must_be_empty stderr
+'
+
- test_expect_success 'fetch output with HEAD and --dry-run' '
++test_expect_success 'fetch porcelain with multiple remotes' '
++ test_when_finished "rm -rf porcelain" &&
++
++ git clone . porcelain &&
++ git -C porcelain remote add second-remote "$PWD" &&
++ git -C porcelain fetch second-remote &&
++
++ test_commit --no-tag multi-commit &&
++ old_commit=$(git rev-parse HEAD~) &&
++ new_commit=$(git rev-parse HEAD) &&
++
++ cat >expect <<-EOF &&
++ $old_commit $new_commit refs/remotes/origin/force-updated
++ $old_commit $new_commit refs/remotes/second-remote/force-updated
++ EOF
++
++ git -C porcelain fetch --porcelain --all >actual 2>stderr &&
++ test_cmp expect actual &&
++ test_must_be_empty stderr
++'
++
++test_expect_success 'fetch porcelain refuses to work with submodules' '
++ test_when_finished "rm -rf porcelain" &&
++
++ cat >expect <<-EOF &&
++ fatal: options ${SQ}--porcelain${SQ} and ${SQ}--recurse-submodules${SQ} cannot be used together
++ EOF
++
++ git init porcelain &&
++ test_must_fail git -C porcelain fetch --porcelain --recurse-submodules=yes 2>stderr &&
++ test_cmp expect stderr &&
++
++ test_must_fail git -C porcelain fetch --porcelain --recurse-submodules=on-demand 2>stderr &&
++ test_cmp expect stderr
++'
++
+ test_expect_success 'fetch output with HEAD' '
test_when_finished "rm -rf head" &&
git clone . head &&
-@@ t/t5574-fetch-output.sh: test_expect_success 'fetch output with HEAD and --dry-run' '
- test_cmp expect actual
+@@ t/t5574-fetch-output.sh: test_expect_success 'fetch output with HEAD' '
+ test_cmp expect actual.err
'
-+test_expect_success 'fetch porcelain output with HEAD and --dry-run' '
++test_expect_success 'fetch porcelain output with HEAD' '
+ 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 &&
++ git -C head fetch --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 &&
++ git -C head fetch --porcelain origin HEAD >actual &&
++ test_cmp expect actual &&
++
++ git -C head fetch --porcelain --dry-run origin HEAD:foo >actual &&
+ cat >expect <<-EOF &&
+ * $ZERO_OID $COMMIT_ID refs/heads/foo
+ EOF
++ test_cmp expect actual &&
++
++ git -C head fetch --porcelain origin HEAD:foo >actual &&
+ test_cmp expect actual
+'
+-- 2.40.1