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

Re: [PATCH 9/9] t1092: test interesting sparse-checkout scenarios

From
Elijah Newren <newren@gmail.com>
Date
Jan 20, 2021, 19:40 UTC
Message-ID
<CABPp-BH=E4bR2K21e-p+ABTG02feDF7YNvrOKhsMW5Dtet6y7Q@mail.gmail.com>
In-Reply-To
<555e210dc03e3966d39b84d5fd5d409038422c40.1611161639.git.gitgitgadget@gmail.com>

On Wed, Jan 20, 2021 at 8:54 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 5 quoted lines
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> These also document some behaviors that differ from a full checkout, and
> possibly in a way that is not intended.

I'm in favor. I should turn some of my noted weird behaviors from [1] into testcases as well.

[1] https://lore.kernel.org/git/CABPp-BGJ_Nvi5TmgriD9Bh6eNXE2EDq2f8e8QKXAeYG3BxZafA@mail.gmail.com/
Show 164 quoted lines
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  t/t1092-sparse-checkout-compatibility.sh | 323 +++++++++++++++++++++++
>  1 file changed, 323 insertions(+)
>  create mode 100755 t/t1092-sparse-checkout-compatibility.sh
>
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> new file mode 100755
> index 00000000000..46f9dc2cdf3
> --- /dev/null
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -0,0 +1,323 @@
> +#!/bin/sh
> +
> +test_description='compare full workdir to sparse workdir'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +       git init initial-repo &&
> +       (
> +               cd initial-repo &&
> +               echo a >a &&
> +               echo "after deep" >e &&
> +               echo "after folder1" >g &&
> +               mkdir folder1 folder2 deep x &&
> +               mkdir deep/deeper1 deep/deeper2 &&
> +               mkdir deep/deeper1/deepest &&
> +               echo "after deeper1" >deep/e &&
> +               echo "after deepest" >deep/deeper1/e &&
> +               cp a folder1 &&
> +               cp a folder2 &&
> +               cp a deep &&
> +               cp a deep/deeper1 &&
> +               cp a deep/deeper2 &&
> +               cp a deep/deeper1/deepest &&
> +               git add . &&
> +               git commit -m "initial commit" &&
> +               git checkout -b base &&
> +               for dir in folder1 folder2 deep
> +               do
> +                       git checkout -b update-$dir &&
> +                       echo "updated $dir" >$dir/a &&
> +                       git commit -a -m "update $dir" || return 1
> +               done &&
> +
> +               git checkout -b rename-base base &&
> +               echo >folder1/larger-content <<-\EOF &&
> +               matching
> +               lines
> +               help
> +               inexact
> +               renames
> +               EOF
> +               cp folder1/larger-content folder2/ &&
> +               cp folder1/larger-content deep/deeper1/ &&
> +               git add . &&
> +               git commit -m "add interesting rename content" &&
> +
> +               git checkout -b rename-out-to-out rename-base &&
> +               mv folder1/a folder2/b &&
> +               mv folder1/larger-content folder2/edited-content &&
> +               echo >>folder2/edited-content &&
> +               git add . &&
> +               git commit -m "rename folder1/... to folder2/..." &&
> +
> +               git checkout -b rename-out-to-in rename-base &&
> +               mv folder1/a deep/deeper1/b &&
> +               mv folder1/larger-content deep/deeper1/edited-content &&
> +               echo >>deep/deeper1/edited-content &&
> +               git add . &&
> +               git commit -m "rename folder1/... to deep/deeper1/..." &&
> +
> +               git checkout -b rename-in-to-out rename-base &&
> +               mv deep/deeper1/a folder1/b &&
> +               mv deep/deeper1/larger-content folder1/edited-content &&
> +               echo >>folder1/edited-content &&
> +               git add . &&
> +               git commit -m "rename deep/deeper1/... to folder1/..." &&
> +
> +               git checkout -b deepest base &&
> +               echo "updated deepest" >deep/deeper1/deepest/a &&
> +               git commit -a -m "update deepest" &&
> +
> +               git checkout -f base &&
> +               git reset --hard
> +       )
> +'
> +
> +init_repos () {
> +       rm -rf full-checkout sparse-checkout sparse-index &&
> +
> +       # create repos in initial state
> +       cp -r initial-repo full-checkout &&
> +       git -C full-checkout reset --hard &&
> +
> +       cp -r initial-repo sparse-checkout &&
> +       git -C sparse-checkout reset --hard &&
> +       git -C sparse-checkout sparse-checkout init --cone &&
> +
> +       # initialize sparse-checkout definitions
> +       git -C sparse-checkout sparse-checkout set deep
> +}
> +
> +run_on_sparse () {
> +       (
> +               cd sparse-checkout &&
> +               $* >../sparse-checkout-out 2>../sparse-checkout-err
> +       )
> +}
> +
> +run_on_all () {
> +       (
> +               cd full-checkout &&
> +               $* >../full-checkout-out 2>../full-checkout-err
> +       ) &&
> +       run_on_sparse $*
> +}
> +
> +test_all_match () {
> +       run_on_all $* &&
> +       test_cmp full-checkout-out sparse-checkout-out &&
> +       test_cmp full-checkout-err sparse-checkout-err
> +}
> +
> +test_expect_success 'status with options' '
> +       init_repos &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git status --porcelain=v2 -z -u &&
> +       test_all_match git status --porcelain=v2 -uno &&
> +       run_on_all "touch README.md" &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git status --porcelain=v2 -z -u &&
> +       test_all_match git status --porcelain=v2 -uno &&
> +       test_all_match git add README.md &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git status --porcelain=v2 -z -u &&
> +       test_all_match git status --porcelain=v2 -uno
> +'
> +
> +test_expect_success 'add, commit, checkout' '
> +       init_repos &&
> +
> +       write_script edit-contents <<-\EOF &&
> +       echo text >>README.md
> +       EOF
> +       run_on_all "../edit-contents" &&
> +
> +       test_all_match git add README.md &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git commit -m "Add README.md" &&
> +
> +       test_all_match git checkout HEAD~1 &&
> +       test_all_match git checkout - &&
> +
> +       run_on_all "../edit-contents" &&
> +
> +       test_all_match git add -A &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git commit -m "Extend README.md" &&
> +
> +       test_all_match git checkout HEAD~1 &&
> +       test_all_match git checkout -
> +'

