Re: [PATCH v2 6/6] t5620: test backfill's unknown argument handling

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <stolee@gmail.com>
>
> Before the recent changes to parse rev-list arguments inside of 'git
> backfill', the builtin would take arbitrary arguments without complaint (and
> ignore them). This was noticed and a patch was sent [1] which motivates this
> change to encode this behavior in test.
>
> [1] https://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/
>
> Reported-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  t/t5620-backfill.sh | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh
> index c6f54ee91c..85740f1f13 100755
> --- a/t/t5620-backfill.sh
> +++ b/t/t5620-backfill.sh
> @@ -7,6 +7,14 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
>  . ./test-lib.sh
>  
> +test_expect_success 'backfill rejects unexpected arguments' '
> +	test_must_fail git backfill unexpected-arg 2>err &&
> +	test_grep "ambiguous argument .*unexpected-arg" err &&
> +
> +	test_must_fail git backfill --all --firt-parent unexpected-arg 2>err &&
> +	test_grep "ambiguous argument .*unexpected-arg" err
> +'

Hmph, I would have expected that an earlier --firt-parent on the
command line would trigger "unknown option" instead.

Having said that, if the code lets the setup_revisions() parse the
command line, the usual "unless disambiguated with a double-dash
'--', stop at the first non-revision and take everything as paths
but for safety all of them must refer to an existing path in the
working tree" behaviour should trigger, and it is not specific to
"backfill", and may already be tested centrally (if not, I do not
object to such a new set of tests).

For any cmd that take revisions and pathspec (e.g., log, rev-list,
grep) these should hold true:

  $ git $cmd [<options>]... Makefile HEAD

    Without disambiguation the command should say "Ah, Makefile
    is not a revision, so we will see no more revisions, and
    everything, including the current one we are looking at, must be
    an existing path on the working tree", and barfs on HEAD that
    does not exist as a file/directory.

  $ git $cmd [<options>]... Makefile -- HEAD

    With disambiguation, the command should verify everything before
    the double-dash to be a rev, and barf that Makefile is not a
    rev.

  $ git $cmd [<options>]... -- Makefile HEAD

    With disambiguation, the command should take everything after
    the double-dash to be a pathspec element without barfing.  After
    all, it may be referring to a path that used to exist in some
    revision the command will look at.

Thanks.