From: Junio C Hamano Date: Fri, 04 Sep 2026 03:38:49 GMT Subject: Re: [PATCH 6/6] fast-import: use early_scan_options() for --allow-unsafe-features Message-ID: In-Reply-To: <20260902161047.476753-7-christian.couder@gmail.com> Christian Couder writes: > The "feature" lines at the start of the stream are processed before the > command line options are parsed, so cmd_fast_import() scans its > arguments early to find out if `--allow-unsafe-features` was given. > > That scan doesn't know which options take their value as a separate > argument, and it stops at the first argument that doesn't start with a > dash. So it disagrees with parse_options(), which accepts values > separated from their option by a space, for a command line like > "--depth 5 --allow-unsafe-features": the scan stops at "5" and never > sees the option, so unsafe "feature" commands from the stream are > refused even though the option was given. Well explained. > @@ -4218,23 +4235,20 @@ 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. > * > + * NEEDSWORK: This scan only matches the exact > + * "--allow-unsafe-features" spelling, while parse_options() below > + * also accepts unambiguous abbreviations, so the two disagree for > + * a command line like "--allow-unsafe": parse_options() accepts > + * the option, but this scan doesn't see it, so unsafe features > + * from the stream are still refused. This errs on the safe side. > */ > - for (int i = 1; i < argc; i++) { > - const char *arg = argv[i]; > - if (*arg != '-' || !strcmp(arg, "--")) > - break; > - if (!strcmp(arg, "--allow-unsafe-features")) > - state.allow_unsafe_features = 1; > - } This is the ad-hoc one that does not know --depth takes a value after it. > + early = early_scan_options_from_options(fast_import_options, > + early_wanted); > + early_scan_options(argc - 1, argv + 1, early, > + EARLY_SCAN_STOP_AT_DASHDASH | > + EARLY_SCAN_STOP_AT_NON_OPTION, > + option_parse_early_allow_unsafe, &state); > + free(early); Interesting. This one now "knows" enough to skip what comes after "--depth" that takes an option ;-) And it is perfectly fine if we skip over "--depth hello" to find "--allow-unsafe", as such a "oops we require number but hello is not a number" will be caught by the real parser anyway. Nicely done.