Skip to content

Commit

Permalink
Merge pull request git-for-windows#473: Sparse Index: fix a checkout …
Browse files Browse the repository at this point in the history
…bug with deep sparse-checkout paths

We got multiple similar reports of failures with the sparse index (git-for-windows#473 as an issue, and another regarding `git checkout` via email). Both were hitting a similar set of paths, which was a hint.

The reason we didn't hit this before is because it requires the following:

1. The sparse-checkout definition needs to have recursive inclusion of deep folders (depth 3 or more).
2. _Adjacent_ to those deep folders, we need a deep sparse directory entry that receives changes.
3. In this particular repo, deep directories are only added to the sparse-checkout in rare occasions and those adjacent folders are rarely updated. They happened to update this week and hit our sparse index dogfooders in surprising ways.

The first commit adds a test that fails without the fix. It requires modifying our test data to make adjacent, deep sparse directory entries possible. It's a rather simple test after we have that data change.

The second commit includes the actual fix. It's really just an error of not understanding the difference between the `name` and `traverse_path` members of the `struct traverse_info` structure. `name` only stores a single tree entry while `traverse_path` actually includes the full name from root. The method we are editing also has an additional `struct name_entry` that fills in the tree entry on top of the `traverse_path`, which explains how this worked to depth two, but not depth three.

Resolves git-for-windows#473 
See also gitgitgadget#1092
  • Loading branch information
derrickstolee authored and dscho committed Jan 15, 2022
2 parents 07ece66 + 3276c83 commit 9fab0b5
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
16 changes: 15 additions & 1 deletion t/t1092-sparse-checkout-compatibility.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ test_expect_success 'setup' '
mkdir folder1 folder2 deep x &&
mkdir deep/deeper1 deep/deeper2 deep/before deep/later &&
mkdir deep/deeper1/deepest &&
mkdir deep/deeper1/deepest2 &&
mkdir deep/deeper1/deepest3 &&
echo "after deeper1" >deep/e &&
echo "after deepest" >deep/deeper1/e &&
cp a folder1 &&
Expand All @@ -30,7 +32,9 @@ test_expect_success 'setup' '
cp a deep/deeper2 &&
cp a deep/later &&
cp a deep/deeper1/deepest &&
cp -r deep/deeper1/deepest deep/deeper2 &&
cp a deep/deeper1/deepest2 &&
cp a deep/deeper1/deepest3 &&
cp -r deep/deeper1/ deep/deeper2 &&
mkdir deep/deeper1/0 &&
mkdir deep/deeper1/0/0 &&
touch deep/deeper1/0/1 &&
Expand Down Expand Up @@ -126,6 +130,8 @@ test_expect_success 'setup' '
git checkout -b deepest base &&
echo "updated deepest" >deep/deeper1/deepest/a &&
echo "updated deepest2" >deep/deeper1/deepest2/a &&
echo "updated deepest3" >deep/deeper1/deepest3/a &&
git commit -a -m "update deepest" &&
git checkout -f base &&
Expand Down Expand Up @@ -302,6 +308,14 @@ test_expect_success 'add, commit, checkout' '
test_all_match git checkout -
'

test_expect_success 'deep changes during checkout' '
init_repos &&
test_sparse_match git sparse-checkout set deep/deeper1/deepest &&
test_all_match git checkout deepest &&
test_all_match git checkout base
'

test_expect_success 'add outside sparse cone' '
init_repos &&
Expand Down
14 changes: 8 additions & 6 deletions unpack-trees.c
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,9 @@ static int find_cache_pos(struct traverse_info *info,

/*
* Given a sparse directory entry 'ce', compare ce->name to
* info->name + '/' + p->path + '/' if info->name is non-empty.
* info->traverse_path + p->path + '/' if info->traverse_path
* is non-empty.
*
* Compare ce->name to p->path + '/' otherwise. Note that
* ce->name must end in a trailing '/' because it is a sparse
* directory entry.
Expand All @@ -1270,11 +1272,11 @@ static int sparse_dir_matches_path(const struct cache_entry *ce,
assert(S_ISSPARSEDIR(ce->ce_mode));
assert(ce->name[ce->ce_namelen - 1] == '/');

if (info->namelen)
return ce->ce_namelen == info->namelen + p->pathlen + 2 &&
ce->name[info->namelen] == '/' &&
!strncmp(ce->name, info->name, info->namelen) &&
!strncmp(ce->name + info->namelen + 1, p->path, p->pathlen);
if (info->pathlen)
return ce->ce_namelen == info->pathlen + p->pathlen + 1 &&
ce->name[info->pathlen - 1] == '/' &&
!strncmp(ce->name, info->traverse_path, info->pathlen) &&
!strncmp(ce->name + info->pathlen, p->path, p->pathlen);
return ce->ce_namelen == p->pathlen + 1 &&
!strncmp(ce->name, p->path, p->pathlen);
}
Expand Down

0 comments on commit 9fab0b5

Please sign in to comment.