Merge branch 'cc/early-scan-options' into seen

The process of parsing command-line options in commands that
perform an early scan over their arguments (such as 'git bisect',
'git rev-parse', and 'git fast-import') has been unified using a
new early-scan sub-API, which parses and skips known options taking
separate values to prevent logic bugs.

* cc/early-scan-options:
  fast-import: use early_scan_options() for --allow-unsafe-features
  parse-options: build early scan options from a struct option array
  parse-options: add parse_options_takes_argument()
  rev-parse: fix "--" detection when it is an option value
  bisect: fix "--" detection when a term name is "--"
  parse-options: add early_scan_options()

Documentation/git-fast-import.adoc

@@ -66,12 +66,10 @@ fast-import stream! This option is enabled automatically for
 remote-helpers that use the `import` capability, as they are
 already trusted to run their own code.
 +
-Note that this option has to be spelled in full, and has to appear
-before any option whose value is separated from it by a space, for
-the unsafe `feature` commands in the stream to be allowed. So
-`--allow-unsafe` or `--depth 5 --allow-unsafe-features` still refuse
-them, while `--allow-unsafe-features --depth 5` and
-`--depth=5 --allow-unsafe-features` allow them.
+Note that this option has to be spelled in full for the unsafe
+`feature` commands in the stream to be allowed. So while
+`--allow-unsafe` is accepted as an unambiguous abbreviation of this
+option, it still refuses them.
 
 `--signed-tags=<mode>`::
 	Specify how to handle signed tags. Behaves in the same way as

builtin/bisect.c

@@ -803,6 +803,19 @@ static enum bisect_error bisect_auto_next(struct bisect_terms *terms,
 	return bisect_next(terms, prefix);
 }
 
+/*
+ * The options "git bisect start" accepts. Only the ones taking their
+ * value as a separate argument matter to the scan looking for "--" below,
+ * as their value has to be skipped along with them.
+ */
+static const struct early_scan_option bisect_start_early_options[] = {
+	EARLY_SCAN_SKIP_VALUE("term-good"),
+	EARLY_SCAN_SKIP_VALUE("term-old"),
+	EARLY_SCAN_SKIP_VALUE("term-bad"),
+	EARLY_SCAN_SKIP_VALUE("term-new"),
+	EARLY_SCAN_END()
+};
+
 static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
 				      const char **argv)
 {
@@ -825,13 +838,15 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
 
 	/*
 	 * Check for one bad and then some good revisions
+	 *
+	 * The scan below has to know about the options taking their value
+	 * as a separate argument, or such a value that happens to be "--"
+	 * would be mistaken for the "--" separating revisions from paths.
 	 */
-	for (i = 0; i < argc; i++) {
-		if (!strcmp(argv[i], "--")) {
-			has_double_dash = 1;
-			break;
-		}
-	}
+	i = early_scan_options(argc, argv, bisect_start_early_options,
+			       EARLY_SCAN_STOP_AT_DASHDASH, NULL, NULL);
+	if (i < argc)
+		has_double_dash = 1;
 
 	for (i = 0; i < argc; i++) {
 		const char *arg = argv[i];

builtin/fast-import.c

@@ -4120,12 +4120,29 @@ static int option_parse_quiet(const struct option *opt UNUSED,
 	return 0;
 }
 
+/*
+ * The only option the early scan below is interested in, as it decides
+ * whether unsafe "feature" commands from the stream are allowed.
+ */
+static const char *early_wanted[] = { "allow-unsafe-features", NULL };
+
+static int option_parse_early_allow_unsafe(
+		const struct early_scan_option *opt UNUSED,
+		const char *value UNUSED, int pos UNUSED, void *data)
+{
+	struct fast_import_state *state = data;
+
+	state->allow_unsafe_features = 1;
+	return 0;
+}
+
 int cmd_fast_import(int argc,
 		    const char **argv,
 		    const char *prefix,
 		    struct repository *repo)
 {
 	struct fast_import_state state;
+	struct early_scan_option *early;
 
 	struct option fast_import_options[] = {
 		OPT_GROUP(N_("Common")),
@@ -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 and stops at the first argument that doesn't start with a
-	 * dash. As parse_options() below also accepts unambiguous abbreviations
-	 * and values separated by a space from their option, the two disagree
-	 * for command lines like "--allow-unsafe" or "--depth 5
-	 * --allow-unsafe-features": 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, but should be fixed by
-	 * teaching this scan about the options that take a value.
+	 * 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;
-	}
+	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);
 
 	rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free));
 	for (unsigned int i = 0; i < (cmd_save - 1); i++)

