Re: [PATCH 3/7] sparse-checkout: pay attention to prefix for {set, add}

>
> On 2/12/2022 7:39 PM, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > 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.  However, these
> > commands can be run successfully from the toplevel directory, and many
> > gitignore-style patterns ARE paths, and bash completion seems to be
> > suggesting directories and files, so perhaps for consistency we pay
> > attention to the prefix?  It's not clear what is okay here, but maybe
> > that's yet another reason to deprecate non-cone mode as we will do later
> > in this series.
> >
> > For now, incorporate prefix into the positional arguments for either
> > cone or non-cone mode.  For additional discussion of this issue, see
> > https://lore.kernel.org/git/29f0410e-6dfa-2e86-394d-b1fb735e7608@gmail.com/
>
> Perhaps this was covered in the issue, but for non-cone mode, it
> matters if there is a leading slash or not in the pattern. Will
> this change make it impossible for a user to input that distinction?
>
> Will there still be a difference between:
>
>         git sparse-checkout set --no-cone /.vs/
>
> and
>
>         git sparse-checkout set --no-cone .vs/
>
> ?

> > Helped-by: Junio Hamano <gitster@pobox.com>
> > Signed-off-by: Elijah Newren <newren@gmail.com>
>
> This could probably use a
>
>   Reported-by: Lessley Dennington <lessleydennington@gmail.com>

> > +static void sanitize_paths(int argc, const char **argv, const char *prefix)
> > +{
> > +     if (!argc)
> > +             return;
> > +
> > +     if (prefix && *prefix) {
> > +             /*
> > +              * The args are not pathspecs, so unfortunately we
> > +              * cannot imitate how cmd_add() uses parse_pathspec().
> > +              */
> > +             int i;
> > +             int prefix_len = strlen(prefix);
> > +
> > +             for (i = 0; i < argc; i++)
> > +                     argv[i] = prefix_path(prefix, prefix_len, argv[i]);
> > +     }
> > +}
> > +
> >  static char const * const builtin_sparse_checkout_add_usage[] = {
> >       N_("git sparse-checkout add (--stdin | <patterns>)"),
> >       NULL
> > @@ -708,6 +726,8 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
> >                            builtin_sparse_checkout_add_usage,
> >                            PARSE_OPT_KEEP_UNKNOWN);
> >
> > +     sanitize_paths(argc, argv, prefix);
> > +
> >       return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
> >  }
> >
> > @@ -759,6 +779,8 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> >       if (!core_sparse_checkout_cone && argc == 0) {
> >               argv = default_patterns;
> >               argc = default_patterns_nr;
> > +     } else {
> > +             sanitize_paths(argc, argv, prefix);
> >       }
>
> Code changes appear to do as described: apply the prefix everywhere
> it matters, no matter the mode.
>
> > +test_expect_success 'set from subdir pays attention to prefix' '
> > +     git -C repo sparse-checkout disable &&
> > +     git -C repo/deep sparse-checkout set --cone deeper2 ../folder1 &&
> > +
> > +     git -C repo sparse-checkout list >actual &&
> > +
> > +     cat >expect <<-\EOF &&
> > +     deep/deeper2
> > +     folder1
> > +     EOF
> > +     test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'add from subdir pays attention to prefix' '
> > +     git -C repo sparse-checkout set --cone deep/deeper2 &&
> > +     git -C repo/deep sparse-checkout add deeper1/deepest ../folder1 &&
> > +
> > +     git -C repo sparse-checkout list >actual &&
> > +
> > +     cat >expect <<-\EOF &&
> > +     deep/deeper1/deepest
> > +     deep/deeper2
> > +     folder1
> > +     EOF
> > +     test_cmp expect actual
> > +'
> > +
> >  test_done
>
> These tests could use a non-cone-mode version to demonstrate the behavior
> in that mode.