I was going to add comments here, but I noticed the next test had the same description and looked very similar, so I'll defer the comments...

Show 24 quoted lines
> +
> +test_expect_success 'add, commit, checkout' '
> +       init_repos &&
> +
> +       write_script edit-contents <<-\EOF &&
> +       echo text >>$1
> +       EOF
> +       run_on_all "../edit-contents README.md" &&
> +
> +       test_all_match git add README.md &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git commit -m "Add README.md" &&
> +
> +       test_all_match git checkout HEAD~1 &&
> +       test_all_match git checkout - &&
> +
> +       run_on_all "../edit-contents README.md" &&
> +
> +       test_all_match git add -A &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git commit -m "Extend README.md" &&
> +
> +       test_all_match git checkout HEAD~1 &&
> +       test_all_match git checkout - &&
Up to here, this test is identical to the previous one.  Why repeat it?
Show 12 quoted lines
> +
> +       run_on_all "../edit-contents deep/newfile" &&
> +
> +       test_all_match git status --porcelain=v2 -uno &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git add . &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git commit -m "add deep/newfile" &&
> +
> +       test_all_match git checkout HEAD~1 &&
> +       test_all_match git checkout -
> +'

Think out loud...so you are only adding files that were not previously tracked and that would have been part of the sparse cone. You aren't trying to add files that would be outside the sparse cone, or manually creating files missing from the working tree due to sparseness and then attempting to add them. (Which is fine, we have to start somewhere with our testing. Also, I think my testcases didn't look at the case you did, and only covered one of these other two cases.)

