Skip to content

Commit

Permalink
reset: preserve skip-worktree bit in mixed reset
Browse files Browse the repository at this point in the history
Change `update_index_from_diff` to set `skip-worktree` when applicable for
new index entries. When `git reset --mixed <tree-ish>` is run, entries in
the index with differences between the pre-reset HEAD and reset <tree-ish>
are identified and handled with `update_index_from_diff`. For each file, a
new cache entry in inserted into the index, created from the <tree-ish> side
of the reset (without changing the working tree). However, the newly-created
entry must have `skip-worktree` explicitly set in either of the following
scenarios:

1. the file is in the current index and has `skip-worktree` set
2. the file is not in the current index but is outside of a defined sparse
   checkout definition

Not setting the `skip-worktree` bit leads to likely-undesirable results for
a user. It causes `skip-worktree` settings to disappear on the
"diff"-containing files (but *only* the diff-containing files), leading to
those files now showing modifications in `git status`. For example, when
running `git reset --mixed` in a sparse checkout, some file entries outside
of sparse checkout could show up as deleted, despite the user never deleting
anything (and not wanting them on-disk anyway).

Additionally, add a test to `t7102` to ensure `skip-worktree` is preserved
in a basic `git reset --mixed` scenario and update a failure-documenting
test from 19a0acc (t1092: test interesting sparse-checkout scenarios,
2021-01-23) with new expected behavior.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Victoria Dye <vdye@github.com>
  • Loading branch information
vdye authored and derrickstolee committed Nov 4, 2021
1 parent fabcc6e commit d5795be
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 104 deletions.
49 changes: 15 additions & 34 deletions builtin/reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "strbuf.h"
#include "quote.h"
#include "dir.h"
#include "entry.h"

#define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)

Expand Down Expand Up @@ -138,45 +137,13 @@ static void update_index_from_diff(struct diff_queue_struct *q,
struct diff_options *opt, void *data)
{
int i;
int pos;
int intent_to_add = *(int *)data;

for (i = 0; i < q->nr; i++) {
int pos;
struct diff_filespec *one = q->queue[i]->one;
struct diff_filespec *two = q->queue[i]->two;
int was_missing = !two->mode && is_null_oid(&two->oid);
int is_in_reset_tree = one->mode && !is_null_oid(&one->oid);
struct cache_entry *ce;
struct cache_entry *ceBefore;
struct checkout state = CHECKOUT_INIT;

/*
* When using the sparse-checkout feature the cache entries that are
* added here will not have the skip-worktree bit set.
* Without this code there is data that is lost because the files that
* would normally be in the working directory are not there and show as
* deleted for the next status or in the case of added files just disappear.
* We need to create the previous version of the files in the working
* directory so that they will have the right content and the next
* status call will show modified or untracked files correctly.
*/
if (core_apply_sparse_checkout && !file_exists(two->path))
{
pos = cache_name_pos(two->path, strlen(two->path));
if ((pos >= 0 && ce_skip_worktree(active_cache[pos])) && (is_in_reset_tree || !was_missing))
{
state.force = 1;
state.refresh_cache = 1;
state.istate = &the_index;
ceBefore = make_cache_entry(&the_index, two->mode, &two->oid, two->path,
0, 0);
if (!ceBefore)
die(_("make_cache_entry failed for path '%s'"),
two->path);

checkout_entry(ceBefore, &state, NULL, NULL);
}
}

if (!is_in_reset_tree && !intent_to_add) {
remove_file_from_cache(one->path);
Expand All @@ -185,6 +152,20 @@ static void update_index_from_diff(struct diff_queue_struct *q,

ce = make_cache_entry(&the_index, one->mode, &one->oid, one->path,
0, 0);

/*
* If the file 1) corresponds to an existing index entry with
* skip-worktree set, or 2) does not exist in the index but is
* outside the sparse checkout definition, add a skip-worktree bit
* to the new index entry. Note that a sparse index will be expanded
* if this entry is outside the sparse cone - this is necessary
* to properly construct the reset sparse directory.
*/
pos = cache_name_pos(one->path, strlen(one->path));
if ((pos >= 0 && ce_skip_worktree(active_cache[pos])) ||
(pos < 0 && !path_in_sparse_checkout(one->path, &the_index)))
ce->ce_flags |= CE_SKIP_WORKTREE;

if (!ce)
die(_("make_cache_entry failed for path '%s'"),
one->path);
Expand Down
16 changes: 4 additions & 12 deletions t/t1092-sparse-checkout-compatibility.sh
Original file line number Diff line number Diff line change
Expand Up @@ -528,25 +528,17 @@ test_expect_success 'blame with pathspec outside sparse definition' '
test_sparse_match test_must_fail git blame deep/deeper2/deepest/a
'

# TODO: This behaves correctly in microsoft/git. Why?
test_expect_success '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
'

# NEEDSWORK: a sparse-checkout behaves differently from a full checkout
# in this scenario, but it shouldn't.
test_expect_success 'checkout and reset (mixed) [sparse]' '
init_repos &&
test_sparse_match git checkout -b reset-test update-deep &&
test_sparse_match git reset deepest &&
# Because skip-worktree is preserved, resetting to update-folder1
# will show worktree changes for full-checkout that are not present
# in sparse-checkout or sparse-index.
test_sparse_match git reset update-folder1 &&
test_sparse_match git reset update-folder2
run_on_sparse test_path_is_missing folder1
'

# NEEDSWORK: with mixed reset, files with differences between HEAD and <commit>
Expand Down
17 changes: 17 additions & 0 deletions t/t7102-reset.sh
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,23 @@ test_expect_success '--mixed refreshes the index' '
test_cmp expect output
'

test_expect_success '--mixed preserves skip-worktree' '
echo 123 >>file2 &&
git add file2 &&
git update-index --skip-worktree file2 &&
git reset --mixed HEAD >output &&
test_must_be_empty output &&
cat >expect <<-\EOF &&
Unstaged changes after reset:
M file2
EOF
git update-index --no-skip-worktree file2 &&
git add file2 &&
git reset --mixed HEAD >output &&
test_cmp expect output
'

test_expect_success 'resetting specific path that is unmerged' '
git rm --cached file2 &&
F1=$(git rev-parse HEAD:file1) &&
Expand Down
58 changes: 0 additions & 58 deletions t/t7114-reset-sparse-checkout.sh

This file was deleted.

0 comments on commit d5795be

Please sign in to comment.