Skip to content

Commit

Permalink
reset: introduce --[no-]refresh option to --mixed
Browse files Browse the repository at this point in the history
Add a new --[no-]refresh option that is intended to explicitly determine
whether a mixed reset should end in an index refresh.

Starting at 9ac8125 (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
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.

Helped-by: Derrick Stolee <[email protected]>
Helped-by: Junio C Hamano <[email protected]>
Signed-off-by: Victoria Dye <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
vdye authored and gitster committed Mar 15, 2022
1 parent e86ec71 commit fd56fba
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 8 deletions.
9 changes: 9 additions & 0 deletions Documentation/git-reset.txt
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,15 @@ OPTIONS
`reset.quiet` config option. `--quiet` and `--no-quiet` will
override the default behavior.

--refresh::
--no-refresh::
Proactively refresh the index after a mixed reset. If unspecified, the
behavior falls back on the `reset.refresh` config option. If neither
`--[no-]refresh` nor `reset.refresh` are set, the default behavior is
decided by the `--[no-]quiet` option and/or `reset.quiet` config.
If `--quiet` is specified or `reset.quiet` is set with no command-line
"quiet" setting, refresh is disabled. Otherwise, refresh is enabled.

--pathspec-from-file=<file>::
Pathspec is passed in `<file>` instead of commandline args. If
`<file>` is exactly `-` then standard input is used. Pathspec
Expand Down
13 changes: 12 additions & 1 deletion builtin/reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -392,13 +392,16 @@ static int git_reset_config(const char *var, const char *value, void *cb)
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
int refresh = -1;
int patch_mode = 0, pathspec_file_nul = 0, unborn;
const char *rev, *pathspec_from_file = NULL;
struct object_id oid;
struct pathspec pathspec;
int intent_to_add = 0;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_BOOL(0, "refresh", &refresh,
N_("skip refreshing the index after reset")),
OPT_SET_INT(0, "mixed", &reset_type,
N_("reset HEAD and index"), MIXED),
OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"), SOFT),
Expand All @@ -421,11 +424,19 @@ int cmd_reset(int argc, const char **argv, const char *prefix)

git_config(git_reset_config, NULL);
git_config_get_bool("reset.quiet", &quiet);
git_config_get_bool("reset.refresh", &refresh);

argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
parse_args(&pathspec, argv, prefix, patch_mode, &rev);

/*
* If refresh is completely unspecified (either by config or by command
* line option), decide based on 'quiet'.
*/
if (refresh < 0)
refresh = !quiet;

if (pathspec_from_file) {
if (patch_mode)
die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--patch");
Expand Down Expand Up @@ -517,7 +528,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (read_from_tree(&pathspec, &oid, intent_to_add))
return 1;
the_index.updated_skipworktree = 1;
if (!quiet && get_git_work_tree()) {
if (refresh && get_git_work_tree()) {
uint64_t t_begin, t_delta_in_ms;

t_begin = getnanotime();
Expand Down
73 changes: 66 additions & 7 deletions t/t7102-reset.sh
Original file line number Diff line number Diff line change
Expand Up @@ -462,14 +462,73 @@ test_expect_success 'resetting an unmodified path is a no-op' '
git diff-index --cached --exit-code HEAD
'

test_reset_refreshes_index () {

# To test whether the index is refreshed in `git reset --mixed` with
# the given options, create a scenario where we clearly see different
# results depending on whether the refresh occurred or not.

# Step 0: start with a clean index
git reset --hard HEAD &&

# Step 1: remove file2, but only in the index (no change to worktree)
git rm --cached file2 &&

# Step 2: reset index & leave worktree unchanged from HEAD
git $1 reset $2 --mixed HEAD &&

# Step 3: verify whether the index is refreshed by checking whether
# file2 still has staged changes in the index differing from HEAD (if
# the refresh occurred, there should be no such changes)
git diff-files >output.log &&
test_must_be_empty output.log
}

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_reset_refreshes_index
'
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_reset_refreshes_index "-c reset.quiet=true" &&
test_reset_refreshes_index "-c reset.quiet=false" &&
# Command line option
! test_reset_refreshes_index "" --quiet &&
test_reset_refreshes_index "" --no-quiet &&
# Command line option overrides config setting
! test_reset_refreshes_index "-c reset.quiet=false" --quiet &&
test_reset_refreshes_index "-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_reset_refreshes_index "-c reset.refresh=true" &&
! test_reset_refreshes_index "-c reset.refresh=false" &&
# Command line option
test_reset_refreshes_index "" --refresh &&
! test_reset_refreshes_index "" --no-refresh &&
# Command line option overrides config setting
test_reset_refreshes_index "-c reset.refresh=false" --refresh &&
! test_reset_refreshes_index "-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_reset_refreshes_index "" "--quiet --refresh" &&
test_reset_refreshes_index "-c reset.quiet=true" --refresh &&
test_reset_refreshes_index "-c reset.refresh=true" --quiet &&
test_reset_refreshes_index "-c reset.refresh=true -c reset.quiet=true"
'

test_expect_success '--mixed preserves skip-worktree' '
Expand Down

0 comments on commit fd56fba

Please sign in to comment.