replay: disallow revision specific options and pathspecs

A previous commit changed `git replay` to make it accept standard
revision ranges using the setup_revisions() function. While this is good
thing to make this command more standard and more flexible, it has the
downside of enabling many revision related options accepted and eaten by
setup_revisions().

Some of these options might make sense, but others, like those
generating non-contiguous history, might not. Anyway those we might want
to allow should probably be tested and perhaps documented a bit, which
could be done in future work.

For now it is just simpler and safer to just disallow all of them, so
let's do that.

Other commands, like `git fast-export`, currently allow all these
revision specific options even though some of them might not make sense,
as these commands also use setup_revisions() but do not check the
options that might be passed to this function.

So a way to fix those commands as well as git replay could be to improve
or refactor the setup_revisions() mechanism to let callers allow and
disallow options in a relevant way for them. Such improvements are
outside the scope of this work though.

Pathspecs, which are also accepted and eaten by setup_revisions(), are
likely to result in disconnected history. That could perhaps be useful,
but that would need tests and documentation, which can be added in
future work. So, while at it, let's disallow them too.

Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

2 files changed, 42 insertions, 1 deletions

diff --git a/builtin/replay.c b/builtin/replay.c
index c855e1a128..7b3c0f2bc4 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -119,7 +119,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct commit *commit;
 	struct merge_options merge_opt;
 	struct merge_result result;
-	int ret = 0;
+	int ret = 0, i;
 
 	const char * const replay_usage[] = {
 		N_("git replay --onto <newbase> <revision-range>..."),
@@ -135,6 +135,20 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
 
+	/*
+	 * TODO: For now, we reject any unknown or invalid option,
+	 * including revision related ones, like --not,
+	 * --first-parent, etc that would be allowed and eaten by
+	 * setup_revisions() below. In the future we should definitely
+	 * accept those that make sense and add related tests and doc
+	 * though.
+	 */
+	for (i = 0; i < argc; i++)
+		if (argv[i][0] == '-') {
+			error(_("invalid option: %s"), argv[i]);
+			usage_with_options(replay_usage, replay_options);
+		}
+
 	if (!onto_name) {
 		error(_("option --onto is mandatory"));
 		usage_with_options(replay_usage, replay_options);
@@ -150,6 +164,17 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		goto cleanup;
 	}
 
+	/*
+	 * TODO: For now, we reject any pathspec. (They are allowed
+	 * and eaten by setup_revisions() above.) In the future we
+	 * should definitely accept them and add related tests and doc
+	 * though.
+	 */
+	if (revs.prune_data.nr) {
+		error(_("invalid pathspec: %s"), revs.prune_data.items[0].match);
+		usage_with_options(replay_usage, replay_options);
+	}
+
 	/* requirements/overrides for revs */
 	revs.reverse = 1;
 	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index a1da4f9ef9..de6e40950e 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -80,4 +80,20 @@ test_expect_success 'using replay on bare repo to rebase with a conflict' '
 	test_expect_code 1 git -C bare replay --onto topic1 B..conflict
 '
 
+test_expect_success 'using replay with (for now) disallowed revision specific option --not' '
+	test_must_fail git replay --onto main topic2 --not topic1
+'
+
+test_expect_success 'using replay on bare repo with (for now) disallowed revision specific option --first-parent' '
+	test_must_fail git -C bare replay --onto main --first-parent topic1..topic2
+'
+
+test_expect_success 'using replay with disallowed pathspec' '
+	test_must_fail git replay --onto main topic1..topic2 A.t
+'
+
+test_expect_success 'using replay on bare repo with disallowed pathspec' '
+	test_must_fail git -C bare replay --onto main topic1..topic2 -- A.t
+'
+
 test_done