git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/5] add: ignore outside the sparse-checkout in refresh()

From
Elijah Newren <newren@gmail.com>
Date
Jul 23, 2021, 19:46 UTC
Message-ID
<CABPp-BHNS=6xX2=NxZnoVgtE8MqR3Tj9RE9m9ocV_+oHRjogoA@mail.gmail.com>
In-Reply-To
<76066a78ce077add4bc69819b017516e12ac0d45.1626901619.git.gitgitgadget@gmail.com>

On Wed, Jul 21, 2021 at 2:07 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 46 quoted lines
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> Since b243012 (refresh_index(): add flag to ignore SKIP_WORKTREE
> entries, 2021-04-08), 'git add --refresh <path>' will output a warning
> message when the path is outside the sparse-checkout definition. The
> implementation of this warning happened in parallel with the
> sparse-index work to add ensure_full_index() calls throughout the
> codebase.
>
> Update this loop to have the proper logic that checks to see if the
> pathspec is outside the sparse-checkout definition. This avoids the need
> to expand the sparse directory entry and determine if the path is
> tracked, untracked, or ignored. We simply avoid updating the stat()
> information because there isn't even an entry that matches the path!
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  builtin/add.c                            | 10 +++++++++-
>  t/t1092-sparse-checkout-compatibility.sh |  6 +-----
>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index c76e6ddd359..d512ece655b 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -192,13 +192,21 @@ static int refresh(int verbose, const struct pathspec *pathspec)
>         struct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;
>         int flags = REFRESH_IGNORE_SKIP_WORKTREE |
>                     (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);
> +       struct pattern_list pl = { 0 };
> +       int sparse_checkout_enabled = !get_sparse_checkout_patterns(&pl);
>
>         seen = xcalloc(pathspec->nr, 1);
>         refresh_index(&the_index, flags, pathspec, seen,
>                       _("Unstaged changes after refreshing the index:"));
>         for (i = 0; i < pathspec->nr; i++) {
>                 if (!seen[i]) {
> -                       if (matches_skip_worktree(pathspec, i, &skip_worktree_seen)) {
> +                       const char *path = pathspec->items[i].original;
> +                       int dtype = DT_REG;
> +
> +                       if (matches_skip_worktree(pathspec, i, &skip_worktree_seen) ||
> +                           (sparse_checkout_enabled &&
> +                            !path_matches_pattern_list(path, strlen(path), NULL,
> +                                                       &dtype, &pl, &the_index))) {

I was slightly worried from the description in the commit message about the case where you have a file without the SKIP_WORKTREE bit set despite not matching sparsity paths. I was worried that you'd skip refreshing it, but I tweaked your testcases and couldn't trigger it.

Show 29 quoted lines
>                                 string_list_append(&only_match_skip_worktree,
>                                                    pathspec->items[i].original);
>                         } else {
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 73c48a71d89..c61424e2074 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -347,7 +347,7 @@ test_expect_success 'status/add: outside sparse cone' '
>         test_all_match git commit -m folder1/newer
>  '
>
> -test_expect_failure 'add: pathspec within sparse directory' '
> +test_expect_success 'add: pathspec within sparse directory' '
>         init_repos &&
>
>         run_on_sparse mkdir folder1 &&
> @@ -357,10 +357,6 @@ test_expect_failure 'add: pathspec within sparse directory' '
>         # This "git add folder1/a" fails with a warning
>         # in the sparse repos, differing from the full
>         # repo. This is intentional.
> -       #
> -       # However, in the sparse-index, folder1/a does not
> -       # match any cache entry and fails with a different
> -       # error message. This needs work.
>         test_sparse_match test_must_fail git add folder1/a &&
>         test_sparse_match test_must_fail git add --refresh folder1/a &&
>         test_all_match git status --porcelain=v2
> --
> gitgitgadget
This and Patch 4/5 look good to me.
Previous: Derrick Stolee via GitGitGadgetNext: Elijah Newren
Message 17 of 38 in “Sparse Index: Integrate with 'git add'”
  1. 0/5 Sparse Index: Integrate with 'git add'Derrick Stolee via GitGitGadget, Jul 21, 2021
  2. 1/5 t1092: test merge conflicts outside coneDerrick Stolee via GitGitGadget, Jul 21, 2021
  3. Elijah NewrenJul 23, 2021
  4. Eric SunshineJul 23, 2021
  5. Elijah NewrenJul 23, 2021
  6. Derrick StoleeJul 26, 2021
  7. 2/5 add: allow operating on a sparse-only indexDerrick Stolee via GitGitGadget, Jul 21, 2021
  8. Junio C HamanoJul 21, 2021
  9. Derrick StoleeJul 21, 2021
  10. Elijah NewrenJul 23, 2021
  11. Derrick StoleeJul 26, 2021
  12. Derrick StoleeJul 26, 2021
  13. 3/5 pathspec: stop calling ensure_full_indexDerrick Stolee via GitGitGadget, Jul 21, 2021
  14. Elijah NewrenJul 23, 2021
  15. 4/5 t1092: 'git add --refresh' difference with sparse-indexDerrick Stolee via GitGitGadget, Jul 21, 2021
  16. 5/5 add: ignore outside the sparse-checkout in refresh()Derrick Stolee via GitGitGadget, Jul 21, 2021
  17. Elijah NewrenJul 23, 2021
  18. Elijah NewrenJul 23, 2021
  19. Elijah NewrenJul 23, 2021
  20. 0/5 Sparse Index: Integrate with 'git add'Derrick Stolee via GitGitGadget, Jul 26, 2021
  21. 1/5 t1092: test merge conflicts outside coneDerrick Stolee via GitGitGadget, Jul 26, 2021
  22. 2/5 add: allow operating on a sparse-only indexDerrick Stolee via GitGitGadget, Jul 26, 2021
  23. 4/5 add: ignore outside the sparse-checkout in refresh()Derrick Stolee via GitGitGadget, Jul 26, 2021
  24. 3/5 pathspec: stop calling ensure_full_indexDerrick Stolee via GitGitGadget, Jul 26, 2021
  25. 5/5 add: remove ensure_full_index() with --renormalizeDerrick Stolee via GitGitGadget, Jul 26, 2021
  26. Elijah NewrenJul 28, 2021
  27. Derrick StoleeJul 29, 2021
  28. Elijah NewrenJul 29, 2021
  29. Derrick StoleeJul 29, 2021
  30. Elijah NewrenJul 30, 2021
  31. 0/5 Sparse Index: Integrate with 'git add'Derrick Stolee via GitGitGadget, Jul 29, 2021
  32. 2/5 add: allow operating on a sparse-only indexDerrick Stolee via GitGitGadget, Jul 29, 2021
  33. 1/5 t1092: test merge conflicts outside coneDerrick Stolee via GitGitGadget, Jul 29, 2021
  34. 4/5 add: ignore outside the sparse-checkout in refresh()Derrick Stolee via GitGitGadget, Jul 29, 2021
  35. 3/5 pathspec: stop calling ensure_full_indexDerrick Stolee via GitGitGadget, Jul 29, 2021
  36. 5/5 add: remove ensure_full_index() with --renormalizeDerrick Stolee via GitGitGadget, Jul 29, 2021
  37. Elijah NewrenJul 29, 2021
  38. Junio C HamanoJul 29, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.