On 3/11/2022 7:08 PM, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@xxxxxxxxxx>
>
> 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
...
> [1] 9ac8125d1a (reset: don't compute unstaged changes after reset when
> --quiet, 2018-10-23)
I believe convention would normally have this listing of the commit in-line
with your discussion of it. The "[1]" probably works, too, but I can't say
that I've seen that used except for URLs. Something like:
Starting at <commit>, the '--quiet' option skips refresh_index()...
> 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
> executing commands after 'git reset --quiet' that do not themselves refresh
> the index (e.g., internals of 'git stash', 'git read-tree').
>
> To mitigate these issues, '--[no-]refresh' and 'reset.refresh' are
> introduced to provide a dedicated mechanism for refreshing the index. When
> either is set, '--quiet' and 'reset.quiet' revert to controlling only
> whether logs are silenced and do not affect index refresh.
Well motivated change.
> +test_index_refreshed () {
> +
> + # To test whether the index is refresh, create a scenario where a
> + # command will fail if the index is *not* refreshed:
> + # 1. update the worktree to match HEAD & remove file2 in the index
> + # 2. reset --mixed to unstage the change from step 1
> + # 3. read-tree HEAD~1 (which differs from HEAD in file2)
> + # If the index is refreshed in step 2, then file2 in the index will be
> + # up-to-date with HEAD and read-tree will succeed (thus failing the
> + # test). If the index is *not* refreshed, however, the staged deletion
> + # of file2 from step 1 will conflict with the changes from the tree read
> + # in step 3, resulting in a failure.
> +
> + # Step 0: start with a clean index
> + git reset --hard HEAD &&
> +
> + # Step 1
> + git rm --cached file2 &&
> +
> + # Step 2
> + git reset $1 --mixed HEAD &&
> +
> + # Step 3
> + git read-tree -m HEAD~1
> +}
> +
> test_expect_success '--mixed refreshes the index' '
> - cat >expect <<-\EOF &&
> - Unstaged changes after reset:
> - M file2
> - EOF
> - echo 123 >>file2 &&
> - git reset --mixed HEAD >output &&
> - test_cmp expect output
> + # Verify default behavior (with no config settings or command line
> + # options)
> + test_index_refreshed &&
> +'
It looks like this test ends with an &&. There's also a missing newline
after the test.
> +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 &&
This is failing, but not for the reason you want: It is running
git reset -c --mixed HEAD
and failing to parse the "-c", I bet.
Perhaps you want to have two arguments: one for config settings
and another for arguments, meaning your call in test_index_refreshed
should be
git $1 reset $2 --mixed HEAD
and calls like this should be
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 --no-quiet &&
If you take a change like I recommend above, these would be
test_index_refreshed "" --no-quiet &&
Thanks,
-Stolee
