From 52a6786aba31426796387fddea45b86a5400a50c Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:44 +0200 Subject: [PATCH 01/12] parse-options: introduce OPT_HIDDEN_GROUP Hidden options are not shown by `git -h`, but are still shown by `git --help-all`. If there are a lot of hidden options or if they don't belong to the same categories as other options, there is currently no way to properly group them. Using `OPT_GROUP("Foo")` means that "Foo" will always be shown which we don't want if that group contains only hidden options. To provide a way to have groups shown only when hidden options are shown, let's implement an OPT_HIDDEN_GROUP macro. To test this new macro, let's also improve `test-tool parse-options` and test its output with `--help-all`. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- parse-options.c | 4 ++-- parse-options.h | 5 +++++ t/helper/test-parse-options.c | 4 ++++ t/t0040-parse-options.sh | 25 ++++++++++++++++++++++++- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/parse-options.c b/parse-options.c index 08c21d9fc0..4519ead9dc 100644 --- a/parse-options.c +++ b/parse-options.c @@ -1414,6 +1414,8 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t if (opts->type == OPTION_SUBCOMMAND) continue; + if (!full && (opts->flags & PARSE_OPT_HIDDEN)) + continue; if (opts->type == OPTION_GROUP) { fputc('\n', outfile); need_newline = 0; @@ -1421,8 +1423,6 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t fprintf(outfile, "%s\n", _(opts->help)); continue; } - if (!full && (opts->flags & PARSE_OPT_HIDDEN)) - continue; if (need_newline) { fputc('\n', outfile); diff --git a/parse-options.h b/parse-options.h index 3ec8ba5cc8..d7f896a933 100644 --- a/parse-options.h +++ b/parse-options.h @@ -237,6 +237,11 @@ struct option { .type = OPTION_GROUP, \ .help = (h), \ } +#define OPT_HIDDEN_GROUP(h) { \ + .type = OPTION_GROUP, \ + .help = (h), \ + .flags = PARSE_OPT_HIDDEN, \ +} #define OPT_BIT(s, l, v, h, b) OPT_BIT_F(s, l, v, h, b, 0) #define OPT_BITOP(s, l, v, h, set, clear) { \ .type = OPTION_BITOP, \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 68579d83f3..f181f0c02d 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -209,6 +209,10 @@ int cmd__parse_options(int argc, const char **argv) OPT_GROUP("Alias"), OPT_STRING('A', "alias-source", &string, "string", "get a string"), OPT_ALIAS('Z', "alias-target", "alias-source"), + OPT_HIDDEN_GROUP("Hidden options"), + OPT_HIDDEN_BOOL(0, "hidden-bool", &boolean, "get a boolean"), + OPT_INTEGER_F('k', "hidden-integer", &integer, "get a integer", + PARSE_OPT_HIDDEN), OPT_END(), }; int ret = 0; diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index a22533f9ed..449fff4d34 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -7,7 +7,7 @@ test_description='our own option parser' . ./test-lib.sh -cat >expect <<\EOF +cat >expect-part1 <<\EOF usage: test-tool parse-options A helper function for the parse-options API. @@ -41,6 +41,9 @@ String options --[no-]string2 get another string --[no-]st get another string (pervert ordering) -o get another string +EOF + +cat >expect-part2 <<\EOF --longhelp help text of this entry spans multiple lines --[no-]list add str to list @@ -67,12 +70,32 @@ Alias EOF +cat >expect-noop <<\EOF + --[no-]obsolete no-op (backward compatibility) +EOF + +cat >expect-hidden <<\EOF +Hidden options + --[no-]hidden-bool get a boolean + -k, --[no-]hidden-integer + get a integer + +EOF + test_expect_success 'test help' ' + cat expect-part1 expect-part2 >expect && test-tool parse-options -h >output 2>output.err && test_must_be_empty output.err && test_cmp expect output ' +test_expect_success 'test --help-all shows hidden group and options' ' + cat expect-part1 expect-noop expect-part2 expect-hidden >expect-help-all && + test-tool parse-options --help-all >output 2>output.err && + test_must_be_empty output.err && + test_cmp expect-help-all output +' + mv expect expect.err check () { From 9227b76d45e4974a31f6bc412e223d811dd1a53e Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:45 +0200 Subject: [PATCH 02/12] api-parse-options.adoc: document per-option flags The "Flags" section in "Documentation/technical/api-parse-options.adoc" documents the flags that can be passed to parse_options() itself. It does not, however, document the flags that can be set on individual options through the `flags` member of `struct option` (and through the `OPT_*_F()` macro variants). These per-option flags are used throughout the codebase (for example `PARSE_OPT_HIDDEN` is used to hide an option from `-h` while still showing it with `--help-all`), but a reader currently has to dig into "parse-options.h" to find them. To remediate that, let's add an "Option flags" subsection to the "Data Structure" section, just before the list of option macros. Let's also make it explicit that these are distinct from the parse_options() flags described earlier, and let's describe the `-h` versus `--help-all` behavior for `PARSE_OPT_HIDDEN`. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- .../technical/api-parse-options.adoc | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/Documentation/technical/api-parse-options.adoc b/Documentation/technical/api-parse-options.adoc index 880eb94642..fb4580e755 100644 --- a/Documentation/technical/api-parse-options.adoc +++ b/Documentation/technical/api-parse-options.adoc @@ -150,6 +150,67 @@ Data Structure The main data structure is an array of the `option` struct, say `static struct option builtin_add_options[]`. + +Option flags +~~~~~~~~~~~~ + +Each option can carry flags in the `flags` field of its `option` +struct. These are per-option flags and are distinct from the +`parse_options()` flags described above; they are usually set through +the `OPT_*_F()` macro variants (see below) rather than by hand. They +are the bitwise-or of: + +`PARSE_OPT_OPTARG`:: + The option's argument is optional, i.e. both `--option` and + `--option=` are accepted. + +`PARSE_OPT_NOARG`:: + The option takes no argument at all. Using `--option=` + is rejected. + +`PARSE_OPT_NONEG`:: + Disable the automatically generated negated `--no-option` + form. + +`PARSE_OPT_HIDDEN`:: + Hide the option: it is omitted from the usage shown by + `git -h`, but is still shown by `git --help-all`. + The option is parsed as usual either way. This is meant for + deprecated, advanced or otherwise uncommon options. + +`PARSE_OPT_LASTARG_DEFAULT`:: + Use the default value (`defval`) when the option is used + without an argument, even for an option that normally requires + one. Only the last argument on the command line takes effect. + +`PARSE_OPT_NODASH`:: + The option is a single character without a leading dash, such + as the `+` used by some commands. + +`PARSE_OPT_LITERAL_ARGHELP`:: + Use the argument help string (`argh`) verbatim in the usage + output instead of surrounding it with `<>` or `[]`. Useful when + `argh` already contains a hand-formatted description. + +`PARSE_OPT_FROM_ALIAS`:: + Internal flag, set on options that were expanded from a + configured alias. It should not be set by callers. + +`PARSE_OPT_NOCOMPLETE`:: + Do not offer this option for completion. + +`PARSE_OPT_COMP_ARG`:: + The option's argument, rather than the option itself, is what + should be completed. + +`PARSE_OPT_CMDMODE`:: + The option is one of several mutually exclusive "command mode" + options that share the same variable. Using more than one of + them at once is rejected. + +Macros +~~~~~~ + There are some macros to easily define options: `OPT__ABBREV(&int_var)`:: From 11d1ca9771c42c0b97b6d6ce0f1a6d191f8d5738 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:46 +0200 Subject: [PATCH 03/12] api-parse-options.adoc: document hidden and OPT_*_F option macros In "Documentation/technical/api-parse-options.adoc", the list of option macros does not mention the `OPT_*_F()` macro variants that take a trailing `flags` argument, nor the `OPT_HIDDEN_GROUP()` and `OPT_HIDDEN_BOOL()` convenience macros. Now that a previous commit documents the per-option flags, let's document these macros too: - Add a paragraph explaining the `OPT_*_F` convention and how it relates to the per-option flags. - Document `OPT_HIDDEN_GROUP()`, introduced in a previous commit, right after `OPT_GROUP()`. - Document `OPT_HIDDEN_BOOL()` right after `OPT_BOOL()`. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- Documentation/technical/api-parse-options.adoc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Documentation/technical/api-parse-options.adoc b/Documentation/technical/api-parse-options.adoc index fb4580e755..0e10327d07 100644 --- a/Documentation/technical/api-parse-options.adoc +++ b/Documentation/technical/api-parse-options.adoc @@ -213,6 +213,13 @@ Macros There are some macros to easily define options: +Many of the macros below have an `_F` variant (for example `OPT_BOOL_F`, +`OPT_STRING_F`, `OPT_INTEGER_F`, `OPT_SET_INT_F`, `OPT_BIT_F` and +`OPT_CALLBACK_F`) that takes an additional trailing `flags` argument. +That argument is the bitwise-or of the per-option flags described in the +"Option flags" section above; the non-`_F` macros are simply defined +with `flags` set to `0`. + `OPT__ABBREV(&int_var)`:: Add `--abbrev[=]`. @@ -236,10 +243,21 @@ There are some macros to easily define options: describes the group or an empty string. Start the description with an upper-case letter. +`OPT_HIDDEN_GROUP(description)`:: + Like `OPT_GROUP()`, but the group header carries + `PARSE_OPT_HIDDEN`, so it is only shown by `--help-all` and not + by `-h`. Use it to label a group that contains only hidden + options, which would otherwise show an empty header under `-h`. + `OPT_BOOL(short, long, &int_var, description)`:: Introduce a boolean option. `int_var` is set to one with `--option` and set to zero with `--no-option`. +`OPT_HIDDEN_BOOL(short, long, &int_var, description)`:: + Like `OPT_BOOL()`, but the option carries `PARSE_OPT_HIDDEN`, + so it is hidden from `-h` while still being shown by + `--help-all`. + `OPT_COUNTUP(short, long, &int_var, description)`:: Introduce a count-up option. Each use of `--option` increments `int_var`, starting from zero From c5a95749d89e21da934651a938ff22f49ab38bb4 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:47 +0200 Subject: [PATCH 04/12] fast-import: localize 'i' into the 'for' loops using it In cmd_fast_import(), a local variable 'i' is defined as an `unsigned int` and then used as a loop counter in four different `for (i = ...; i < ...; i++) { ... }` loops. But in three out of the four cases, `unsigned int` isn't the best type to use. To give each loop counter the type matching its bound (int/unsigned/size_t), let's localize 'i' into each loop that uses it. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 6692f7cd81..9fc9ebe65a 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3937,8 +3937,6 @@ int cmd_fast_import(int argc, const char *prefix, struct repository *repo) { - unsigned int i; - show_usage_if_asked(argc, argv, fast_import_usage); reset_pack_idx_option(&pack_idx_opts); @@ -3959,7 +3957,7 @@ int cmd_fast_import(int argc, * line to override stream data). But we must do an early parse of any * command-line options that impact how we interpret the feature lines. */ - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) { const char *arg = argv[i]; if (*arg != '-' || !strcmp(arg, "--")) break; @@ -3972,7 +3970,7 @@ int cmd_fast_import(int argc, global_prefix = prefix; rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free)); - for (i = 0; i < (cmd_save - 1); i++) + for (unsigned int i = 0; i < (cmd_save - 1); i++) rc_free[i].next = &rc_free[i + 1]; rc_free[cmd_save - 1].next = NULL; @@ -4035,9 +4033,9 @@ int cmd_fast_import(int argc, if (show_stats) { uintmax_t total_count = 0, duplicate_count = 0; - for (i = 0; i < ARRAY_SIZE(object_count_by_type); i++) + for (size_t i = 0; i < ARRAY_SIZE(object_count_by_type); i++) total_count += object_count_by_type[i]; - for (i = 0; i < ARRAY_SIZE(duplicate_count_by_type); i++) + for (size_t i = 0; i < ARRAY_SIZE(duplicate_count_by_type); i++) duplicate_count += duplicate_count_by_type[i]; fprintf(stderr, "%s statistics:\n", argv[0]); From 4c89f21a5006039d2c43a8c139876738d0c998c1 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:48 +0200 Subject: [PATCH 05/12] fast-import: use int for some bool flags The `show_stats` and `quiet` flags are meant to be parsed and used as boolean flags. To easily parse them using OPT_BOOL in a following commit, let's change their type from 'unsigned int' to just 'int'. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 9fc9ebe65a..9c8edd7c89 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -182,8 +182,8 @@ static unsigned long branch_count; static unsigned long branch_load_count; static int failure; static FILE *pack_edges; -static unsigned int show_stats = 1; -static unsigned int quiet; +static int show_stats = 1; +static int quiet; static int global_argc; static const char **global_argv; static const char *global_prefix; From 5442c3f445131eaf43cf42c13e5ecf17abcf4b19 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:49 +0200 Subject: [PATCH 06/12] fast-import: factor out option_*() functions In a following commit we are going to use the parse-options API to start parsing options. Some options will have to be parsed using OPT_CALLBACK as they process their arguments in special ways. When the processing code is already factored out in an option_*() function, like for `--date-format`, we can reuse that function. Unfortunately for other options the processing code has not been factored out yet. Let's do it now and factor out the code that handles the following options: - `--max-pack-size=` - `--big-file-threshold=` - `--signed-commits=` - `--signed-tags=` - `--quiet` into new option_*() functions: - option_max_pack_size() - option_big_file_threshold() - option_signed_commits() - option_signed_tags() - option_quiet() so that we can reuse these functions in following commits when the parse-option API will be used. Note that there are some behavior changes as we now die() with a proper error message when git_parse_ulong() cannot parse the argument from --max-pack-size or from --big-file-threshold. Previously we would end up calling die("unknown option") instead. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 69 ++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 9c8edd7c89..a6e3cc0033 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3751,25 +3751,55 @@ static void option_rewrite_submodules(const char *arg, struct string_list *list) free(s); } +static void option_max_pack_size(const char *arg) +{ + unsigned long v; + + if (!git_parse_ulong(arg, &v)) + die(_("--max-pack-size: argument must be a non-negative integer")); + if (v < 8192) { + warning(_("max-pack-size is now in bytes, assuming --max-pack-size=%lum"), v); + v *= 1024 * 1024; + } else if (v < 1024 * 1024) { + warning(_("minimum max-pack-size is 1 MiB")); + v = 1024 * 1024; + } + max_packsize = v; +} + +static void option_big_file_threshold(const char *arg) +{ + unsigned long v; + + if (!git_parse_ulong(arg, &v)) + die(_("--big-file-threshold: argument must be a non-negative integer")); + repo_settings_set_big_file_threshold(the_repository, v); +} + +static void option_signed_commits(const char *arg) +{ + if (parse_sign_mode(arg, &signed_commit_mode, &signed_commit_keyid)) + usagef(_("unknown --signed-commits mode '%s'"), arg); +} + +static void option_signed_tags(const char *arg) +{ + if (parse_sign_mode(arg, &signed_tag_mode, &signed_tag_keyid)) + usagef(_("unknown --signed-tags mode '%s'"), arg); +} + +static void option_quiet(void) +{ + show_stats = 0; + quiet = 1; +} + static int parse_one_option(const char *option) { if (skip_prefix(option, "max-pack-size=", &option)) { - unsigned long v; - if (!git_parse_ulong(option, &v)) - return 0; - if (v < 8192) { - warning(_("max-pack-size is now in bytes, assuming --max-pack-size=%lum"), v); - v *= 1024 * 1024; - } else if (v < 1024 * 1024) { - warning(_("minimum max-pack-size is 1 MiB")); - v = 1024 * 1024; - } - max_packsize = v; + option_max_pack_size(option); } else if (skip_prefix(option, "big-file-threshold=", &option)) { - unsigned long v; - if (!git_parse_ulong(option, &v)) - return 0; - repo_settings_set_big_file_threshold(the_repository, v); + option_big_file_threshold(option); } else if (skip_prefix(option, "depth=", &option)) { option_depth(option); } else if (skip_prefix(option, "active-branches=", &option)) { @@ -3777,14 +3807,11 @@ static int parse_one_option(const char *option) } else if (skip_prefix(option, "export-pack-edges=", &option)) { option_export_pack_edges(option); } else if (skip_prefix(option, "signed-commits=", &option)) { - if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid)) - usagef(_("unknown --signed-commits mode '%s'"), option); + option_signed_commits(option); } else if (skip_prefix(option, "signed-tags=", &option)) { - if (parse_sign_mode(option, &signed_tag_mode, &signed_tag_keyid)) - usagef(_("unknown --signed-tags mode '%s'"), option); + option_signed_tags(option); } else if (!strcmp(option, "quiet")) { - show_stats = 0; - quiet = 1; + option_quiet(); } else if (!strcmp(option, "stats")) { show_stats = 1; } else if (!strcmp(option, "allow-unsafe-features")) { From db106bcb493c017497b5574fcedf5d89cac18531 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:50 +0200 Subject: [PATCH 07/12] fast-import: introduce 'struct fast_import_state' "builtin/fast-import.c" uses a large number of global variables. This makes it harder than necessary to reason about and improve. Especially adding new features requires adding more global variables, while modernizing and eventually libifying the code becomes more and more difficult. To start reverting the sad trend to more and more globals and to start cleaning things up, let's introduce a 'struct fast_import_state' and pass an instance of it as the first argument to many functions. This is similar to what was done for "builtin/apply.c" by introducing a 'struct apply_state', see 07d7e290ff (apply: move 'struct apply_state' to a header file, 2016-08-11) and related commits. As a first step only the 'global_argc', 'global_argv' and 'global_prefix' variables are moved into the new struct. More variables will be moved into it in the following commits. Some functions receive the new 'state' parameter only to pass it along or for future use, so they are marked with UNUSED for now to satisfy '-Werror=unused-parameter'. This is a mostly mechanical refactoring with no intended behavior change. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 262 ++++++++++++++++++++++-------------------- 1 file changed, 138 insertions(+), 124 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index a6e3cc0033..cb0c856e41 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -184,10 +184,6 @@ static int failure; static FILE *pack_edges; static int show_stats = 1; static int quiet; -static int global_argc; -static const char **global_argv; -static const char *global_prefix; - static enum sign_mode signed_tag_mode = SIGN_VERBATIM; static enum sign_mode signed_commit_mode = SIGN_VERBATIM; static const char *signed_commit_keyid; @@ -276,10 +272,27 @@ static kh_oid_map_t *sub_oid_map; /* Where to write output of cat-blob commands */ static int cat_blob_fd = STDOUT_FILENO; -static void parse_argv(void); -static void parse_get_mark(const char *p); -static void parse_cat_blob(const char *p); -static void parse_ls(const char *p, struct branch *b); +/* Command state */ +struct fast_import_state { + int argc; + const char **argv; + const char *prefix; +}; + +static void fast_import_state_init(struct fast_import_state *state, + int argc, const char **argv, + const char *prefix) +{ + memset(state, 0, sizeof(*state)); + state->argc = argc; + state->argv = argv; + state->prefix = prefix; +} + +static void parse_argv(struct fast_import_state *state); +static void parse_get_mark(struct fast_import_state *state, const char *p); +static void parse_cat_blob(struct fast_import_state *state, const char *p); +static void parse_ls(struct fast_import_state *state, const char *p, struct branch *b); static void for_each_mark(struct mark_set *m, uintmax_t base, each_mark_fn_t callback, void *p) { @@ -1845,7 +1858,7 @@ done: } -static int read_next_command(void) +static int read_next_command(struct fast_import_state *state) { static int stdin_eof = 0; @@ -1867,7 +1880,7 @@ static int read_next_command(void) if (!seen_data_command && !starts_with(command_buf.buf, "feature ") && !starts_with(command_buf.buf, "option ")) { - parse_argv(); + parse_argv(state); } rc = rc_free; @@ -1899,22 +1912,22 @@ static void skip_optional_lf(void) ungetc(term_char, stdin); } -static void parse_mark(void) +static void parse_mark(struct fast_import_state *state) { const char *v; if (skip_prefix(command_buf.buf, "mark :", &v)) { next_mark = strtoumax(v, NULL, 10); - read_next_command(); + read_next_command(state); } else next_mark = 0; } -static void parse_original_identifier(void) +static void parse_original_identifier(struct fast_import_state *state) { const char *v; if (skip_prefix(command_buf.buf, "original-oid ", &v)) - read_next_command(); + read_next_command(state); } static int parse_data(struct strbuf *sb, uintmax_t limit, uintmax_t *len_res) @@ -2068,11 +2081,11 @@ static void parse_and_store_blob( } } -static void parse_new_blob(void) +static void parse_new_blob(struct fast_import_state *state) { - read_next_command(); - parse_mark(); - parse_original_identifier(); + read_next_command(state); + parse_mark(state); + parse_original_identifier(state); parse_and_store_blob(&last_blob, NULL, next_mark); } @@ -2368,7 +2381,7 @@ static void parse_path_space(struct strbuf *sb, const char *p, (*endp)++; } -static void file_change_m(const char *p, struct branch *b) +static void file_change_m(struct fast_import_state *state, const char *p, struct branch *b) { static struct strbuf path = STRBUF_INIT; struct object_entry *oe; @@ -2435,10 +2448,10 @@ static void file_change_m(const char *p, struct branch *b) if (S_ISDIR(mode)) die(_("directories cannot be specified 'inline': %s"), command_buf.buf); - while (read_next_command() != EOF) { + while (read_next_command(state) != EOF) { const char *v; if (skip_prefix(command_buf.buf, "cat-blob ", &v)) - parse_cat_blob(v); + parse_cat_blob(state, v); else { parse_and_store_blob(&last_blob, &oid, 0); break; @@ -2512,7 +2525,7 @@ static void file_change_cr(const char *p, struct branch *b, int rename) leaf.tree); } -static void note_change_n(const char *p, struct branch *b, unsigned char *old_fanout) +static void note_change_n(struct fast_import_state *state, const char *p, struct branch *b, unsigned char *old_fanout) { struct object_entry *oe; struct branch *s; @@ -2577,7 +2590,7 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa die(_("invalid ref name or SHA1 expression: %s"), p); if (inline_data) { - read_next_command(); + read_next_command(state); parse_and_store_blob(&last_blob, &oid, 0); } else if (oe) { if (oe->type != OBJ_BLOB) @@ -2644,7 +2657,7 @@ static void parse_from_existing(struct branch *b) } } -static int parse_objectish(struct branch *b, const char *objectish) +static int parse_objectish(struct fast_import_state *state, struct branch *b, const char *objectish) { struct branch *s; struct object_id oid; @@ -2687,31 +2700,31 @@ static int parse_objectish(struct branch *b, const char *objectish) b->branch_tree.tree = NULL; } - read_next_command(); + read_next_command(state); return 1; } -static int parse_from(struct branch *b) +static int parse_from(struct fast_import_state *state, struct branch *b) { const char *from; if (!skip_prefix(command_buf.buf, "from ", &from)) return 0; - return parse_objectish(b, from); + return parse_objectish(state, b, from); } -static int parse_objectish_with_prefix(struct branch *b, const char *prefix) +static int parse_objectish_with_prefix(struct fast_import_state *state, struct branch *b, const char *prefix) { const char *base; if (!skip_prefix(command_buf.buf, prefix, &base)) return 0; - return parse_objectish(b, base); + return parse_objectish(state, b, base); } -static struct hash_list *parse_merge(unsigned int *count) +static struct hash_list *parse_merge(struct fast_import_state *state, unsigned int *count) { struct hash_list *list = NULL, **tail = &list, *n; const char *from; @@ -2745,7 +2758,7 @@ static struct hash_list *parse_merge(unsigned int *count) tail = &n->next; (*count)++; - read_next_command(); + read_next_command(state); } return list; } @@ -2756,7 +2769,7 @@ struct signature_data { struct strbuf data; /* The actual signature data */ }; -static void parse_one_signature(struct signature_data *sig, const char *v) +static void parse_one_signature(struct fast_import_state *state, struct signature_data *sig, const char *v) { char *args = xstrdup(v); /* Will be freed when sig->hash_algo is freed */ char *space = strchr(args, ' '); @@ -2781,15 +2794,15 @@ static void parse_one_signature(struct signature_data *sig, const char *v) warning(_("'unknown' signature format in gpgsig")); /* Read signature data */ - read_next_command(); + read_next_command(state); parse_data(&sig->data, 0, NULL); } -static void discard_one_signature(void) +static void discard_one_signature(struct fast_import_state *state) { struct strbuf data = STRBUF_INIT; - read_next_command(); + read_next_command(state); parse_data(&data, 0, NULL); strbuf_release(&data); } @@ -2827,13 +2840,14 @@ static void store_signature(struct signature_data *stored_sig, } } -static void import_one_signature(struct signature_data *sig_sha1, +static void import_one_signature(struct fast_import_state *state, + struct signature_data *sig_sha1, struct signature_data *sig_sha256, const char *v) { struct signature_data sig = { NULL, NULL, STRBUF_INIT }; - parse_one_signature(&sig, v); + parse_one_signature(state, &sig, v); if (!strcmp(sig.hash_algo, "sha1")) store_signature(sig_sha1, &sig, "SHA-1"); @@ -2947,7 +2961,7 @@ static void handle_signature_if_invalid(struct strbuf *new_data, strbuf_release(&tmp_buf); } -static void parse_new_commit(const char *arg) +static void parse_new_commit(struct fast_import_state *state, const char *arg) { static struct strbuf msg = STRBUF_INIT; struct signature_data sig_sha1 = { NULL, NULL, STRBUF_INIT }; @@ -2965,16 +2979,16 @@ static void parse_new_commit(const char *arg) if (!b) b = new_branch(arg); - read_next_command(); - parse_mark(); - parse_original_identifier(); + read_next_command(state); + parse_mark(state); + parse_original_identifier(state); if (skip_prefix(command_buf.buf, "author ", &v)) { author = parse_ident(v); - read_next_command(); + read_next_command(state); } if (skip_prefix(command_buf.buf, "committer ", &v)) { committer = parse_ident(v); - read_next_command(); + read_next_command(state); } if (!committer) die(_("expected committer but didn't get one")); @@ -2990,7 +3004,7 @@ static void parse_new_commit(const char *arg) warning(_("stripping a commit signature")); /* fallthru */ case SIGN_STRIP: - discard_one_signature(); + discard_one_signature(state); break; /* Second, modes that parse the signature */ @@ -3001,24 +3015,24 @@ static void parse_new_commit(const char *arg) case SIGN_STRIP_IF_INVALID: case SIGN_SIGN_IF_INVALID: case SIGN_ABORT_IF_INVALID: - import_one_signature(&sig_sha1, &sig_sha256, v); + import_one_signature(state, &sig_sha1, &sig_sha256, v); break; /* Third, BUG */ default: BUG("invalid signed_commit_mode value %d", signed_commit_mode); } - read_next_command(); + read_next_command(state); } if (skip_prefix(command_buf.buf, "encoding ", &v)) { encoding = xstrdup(v); - read_next_command(); + read_next_command(state); } parse_data(&msg, 0, NULL); - read_next_command(); - parse_from(b); - merge_list = parse_merge(&merge_count); + read_next_command(state); + parse_from(state, b); + merge_list = parse_merge(state, &merge_count); /* ensure the branch is active/loaded */ if (!b->branch_tree.tree || !max_active_branches) { @@ -3031,7 +3045,7 @@ static void parse_new_commit(const char *arg) /* file_change* */ while (command_buf.len > 0) { if (skip_prefix(command_buf.buf, "M ", &v)) - file_change_m(v, b); + file_change_m(state, v, b); else if (skip_prefix(command_buf.buf, "D ", &v)) file_change_d(v, b); else if (skip_prefix(command_buf.buf, "R ", &v)) @@ -3039,18 +3053,18 @@ static void parse_new_commit(const char *arg) else if (skip_prefix(command_buf.buf, "C ", &v)) file_change_cr(v, b, 0); else if (skip_prefix(command_buf.buf, "N ", &v)) - note_change_n(v, b, &prev_fanout); + note_change_n(state, v, b, &prev_fanout); else if (!strcmp("deleteall", command_buf.buf)) file_change_deleteall(b); else if (skip_prefix(command_buf.buf, "ls ", &v)) - parse_ls(v, b); + parse_ls(state, v, b); else if (skip_prefix(command_buf.buf, "cat-blob ", &v)) - parse_cat_blob(v); + parse_cat_blob(state, v); else { unread_command_buf = 1; break; } - if (read_next_command() == EOF) + if (read_next_command(state) == EOF) break; } @@ -3188,7 +3202,7 @@ static void handle_tag_signature(struct strbuf *buf, struct strbuf *msg, const c } } -static void parse_new_tag(const char *arg) +static void parse_new_tag(struct fast_import_state *state, const char *arg) { static struct strbuf msg = STRBUF_INIT; const char *from; @@ -3207,8 +3221,8 @@ static void parse_new_tag(const char *arg) else first_tag = t; last_tag = t; - read_next_command(); - parse_mark(); + read_next_command(state); + parse_mark(state); /* from ... */ if (!skip_prefix(command_buf.buf, "from ", &from)) @@ -3236,15 +3250,15 @@ static void parse_new_tag(const char *arg) type = oe->type; } else die(_("invalid ref name or SHA1 expression: %s"), from); - read_next_command(); + read_next_command(state); /* original-oid ... */ - parse_original_identifier(); + parse_original_identifier(state); /* tagger ... */ if (skip_prefix(command_buf.buf, "tagger ", &v)) { tagger = parse_ident(v); - read_next_command(); + read_next_command(state); } else tagger = NULL; @@ -3275,7 +3289,7 @@ static void parse_new_tag(const char *arg) t->pack_id = pack_id; } -static void parse_reset_branch(const char *arg) +static void parse_reset_branch(struct fast_import_state *state, const char *arg) { struct branch *b; const char *tag_name; @@ -3292,8 +3306,8 @@ static void parse_reset_branch(const char *arg) } else b = new_branch(arg); - read_next_command(); - parse_from(b); + read_next_command(state); + parse_from(state, b); if (b->delete && skip_prefix(b->name, "refs/tags/", &tag_name)) { /* * Elsewhere, we call dump_branches() before dump_tags(), @@ -3378,7 +3392,7 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid) free(buf); } -static void parse_get_mark(const char *p) +static void parse_get_mark(struct fast_import_state *state UNUSED, const char *p) { struct object_entry *oe; char output[GIT_MAX_HEXSZ + 2]; @@ -3395,7 +3409,7 @@ static void parse_get_mark(const char *p) cat_blob_write(output, the_hash_algo->hexsz + 1); } -static void parse_cat_blob(const char *p) +static void parse_cat_blob(struct fast_import_state *state UNUSED, const char *p) { struct object_entry *oe; struct object_id oid; @@ -3560,7 +3574,7 @@ static void print_ls(int mode, const unsigned char *hash, const char *path) cat_blob_write(line.buf, line.len); } -static void parse_ls(const char *p, struct branch *b) +static void parse_ls(struct fast_import_state *state UNUSED, const char *p, struct branch *b) { static struct strbuf path = STRBUF_INIT; struct tree_entry *root = NULL; @@ -3607,13 +3621,13 @@ static void checkpoint(void) dump_marks(); } -static void parse_checkpoint(void) +static void parse_checkpoint(struct fast_import_state *state UNUSED) { checkpoint_requested = 1; skip_optional_lf(); } -static void parse_progress(void) +static void parse_progress(struct fast_import_state *state UNUSED) { fwrite(command_buf.buf, 1, command_buf.len, stdout); fputc('\n', stdout); @@ -3621,36 +3635,36 @@ static void parse_progress(void) skip_optional_lf(); } -static void parse_alias(void) +static void parse_alias(struct fast_import_state *state) { struct object_entry *e; struct branch b; skip_optional_lf(); - read_next_command(); + read_next_command(state); /* mark ... */ - parse_mark(); + parse_mark(state); if (!next_mark) die(_("expected 'mark' command, got %s"), command_buf.buf); /* to ... */ memset(&b, 0, sizeof(b)); - if (!parse_objectish_with_prefix(&b, "to ")) + if (!parse_objectish_with_prefix(state, &b, "to ")) die(_("expected 'to' command, got %s"), command_buf.buf); e = find_object(&b.oid); assert(e); insert_mark(&marks, next_mark, e); } -static char* make_fast_import_path(const char *path) +static char* make_fast_import_path(struct fast_import_state *state, const char *path) { if (!relative_marks_paths || is_absolute_path(path)) - return prefix_filename(global_prefix, path); + return prefix_filename(state->prefix, path); return repo_git_path(the_repository, "info/fast-import/%s", path); } -static void option_import_marks(const char *marks, +static void option_import_marks(struct fast_import_state *state, const char *marks, int from_stream, int ignore_missing) { if (import_marks_file) { @@ -3663,7 +3677,7 @@ static void option_import_marks(const char *marks, } free(import_marks_file); - import_marks_file = make_fast_import_path(marks); + import_marks_file = make_fast_import_path(state, marks); import_marks_file_from_stream = from_stream; import_marks_file_ignore_missing = ignore_missing; } @@ -3703,13 +3717,13 @@ static void option_active_branches(const char *branches) max_active_branches = ulong_arg("--active-branches", branches); } -static void option_export_marks(const char *marks) +static void option_export_marks(struct fast_import_state *state, const char *marks) { free(export_marks_file); - export_marks_file = make_fast_import_path(marks); + export_marks_file = make_fast_import_path(state, marks); } -static void option_cat_blob_fd(const char *fd) +static void option_cat_blob_fd(struct fast_import_state *state UNUSED, const char *fd) { unsigned long n = ulong_arg("--cat-blob-fd", fd); if (n > (unsigned long) INT_MAX) @@ -3717,16 +3731,16 @@ static void option_cat_blob_fd(const char *fd) cat_blob_fd = (int) n; } -static void option_export_pack_edges(const char *edges) +static void option_export_pack_edges(struct fast_import_state *state, const char *edges) { - char *fn = prefix_filename(global_prefix, edges); + char *fn = prefix_filename(state->prefix, edges); if (pack_edges) fclose(pack_edges); pack_edges = xfopen(fn, "a"); free(fn); } -static void option_rewrite_submodules(const char *arg, struct string_list *list) +static void option_rewrite_submodules(struct fast_import_state *state, const char *arg, struct string_list *list) { struct mark_set *ms; FILE *fp; @@ -3738,7 +3752,7 @@ static void option_rewrite_submodules(const char *arg, struct string_list *list) f++; CALLOC_ARRAY(ms, 1); - f = prefix_filename(global_prefix, f); + f = prefix_filename(state->prefix, f); fp = fopen(f, "r"); if (!fp) die_errno(_("cannot read '%s'"), f); @@ -3794,7 +3808,7 @@ static void option_quiet(void) quiet = 1; } -static int parse_one_option(const char *option) +static int parse_one_option(struct fast_import_state *state, const char *option) { if (skip_prefix(option, "max-pack-size=", &option)) { option_max_pack_size(option); @@ -3805,7 +3819,7 @@ static int parse_one_option(const char *option) } else if (skip_prefix(option, "active-branches=", &option)) { option_active_branches(option); } else if (skip_prefix(option, "export-pack-edges=", &option)) { - option_export_pack_edges(option); + option_export_pack_edges(state, option); } else if (skip_prefix(option, "signed-commits=", &option)) { option_signed_commits(option); } else if (skip_prefix(option, "signed-tags=", &option)) { @@ -3823,34 +3837,34 @@ static int parse_one_option(const char *option) return 1; } -static void check_unsafe_feature(const char *feature, int from_stream) +static void check_unsafe_feature(struct fast_import_state *state UNUSED, const char *feature, int from_stream) { if (from_stream && !allow_unsafe_features) die(_("feature '%s' forbidden in input without --allow-unsafe-features"), feature); } -static int parse_one_feature(const char *feature, int from_stream) +static int parse_one_feature(struct fast_import_state *state, const char *feature, int from_stream) { const char *arg; if (skip_prefix(feature, "date-format=", &arg)) { option_date_format(arg); } else if (skip_prefix(feature, "import-marks=", &arg)) { - check_unsafe_feature("import-marks", from_stream); - option_import_marks(arg, from_stream, 0); + check_unsafe_feature(state, "import-marks", from_stream); + option_import_marks(state, arg, from_stream, 0); } else if (skip_prefix(feature, "import-marks-if-exists=", &arg)) { - check_unsafe_feature("import-marks-if-exists", from_stream); - option_import_marks(arg, from_stream, 1); + check_unsafe_feature(state, "import-marks-if-exists", from_stream); + option_import_marks(state, arg, from_stream, 1); } else if (skip_prefix(feature, "export-marks=", &arg)) { - check_unsafe_feature(feature, from_stream); - option_export_marks(arg); + check_unsafe_feature(state, feature, from_stream); + option_export_marks(state, arg); } else if (!strcmp(feature, "alias")) { ; /* Don't die - this feature is supported */ } else if (skip_prefix(feature, "rewrite-submodules-to=", &arg)) { - option_rewrite_submodules(arg, &sub_marks_to); + option_rewrite_submodules(state, arg, &sub_marks_to); } else if (skip_prefix(feature, "rewrite-submodules-from=", &arg)) { - option_rewrite_submodules(arg, &sub_marks_from); + option_rewrite_submodules(state, arg, &sub_marks_from); } else if (!strcmp(feature, "get-mark")) { ; /* Don't die - this feature is supported */ } else if (!strcmp(feature, "cat-blob")) { @@ -3872,23 +3886,23 @@ static int parse_one_feature(const char *feature, int from_stream) return 1; } -static void parse_feature(const char *feature) +static void parse_feature(struct fast_import_state *state, const char *feature) { if (seen_data_command) die(_("got feature command '%s' after data command"), feature); - if (parse_one_feature(feature, 1)) + if (parse_one_feature(state, feature, 1)) return; die(_("this version of fast-import does not support feature %s."), feature); } -static void parse_option(const char *option) +static void parse_option(struct fast_import_state *state, const char *option) { if (seen_data_command) die(_("got option command '%s' after data command"), option); - if (parse_one_option(option)) + if (parse_one_option(state, option)) return; die(_("this version of fast-import does not support option: %s"), option); @@ -3924,12 +3938,12 @@ static void git_pack_config(void) static const char fast_import_usage[] = "git fast-import [--date-format=] [--max-pack-size=] [--big-file-threshold=] [--depth=] [--active-branches=] [--export-marks=]"; -static void parse_argv(void) +static void parse_argv(struct fast_import_state *state) { unsigned int i; - for (i = 1; i < global_argc; i++) { - const char *a = global_argv[i]; + for (i = 1; i < state->argc; i++) { + const char *a = state->argv[i]; if (*a != '-' || !strcmp(a, "--")) break; @@ -3937,20 +3951,20 @@ static void parse_argv(void) if (!skip_prefix(a, "--", &a)) die(_("unknown option %s"), a); - if (parse_one_option(a)) + if (parse_one_option(state, a)) continue; - if (parse_one_feature(a, 0)) + if (parse_one_feature(state, a, 0)) continue; if (skip_prefix(a, "cat-blob-fd=", &a)) { - option_cat_blob_fd(a); + option_cat_blob_fd(state, a); continue; } die(_("unknown option --%s"), a); } - if (i != global_argc) + if (i != state->argc) usage(fast_import_usage); seen_data_command = 1; @@ -3964,6 +3978,8 @@ int cmd_fast_import(int argc, const char *prefix, struct repository *repo) { + struct fast_import_state state; + show_usage_if_asked(argc, argv, fast_import_usage); reset_pack_idx_option(&pack_idx_opts); @@ -3992,9 +4008,7 @@ int cmd_fast_import(int argc, allow_unsafe_features = 1; } - global_argc = argc; - global_argv = argv; - global_prefix = prefix; + fast_import_state_init(&state, argc, argv, prefix); rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free)); for (unsigned int i = 0; i < (cmd_save - 1); i++) @@ -4004,34 +4018,34 @@ int cmd_fast_import(int argc, start_packfile(); set_die_routine(die_nicely); set_checkpoint_signal(); - while (read_next_command() != EOF) { + while (read_next_command(&state) != EOF) { const char *v; if (!strcmp("blob", command_buf.buf)) - parse_new_blob(); + parse_new_blob(&state); else if (skip_prefix(command_buf.buf, "commit ", &v)) - parse_new_commit(v); + parse_new_commit(&state, v); else if (skip_prefix(command_buf.buf, "tag ", &v)) - parse_new_tag(v); + parse_new_tag(&state, v); else if (skip_prefix(command_buf.buf, "reset ", &v)) - parse_reset_branch(v); + parse_reset_branch(&state, v); else if (skip_prefix(command_buf.buf, "ls ", &v)) - parse_ls(v, NULL); + parse_ls(&state, v, NULL); else if (skip_prefix(command_buf.buf, "cat-blob ", &v)) - parse_cat_blob(v); + parse_cat_blob(&state, v); else if (skip_prefix(command_buf.buf, "get-mark ", &v)) - parse_get_mark(v); + parse_get_mark(&state, v); else if (!strcmp("checkpoint", command_buf.buf)) - parse_checkpoint(); + parse_checkpoint(&state); else if (!strcmp("done", command_buf.buf)) break; else if (!strcmp("alias", command_buf.buf)) - parse_alias(); + parse_alias(&state); else if (starts_with(command_buf.buf, "progress ")) - parse_progress(); + parse_progress(&state); else if (skip_prefix(command_buf.buf, "feature ", &v)) - parse_feature(v); + parse_feature(&state, v); else if (skip_prefix(command_buf.buf, "option git ", &v)) - parse_option(v); + parse_option(&state, v); else if (starts_with(command_buf.buf, "option ")) /* ignore non-git options*/; else @@ -4043,7 +4057,7 @@ int cmd_fast_import(int argc, /* argv hasn't been parsed yet, do so */ if (!seen_data_command) - parse_argv(); + parse_argv(&state); if (require_explicit_termination && feof(stdin)) die(_("stream ends early")); From 9c89f2d58c76e9701176a9037749e47bea96a9b8 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:51 +0200 Subject: [PATCH 08/12] fast-import: move command state globals into 'struct fast_import_state' A previous commit introduced 'struct fast_import_state' to hold some command state, and reduce the need for global variables. Let's continue in the same direction and move two more global variables that describe the command state into it: 'seen_data_command' and 'allow_unsafe_features'. All the sites accessing these variables are already in functions that receive the 'state' parameter (or in cmd_fast_import() which owns the struct), so no additional threading is needed. As 'state->allow_unsafe_features' is now dereferenced in check_unsafe_feature(), its 'state' parameter is no longer unused, so the UNUSED marker is removed. The fast_import_state_init() call is moved up before the early command-line scan for '--allow-unsafe-features', so that this option can be recorded directly into the struct without being clobbered by the memset() in fast_import_state_init(). This is a mechanical refactoring with no intended behavior change. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index cb0c856e41..926afde954 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -257,9 +257,7 @@ static struct recent_command *rc_free; static unsigned int cmd_save = 100; static uintmax_t next_mark; static struct strbuf new_data = STRBUF_INIT; -static int seen_data_command; static int require_explicit_termination; -static int allow_unsafe_features; /* Signal handling */ static volatile sig_atomic_t checkpoint_requested; @@ -277,6 +275,8 @@ struct fast_import_state { int argc; const char **argv; const char *prefix; + int seen_data_command; + int allow_unsafe_features; }; static void fast_import_state_init(struct fast_import_state *state, @@ -1877,7 +1877,7 @@ static int read_next_command(struct fast_import_state *state) if (stdin_eof) return EOF; - if (!seen_data_command + if (!state->seen_data_command && !starts_with(command_buf.buf, "feature ") && !starts_with(command_buf.buf, "option ")) { parse_argv(state); @@ -3837,9 +3837,9 @@ static int parse_one_option(struct fast_import_state *state, const char *option) return 1; } -static void check_unsafe_feature(struct fast_import_state *state UNUSED, const char *feature, int from_stream) +static void check_unsafe_feature(struct fast_import_state *state, const char *feature, int from_stream) { - if (from_stream && !allow_unsafe_features) + if (from_stream && !state->allow_unsafe_features) die(_("feature '%s' forbidden in input without --allow-unsafe-features"), feature); } @@ -3888,7 +3888,7 @@ static int parse_one_feature(struct fast_import_state *state, const char *featur static void parse_feature(struct fast_import_state *state, const char *feature) { - if (seen_data_command) + if (state->seen_data_command) die(_("got feature command '%s' after data command"), feature); if (parse_one_feature(state, feature, 1)) @@ -3899,7 +3899,7 @@ static void parse_feature(struct fast_import_state *state, const char *feature) static void parse_option(struct fast_import_state *state, const char *option) { - if (seen_data_command) + if (state->seen_data_command) die(_("got option command '%s' after data command"), option); if (parse_one_option(state, option)) @@ -3967,7 +3967,7 @@ static void parse_argv(struct fast_import_state *state) if (i != state->argc) usage(fast_import_usage); - seen_data_command = 1; + state->seen_data_command = 1; if (import_marks_file) read_marks(); build_mark_map(&sub_marks_from, &sub_marks_to); @@ -3982,6 +3982,8 @@ int cmd_fast_import(int argc, show_usage_if_asked(argc, argv, fast_import_usage); + fast_import_state_init(&state, argc, argv, prefix); + reset_pack_idx_option(&pack_idx_opts); git_pack_config(); @@ -4005,11 +4007,9 @@ int cmd_fast_import(int argc, if (*arg != '-' || !strcmp(arg, "--")) break; if (!strcmp(arg, "--allow-unsafe-features")) - allow_unsafe_features = 1; + state.allow_unsafe_features = 1; } - fast_import_state_init(&state, argc, argv, prefix); - rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free)); for (unsigned int i = 0; i < (cmd_save - 1); i++) rc_free[i].next = &rc_free[i + 1]; @@ -4056,7 +4056,7 @@ int cmd_fast_import(int argc, } /* argv hasn't been parsed yet, do so */ - if (!seen_data_command) + if (!state.seen_data_command) parse_argv(&state); if (require_explicit_termination && feof(stdin)) From 4da67625c73b7b7ea39803e8ec6ad28e05801806 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:52 +0200 Subject: [PATCH 09/12] fast-import: use struct option for usage string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently `git fast-import -h` shows the following on a single line: usage : git fast-import [--date-format=] [--max-pack-size=] \ [--big-file-threshold=] [--depth=] \ [--active-branches=] \ [--export-marks=] This output has a number of issues like: - It's missing a lot of options. - It's not consistent with the SYNOPSIS section of the doc. - With `--help-all` instead of `-h` additional hidden options should be shown, but that's not the case. - It's not standard style anymore. - Most other Git commands show additional lines for most of the options they support. Also while most commands use the parse-options API to handle their options, "builtin/fast-import.c" still doesn't use it. Let's improve on that by using the parse-options API to display the options when `-h` and `--help-all` are used. While at it, let's make the SYNOPSIS section of "Documentation/git-fast-import.adoc" consistent with the new usage string. This deliberately leaves it to future work to also use the parse-options API to actually parse the options. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- Documentation/git-fast-import.adoc | 2 +- builtin/fast-import.c | 83 +++++++++++++++++++++++++++--- t/t0450/adoc-help-mismatches | 1 - 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc index d68bc52b7e..7c5900e048 100644 --- a/Documentation/git-fast-import.adoc +++ b/Documentation/git-fast-import.adoc @@ -9,7 +9,7 @@ git-fast-import - Backend for fast Git data importers SYNOPSIS -------- [verse] -frontend | 'git fast-import' [] +'git fast-import' [] DESCRIPTION ----------- diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 926afde954..bf72adf62b 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -30,6 +30,7 @@ #include "khash.h" #include "date.h" #include "gpg-interface.h" +#include "parse-options.h" #define PACK_ID_BITS 16 #define MAX_PACK_ID ((1<argc = argc; state->argv = argv; state->prefix = prefix; + state->option = option; } static void parse_argv(struct fast_import_state *state); @@ -3935,8 +3938,10 @@ static void git_pack_config(void) repo_config(the_repository, git_default_config, NULL); } -static const char fast_import_usage[] = -"git fast-import [--date-format=] [--max-pack-size=] [--big-file-threshold=] [--depth=] [--active-branches=] [--export-marks=]"; +static const char *const fast_import_usage[] = { + N_("git fast-import []"), + NULL +}; static void parse_argv(struct fast_import_state *state) { @@ -3965,7 +3970,7 @@ static void parse_argv(struct fast_import_state *state) die(_("unknown option --%s"), a); } if (i != state->argc) - usage(fast_import_usage); + usage_with_options(fast_import_usage, state->option); state->seen_data_command = 1; if (import_marks_file) @@ -3980,9 +3985,75 @@ int cmd_fast_import(int argc, { struct fast_import_state state; - show_usage_if_asked(argc, argv, fast_import_usage); + unsigned long pack_size_limit, big_file_threshold; + char *edges, *signed_commits, *signed_tags, *date_format; + char *import_marks_if_exists, *submodules_from, *submodules_to; - fast_import_state_init(&state, argc, argv, prefix); + /* + * NEEDSWORK: For now this is used only to render + * `-h`/`--help-all` usage messages. The actual parsing is + * done by parse_one_option()/parse_one_feature(). + */ + struct option fast_import_options[] = { + OPT_GROUP(N_("Common")), + OPT_STRING_F(0, "date-format", &date_format, N_("fmt"), + N_("format of the commit/tag dates"), PARSE_OPT_NONEG), + OPT_BOOL_F(0, "stats", &show_stats, + N_("display some basic statistics (objects, packfiles and memory)"), + PARSE_OPT_NONEG), + OPT_BOOL_F(0, "quiet", &quiet, + N_("disable the output shown by --stats"), PARSE_OPT_NONEG), + OPT_BOOL_F(0, "force", &force_update, + N_("force updating modified existing branches"), PARSE_OPT_NONEG), + OPT_BOOL_F(0, "done", &require_explicit_termination, + N_("require a terminating 'done' command"), PARSE_OPT_NONEG), + OPT_UNSIGNED(0, "max-pack-size", &pack_size_limit, + N_("maximum size of each output pack file")), + OPT_UNSIGNED(0, "big-file-threshold", &big_file_threshold, + N_("maximum size of a blob that will be deltified")), + OPT_UNSIGNED(0, "depth", &max_depth, + N_("maximum delta depth")), + OPT_UNSIGNED(0, "active-branches", &max_active_branches, + N_("maximum number of branches to maintain active")), + OPT_GROUP(N_("Marks")), + OPT_STRING_F(0, "import-marks", &import_marks_file, N_("file"), + N_("import marks from "), PARSE_OPT_NONEG), + OPT_STRING_F(0, "import-marks-if-exists", &import_marks_if_exists, N_("file"), + N_("import marks from if it exists"), PARSE_OPT_NONEG), + OPT_STRING_F(0, "export-marks", &export_marks_file, N_("file"), + N_("dump marks to "), PARSE_OPT_NONEG), + OPT_BOOL(0, "relative-marks", &relative_marks_paths, + N_("are --(import|export)-marks= paths relative to '.git/info/fast-import'?")), + OPT_GROUP(N_("Submodule rewrite")), + OPT_STRING_F(0, "rewrite-submodules-from", &submodules_from, N_("name:filename"), + N_("rewrite object IDs for submodule from "), + PARSE_OPT_NONEG), + OPT_STRING_F(0, "rewrite-submodules-to", &submodules_to, N_("name:filename"), + N_("rewrite object IDs for submodule to "), + PARSE_OPT_NONEG), + OPT_GROUP(N_("Signing")), + OPT_STRING_F(0, "signed-commits", &signed_commits, N_("mode"), + N_("how to handle signed commits"), + PARSE_OPT_NONEG), + OPT_STRING_F(0, "signed-tags", &signed_tags, N_("mode"), + N_("how to handle signed tags"), + PARSE_OPT_NONEG), + OPT_HIDDEN_GROUP(N_("Advanced")), + OPT_BOOL_F(0, "allow-unsafe-features", &state.allow_unsafe_features, + N_("allow unsafe mark commands from the stream"), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), + OPT_STRING_F(0, "export-pack-edges", &edges, N_("file"), + N_("dump edge commits to "), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), + OPT_INTEGER_F(0, "cat-blob-fd", &cat_blob_fd, + N_("write some responses to instead of stdout"), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), + OPT_END() + }; + + show_usage_with_options_if_asked(argc, argv, fast_import_usage, fast_import_options); + + fast_import_state_init(&state, argc, argv, prefix, fast_import_options); reset_pack_idx_option(&pack_idx_opts); git_pack_config(); diff --git a/t/t0450/adoc-help-mismatches b/t/t0450/adoc-help-mismatches index c4a55ff4e3..baf3b1d809 100644 --- a/t/t0450/adoc-help-mismatches +++ b/t/t0450/adoc-help-mismatches @@ -12,7 +12,6 @@ column credential credential-cache credential-store -fast-import fetch-pack fmt-merge-msg format-patch From 873854d8d45e416bcacc89f7676fbe9fe8665488 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:53 +0200 Subject: [PATCH 10/12] fast-import: use callbacks to parse some options A previous commit started using the parse-option API to generate proper `git fast-import -h` and `git fast-import --help-all` output. Let's prepare for when we can use that API to also parse the options by using OPT_CALLBACK for some options that require special processing of their arguments. A following commit will actually parse the options using these callbacks. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 208 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 168 insertions(+), 40 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index bf72adf62b..f3c46fb567 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3978,6 +3978,126 @@ static void parse_argv(struct fast_import_state *state) build_mark_map(&sub_marks_from, &sub_marks_to); } +static int option_parse_date_format(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_date_format(arg); + return 0; +} + +static int option_parse_export_pack_edges(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_export_pack_edges(opt->value, arg); + return 0; +} + +static int option_parse_max_pack_size(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_max_pack_size(arg); + return 0; +} + +static int option_parse_big_file_threshold(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_big_file_threshold(arg); + return 0; +} + +static int option_parse_signed_commits(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_signed_commits(arg); + return 0; +} + +static int option_parse_signed_tags(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_signed_tags(arg); + return 0; +} + +static int option_parse_rewrite_submodules_from(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_rewrite_submodules(opt->value, arg, &sub_marks_from); + return 0; +} + +static int option_parse_rewrite_submodules_to(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_rewrite_submodules(opt->value, arg, &sub_marks_to); + return 0; +} + +static int option_parse_cat_blob_fd(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_cat_blob_fd(opt->value, arg); + return 0; +} + +static int option_parse_import_marks(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_import_marks(opt->value, arg, 0, 0); + return 0; +} + +static int option_parse_import_marks_if_exists(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_import_marks(opt->value, arg, 0, 1); + return 0; +} + +static int option_parse_export_marks(const struct option *opt, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_export_marks(opt->value, arg); + return 0; +} + +static int option_parse_depth(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_depth(arg); + return 0; +} + +static int option_parse_active_branches(const struct option *opt UNUSED, + const char *arg, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_active_branches(arg); + return 0; +} + +static int option_parse_quiet(const struct option *opt UNUSED, + const char *arg UNUSED, int unset) +{ + BUG_ON_OPT_NEG(unset); + option_quiet(); + return 0; +} + int cmd_fast_import(int argc, const char **argv, const char *prefix, @@ -3985,10 +4105,6 @@ int cmd_fast_import(int argc, { struct fast_import_state state; - unsigned long pack_size_limit, big_file_threshold; - char *edges, *signed_commits, *signed_tags, *date_format; - char *import_marks_if_exists, *submodules_from, *submodules_to; - /* * NEEDSWORK: For now this is used only to render * `-h`/`--help-all` usage messages. The actual parsing is @@ -3996,58 +4112,70 @@ int cmd_fast_import(int argc, */ struct option fast_import_options[] = { OPT_GROUP(N_("Common")), - OPT_STRING_F(0, "date-format", &date_format, N_("fmt"), - N_("format of the commit/tag dates"), PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "date-format", NULL, N_("fmt"), + N_("format of the commit/tag dates"), + PARSE_OPT_NONEG, option_parse_date_format), OPT_BOOL_F(0, "stats", &show_stats, N_("display some basic statistics (objects, packfiles and memory)"), PARSE_OPT_NONEG), - OPT_BOOL_F(0, "quiet", &quiet, - N_("disable the output shown by --stats"), PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "quiet", NULL, NULL, + N_("disable the output shown by --stats"), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, + option_parse_quiet), OPT_BOOL_F(0, "force", &force_update, N_("force updating modified existing branches"), PARSE_OPT_NONEG), OPT_BOOL_F(0, "done", &require_explicit_termination, N_("require a terminating 'done' command"), PARSE_OPT_NONEG), - OPT_UNSIGNED(0, "max-pack-size", &pack_size_limit, - N_("maximum size of each output pack file")), - OPT_UNSIGNED(0, "big-file-threshold", &big_file_threshold, - N_("maximum size of a blob that will be deltified")), - OPT_UNSIGNED(0, "depth", &max_depth, - N_("maximum delta depth")), - OPT_UNSIGNED(0, "active-branches", &max_active_branches, - N_("maximum number of branches to maintain active")), + OPT_CALLBACK_F(0, "max-pack-size", NULL, N_("n"), + N_("maximum size of each output pack file"), + PARSE_OPT_NONEG, option_parse_max_pack_size), + OPT_CALLBACK_F(0, "big-file-threshold", NULL, N_("n"), + N_("maximum size of a blob that will be deltified"), + PARSE_OPT_NONEG, option_parse_big_file_threshold), + OPT_CALLBACK_F(0, "depth", NULL, N_("n"), + N_("maximum delta depth"), + PARSE_OPT_NONEG, option_parse_depth), + OPT_CALLBACK_F(0, "active-branches", NULL, N_("n"), + N_("maximum number of branches to maintain active"), + PARSE_OPT_NONEG, option_parse_active_branches), OPT_GROUP(N_("Marks")), - OPT_STRING_F(0, "import-marks", &import_marks_file, N_("file"), - N_("import marks from "), PARSE_OPT_NONEG), - OPT_STRING_F(0, "import-marks-if-exists", &import_marks_if_exists, N_("file"), - N_("import marks from if it exists"), PARSE_OPT_NONEG), - OPT_STRING_F(0, "export-marks", &export_marks_file, N_("file"), - N_("dump marks to "), PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "import-marks", &state, N_("file"), + N_("import marks from "), + PARSE_OPT_NONEG, option_parse_import_marks), + OPT_CALLBACK_F(0, "import-marks-if-exists", &state, N_("file"), + N_("import marks from if it exists"), + PARSE_OPT_NONEG, option_parse_import_marks_if_exists), + OPT_CALLBACK_F(0, "export-marks", &state, N_("file"), + N_("dump marks to "), + PARSE_OPT_NONEG, option_parse_export_marks), OPT_BOOL(0, "relative-marks", &relative_marks_paths, N_("are --(import|export)-marks= paths relative to '.git/info/fast-import'?")), OPT_GROUP(N_("Submodule rewrite")), - OPT_STRING_F(0, "rewrite-submodules-from", &submodules_from, N_("name:filename"), - N_("rewrite object IDs for submodule from "), - PARSE_OPT_NONEG), - OPT_STRING_F(0, "rewrite-submodules-to", &submodules_to, N_("name:filename"), - N_("rewrite object IDs for submodule to "), - PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "rewrite-submodules-from", &state, N_("name:filename"), + N_("rewrite object IDs for submodule from "), + PARSE_OPT_NONEG, option_parse_rewrite_submodules_from), + OPT_CALLBACK_F(0, "rewrite-submodules-to", &state, N_("name:filename"), + N_("rewrite object IDs for submodule to "), + PARSE_OPT_NONEG, option_parse_rewrite_submodules_to), OPT_GROUP(N_("Signing")), - OPT_STRING_F(0, "signed-commits", &signed_commits, N_("mode"), - N_("how to handle signed commits"), - PARSE_OPT_NONEG), - OPT_STRING_F(0, "signed-tags", &signed_tags, N_("mode"), - N_("how to handle signed tags"), - PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "signed-commits", NULL, N_("mode"), + N_("how to handle signed commits"), + PARSE_OPT_NONEG, option_parse_signed_commits), + OPT_CALLBACK_F(0, "signed-tags", NULL, N_("mode"), + N_("how to handle signed tags"), + PARSE_OPT_NONEG, option_parse_signed_tags), OPT_HIDDEN_GROUP(N_("Advanced")), OPT_BOOL_F(0, "allow-unsafe-features", &state.allow_unsafe_features, N_("allow unsafe mark commands from the stream"), PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), - OPT_STRING_F(0, "export-pack-edges", &edges, N_("file"), - N_("dump edge commits to "), - PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), - OPT_INTEGER_F(0, "cat-blob-fd", &cat_blob_fd, - N_("write some responses to instead of stdout"), - PARSE_OPT_HIDDEN | PARSE_OPT_NONEG), + OPT_CALLBACK_F(0, "export-pack-edges", &state, N_("file"), + N_("dump edge commits to "), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG, + option_parse_export_pack_edges), + OPT_CALLBACK_F(0, "cat-blob-fd", &state, N_("fd"), + N_("write some responses to instead of stdout"), + PARSE_OPT_HIDDEN | PARSE_OPT_NONEG, + option_parse_cat_blob_fd), OPT_END() }; From 79e34302481969f5f25310ab730f1314e2aa4c3b Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 4 Aug 2026 12:03:54 +0200 Subject: [PATCH 11/12] fast-import: use parse_options() for command line options Previous commits have started to use the parse-options API to display output from `git fast-import -h` and `git fast-import --help-all` and to prepare for parsing the command line options using this API. Let's now actually use the API to parse command line options. This brings a number of changes that are mostly beneficial: - The `--alias`, `--get-mark`, `--cat-blob`, `--ls` and `--notes` options are no longer accepted on the command line. They were previously accepted as no-ops because parse_argv() fell through to parse_one_feature(). They are not documented in the OPTIONS section and are only meaningful as in-stream feature assertions, so accepting them on the command line was an accident of code sharing dating back to 9c8398f0c9 (fast-import: add option command, 2009-12-04). - Abbreviated options like `--dep=5` now work since parse_options() allows unambiguous prefixes. - As `--cat-blob` is an abbreviation of `--cat-blob-fd`, using the former on the command line will fail with "option `cat-blob-fd' requires a value" unlike the other four options that are not accepted anymore on the command line (see above). - The error messages for some options might differ a bit. - The code is shorter and more standard. Note that parse_one_feature() is now always called with its `from_stream` argument set to 1, but the code simplifications that can be made are left for a following clean-up commit. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 33 ++++----------------------------- t/t9300-fast-import.sh | 7 +++++++ 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index f3c46fb567..0df7a31014 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3945,31 +3945,11 @@ static const char *const fast_import_usage[] = { static void parse_argv(struct fast_import_state *state) { - unsigned int i; + int argc = parse_options(state->argc, state->argv, state->prefix, + state->option, fast_import_usage, + PARSE_OPT_KEEP_ARGV0); - for (i = 1; i < state->argc; i++) { - const char *a = state->argv[i]; - - if (*a != '-' || !strcmp(a, "--")) - break; - - if (!skip_prefix(a, "--", &a)) - die(_("unknown option %s"), a); - - if (parse_one_option(state, a)) - continue; - - if (parse_one_feature(state, a, 0)) - continue; - - if (skip_prefix(a, "cat-blob-fd=", &a)) { - option_cat_blob_fd(state, a); - continue; - } - - die(_("unknown option --%s"), a); - } - if (i != state->argc) + if (argc > 1) usage_with_options(fast_import_usage, state->option); state->seen_data_command = 1; @@ -4105,11 +4085,6 @@ int cmd_fast_import(int argc, { struct fast_import_state state; - /* - * NEEDSWORK: For now this is used only to render - * `-h`/`--help-all` usage messages. The actual parsing is - * done by parse_one_option()/parse_one_feature(). - */ struct option fast_import_options[] = { OPT_GROUP(N_("Common")), OPT_CALLBACK_F(0, "date-format", NULL, N_("fmt"), diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index fe6c2617ac..d9de2ef0d8 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -2827,6 +2827,13 @@ test_expect_success 'R: unknown commandline options are rejected' '\ test_must_fail git fast-import --non-existing-option < /dev/null ' +test_expect_success 'R: feature-only names are rejected on the command line' ' + for opt in --alias --get-mark --ls --notes + do + test_must_fail git fast-import "$opt" Date: Tue, 4 Aug 2026 12:03:55 +0200 Subject: [PATCH 12/12] fast-import: remove useless from_stream argument Now that a previous commit has removed a call to parse_one_feature() from parse_argv(), the former is always called with its `from_stream` argument set to 1. Let's take advantage of that to simplify and cleanup the code a bit. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 0df7a31014..9d827f2224 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3840,27 +3840,27 @@ static int parse_one_option(struct fast_import_state *state, const char *option) return 1; } -static void check_unsafe_feature(struct fast_import_state *state, const char *feature, int from_stream) +static void check_unsafe_feature(struct fast_import_state *state, const char *feature) { - if (from_stream && !state->allow_unsafe_features) + if (!state->allow_unsafe_features) die(_("feature '%s' forbidden in input without --allow-unsafe-features"), feature); } -static int parse_one_feature(struct fast_import_state *state, const char *feature, int from_stream) +static int parse_one_feature(struct fast_import_state *state, const char *feature) { const char *arg; if (skip_prefix(feature, "date-format=", &arg)) { option_date_format(arg); } else if (skip_prefix(feature, "import-marks=", &arg)) { - check_unsafe_feature(state, "import-marks", from_stream); - option_import_marks(state, arg, from_stream, 0); + check_unsafe_feature(state, "import-marks"); + option_import_marks(state, arg, 1, 0); } else if (skip_prefix(feature, "import-marks-if-exists=", &arg)) { - check_unsafe_feature(state, "import-marks-if-exists", from_stream); - option_import_marks(state, arg, from_stream, 1); + check_unsafe_feature(state, "import-marks-if-exists"); + option_import_marks(state, arg, 1, 1); } else if (skip_prefix(feature, "export-marks=", &arg)) { - check_unsafe_feature(state, feature, from_stream); + check_unsafe_feature(state, feature); option_export_marks(state, arg); } else if (!strcmp(feature, "alias")) { ; /* Don't die - this feature is supported */ @@ -3894,7 +3894,7 @@ static void parse_feature(struct fast_import_state *state, const char *feature) if (state->seen_data_command) die(_("got feature command '%s' after data command"), feature); - if (parse_one_feature(state, feature, 1)) + if (parse_one_feature(state, feature)) return; die(_("this version of fast-import does not support feature %s."), feature);