Message ID | f6cf05a5bee9e4ebc174bab0385a13cc1cdb4014.1650908958.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Sparse index: integrate with 'git stash' | expand |
"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Victoria Dye <vdye@github.com> > > Enable sparse index in 'git stash' by disabling > 'command_requires_full_index'. > > With sparse index enabled, some subcommands of 'stash' work without > expanding the index, e.g., 'git stash', 'git stash list', 'git stash drop', > etc. Others ensure the index is expanded either directly (as in the case of > 'git stash [pop|apply]', where the call to 'merge_recursive_generic()' in > 'do_apply_stash()' triggers the expansion), or in a command called > internally by stash (e.g., 'git update-index' in 'git stash -u'). So, in > addition to enabling sparse index, add tests to 't1092' demonstrating which > variants of 'git stash' expand the index, and which do not. As always, it is suprising that the change id deceptively easy, but it is only true because various components like the merge machinery used by the code have been taught to expand the sparse index entries as needed. Looking good so far.
On 4/25/2022 1:49 PM, Victoria Dye via GitGitGadget wrote: > From: Victoria Dye <vdye@github.com> > > Enable sparse index in 'git stash' by disabling > 'command_requires_full_index'. > ensure_not_expanded () { > rm -f trace2.txt && > - echo >>sparse-index/untracked.txt && > + if test -z $WITHOUT_UNTRACKED_TXT Do we need quotes around "$WITHOUT_UNTRACKED_TXT"? I mean, I suppose we don't since the tests pass when this variable is unset, but I think it is a good practice to be careful about empty variables. Or am I wrong? > + then > + echo >>sparse-index/untracked.txt > + fi && > - ensure_not_expanded checkout-index -f a && > - ensure_not_expanded checkout-index -f --all && > - for ref in update-deep update-folder1 update-folder2 update-deep > - do > - echo >>sparse-index/README.md && > - ensure_not_expanded reset --hard $ref || return 1 > - done && > - Moving these to be within the stash tests is interesting. > +test_expect_success 'sparse-index is not expanded: stash' ' > + init_repos && > + > + echo >>sparse-index/a && > + ensure_not_expanded stash && > + ensure_not_expanded stash list && > + ensure_not_expanded stash show stash@{0} && > + ! ensure_not_expanded stash apply stash@{0} && > + ensure_not_expanded stash drop stash@{0} && > + > + echo >>sparse-index/deep/new && > + ! ensure_not_expanded stash -u && > + ( > + WITHOUT_UNTRACKED_TXT=1 && > + ! ensure_not_expanded stash pop > + ) && > + > + ensure_not_expanded stash create && > + oid=$(git -C sparse-index stash create) && > + ensure_not_expanded stash store -m "test" $oid && > + ensure_not_expanded reset --hard && > + ! ensure_not_expanded stash pop && > + > + ensure_not_expanded checkout-index -f a && > + ensure_not_expanded checkout-index -f --all && > + for ref in update-deep update-folder1 update-folder2 update-deep > + do > + echo >>sparse-index/README.md && > + ensure_not_expanded reset --hard $ref || return 1 > + done It is not obvious why that's necessary here. Perhaps a later change will build upon these checkout-index commands within this test? Thanks, -Stolee
Derrick Stolee wrote: > On 4/25/2022 1:49 PM, Victoria Dye via GitGitGadget wrote: >> From: Victoria Dye <vdye@github.com> >> >> Enable sparse index in 'git stash' by disabling >> 'command_requires_full_index'. > >> ensure_not_expanded () { >> rm -f trace2.txt && >> - echo >>sparse-index/untracked.txt && >> + if test -z $WITHOUT_UNTRACKED_TXT > > Do we need quotes around "$WITHOUT_UNTRACKED_TXT"? > > I mean, I suppose we don't since the tests pass when this variable > is unset, but I think it is a good practice to be careful about > empty variables. Or am I wrong? > >> + then >> + echo >>sparse-index/untracked.txt >> + fi && > > >> - ensure_not_expanded checkout-index -f a && >> - ensure_not_expanded checkout-index -f --all && >> - for ref in update-deep update-folder1 update-folder2 update-deep >> - do >> - echo >>sparse-index/README.md && >> - ensure_not_expanded reset --hard $ref || return 1 >> - done && >> - > > Moving these to be within the stash tests is interesting. > That was unintentional. I originally had the 'stash' cases in the big 'sparse-index is not expanded' test, but decided to move them into their own test for more appropriate granularity. In doing that, I accidentally took some of the 'checkout-index' tests with it. I'll move them back in V2. Thanks for the catch! >> +test_expect_success 'sparse-index is not expanded: stash' ' >> + init_repos && >> + >> + echo >>sparse-index/a && >> + ensure_not_expanded stash && >> + ensure_not_expanded stash list && >> + ensure_not_expanded stash show stash@{0} && >> + ! ensure_not_expanded stash apply stash@{0} && >> + ensure_not_expanded stash drop stash@{0} && >> + >> + echo >>sparse-index/deep/new && >> + ! ensure_not_expanded stash -u && >> + ( >> + WITHOUT_UNTRACKED_TXT=1 && >> + ! ensure_not_expanded stash pop >> + ) && >> + >> + ensure_not_expanded stash create && >> + oid=$(git -C sparse-index stash create) && >> + ensure_not_expanded stash store -m "test" $oid && >> + ensure_not_expanded reset --hard && >> + ! ensure_not_expanded stash pop && >> + >> + ensure_not_expanded checkout-index -f a && >> + ensure_not_expanded checkout-index -f --all && >> + for ref in update-deep update-folder1 update-folder2 update-deep >> + do >> + echo >>sparse-index/README.md && >> + ensure_not_expanded reset --hard $ref || return 1 >> + done > > It is not obvious why that's necessary here. Perhaps a later > change will build upon these checkout-index commands within > this test? > > Thanks, > -Stolee
Derrick Stolee <derrickstolee@github.com> writes: > On 4/25/2022 1:49 PM, Victoria Dye via GitGitGadget wrote: >> From: Victoria Dye <vdye@github.com> >> >> Enable sparse index in 'git stash' by disabling >> 'command_requires_full_index'. > >> ensure_not_expanded () { >> rm -f trace2.txt && >> - echo >>sparse-index/untracked.txt && >> + if test -z $WITHOUT_UNTRACKED_TXT > > Do we need quotes around "$WITHOUT_UNTRACKED_TXT"? > > I mean, I suppose we don't since the tests pass when this variable > is unset, but I think it is a good practice to be careful about > empty variables. Or am I wrong? Interesting. I do not think the reason why test -z && echo true says "true" is "-z" noticed an empty string---it is "test $string" that says "true" when "$string" is not a recognised command and is not an empty string, no? You are absolutely right that the above must be quoted. Thanks for carefully reading.
diff --git a/builtin/stash.c b/builtin/stash.c index 0c7b6a95882..1bfba532044 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1770,6 +1770,9 @@ int cmd_stash(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_stash_usage, PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH); + prepare_repo_settings(the_repository); + the_repository->settings.command_requires_full_index = 0; + index_file = get_index_file(); strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file, (uintmax_t)pid); diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index baf95906729..b00c65c7770 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1271,7 +1271,10 @@ test_expect_success 'index.sparse disabled inline uses full index' ' ensure_not_expanded () { rm -f trace2.txt && - echo >>sparse-index/untracked.txt && + if test -z $WITHOUT_UNTRACKED_TXT + then + echo >>sparse-index/untracked.txt + fi && if test "$1" = "!" then @@ -1314,14 +1317,6 @@ test_expect_success 'sparse-index is not expanded' ' echo >>sparse-index/untracked.txt && ensure_not_expanded add . && - ensure_not_expanded checkout-index -f a && - ensure_not_expanded checkout-index -f --all && - for ref in update-deep update-folder1 update-folder2 update-deep - do - echo >>sparse-index/README.md && - ensure_not_expanded reset --hard $ref || return 1 - done && - ensure_not_expanded reset --mixed base && ensure_not_expanded reset --hard update-deep && ensure_not_expanded reset --keep base && @@ -1375,6 +1370,38 @@ test_expect_success 'sparse-index is not expanded: merge conflict in cone' ' ) ' +test_expect_success 'sparse-index is not expanded: stash' ' + init_repos && + + echo >>sparse-index/a && + ensure_not_expanded stash && + ensure_not_expanded stash list && + ensure_not_expanded stash show stash@{0} && + ! ensure_not_expanded stash apply stash@{0} && + ensure_not_expanded stash drop stash@{0} && + + echo >>sparse-index/deep/new && + ! ensure_not_expanded stash -u && + ( + WITHOUT_UNTRACKED_TXT=1 && + ! ensure_not_expanded stash pop + ) && + + ensure_not_expanded stash create && + oid=$(git -C sparse-index stash create) && + ensure_not_expanded stash store -m "test" $oid && + ensure_not_expanded reset --hard && + ! ensure_not_expanded stash pop && + + ensure_not_expanded checkout-index -f a && + ensure_not_expanded checkout-index -f --all && + for ref in update-deep update-folder1 update-folder2 update-deep + do + echo >>sparse-index/README.md && + ensure_not_expanded reset --hard $ref || return 1 + done +' + test_expect_success 'sparse index is not expanded: diff' ' init_repos &&