Show 30 quoted lines
> +
> +test_expect_success 'checkout and reset --hard' '
> +       init_repos &&
> +
> +       test_all_match git checkout update-folder1 &&
> +       test_all_match git status --porcelain=v2 &&
> +
> +       test_all_match git checkout update-deep &&
> +       test_all_match git status --porcelain=v2 &&
> +
> +       test_all_match git checkout -b reset-test &&
> +       test_all_match git reset --hard deepest &&
> +       test_all_match git reset --hard update-folder1 &&
> +       test_all_match git reset --hard update-folder2
> +'
> +
> +test_expect_success 'diff --staged' '
> +       init_repos &&
> +
> +       write_script edit-contents <<-\EOF &&
> +       echo text >>README.md
> +       EOF
> +       run_on_all "../edit-contents" &&
> +
> +       test_all_match git diff &&
> +       test_all_match git diff --staged &&
> +       test_all_match git add README.md &&
> +       test_all_match git diff &&
> +       test_all_match git diff --staged
> +'

Also a case where you're operating on a file that matches the sparsity patterns (in cone mode, everything in the root directory is included).

Show 9 quoted lines
> +test_expect_success 'diff with renames' '
> +       init_repos &&
> +
> +       for branch in rename-out-to-out rename-out-to-in rename-in-to-out
> +       do
> +               test_all_match git checkout rename-base &&
> +               test_all_match git checkout $branch -- .&&
> +               test_all_match git diff --staged &&
> +               test_all_match git diff --staged --find-renames || return 1

Aren't these last two lines the same? (diff.renames defaults to true ever since commit 5404c116aa, "diff: activate diff.renames by default", 2016-02-25) Are they only different because you have a tweaked config that turns off renames by default?

Perhaps the first diff line should have a --no-renames flag.
Show 25 quoted lines
> +       done
> +'
> +
> +test_expect_success 'log with pathspec outside sparse definition' '
> +       init_repos &&
> +
> +       test_all_match git log -- a &&
> +       test_all_match git log -- folder1/a &&
> +       test_all_match git log -- folder2/a &&
> +       test_all_match git log -- deep/a &&
> +       test_all_match git log -- deep/deeper1/a &&
> +       test_all_match git log -- deep/deeper1/deepest/a &&
> +
> +       test_all_match git checkout update-folder1 &&
> +       test_all_match git log -- folder1/a
> +'
> +
> +test_expect_success 'blame with pathspec inside sparse definition' '
> +       init_repos &&
> +
> +       test_all_match git blame a &&
> +       test_all_match git blame deep/a &&
> +       test_all_match git blame deep/deeper1/a &&
> +       test_all_match git blame deep/deeper1/deepest/a
> +'
Good check.
On a side note going back to a piece of the other thread I didn't get
a response to, I'm still curious whether
    git blame -C -C $PATH_INSIDE_SPARSE_DEFINITION
should (optionally?) behave differently in a sparse checkout.  In
particular, should it limit its copy detection to other paths also in
the sparse checkout, or should it always search all other files within
the repository for copied lines?  Searching just within the sparse
checkout seems like it could be a really nice performance
optimization.
> +
> +# TODO: blame currently does not support blaming files outside of the
> +# sparse definition. It complains that the file doesn't exist locally.

Nice catch. Yeah, blame tries to check the local working copy for changes, and shows those lines with a changed in commit 0000000000. We should add a check that says that if the file is SKIP_WORKTREE, then we treat it the same as `git blame $PATH HEAD`.

Show 11 quoted lines
> +test_expect_failure 'blame with pathspec outside sparse definition' '
> +       init_repos &&
> +
> +       test_all_match git blame folder1/a &&
> +       test_all_match git blame folder2/a &&
> +       test_all_match git blame deep/deeper2/a &&
> +       test_all_match git blame deep/deeper2/deepest/a
> +'
> +
> +# TODO: reset currently does not behave as expected when in a
> +# sparse-checkout.

