Jeff King <peff@xxxxxxxx> writes: > On Wed, Dec 20, 2023 at 06:46:51PM -0800, Junio C Hamano wrote: > >> Josh Steadmon <steadmon@xxxxxxxxxx> writes: >> >> > I can confirm that this fixes an issue noticed by sparse-checkout users >> > at $DAYJOB. Looks good to me. Thanks! >> >> Heh, there is another one that is not converted in the same file for >> "check-rules" subcommand, so the posted patch is way incomplete, I >> think. > > Yeah. I think it is in the same boat as the other two, in that I believe > that the KEEP_UNKNOWN_OPT flag is counter-productive and should just be > dropped. If we dropped KEEP_UNKNOWN_OPT, however, the pattern that is currently accepted will stop working, e.g., $ git sparse-checkout [add/set] --[no-]cone --foo bar as we would barf with "--foo: unknown option", and the users who are used to this sloppy command line parsing we had for the past few years must now write "--end-of-options" before "--foo". After all, the reason why the original authors of this code used KEEP_UNKNOWN is likely to deal with path patterns that begin with dashes. The patch in the message that started this thread may not be correct, either, I am afraid. For either of these: $ git sparse-checkout [add/set] --[no-]cone foo --end-of-options bar $ git sparse-checkout [add/set] --[no-]cone --foo --end-of-options bar we would see that "foo" (or "--foo") is not "--end-of-options", and we end up using three patterns "foo" (or "--foo"), "--end-of-options", and "bar", I suspect. I wonder if we should notice the "foo" or "--foo" that were not understood and error out, instead. But after all, it is not absolutely necessary to notice and barf. The ONLY practical use of the "--end-of-options" mechanism is to allow us to write (this applies to any git subcommand): #!/bin/sh git cmd --hard --coded --options --end-of-options "$@" in scripts to protect the intended operation from mistaking the end-user input as options. And with a script written carefully to do so, all the args that appear before "--end-of-options" would be recognizable by the command line parser. On the other hand, such a "notice and barf" would not help a script that is written without "--end-of-options", when it is fed "$@" that happens to begin with "--end-of-options". We would silently swallow "--end-of-options" without any chance to notice the lack of "--end-of-options" in the script. So I dunno. Here is an additional test on top of [1/3] <20231221065925.3234048-2-gitster@xxxxxxxxx> that demonstrates and summarizes the idea. t/t1090-sparse-checkout-scope.sh | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git c/t/t1090-sparse-checkout-scope.sh w/t/t1090-sparse-checkout-scope.sh index 5b96716235..da9534d222 100755 --- c/t/t1090-sparse-checkout-scope.sh +++ w/t/t1090-sparse-checkout-scope.sh @@ -54,6 +54,40 @@ test_expect_success 'return to full checkout of main' ' test "$(cat b)" = "modified" ' +test_expect_success 'sparse-checkout set command line parsing' ' + # baseline + git sparse-checkout disable && + git sparse-checkout set --no-cone "a*" && + echo "a*" >expect && + test_cmp .git/info/sparse-checkout expect && + + # unknown string that looks like a dashed option is + # taken as a mere pattern + git sparse-checkout disable && + git sparse-checkout set --no-cone --foo "a*" && + test_write_lines "--foo" "a*" >expect && + test_cmp .git/info/sparse-checkout expect && + + # --end-of-options can be used to protect parameters that + # potentially begin with dashes + set x --cone "a*" && shift && + git sparse-checkout disable && + git sparse-checkout set --no-cone --end-of-options "$@" && + test_write_lines "$@" >expect && + test_cmp .git/info/sparse-checkout expect && + + # but writing --end-of-options after what the command does not + # recognize is too late; it becomes one of the patterns, so + # that the end-user input that happens to be "--end-of-options" + # can be passed through. To be absoutely sure, you should write + # --end-of-options yourself before taking "$@" in. + set x --foo --end-of-options "a*" && shift && + git sparse-checkout disable && + git sparse-checkout set --no-cone "$@" && + test_write_lines "$@" >expect && + test_cmp .git/info/sparse-checkout expect +' + test_expect_success 'skip-worktree on files outside sparse patterns' ' git sparse-checkout disable && git sparse-checkout set --no-cone "a*" &&
