Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sparse Index] Integrate with git status #374

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
2a4a725
sparse-index: skip indexes with unmerged entries
derrickstolee Apr 29, 2021
f5bae86
sparse-index: include EXTENDED flag when expanding
derrickstolee May 4, 2021
d965669
t1092: replace incorrect 'echo' with 'cat'
derrickstolee May 26, 2021
44a9402
t1092: expand repository data shape
derrickstolee May 14, 2021
701ac0e
t1092: add tests for status/add and sparse files
derrickstolee Apr 12, 2021
587333f
unpack-trees: preserve cache_bottom
derrickstolee Apr 21, 2021
6fc898a
unpack-trees: compare sparse directories correctly
derrickstolee Apr 21, 2021
b676ef4
unpack-trees: unpack sparse directory entries
derrickstolee May 27, 2021
d693f00
dir.c: accept a directory as part of cone-mode patterns
derrickstolee Jan 11, 2021
ed11cfc
diff-lib: handle index diffs with sparse dirs
derrickstolee Jun 1, 2021
48fd25a
status: skip sparse-checkout percentage with sparse-index
derrickstolee Jan 15, 2021
3499105
status: use sparse-index throughout
derrickstolee Jan 8, 2021
60a6706
wt-status: expand added sparse directory entries
derrickstolee May 4, 2021
76bd8ec
fsmonitor: integrate with sparse index
derrickstolee Jan 15, 2021
a1a570a
Merge sparse-aware 'git status' into vfs-2.32.0
derrickstolee Jun 15, 2021
093a832
t1092: cleanups
derrickstolee Jun 17, 2021
722e7cd
fixup! unpack-trees: unpack sparse directory entries
derrickstolee Jun 17, 2021
9edbebf
fixup! unpack-trees: unpack sparse directory entries
derrickstolee Jun 17, 2021
0fda21d
fixup! t1092: expand repository data shape
derrickstolee Jun 17, 2021
610518c
fixup! unpack-trees: unpack sparse directory entries
derrickstolee Jun 17, 2021
320586f
fixup! dir.c: accept a directory as part of cone-mode patterns
derrickstolee Jun 17, 2021
ddaebb7
fixup! wt-status: expand added sparse directory entries
derrickstolee Jun 17, 2021
44bfb50
fixup! sparse-index: implement ensure_full_index()
derrickstolee Jun 17, 2021
0c20b94
fixup! fsmonitor: integrate with sparse index
derrickstolee Jun 17, 2021
f462956
fixup! fixup! t1092: expand repository data shape
derrickstolee Jun 18, 2021
87a3a29
fixup! wt-status: expand added sparse directory entries
derrickstolee Jun 21, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1397,6 +1397,17 @@ enum pattern_match_result path_matches_pattern_list(
strbuf_addch(&parent_pathname, '/');
strbuf_add(&parent_pathname, pathname, pathlen);

/*
* Directory entries are matched if and only if a file
* contained immediately within them is matched. For the
* case of a directory entry, modify the path to create
* a fake filename within this directory, allowing us to
* use the file-base matching logic in an equivalent way.
*/
if (parent_pathname.len > 0 &&
parent_pathname.buf[parent_pathname.len - 1] == '/')
strbuf_add(&parent_pathname, "-", 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would . make sense here instead of -?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason having a filename as . seems stranger to me than -. Perhaps _ would seem less odd?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked with @dscho offline here and here is the conclusion:

  • The recommendation here is clever: path/to/directory/. is semantically identical to path/to/directory, so . is a natural option here.
  • This relies on the cone-mode path-checking logic to be ignorant of this meaning of the trailing /.. While it is dumb enough to ignore that, we would be adding an expectation that it remains dumb and otherwise doesn't check that the input path is a valid name for a file.

The conclusion is that this is a clever idea, but perhaps too clever.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure there is a correct answer here. But it makes me want to say that we want to treat the ce's as a {type,path} tuple with an accessor and all that and get away from the pattern of just iterating over a dumb list of strings (that all callers know is a dumb list of strings).

Is there existing code to detect file -vs- symlink collisions? or file -vs- submodule collisions? How are they handled?
I mean we're adding the concept of (sparse) directory nodes into the index; it would be nice if they collided the same way. (I'm not trying to cause problems, just asking dumb questions.)


derrickstolee marked this conversation as resolved.
Show resolved Hide resolved
if (hashmap_contains_path(&pl->recursive_hashmap,
&parent_pathname)) {
result = MATCHED_RECURSIVE;
Expand Down
20 changes: 19 additions & 1 deletion sparse-index.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,17 @@ int set_sparse_index_config(struct repository *repo, int enable)
return res;
}

static int index_has_unmerged_entries(struct index_state *istate)
{
int i;
for (i = 0; i < istate->cache_nr; i++) {
if (ce_stage(istate->cache[i]))
return 1;
}

return 0;
}

int convert_to_sparse(struct index_state *istate)
{
int test_env;
Expand Down Expand Up @@ -152,6 +163,13 @@ int convert_to_sparse(struct index_state *istate)
return -1;
}

/*
* NEEDSWORK: If we have unmerged entries, then stay full.
* Unmerged entries prevent the cache-tree extension from working.
*/
if (index_has_unmerged_entries(istate))
return 0;

if (cache_tree_update(istate, 0)) {
warning(_("unable to update cache-tree, staying full"));
return -1;
Expand Down Expand Up @@ -195,7 +213,7 @@ static int add_path_to_index(const struct object_id *oid,
strbuf_addstr(base, path);

ce = make_cache_entry(istate, mode, oid, base->buf, 0, 0);
ce->ce_flags |= CE_SKIP_WORKTREE;
ce->ce_flags |= CE_SKIP_WORKTREE | CE_EXTENDED;
set_index_entry(istate, istate->cache_nr++, ce);

strbuf_setlen(base, len);
Expand Down
101 changes: 98 additions & 3 deletions t/t1092-sparse-checkout-compatibility.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,28 @@ test_expect_success 'setup' '
echo "after folder1" >g &&
echo "after x" >z &&
mkdir folder1 folder2 deep x &&
mkdir deep/deeper1 deep/deeper2 &&
mkdir deep/deeper1 deep/deeper2 deep/before deep/later &&
mkdir deep/deeper1/deepest &&
echo "after deeper1" >deep/e &&
echo "after deepest" >deep/deeper1/e &&
cp a folder1 &&
cp a folder2 &&
cp a x &&
cp a deep &&
cp a deep/before &&
cp a deep/deeper1 &&
cp a deep/deeper2 &&
cp a deep/later &&
cp a deep/deeper1/deepest &&
cp -r deep/deeper1/deepest deep/deeper2 &&
mkdir deep/deeper1/0 &&
mkdir deep/deeper1/0/0 &&
touch deep/deeper1/0/1 &&
touch deep/deeper1/0/0/0 &&
cp -r deep/deeper1/0 folder1 &&
cp -r deep/deeper1/0 folder2 &&
echo >>folder1/0/0/0 &&
echo >>folder2/0/1 &&
git add . &&
git commit -m "initial commit" &&
git checkout -b base &&
Expand All @@ -40,7 +50,7 @@ test_expect_success 'setup' '
done &&

git checkout -b rename-base base &&
echo >folder1/larger-content <<-\EOF &&
cat >folder1/larger-content <<-\EOF &&
matching
lines
help
Expand All @@ -56,18 +66,27 @@ test_expect_success 'setup' '
mv folder1/a folder2/b &&
mv folder1/larger-content folder2/edited-content &&
echo >>folder2/edited-content &&
echo >>folder2/0/1 &&
echo stuff >>deep/deeper1/a &&
git add . &&
git commit -m "rename folder1/... to folder2/..." &&

git checkout -b rename-out-to-in rename-base &&
mv folder1/a deep/deeper1/b &&
echo more stuff >>deep/deeper1/a &&
rm folder2/0/1 &&
mkdir folder2/0/1 &&
echo >>folder2/0/1/1 &&
mv folder1/larger-content deep/deeper1/edited-content &&
echo >>deep/deeper1/edited-content &&
git add . &&
git commit -m "rename folder1/... to deep/deeper1/..." &&

git checkout -b rename-in-to-out rename-base &&
mv deep/deeper1/a folder1/b &&
echo >>folder2/0/1 &&
rm -rf folder1/0/0 &&
echo >>folder1/0/0 &&
mv deep/deeper1/larger-content folder1/edited-content &&
echo >>folder1/edited-content &&
git add . &&
Expand Down Expand Up @@ -232,6 +251,44 @@ test_expect_success 'add, commit, checkout' '
test_all_match git checkout -
'

test_expect_success 'status/add: outside sparse cone' '
init_repos &&

# adding a "missing" file outside the cone should fail
test_sparse_match test_must_fail git add folder1/a &&

# folder1 is at HEAD, but outside the sparse cone
run_on_sparse mkdir folder1 &&
cp initial-repo/folder1/a sparse-checkout/folder1/a &&
cp initial-repo/folder1/a sparse-index/folder1/a &&

test_sparse_match git status &&

write_script edit-contents <<-\EOF &&
echo text >>$1
EOF
run_on_sparse ../edit-contents folder1/a &&
run_on_all ../edit-contents folder1/new &&

test_sparse_match git status --porcelain=v2 &&

# This "git add folder1/a" fails with a warning
# in the sparse repos, differing from the full
# repo. This is intentional.
test_sparse_match test_must_fail git add folder1/a &&
test_sparse_match test_must_fail git add --refresh folder1/a &&
test_all_match git status --porcelain=v2 &&

test_all_match git add . &&
test_all_match git status --porcelain=v2 &&
test_all_match git commit -m folder1/new &&

run_on_all ../edit-contents folder1/newer &&
test_all_match git add folder1/ &&
test_all_match git status --porcelain=v2 &&
test_all_match git commit -m folder1/newer
'

test_expect_success 'checkout and reset --hard' '
init_repos &&

Expand Down Expand Up @@ -262,13 +319,29 @@ test_expect_success 'diff --staged' '
test_all_match git diff --staged
'

test_expect_success 'diff with renames' '
test_expect_success 'diff with renames and conflicts' '
init_repos &&

for branch in rename-out-to-out rename-out-to-in rename-in-to-out
do
test_all_match git checkout rename-base &&
test_all_match git checkout $branch -- .&&
derrickstolee marked this conversation as resolved.
Show resolved Hide resolved
test_all_match git status --porcelain=v2 &&
test_all_match git diff --staged --no-renames &&
test_all_match git diff --staged --find-renames || return 1
done
'

test_expect_success 'diff with directory/file conflicts' '
init_repos &&

for branch in rename-out-to-out rename-out-to-in rename-in-to-out
do
git -C full-checkout reset --hard &&
test_sparse_match git reset --hard &&
test_all_match git checkout $branch &&
test_all_match git checkout rename-base -- . &&
test_all_match git status --porcelain=v2 &&
test_all_match git diff --staged --no-renames &&
test_all_match git diff --staged --find-renames || return 1
done
Expand Down Expand Up @@ -352,6 +425,28 @@ test_expect_success 'merge with outside renames' '
done
'

# Sparse-index fails to convert the index in the
# final 'git cherry-pick' command.
test_expect_success 'cherry-pick with conflicts' '
init_repos &&

write_script edit-conflict <<-\EOF &&
echo $1 >conflict
EOF

test_all_match git checkout -b to-cherry-pick &&
run_on_all ../edit-conflict ABC &&
test_all_match git add conflict &&
test_all_match git commit -m "conflict to pick" &&

test_all_match git checkout -B base HEAD~1 &&
run_on_all ../edit-conflict DEF &&
test_all_match git add conflict &&
test_all_match git commit -m "conflict in base" &&

test_all_match test_must_fail git cherry-pick to-cherry-pick
'

test_expect_success 'clean' '
init_repos &&

Expand Down
Loading