I'm going to go to test this out to see what it does. It's the first testcase you listed that I didn't know how it worked and couldn't figure it out from your comments. However it turns out, definitely a good test to have.

Show 57 quoted lines
> +test_expect_failure 'checkout and reset (mixed)' '
> +       init_repos &&
> +
> +       test_all_match git checkout -b reset-test update-deep &&
> +       test_all_match git reset deepest &&
> +       test_all_match git reset update-folder1 &&
> +       test_all_match git reset update-folder2
> +'
> +
> +test_expect_success 'merge' '
> +       init_repos &&
> +
> +       test_all_match git checkout -b merge update-deep &&
> +       test_all_match git merge -m "folder1" update-folder1 &&
> +       test_all_match git rev-parse HEAD^{tree} &&
> +       test_all_match git merge -m "folder2" update-folder2 &&
> +       test_all_match git rev-parse HEAD^{tree}
> +'
> +
> +test_expect_success 'merge with outside renames' '
> +       init_repos &&
> +
> +       for type in out-to-out out-to-in in-to-out
> +       do
> +               test_all_match git reset --hard &&
> +               test_all_match git checkout -f -b merge-$type update-deep &&
> +               test_all_match git merge -m "$type" rename-$type &&
> +               test_all_match git rev-parse HEAD^{tree} || return 1
> +       done
> +'
> +
> +test_expect_success 'clean' '
> +       init_repos &&
> +
> +       echo bogus >>.gitignore &&
> +       run_on_all cp ../.gitignore . &&
> +       test_all_match git add .gitignore &&
> +       test_all_match git commit -m ignore-bogus-files &&
> +
> +       run_on_sparse mkdir folder1 &&
> +       run_on_all touch folder1/bogus &&
> +
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git clean -f &&
> +       test_all_match git status --porcelain=v2 &&
> +
> +       test_all_match git clean -xf &&
> +       test_all_match git status --porcelain=v2 &&
> +
> +       test_all_match git clean -xdf &&
> +       test_all_match git status --porcelain=v2 &&
> +
> +       test_path_is_dir sparse-checkout/folder1
> +'
> +
> +test_done
> --

