From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 22 Feb 2024 16:35:54 GMT
Subject: Re: [PATCH v2 7/8] cherry-pick: enforce `--keep-redundant-commits` incompatibility
Message-ID: <8c2eec1b-9a59-4739-a903-b2e8955f3ff5@gmail.com>
In-Reply-To: <20240210074859.552497-8-brianmlyles@gmail.com>

Hi Brian

On 10/02/2024 07:43, Brian Lyles wrote:
> When `--keep-redundant-commits` was added in  b27cfb0d8d
> (git-cherry-pick: Add keep-redundant-commits option, 2012-04-20), it was
> not marked as incompatible with the various operations needed to
> continue or exit a cherry-pick (`--continue`, `--skip`, `--abort`, and
> `--quit`).
> 
> Enforce this incompatibility via `verify_opt_compatible` like we do for
> the other various options.
> 
> Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
> ---
> 
> This commit was not present in v1 either. It addresses an existing issue
> that I noticed after Phillip pointed out the same deficiency for my new
> `--empty` option introduced in the ultimate commit in this series.

Well spotted, do we really need a new test file just for this though? I 
wonder if the new test would be better off living in 
t3505-cherry-pick-empty.sh or t3507-cherry-pick-conflict.sh

Best Wishes

Phillip

> [1]: https://lore.kernel.org/git/CAHPHrSf+joHe6ikErHLgWrk-_qjSROS-dXCHagxWGDAF=2deDg@mail.gmail.com/
> 
>   builtin/revert.c                            |  1 +
>   t/t3515-cherry-pick-incompatible-options.sh | 34 +++++++++++++++++++++
>   2 files changed, 35 insertions(+)
>   create mode 100755 t/t3515-cherry-pick-incompatible-options.sh
> 
> diff --git a/builtin/revert.c b/builtin/revert.c
> index d83977e36e..48c426f277 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -163,6 +163,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
>   				"--ff", opts->allow_ff,
>   				"--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
>   				"--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
> +				"--keep-redundant-commits", opts->keep_redundant_commits,
>   				NULL);
>   	}
>   
> diff --git a/t/t3515-cherry-pick-incompatible-options.sh b/t/t3515-cherry-pick-incompatible-options.sh
> new file mode 100755
> index 0000000000..6100ab64fd
> --- /dev/null
> +++ b/t/t3515-cherry-pick-incompatible-options.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='test if cherry-pick detects and aborts on incompatible options'
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +
> +	echo first > file1 &&
> +	git add file1 &&
> +	test_tick &&
> +	git commit -m "first" &&
> +
> +	echo second > file1 &&
> +	git add file1 &&
> +	test_tick &&
> +	git commit -m "second"
> +'
> +
> +test_expect_success '--keep-redundant-commits is incompatible with operations' '
> +	test_must_fail git cherry-pick HEAD 2>output &&
> +	test_grep "The previous cherry-pick is now empty" output &&
> +	test_must_fail git cherry-pick --keep-redundant-commits --continue 2>output &&
> +	test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --continue" output &&
> +	test_must_fail git cherry-pick --keep-redundant-commits --skip 2>output &&
> +	test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --skip" output &&
> +	test_must_fail git cherry-pick --keep-redundant-commits --abort 2>output &&
> +	test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --abort" output &&
> +	test_must_fail git cherry-pick --keep-redundant-commits --quit 2>output &&
> +	test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --quit" output &&
> +	git cherry-pick --abort
> +'
> +
> +test_done