Skip to content

Commit

Permalink
reset: fix mixed reset when using virtual filesystem
Browse files Browse the repository at this point in the history
During the 2.35.0 rebase, we ejected 570f64b (Fix reset when using the
sparse-checkout feature., 2017-03-15) because of a similar change
upstream that actually works with the expected behavior of
sparse-checkout.

That commit only ever existed in microsoft/git, but when it was
considered for upstream we realized that it behaved strangely for a
sparse-checkout scenario.

The root problem is that during a mixed reset, 'git reset <commit>'
updates the index to aggree with <commit> but leaves the worktree the
same as it was before. The issue with sparse-checkout is that some files
might not be in the worktree and thus the information from those files
would be "lost".

The upstream decision was to leave these files as ignored, because
that's what the SKIP_WORKTREE bit means: don't put these files in the
worktree and ignore their contents. If there already were files in the
worktree, then Git does not change them. The case for "losing" data is
if a committed change outside of the sparse-checkout was in the previous
HEAD position. However, this information could be recovered from the
reflog.

The case where this is different is in a virtualized filesystem. The
virtualization is projecting the index contents onto the filesystem, so
we need to do something different here. In a virtual environment, every
file is considered "important" and we abuse the SKIP_WORKTREE bit to
indicate that Git does not need to process a projected file. When a file
is populated, the virtual filesystem hook provides the information for
removing the SKIP_WORKTREE bit.

In the case of these mixed resets, we have the issue where we change the
projection of the worktree for these cache entries that change. If a
file is populated in the worktree, then the populated file will persist
and appear in a follow-up 'git status'. However, if the file is not
populated and only projected, we change the projection from the current
value to the new value, leaving a clean 'git status'.

The previous version of this commit includes a call to checkout_entry(),
which populates the file. This causes the file to be actually in the
working tree and no longer projected.

To make this work with the upstream changes, stop setting the
skip-worktree bit for the new cache entry. This seemed to work fine
without this change, but it's likely due to some indirection with the
virtual filesystem. Better to do the best-possible thing here so we
don't hide a corner-case bug by accident.

Helped-by: Victoria Dye <vdye@github.com>
Signed-off-by: Kevin Willford <kewillf@microsoft.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
  • Loading branch information
Kevin Willford authored and dscho committed Nov 3, 2023
1 parent c4390e2 commit dd7f63f
Showing 1 changed file with 48 additions and 2 deletions.
50 changes: 48 additions & 2 deletions builtin/reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
#include "add-interactive.h"
#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 @@ -156,9 +158,47 @@ static void update_index_from_diff(struct diff_queue_struct *q,

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

/*
* When using the virtual filesystem 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_virtualfilesystem && !file_exists(two->path))
{
pos = index_name_pos(&the_index, two->path, strlen(two->path));
if ((pos >= 0 && ce_skip_worktree(the_index.cache[pos])) &&
(is_missing || !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);
respect_skip_worktree = 0;
}
}

if (!is_in_reset_tree && !intent_to_add) {
remove_file_from_index(&the_index, one->path);
Expand All @@ -177,8 +217,14 @@ static void update_index_from_diff(struct diff_queue_struct *q,
* to properly construct the reset sparse directory.
*/
pos = index_name_pos(&the_index, one->path, strlen(one->path));
if ((pos >= 0 && ce_skip_worktree(the_index.cache[pos])) ||
(pos < 0 && !path_in_sparse_checkout(one->path, &the_index)))

/*
* Do not add the SKIP_WORKTREE bit back if we populated the
* file on purpose in a virtual filesystem scenario.
*/
if (respect_skip_worktree &&
((pos >= 0 && ce_skip_worktree(the_index.cache[pos])) ||
(pos < 0 && !path_in_sparse_checkout(one->path, &the_index))))
ce->ce_flags |= CE_SKIP_WORKTREE;

if (!ce)
Expand Down

0 comments on commit dd7f63f

Please sign in to comment.