I made lots of comments, but overall these tests look good to me other than just one question about test duplication and another about using --no-renames for diff when rename detection isn't wanted.

Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee
Message 26 of 65 in “More index cleanups”
  1. 0/9 More index cleanupsDerrick Stolee via GitGitGadget, Jan 20, 2021
  2. 1/9 cache-tree: clean up cache_tree_update()Derrick Stolee via GitGitGadget, Jan 20, 2021
  3. Elijah NewrenJan 20, 2021
  4. Derrick StoleeJan 20, 2021
  5. 3/9 fsmonitor: de-duplicate BUG()s around dirty bitsDerrick Stolee via GitGitGadget, Jan 20, 2021
  6. Elijah NewrenJan 20, 2021
  7. Chris TorekJan 21, 2021
  8. Derrick StoleeJan 21, 2021
  9. 4/9 repository: add repo reference to index_stateDerrick Stolee via GitGitGadget, Jan 20, 2021
  10. Elijah NewrenJan 20, 2021
  11. Derrick StoleeJan 20, 2021
  12. Elijah NewrenJan 20, 2021
  13. 5/9 name-hash: use trace2 regions for initDerrick Stolee via GitGitGadget, Jan 20, 2021
  14. Elijah NewrenJan 20, 2021
  15. 6/9 sparse-checkout: load sparse-checkout patternsDerrick Stolee via GitGitGadget, Jan 20, 2021
  16. Elijah NewrenJan 20, 2021
  17. 7/9 sparse-checkout: hold pattern list in indexDerrick Stolee via GitGitGadget, Jan 20, 2021
  18. Elijah NewrenJan 20, 2021
  19. Derrick StoleeJan 20, 2021
  20. 8/9 test-lib: test_region looks for trace2 regionsDerrick Stolee via GitGitGadget, Jan 20, 2021
  21. Elijah NewrenJan 20, 2021
  22. Derrick StoleeJan 20, 2021
  23. 2/9 cache-tree: extract subtree_pos()Derrick Stolee via GitGitGadget, Jan 20, 2021
  24. Elijah NewrenJan 20, 2021
  25. 9/9 t1092: test interesting sparse-checkout scenariosDerrick Stolee via GitGitGadget, Jan 20, 2021
  26. Elijah NewrenJan 20, 2021
  27. Derrick StoleeJan 21, 2021
  28. 0/8 More index cleanupsDerrick Stolee via GitGitGadget, Jan 22, 2021
  29. 7/8 test-lib: test_region looks for trace2 regionsDerrick Stolee via GitGitGadget, Jan 22, 2021
  30. Junio C HamanoJan 22, 2021
  31. Derrick StoleeJan 23, 2021
  32. Junio C HamanoJan 23, 2021
  33. 8/8 t1092: test interesting sparse-checkout scenariosDerrick Stolee via GitGitGadget, Jan 22, 2021
  34. 6/8 sparse-checkout: load sparse-checkout patternsDerrick Stolee via GitGitGadget, Jan 22, 2021
  35. 4/8 repository: add repo reference to index_stateDerrick Stolee via GitGitGadget, Jan 22, 2021
  36. Junio C HamanoJan 22, 2021
  37. 5/8 name-hash: use trace2 regions for initDerrick Stolee via GitGitGadget, Jan 22, 2021
  38. 3/8 fsmonitor: de-duplicate BUG()s around dirty bitsDerrick Stolee via GitGitGadget, Jan 22, 2021
  39. Junio C HamanoJan 22, 2021
  40. 2/8 cache-tree: extract subtree_pos()Derrick Stolee via GitGitGadget, Jan 22, 2021
  41. 1/8 cache-tree: clean up cache_tree_update()Derrick Stolee via GitGitGadget, Jan 22, 2021
  42. Junio C HamanoJan 22, 2021
  43. Elijah NewrenJan 22, 2021
  44. Derrick StoleeJan 23, 2021
  45. 0/9 More index cleanupsDerrick Stolee via GitGitGadget, Jan 23, 2021
  46. 3/9 cache-tree: extract subtree_pos()Derrick Stolee via GitGitGadget, Jan 23, 2021
  47. 2/9 cache-tree: simplify verify_cache() prototypeDerrick Stolee via GitGitGadget, Jan 23, 2021
  48. Elijah NewrenJan 23, 2021
  49. Derrick StoleeJan 23, 2021
  50. Elijah NewrenJan 23, 2021
  51. Junio C HamanoJan 23, 2021
  52. Junio C HamanoJan 23, 2021
  53. Derrick StoleeJan 23, 2021
  54. 4/9 fsmonitor: de-duplicate BUG()s around dirty bitsDerrick Stolee via GitGitGadget, Jan 23, 2021
  55. 1/9 cache-tree: clean up cache_tree_update()Derrick Stolee via GitGitGadget, Jan 23, 2021
  56. 5/9 repository: add repo reference to index_stateDerrick Stolee via GitGitGadget, Jan 23, 2021
  57. 6/9 name-hash: use trace2 regions for initDerrick Stolee via GitGitGadget, Jan 23, 2021
  58. 8/9 test-lib: test_region looks for trace2 regionsDerrick Stolee via GitGitGadget, Jan 23, 2021
  59. Derrick StoleeJan 23, 2021
  60. 7/9 sparse-checkout: load sparse-checkout patternsDerrick Stolee via GitGitGadget, Jan 23, 2021
  61. 9/9 t1092: test interesting sparse-checkout scenariosDerrick Stolee via GitGitGadget, Jan 23, 2021
  62. Elijah NewrenJan 25, 2021
  63. Elijah NewrenJan 23, 2021
  64. Derrick StoleeJan 23, 2021
  65. Junio C HamanoJan 23, 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.