builtin/rev-parse.c

@@ -695,6 +695,17 @@ static void print_path(const char *path, const char *prefix,
 	strbuf_release(&sb);
 }
 
+/*
+ * The options taking their value as a separate argument, which the scan
+ * looking for "--" below has to skip along with their value.
+ */
+static const struct early_scan_option rev_parse_early_options[] = {
+	EARLY_SCAN_SKIP_VALUE("default"),
+	EARLY_SCAN_SKIP_VALUE("prefix"),
+	EARLY_SCAN_SKIP_VALUE("resolve-git-dir"),
+	EARLY_SCAN_END()
+};
+
 int cmd_rev_parse(int argc,
 		  const char **argv,
 		  const char *prefix,
@@ -724,12 +735,15 @@ int cmd_rev_parse(int argc,
 	if (argc > 1 && !strcmp("-h", argv[1]))
 		usage(builtin_rev_parse_usage);
 
-	for (i = 1; i < argc; i++) {
-		if (!strcmp(argv[i], "--")) {
-			has_dashdash = 1;
-			break;
-		}
-	}
+	/*
+	 * The scan below has to know about the options taking their value
+	 * as a separate argument, or such a value that happens to be "--"
+	 * would be mistaken for the "--" separating revisions from paths.
+	 */
+	i = early_scan_options(argc - 1, argv + 1, rev_parse_early_options,
+			       EARLY_SCAN_STOP_AT_DASHDASH, NULL, NULL);
+	if (i < argc - 1)
+		has_dashdash = 1;
 
 	/* No options; just report on whether we're in a git repo or not. */
 	if (argc == 1) {

parse-options.c

@@ -841,6 +841,26 @@ static void show_negated_gitcomp(const struct option *opts, int show_all,
 	}
 }
 
+int parse_options_takes_argument(const struct option *opt)
+{
+	switch (opt->type) {
+	case OPTION_STRING:
+	case OPTION_FILENAME:
+	case OPTION_INTEGER:
+	case OPTION_UNSIGNED:
+	case OPTION_CALLBACK:
+		break;
+	default:
+		return 0;
+	}
+
+	if (opt->flags & (PARSE_OPT_NOARG | PARSE_OPT_OPTARG |
+			  PARSE_OPT_LASTARG_DEFAULT))
+		return 0;
+
+	return 1;
+}
+
 static int show_gitcomp(const struct option *opts, int show_all)
 {
 	const struct option *original_opts = opts;
@@ -862,20 +882,9 @@ static int show_gitcomp(const struct option *opts, int show_all)
 			break;
 		case OPTION_GROUP:
 			continue;
-		case OPTION_STRING:
-		case OPTION_FILENAME:
-		case OPTION_INTEGER:
-		case OPTION_UNSIGNED:
-		case OPTION_CALLBACK:
-			if (opts->flags & PARSE_OPT_NOARG)
-				break;
-			if (opts->flags & PARSE_OPT_OPTARG)
-				break;
-			if (opts->flags & PARSE_OPT_LASTARG_DEFAULT)
-				break;
-			suffix = "=";
-			break;
 		default:
+			if (parse_options_takes_argument(opts))
+				suffix = "=";
 			break;
 		}
 		if (opts->flags & PARSE_OPT_COMP_ARG)
@@ -1246,6 +1255,115 @@ int parse_options(int argc, const char **argv,
 	return parse_options_end(&ctx);
 }
 
+/*
+ * Look for `arg` among `options`. On success, return the matching option
+ * and set `value` to the value stuck to it, if any, or to NULL.
+ */
+static const struct early_scan_option *
+find_early_scan_option(const char *arg,
+		       const struct early_scan_option *options,
+		       const char **value)
+{
+	if (!skip_prefix(arg, "--", &arg))
+		return NULL;
+
+	for (; options->name; options++) {
+		const char *rest;
+
+		if (!skip_prefix(arg, options->name, &rest))
+			continue;
+		if (!*rest) {
+			*value = NULL;
+			return options;
+		}
+		/* Only an option taking a value can be stuck to one. */
+		if (*rest == '=' && options->takes_value) {
+			*value = rest + 1;
+			return options;
+		}
+	}
+
+	return NULL;
+}
+
+int early_scan_options(int argc, const char **argv,
+		       const struct early_scan_option *options,
+		       enum early_scan_flags flags,
+		       early_scan_fn *fn, void *data)
+{
+	for (int i = 0; i < argc; i++) {
+		const char *arg = argv[i];
+		const char *value;
+		const struct early_scan_option *opt;
+		int pos = i;
+
+		if ((flags & EARLY_SCAN_STOP_AT_DASHDASH) &&
+		    !strcmp(arg, "--"))
+			return i;
+
+		opt = find_early_scan_option(arg, options, &value);
+		if (!opt) {
+			if ((flags & EARLY_SCAN_STOP_AT_NON_OPTION) &&
+			    (*arg != '-' || !arg[1]))
+				return i;
+			continue;
+		}
+
+		/*
+		 * When an option takes a value, but that value is not
+		 * stuck to it with '=', then the next argument is the
+		 * value and it has to be skipped so that it isn't
+		 * taken for an option itself.
+		 */
+		if (opt->takes_value && !value && i + 1 < argc)
+			value = argv[++i];
+
+		if (opt->wanted && fn(opt, value, pos, data))
+			return i;
+	}
+
+	return argc;
+}
+
+struct early_scan_option *
+early_scan_options_from_options(const struct option *options,
+				const char **wanted)
+{
+	struct early_scan_option *early;
+	size_t nr = 0;
+
+	for (const struct option *opt = options; opt->type != OPTION_END; opt++)
+		if (opt->long_name)
+			nr++;
+
+	CALLOC_ARRAY(early, nr + 1);
+
+	nr = 0;
+	for (const struct option *opt = options; opt->type != OPTION_END; opt++) {
+		if (!opt->long_name)
+			continue;
+		early[nr].name = opt->long_name;
+		early[nr].takes_value = !!parse_options_takes_argument(opt);
+		nr++;
+	}
+
+	for (; wanted && *wanted; wanted++) {
+		size_t i;
+
+		for (i = 0; i < nr; i++) {
+			if (strcmp(early[i].name, *wanted))
+				continue;
+			early[i].wanted = 1;
+			break;
+		}
+		if (i == nr)
+			BUG("wanted option '%s' is not in the options array",
+			    *wanted);
+	}
+
+	return early;
+}
+
 static int usage_argh(const struct option *opts, FILE *outfile)
 {
 	const char *s;

parse-options.h

@@ -423,6 +423,16 @@ int parse_options(int argc, const char **argv, const char *prefix,
 		  const char * const usagestr[],
 		  enum parse_opt_flags flags);
 
+/*
+ * Return non-zero if `opt` takes a value, which means that it consumes
+ * the next argument when that value is not stuck to it with an '='.
+ *
+ * Note that an option with PARSE_OPT_LASTARG_DEFAULT only consumes the
+ * next argument when it isn't the last one, so it is not considered as
+ * taking a value here.
+ */
+int parse_options_takes_argument(const struct option *opt);
+
 NORETURN void usage_with_options(const char * const *usagestr,
 				 const struct option *options);
 
@@ -494,6 +504,88 @@ static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
 		BUG("option callback expects an argument"); \
 } while(0)
 
+/*----- Early scan: scanning argv before the actual option parsing -----*/
+
+/*
+ * Some commands need to look at a few options before they can parse
+ * their command line for real, for example because the result decides
+ * whether a repository is needed at all.
+ *
+ * Such an early scan has to know which options take their value as a
+ * separate argument, or it could mistake such a value for an option. The
+ * `struct early_scan_option` array passed to early_scan_options() below
+ * describes the options to look for, as well as the ones that only need
+ * to be skipped along with their value.
+ */
+struct early_scan_option {
+	const char *name; 	/* Option name, without the leading dashes */
+	unsigned takes_value:1; /* "--option=value" or "--option value" expected? */
+	unsigned wanted:1;      /* Report option to callback? */
+};
+
+#define EARLY_SCAN_SKIP_VALUE(n) { .name = (n), .takes_value = 1 }
+#define EARLY_SCAN_WANT(n) { .name = (n), .wanted = 1 }
+#define EARLY_SCAN_WANT_VALUE(n) { .name = (n), .takes_value = 1, .wanted = 1 }
+#define EARLY_SCAN_END() { NULL }
+
+/*
+ * Called by early_scan_options() for each argument matching a
+ * `struct early_scan_option` that has its `wanted` bit set.
+ *
+ * `option` is the matching option, `value` its value or NULL if it
+ * doesn't take one, and `pos` the index of the option in argv.
+ *
+ * Returning a non-zero value stops the scan.
+ */
+typedef int early_scan_fn(const struct early_scan_option *option,
+			  const char *value, int pos, void *data);
+
+enum early_scan_flags {
+	EARLY_SCAN_STOP_AT_DASHDASH = 1 << 0, /* Stop at "--" */
+	EARLY_SCAN_STOP_AT_NON_OPTION = 1 << 1,
+};
+
+/*
+ * Scan `argv` for the options described by `options`, calling `fn`
+ * for each of those that are `wanted`. `argv` is not modified.
+ *
+ * `fn` may be NULL when no option is `wanted`, which is useful to only
+ * find out where the scan stops.
+ *
+ * Note that abbreviated options are not recognized, as a scan cannot
+ * know about the options it hasn't been told about, and would then
+ * resolve abbreviations differently from the actual option parsing.
+ *
+ * Returns the index at which the scan stopped, which is `argc` when the
+ * whole array was scanned.
+ */
+int early_scan_options(int argc, const char **argv,
+		       const struct early_scan_option *options,
+		       enum early_scan_flags flags,
+		       early_scan_fn *fn, void *data);
+
+/*
+ * Build the `struct early_scan_option` array to pass to
+ * early_scan_options() from the `options` array that the actual option
+ * parsing uses, so that both agree on which options take a value.
+ *
+ * Note some intentional limitations to keep the scan simple and fast:
+ * short options are ignored, options with PARSE_OPT_LASTARG_DEFAULT or
+ * PARSE_OPT_OPTARG are treated as not taking a separate value, negated
+ * options ("--no-...") are not automatically generated, and abbreviated
+ * options will not be matched.
+ *
+ * The options named in the NULL terminated `wanted` array get their
+ * `wanted` bit set, the other ones are only there to be skipped along
+ * with their value. It is a BUG() for a name in `wanted` not to appear
+ * in `options`.
+ *
+ * The returned array is allocated and should be free()d by the caller.
+ */
+struct early_scan_option *
+early_scan_options_from_options(const struct option *options,
+				const char **wanted);
+
 /*----- incremental advanced APIs -----*/
 
 struct parse_opt_cmdmode_list;

t/helper/test-parse-options.c

@@ -383,3 +383,74 @@ int cmd__parse_subcommand(int argc, const char **argv)
 
 	return parse_subcommand__cmd(argc, argv, test_flags);
 }
+
+static int show_early_option(const struct early_scan_option *opt,
+			     const char *value, int pos, void *data UNUSED)
+{
+	printf("found: %s at %d", opt->name, pos);
+	if (value)
+		printf(" value: %s", value);
+	putchar('\n');
+	return 0;
+}
+
+int cmd__early_scan_options(int argc, const char **argv)
+{
+	static const struct early_scan_option options[] = {
+		EARLY_SCAN_WANT("wanted"),
+		EARLY_SCAN_WANT_VALUE("wanted-value"),
+		EARLY_SCAN_SKIP_VALUE("skipped-value"),
+		EARLY_SCAN_END()
+	};
+	enum early_scan_flags flags = 0;
+	int stopped;
+
+	while (argc > 1 && *argv[1] == '-') {
+		if (!strcmp(argv[1], "--stop-at-dashdash"))
+			flags |= EARLY_SCAN_STOP_AT_DASHDASH;
+		else if (!strcmp(argv[1], "--stop-at-non-option"))
+			flags |= EARLY_SCAN_STOP_AT_NON_OPTION;
+		else
+			break;
+		argc--;
+		argv++;
+	}
+
+	stopped = early_scan_options(argc - 1, argv + 1, options, flags,
+				    show_early_option, NULL);
+	printf("stopped at: %d of %d\n", stopped, argc - 1);
+
+	return 0;
+}
+
+int cmd__early_scan_from_options(int argc, const char **argv)
+{
+	int an_int = 0, a_bool = 0;
+	char *a_string = NULL;
+	const struct option options[] = {
+		OPT_STRING(0, "string", &a_string, "str", "get a string"),
+		OPT_INTEGER(0, "int", &an_int, "get an integer"),
+		OPT_BOOL(0, "bool", &a_bool, "get a boolean"),
+		OPT_STRING_F(0, "optarg", &a_string, "str",
+			     "string with an optional value",
+			     PARSE_OPT_OPTARG),
+		OPT_END()
+	};
+	static const char *wanted[] = { "bool", NULL };
+	struct early_scan_option *early;
+	int stopped;
+
+	early = early_scan_options_from_options(options, wanted);
+
+	for (const struct early_scan_option *o = early; o->name; o++)
+		printf("option: %s takes_value: %d wanted: %d\n",
+		       o->name, o->takes_value, o->wanted);
+
+	stopped = early_scan_options(argc - 1, argv + 1, early, 0,
+				     show_early_option, NULL);
+	printf("stopped at: %d of %d\n", stopped, argc - 1);
+
+	free(early);
+
+	return 0;
+}

t/helper/test-tool.c

@@ -50,6 +50,8 @@ static struct test_cmd cmds[] = {
 	{ "pack-mtimes", cmd__pack_mtimes },
 	{ "parse-options", cmd__parse_options },
 	{ "parse-options-flags", cmd__parse_options_flags },
+	{ "early-scan-options", cmd__early_scan_options },
+	{ "early-scan-from-options", cmd__early_scan_from_options },
 	{ "parse-pathspec-file", cmd__parse_pathspec_file },
 	{ "parse-subcommand", cmd__parse_subcommand },
 	{ "partial-clone", cmd__partial_clone },

t/helper/test-tool.h

@@ -43,6 +43,8 @@ int cmd__pack_deltas(int argc, const char **argv);
 int cmd__pack_mtimes(int argc, const char **argv);
 int cmd__parse_options(int argc, const char **argv);
 int cmd__parse_options_flags(int argc, const char **argv);
+int cmd__early_scan_options(int argc, const char **argv);
+int cmd__early_scan_from_options(int argc, const char **argv);
 int cmd__parse_pathspec_file(int argc, const char** argv);
 int cmd__parse_subcommand(int argc, const char **argv);
 int cmd__partial_clone(int argc, const char **argv);

t/t0040-parse-options.sh

@@ -845,4 +845,107 @@ test_expect_success 'u16 limits range' '
 	test_grep "value 65536 for option .u16. not in range \[0,65535\]" err
 '
 
+test_expect_success 'early_scan_options() finds a wanted option' '
+	test-tool early-scan-options --wanted >actual &&
+	cat >expect <<-\EOF &&
+	found: wanted at 0
+	stopped at: 1 of 1
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'early_scan_options() reads a stuck or separate value' '
+	test-tool early-scan-options --wanted-value=one >actual &&
+	cat >expect <<-\EOF &&
+	found: wanted-value at 0 value: one
+	stopped at: 1 of 1
+	EOF
+	test_cmp expect actual &&
+	test-tool early-scan-options --wanted-value two >actual &&
+	cat >expect <<-\EOF &&
+	found: wanted-value at 0 value: two
+	stopped at: 2 of 2
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'early_scan_options() skips the value of other options' '
+	test-tool early-scan-options --skipped-value --wanted >actual &&
+	cat >expect <<-\EOF &&
+	stopped at: 2 of 2
+	EOF
+	test_cmp expect actual &&
+	test-tool early-scan-options --skipped-value one --wanted >actual &&
+	cat >expect <<-\EOF &&
+	found: wanted at 2
+	stopped at: 3 of 3
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'early_scan_options() can stop at "--"' '
+	test-tool early-scan-options --stop-at-dashdash -- --wanted >actual &&
+	cat >expect <<-\EOF &&
+	stopped at: 0 of 2
+	EOF
+	test_cmp expect actual &&
+	test-tool early-scan-options --stop-at-dashdash \
+		--skipped-value -- --wanted >actual &&
+	cat >expect <<-\EOF &&
+	found: wanted at 2
+	stopped at: 3 of 3
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'early_scan_options() can stop at a non-option' '
+	test-tool early-scan-options --stop-at-non-option \
+		arg --wanted >actual &&
+	cat >expect <<-\EOF &&
+	stopped at: 0 of 2
+	EOF
+	test_cmp expect actual &&
+	test-tool early-scan-options --stop-at-non-option \
+		--skipped-value arg --wanted >actual &&
+	cat >expect <<-\EOF &&
+	found: wanted at 2
+	stopped at: 3 of 3
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'early_scan_options() ignores abbreviated options' '
+	test-tool early-scan-options --want >actual &&
+	cat >expect <<-\EOF &&
+	stopped at: 1 of 1
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'early_scan_options_from_options() derives takes_value' '
+	test-tool early-scan-from-options >actual &&
+	cat >expect <<-\EOF &&
+	option: string takes_value: 1 wanted: 0
+	option: int takes_value: 1 wanted: 0
+	option: bool takes_value: 0 wanted: 1
+	option: optarg takes_value: 0 wanted: 0
+	stopped at: 0 of 0
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'early_scan_options_from_options() skips values' '
+	test-tool early-scan-from-options --string --bool >out &&
+	tail -1 out >actual &&
+	echo "stopped at: 2 of 2" >expect &&
+	test_cmp expect actual &&
+	test-tool early-scan-from-options --string v --bool >out &&
+	tail -2 out >actual &&
+	cat >expect <<-\EOF &&
+	found: bool at 2
+	stopped at: 3 of 3
+	EOF
+	test_cmp expect actual
+'
+
 test_done

t/t1500-rev-parse.sh

@@ -383,4 +383,9 @@ test_expect_success ':/ and HEAD^{/} favor more recent matching commits' '
 	)
 '
 
+test_expect_success 'rev-parse with "--" as an option value' '
+	test_must_fail git rev-parse --default -- notarev 2>err &&
+	test_grep "ambiguous argument .notarev." err
+'
+
 test_done

t/t6030-bisect-porcelain.sh

@@ -1297,6 +1297,14 @@ test_expect_success 'bisect start takes options and revs in any order' '
 	test_cmp expected actual
 '
 
+test_expect_success 'bisect start with "--" as a term name' '
+	git bisect reset &&
+	git bisect start --term-good -- hello &&
+	git bisect terms --term-good >actual &&
+	echo -- >expected &&
+	test_cmp expected actual
+'
+
 # Bisect is started with --term-new and --term-old arguments,
 # then skip. The HEAD should be changed.
 test_expect_success 'bisect skip works with --term*' '

t/t9300-fast-import.sh

@@ -2344,6 +2344,20 @@ test_expect_success 'R: export-marks options can be overridden by commandline op
 	test_path_is_missing feature-sub
 '
 
+test_expect_success 'R: --allow-unsafe-features found after a value' '
+	echo "feature import-marks-if-exists=nonexistent.marks" >input &&
+	git fast-import --allow-unsafe-features <input &&
+	git fast-import --depth=5 --allow-unsafe-features <input &&
+	git fast-import --depth 5 --allow-unsafe-features <input &&
+	git fast-import --date-format raw --allow-unsafe-features <input
+'
+
+test_expect_success 'R: --allow-unsafe-features has to be spelled in full' '
+	echo "feature import-marks-if-exists=nonexistent.marks" >input &&
+	test_must_fail git fast-import --allow-unsafe <input 2>err &&
+	test_grep "forbidden in input without --allow-unsafe-features" err
+'
+
 test_expect_success 'R: catch typo in marks file name' '
 	test_must_fail git fast-import --import-marks=nonexistent.marks </dev/null &&
 	echo "feature import-marks=nonexistent.marks" |