[PATCH v2 0/5] Allow 'reset --quiet' to refresh the index, use 'reset --quiet' in 'stash'
- From
Victoria Dye via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Mar 14, 2022, 16:10 UTC
- Message-ID
- <pull.1170.v2.git.1647274230.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1170.git.1647043729.gitgitgadget@gmail.com>
In the process of working on tests for 'git stash' sparse index integration, I found that the '--quiet' option in 'git stash' does not suppress all non-error output when used with '--index'. Specifically, this comes from an invocation of 'git reset' without the '--quiet' flag in 'reset_head()'. Upon enabling that flag, though, I discovered that 1) 'reset' does not refresh the index if '--quiet' is specified (as of [1]) and 2) 'git stash' needs the index to be refreshed after the reset.
This series aims to decouple the "suppress logging" and "skip index refresh" behaviors in 'git reset --mixed', then allow 'stash' to internally use reset with logs suppressed but index refresh enabled. This is accomplished by introducing the '--[no-]refresh' option and 'reset.refresh' config setting to 'git reset'. Additionally, in the spirit of backward-compatibility, '--quiet' and/or 'reset.quiet=true' without any specified "refresh" option/config will continue to skip 'refresh_index(...)'.
There are also some minor updates to the advice that suggests skipping the index refresh:
* replace recommendation to use "--quiet" with "--no-refresh" * use 'advise()' rather than 'printf()' * rename the advice config setting from 'advice.resetQuiet' to to 'advice.resetNoRefresh' * suppress advice if '--quiet' is specified in 'reset'
Finally, tests are added to 't7102-reset.sh' verifying whether index refresh happens when expected and to 't3903-stash.sh' verifying that 'apply --quiet' no longer prints extraneous logs.
Changes since V1 ================
* Update cover letter title * Squash 't7102' test fixes from patch 5 into patch 2 * Refactor 't7102' tests to properly use inline config settings
[1] https://lore.kernel.org/git/20181023190423.5772-2-peartben@gmail.com/
Thanks! -Victoria
Victoria Dye (5): reset: revise index refresh advice reset: introduce --[no-]refresh option to --mixed reset: replace '--quiet' with '--no-refresh' in performance advice reset: suppress '--no-refresh' advice if logging is silenced stash: make internal resets quiet and refresh index
Documentation/config/advice.txt | 8 ++-- Documentation/git-reset.txt | 9 ++++ advice.c | 2 +- advice.h | 2 +- builtin/reset.c | 21 ++++++--- builtin/stash.c | 5 ++- t/t3903-stash.sh | 12 +++++ t/t7102-reset.sh | 77 ++++++++++++++++++++++++++++++--- 8 files changed, 116 insertions(+), 20 deletions(-)
base-commit: 1a4874565fa3b6668042216189551b98b4dc0b1b Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1170%2Fvdye%2Fstash%2Fmake-reset-quiet-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1170/vdye/stash/make-reset-quiet-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1170
Range-diff vs v1:
1: 7206ef8dd8a = 1: 7206ef8dd8a reset: revise index refresh advice
2: bda93703013 ! 2: 7f0226bc3e6 reset: introduce --[no-]refresh option to --mixed
@@ Commit message
Add a new --[no-]refresh option that is intended to explicitly determine
whether a mixed reset should end in an index refresh.
- A few years ago, [1] introduced behavior to the '--quiet' option to skip the
- call to 'refresh_index(...)' at the end of a mixed reset with the goal of
- improving performance. However, by coupling behavior that modifies the index
- with the option that silences logs, there is no way for users to have one
- without the other (i.e., silenced logs with a refreshed index) without
+ Starting at 9ac8125d1a (reset: don't compute unstaged changes after reset
+ when --quiet, 2018-10-23), using the '--quiet' option results in skipping
+ the call to 'refresh_index(...)' at the end of a mixed reset with the goal
+ of improving performance. However, by coupling behavior that modifies the
+ index with the option that silences logs, there is no way for users to have
+ one without the other (i.e., silenced logs with a refreshed index) without
incurring the overhead of a separate call to 'git update-index --refresh'.
Furthermore, there is minimal user-facing documentation indicating that
--quiet skips the index refresh, potentially leading to unexpected issues
@@ Commit message
either is set, '--quiet' and 'reset.quiet' revert to controlling only
whether logs are silenced and do not affect index refresh.
- [1] 9ac8125d1a (reset: don't compute unstaged changes after reset when
- --quiet, 2018-10-23)
-
+ Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Victoria Dye <vdye@github.com>
## Documentation/git-reset.txt ##
@@ t/t7102-reset.sh: test_expect_success 'resetting an unmodified path is a no-op'
+ git rm --cached file2 &&
+
+ # Step 2
-+ git reset $1 --mixed HEAD &&
++ git $1 reset $2 --mixed HEAD &&
+
+ # Step 3
+ git read-tree -m HEAD~1
@@ t/t7102-reset.sh: test_expect_success 'resetting an unmodified path is a no-op'
- test_cmp expect output
+ # Verify default behavior (with no config settings or command line
+ # options)
-+ test_index_refreshed &&
++ test_index_refreshed
+'
+test_expect_success '--mixed --[no-]quiet sets default refresh behavior' '
+ # Verify that --[no-]quiet and `reset.quiet` (without --[no-]refresh)
+ # determine refresh behavior
+
+ # Config setting
-+ test_must_fail test_index_refreshed -c reset.quiet=true &&
-+ test_index_refreshed -c reset.quiet=true &&
++ ! test_index_refreshed "-c reset.quiet=true" &&
++ test_index_refreshed "-c reset.quiet=false" &&
+
+ # Command line option
-+ test_must_fail test_index_refreshed --quiet &&
-+ test_index_refreshed --no-quiet &&
++ ! test_index_refreshed "" --quiet &&
++ test_index_refreshed "" --no-quiet &&
+
+ # Command line option overrides config setting
-+ test_must_fail test_index_refreshed -c reset.quiet=false --quiet &&
-+ test_index_refreshed -c reset.refresh=true --no-quiet
++ ! test_index_refreshed "-c reset.quiet=false" --quiet &&
++ test_index_refreshed "-c reset.refresh=true" --no-quiet
+'
+
+test_expect_success '--mixed --[no-]refresh sets refresh behavior' '
+ # Verify that --[no-]refresh and `reset.refresh` control index refresh
+
+ # Config setting
-+ test_index_refreshed -c reset.refresh=true &&
-+ test_must_fail test_index_refreshed -c reset.refresh=false &&
++ test_index_refreshed "-c reset.refresh=true" &&
++ ! test_index_refreshed "-c reset.refresh=false" &&
+
+ # Command line option
-+ test_index_refreshed --refresh &&
-+ test_must_fail test_index_refreshed --no-refresh &&
++ test_index_refreshed "" --refresh &&
++ ! test_index_refreshed "" --no-refresh &&
+
+ # Command line option overrides config setting
-+ test_index_refreshed -c reset.refresh=false --refresh &&
-+ test_must_fail test_index_refreshed -c reset.refresh=true --no-refresh
++ test_index_refreshed "-c reset.refresh=false" --refresh &&
++ ! test_index_refreshed "-c reset.refresh=true" --no-refresh
+'
+
+test_expect_success '--mixed --refresh overrides --quiet refresh behavior' '
+ # Verify that *both* --refresh and `reset.refresh` override the
+ # default non-refresh behavior of --quiet
-+ test_index_refreshed --refresh --quiet &&
-+ test_index_refreshed --refresh -c reset.quiet=true &&
-+ test_index_refreshed -c reset.refresh=true --quiet &&
-+ test_index_refreshed -c reset.refresh=true -c reset.quiet=true
++ test_index_refreshed "" "--quiet --refresh" &&
++ test_index_refreshed "-c reset.quiet=true" --refresh &&
++ test_index_refreshed "-c reset.refresh=true" --quiet &&
++ test_index_refreshed "-c reset.refresh=true -c reset.quiet=true"
'
test_expect_success '--mixed preserves skip-worktree' '
3: 63c5ee36feb = 3: f869723d64f reset: replace '--quiet' with '--no-refresh' in performance advice
4: 3c65a9f1993 = 4: cffca0ea5c6 reset: suppress '--no-refresh' advice if logging is silenced
5: 052499bbc93 ! 5: 3334d4cb6f3 stash: make internal resets quiet and refresh index
@@ t/t3903-stash.sh: test_expect_success 'apply -q is quiet' '
test_expect_success 'save -q is quiet' '
git stash save --quiet >output.out 2>&1 &&
test_must_be_empty output.out
-
- ## t/t7102-reset.sh ##
-@@ t/t7102-reset.sh: test_index_refreshed () {
- git rm --cached file2 &&
-
- # Step 2
-- git reset $1 --mixed HEAD &&
-+ git reset $@ --mixed HEAD &&
-
- # Step 3
- git read-tree -m HEAD~1
-@@ t/t7102-reset.sh: test_index_refreshed () {
- test_expect_success '--mixed refreshes the index' '
- # Verify default behavior (with no config settings or command line
- # options)
-- test_index_refreshed &&
-+ test_index_refreshed
- '
- test_expect_success '--mixed --[no-]quiet sets default refresh behavior' '
- # Verify that --[no-]quiet and `reset.quiet` (without --[no-]refresh)
- # determine refresh behavior
-
-- # Config setting
-- test_must_fail test_index_refreshed -c reset.quiet=true &&
-- test_index_refreshed -c reset.quiet=true &&
--
- # Command line option
-- test_must_fail test_index_refreshed --quiet &&
-+ ! test_index_refreshed --quiet &&
- test_index_refreshed --no-quiet &&
-
-- # Command line option overrides config setting
-- test_must_fail test_index_refreshed -c reset.quiet=false --quiet &&
-- test_index_refreshed -c reset.refresh=true --no-quiet
-+ # Config: reset.quiet=false
-+ test_config reset.quiet false &&
-+ (
-+ test_index_refreshed &&
-+ ! test_index_refreshed --quiet
-+ ) &&
-+
-+ # Config: reset.quiet=true
-+ test_config reset.quiet true &&
-+ (
-+ ! test_index_refreshed &&
-+ test_index_refreshed --no-quiet
-+ )
- '
-
- test_expect_success '--mixed --[no-]refresh sets refresh behavior' '
- # Verify that --[no-]refresh and `reset.refresh` control index refresh
-
-- # Config setting
-- test_index_refreshed -c reset.refresh=true &&
-- test_must_fail test_index_refreshed -c reset.refresh=false &&
--
- # Command line option
- test_index_refreshed --refresh &&
-- test_must_fail test_index_refreshed --no-refresh &&
-+ ! test_index_refreshed --no-refresh &&
-+
-+ # Config: reset.refresh=false
-+ test_config reset.refresh false &&
-+ (
-+ ! test_index_refreshed &&
-+ test_index_refreshed --refresh
-+ ) &&
-
-- # Command line option overrides config setting
-- test_index_refreshed -c reset.refresh=false --refresh &&
-- test_must_fail test_index_refreshed -c reset.refresh=true --no-refresh
-+ # Config: reset.refresh=true
-+ test_config reset.refresh true &&
-+ (
-+ test_index_refreshed &&
-+ ! test_index_refreshed --no-refresh
-+ )
- '
-
- test_expect_success '--mixed --refresh overrides --quiet refresh behavior' '
- # Verify that *both* --refresh and `reset.refresh` override the
- # default non-refresh behavior of --quiet
-+
- test_index_refreshed --refresh --quiet &&
-- test_index_refreshed --refresh -c reset.quiet=true &&
-- test_index_refreshed -c reset.refresh=true --quiet &&
-- test_index_refreshed -c reset.refresh=true -c reset.quiet=true
-+
-+ # Config: reset.quiet=true
-+ test_config reset.quiet true &&
-+ test_index_refreshed --refresh &&
-+
-+ # Config: reset.quiet=true, reset.refresh=true
-+ test_config reset.refresh true &&
-+ test_index_refreshed
- '
-
- test_expect_success '--mixed preserves skip-worktree' '-- gitgitgadget