sparse-checkout: pay attention to prefix for {set, add}

In cone mode, non-option arguments to set & add are clearly paths, and as such, we should pay attention to prefix. In non-cone mode, it is not clear that folks intend to provide paths since the inputs are gitignore-style patterns. Paying attention to prefix would prevent folks from doing things like git sparse-checkout add /.gitattributes git sparse-checkout add '/toplevel-dir/*' In fact, the former will result in fatal: '/.gitattributes' is outside repository... while the later will result in fatal: Invalid path '/toplevel-dir': No such file or directory despite the fact that both are valid gitignore-style patterns that would select real files if added to the sparse-checkout file. This might lead people to just use the path without the leading slash, potentially resulting in them grabbing files with the same name throughout the directory hierarchy contrary to their expectations. See also [1] and [2]. Adding prefix seems to just be fraught with error; so for now simply throw an error in non-cone mode when sparse-checkout set/add are run from a subdirectory. [1] https://lore.kernel.org/git/e1934710-e228-adc4-d37c-f706883bd27c@gmail.com/ [2] https://lore.kernel.org/git/CABPp-BHXZ-XLxY0a3wCATfdq=6-EjW62RzbxKAoFPeXfJswD2w@mail.gmail.com/ Helped-by: Junio Hamano <gitster@pobox.com> Reviewed-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Feb 19, 2022 at 16:44 UTC bb8b5e9a90d607ec6144527c5019f56b6a81d862
2 files changed +67
builtin/sparse-checkout.c
+26
@@ -684,6 +684,28 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
684 return result;
685 }
686
687 +static void sanitize_paths(int argc, const char **argv, const char *prefix)
688 +{
689 + if (!argc)
690 + return;
691 +
692 + if (prefix && *prefix && core_sparse_checkout_cone) {
693 + /*
694 + * The args are not pathspecs, so unfortunately we
695 + * cannot imitate how cmd_add() uses parse_pathspec().
696 + */
697 + int i;
698 + int prefix_len = strlen(prefix);
699 +
700 + for (i = 0; i < argc; i++)
701 + argv[i] = prefix_path(prefix, prefix_len, argv[i]);
702 + }
703 +
704 + if (prefix && *prefix && !core_sparse_checkout_cone)
705 + die(_("please run from the toplevel directory in non-cone mode"));
706 +
707 +}
708 +
709 static char const * const builtin_sparse_checkout_add_usage[] = {
710 N_("git sparse-checkout add (--stdin | <patterns>)"),
711 NULL
@@ -711,6 +733,8 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
733 builtin_sparse_checkout_add_usage,
734 PARSE_OPT_KEEP_UNKNOWN);
735
736 + sanitize_paths(argc, argv, prefix);
737 +
738 return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
739 }
740
@@ -762,6 +786,8 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
786 if (!core_sparse_checkout_cone && argc == 0) {
787 argv = default_patterns;
788 argc = default_patterns_nr;
789 + } else {
790 + sanitize_paths(argc, argv, prefix);
791 }
792
793 return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
t/t1091-sparse-checkout-builtin.sh
+41
@@ -798,4 +798,45 @@ test_expect_success 'malformed cone-mode patterns' '
798 grep "warning: disabling cone pattern matching" err
799 '
800
801 +test_expect_success 'set from subdir pays attention to prefix' '
802 + git -C repo sparse-checkout disable &&
803 + git -C repo/deep sparse-checkout set --cone deeper2 ../folder1 &&
804 +
805 + git -C repo sparse-checkout list >actual &&
806 +
807 + cat >expect <<-\EOF &&
808 + deep/deeper2
809 + folder1
810 + EOF
811 + test_cmp expect actual
812 +'
813 +
814 +test_expect_success 'add from subdir pays attention to prefix' '
815 + git -C repo sparse-checkout set --cone deep/deeper2 &&
816 + git -C repo/deep sparse-checkout add deeper1/deepest ../folder1 &&
817 +
818 + git -C repo sparse-checkout list >actual &&
819 +
820 + cat >expect <<-\EOF &&
821 + deep/deeper1/deepest
822 + deep/deeper2
823 + folder1
824 + EOF
825 + test_cmp expect actual
826 +'
827 +
828 +test_expect_success 'set from subdir in non-cone mode throws an error' '
829 + git -C repo sparse-checkout disable &&
830 + test_must_fail git -C repo/deep sparse-checkout set --no-cone deeper2 ../folder1 2>error &&
831 +
832 + grep "run from the toplevel directory in non-cone mode" error
833 +'
834 +
835 +test_expect_success 'set from subdir in non-cone mode throws an error' '
836 + git -C repo sparse-checkout set --no-cone deep/deeper2 &&
837 + test_must_fail git -C repo/deep sparse-checkout add deeper1/deepest ../folder1 2>error &&
838 +
839 + grep "run from the toplevel directory in non-cone mode" error
840 +'
841 +
842 test_done