Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add

[Josh Steadmon <steadmon@google.com>]

On 2023.12.20 15:19, Junio C Hamano wrote:
> 93851746 (parse-options: decouple "--end-of-options" and "--",
> 2023-12-06) updated the world order to make callers of parse-options
> that set PARSE_OPT_KEEP_UNKNOWN_OPT responsible for deciding what to
> do with "--end-of-options" they may see after parse_options() returns.
> 
> This unfortunately broke "sparse-checkout set/add", and from this
> invocation,
> 
>   "git sparse-checkout [add|set] --[no-]cone --end-of-options pattern..."
> 
> we now see "--end-of-options" listed in .git/info/sparse-checkout as if
> it is one of the path patterns.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/sparse-checkout.c          | 9 +++++++++
>  t/t1090-sparse-checkout-scope.sh   | 8 ++++++++
>  t/t1091-sparse-checkout-builtin.sh | 2 +-
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git c/builtin/sparse-checkout.c w/builtin/sparse-checkout.c
> index 80227f3df1..226a458b10 100644
> --- c/builtin/sparse-checkout.c
> +++ w/builtin/sparse-checkout.c
> @@ -776,6 +776,10 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
>  			     builtin_sparse_checkout_add_usage,
>  			     PARSE_OPT_KEEP_UNKNOWN_OPT);
>  
> +	if (argc && !strcmp(*argv, "--end-of-options")) {
> +		argc--;
> +		argv++;
> +	}
>  	sanitize_paths(argc, argv, prefix, add_opts.skip_checks);
>  
>  	return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
> @@ -823,6 +827,11 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>  			     builtin_sparse_checkout_set_usage,
>  			     PARSE_OPT_KEEP_UNKNOWN_OPT);
>  
> +	if (argc && !strcmp(*argv, "--end-of-options")) {
> +		argc--;
> +		argv++;
> +	}
> +
>  	if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
>  		return 1;
>  
> diff --git c/t/t1090-sparse-checkout-scope.sh w/t/t1090-sparse-checkout-scope.sh
> index 3a14218b24..5b96716235 100755
> --- c/t/t1090-sparse-checkout-scope.sh
> +++ w/t/t1090-sparse-checkout-scope.sh
> @@ -57,6 +57,14 @@ test_expect_success 'return to full checkout of main' '
>  test_expect_success 'skip-worktree on files outside sparse patterns' '
>  	git sparse-checkout disable &&
>  	git sparse-checkout set --no-cone "a*" &&
> +	cat .git/info/sparse-checkout >wo-eoo &&
> +
> +	git sparse-checkout disable &&
> +	git sparse-checkout set --no-cone --end-of-options "a*" &&
> +	cat .git/info/sparse-checkout >w-eoo &&
> +
> +	test_cmp wo-eoo w-eoo &&
> +
>  	git checkout-index --all --ignore-skip-worktree-bits &&
>  
>  	git ls-files -t >output &&
> diff --git c/t/t1091-sparse-checkout-builtin.sh w/t/t1091-sparse-checkout-builtin.sh
> index f67611da28..e33a6ed1b4 100755
> --- c/t/t1091-sparse-checkout-builtin.sh
> +++ w/t/t1091-sparse-checkout-builtin.sh
> @@ -334,7 +334,7 @@ test_expect_success 'cone mode: set with nested folders' '
>  
>  test_expect_success 'cone mode: add independent path' '
>  	git -C repo sparse-checkout set deep/deeper1 &&
> -	git -C repo sparse-checkout add folder1 &&
> +	git -C repo sparse-checkout add --end-of-options folder1 &&
>  	cat >expect <<-\EOF &&
>  	/*
>  	!/*/

I can confirm that this fixes an issue noticed by sparse-checkout users
at $DAYJOB. Looks good to me. Thanks!

Reviewed-by: Josh Steadmon <steadmon@google.com>