--- v6
+++ v5
@@ -26,136 +26,80 @@
Thanks, -Stolee
-Update in V6
-============
-
-I'm sorry that this revision took so long. Initially I was blocked on
-getting the directory/file conflict figured out (I did), but also my team
-was very busy with some things. Eventually, we reached an internal deadline
-to make an experimental release available [1] with initial sparse-index
-performance boosts. Creating that included some additional review by Jeff
-Hostetler and Johannes Schindelin which led to more changes in this version.
-
-The good news is that this series has now created the basis for many Git
-commands to integrate with the sparse-index without much additional work.
-This effort was unfortunately overloaded on this series because the changes
-needed for things like 'git checkout' or 'git add' all intersect with the
-changes needed for 'git status'. Might as well get it right the first time.
-
-Because the range-diff is a big difficult to read this time, I'll break the
-changes down on a patch-by-patch basis.
-
- 1. sparse-index: skip indexes with unmerged entries
-
- (no change)
-
- 2. sparse-index: include EXTENDED flag when expanding
-
- * Commit message better describes the purpose of the change.
-
- 3. t1092: replace incorrect 'echo' with 'cat'
-
- * Typo fix
-
- 4. t1092: expand repository data shape
-
- * some files are added that surround "folder1/" immediately before and
- after, based on the sorting with the trailing slash. This provides extra
- coverage.
-
- 5. t1092: add tests for status/add and sparse files
-
- (no change)
-
- 6. unpack-trees: preserve cache_bottom
-
- (no change)
-
- 7. unpack-trees: compare sparse directories correctly
-
- * We were previosly not comparing the path lengths, which causes a problem
- (with later updates) when a sparse directory such as "folder1/0/" gets
- compared to a tree name "folder1".
-
- 8. unpack-trees: rename unpack_nondirectories()
-
- * This new commit changes the name to make more sense with its new behavior
- that could modify a sparse directory entry. The point of the method is in
- contrast to recursing into trees.
-
- 9. unpack-trees: unpack sparse directory entries
-
- * THIS is the biggest change from previous versions. There were a few
- things going on that were tricky to get right, especially with the
- directory/file conflict (handled in an update in the following, new
- patch).
-
- * The changes to create_ce_entry() regarding alloc_len missed a spot that
- was critical to getting the length right in the allocated entry.
-
- * Use '\0' over 0 to represent the terminating character.
-
- * We don't need a "sparse_directory" parameter to unpack_nondirectories()
- (which was renamed to unpack_single_entry() by the previous new patch)
- because we can use dirmask to discover if src[0] (or any other value)
- should be a sparse directory entry.
-
- * Similarly, we don't need to call the method twice from unpack_callback().
-
- * The 'conflicts' variable is set to match the dirmask in the beginning,
- but it should depend on whether or not we have a sparse directory entry
- instead, and if all trees that have the path have a directory.
-
- * The implementation of find_cache_entry() uses find_cache_pos() to find an
- insertion position for a path if it doesn't find an exact match. Before,
- we subtracted one to find the sparse directory entry, but there could be
- multiple paths between the sparse directory entry and the insertion
- point, so we need to walk backwards until we find it. This requires many
- paths having the same prefix, so hopefully is a rare case. Some of the
- test data changes were added to cover the need for this logic. This uses
- a helper method, sparse_dir_matches_path, which is also used by
- is_sparse_directory_entry.
-
- 10. unpack-trees: handle dir/file conflict of sparse entries
-
- * This new logic inside twoway_merge handles the special case for dealing
- with a directory/file conflict during a 'git checkout'. The necessarily
- data and tests are also added here, though the logic will only take
- serious effect when we integrate with 'git checkout' later.
-
- 11. dir.c: accept a directory as part of cone-mode patterns
-
- * The value slash_pos was previously a pointer within a strbuf, but in some
- cases we add to that strbuf and that could reallocate the pointer, making
- slash_pos be invalid. The replacement is to have slash_pos be an integer
- position within the string, so it is consistent even if the string is
- reallocated for an append.
-
- 12. diff-lib: handle index diffs with sparse dirs
-
- * As recommended in the previous review, a simple diff_tree_oid() replaces
- the complicated use of read_tree_at() and traverse_trees() in the
- previous version.
-
- 13. status: skip sparse-checkout percentage with sparse-index
-
- (no change)
-
- 14. status: use sparse-index throughout
-
- (no change)
-
- 15. wt-status: expand added sparse directory entries
-
- * Duplicate 'git status --porcelain=v2' lines are removed from tests.
-
- * The pathspec is initialized using "= { 0 }" instead of memset().
-
- 16. fsmonitor: integrate with sparse index
-
- * An extra test_region is added to ensure that the filesystem monitor hook
- is still being called, and we are not simply disabling the feature
- entirely.
+Updates in V5
+=============
+
+I replaced one patch with a few that are more complicated. The reason is
+that I started integrating with git checkout and realized that some of the
+changes I was making in unpack-trees.c were incorrect for that situation, so
+I might as well do them right here. The tests can't demonstrate the bugs
+with the previous case until we integrate with git checkout, which will
+follow in another series after this one is submitted.
+
+For testing, I've integrated this series along with extensions that work for
+git commit and git checkout into the Scalar functional tests, which test
+many scenarios with cone mode sparse-checkout and hence provides good
+evidence that this is working correctly.
+
+
+Updates in V4
+=============
+
+ * The previous patch "unpack-trees: stop recursing into sparse directories"
+ was confusing, and actually a bit sloppy.
+ * It has been replaced with "unpack-trees: be careful around sparse
+ directory entries" which takes the sparse-directory checks and raises
+ them higher up into unpack_trees.c instead of in diff-lib.c.
+
+
+Updates in V3
+=============
+
+Sorry that this was a long time coming. I got a little side-tracked on other
+projects, but I also worked to get the sparse-index feature working against
+the Scalar functional tests, which contain many special cases around the
+sparse-checkout feature as they were inherited from special cases that arose
+in the virtualized environment of VFS for Git. This version contains my
+fixes based on that investigation. Most of these were easy to identify and
+fix, but I was blocked for a long time struggling with a bug when combining
+the sparse-index with the builtin FS Monitor feature, but I've reported my
+findings already [1].
+
+[1]
+https://lore.kernel.org/git/0b9e54ba-ac27-e537-7bef-1b4448f92352@gmail.com/
+
+ * Updated comments and tests based on the v2 feedback.
+ * Expanded the test repository data shape based on the special cases found
+ during my investigation.
+ * Added several commits that either fix errors in the status code, or fix
+ errors in the previous sparse-index series, specifically:
+ * When in a conflict state, the cache-tree fails to update. For now, skip
+ writing a sparse-index until this can be resolved more carefully.
+ * When expanding a sparse-directory entry, we set the CE_SKIP_WORKTREE
+ bit but forgot the CE_EXTENDED bit.
+ * git status had failures if there was a sparse-directory entry as the
+ first entry within a directory.
+ * When expanding a directory to report its status, such as when a
+ sparse-directory is staged but doesn't exist at HEAD (such as in an
+ orphaned commit) we did not previously recurse correctly into
+ subdirectories.
+ * Be extra careful with the FS Monitor data when expanding or contracting
+ an index. This version now abandons all FS Monitor data at these
+ conversion points with the expectation that in the future these
+ conversions will be rare so the FS Monitor feature can work
+ efficiently. Updates in V2
+
+----------------------------------------------------------------------------
+
+ * Based on the feedback, it is clear that 'git add' will require much more
+ careful testing and thought. I'm splitting it out of this series and it
+ will return with a follow-up.
+ * Test cases are improved, both in coverage and organization.
+ * The previous "unpack-trees: make sparse aware" patch is split into three
+ now.
+ * Stale messages based on an old implementation of the "protections" topic
+ are now fixed.
+ * Performance tests were re-run.
Derrick Stolee (14):
sparse-index: skip indexes with unmerged entries
@@ -174,531 +118,353 @@
fsmonitor: integrate with sparse index
builtin/commit.c | 3 +
- diff-lib.c | 16 +++
+ diff-lib.c | 188 +++++++++++++++++++++++
dir.c | 11 ++
read-cache.c | 10 +-
sparse-index.c | 27 +++-
- t/t1092-sparse-checkout-compatibility.sh | 155 ++++++++++++++++++++++-
- t/t7519-status-fsmonitor.sh | 48 +++++++
- unpack-trees.c | 130 ++++++++++++++++---
- wt-status.c | 63 ++++++++-
+ t/t1092-sparse-checkout-compatibility.sh | 158 ++++++++++++++++++-
+ t/t7519-status-fsmonitor.sh | 48 ++++++
+ unpack-trees.c | 121 +++++++++++++--
+ wt-status.c | 64 +++++++-
wt-status.h | 1 +
- 10 files changed, 436 insertions(+), 28 deletions(-)
-
-
-base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb
-Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-932%2Fderrickstolee%2Fsparse-index%2Fstatus-and-add-v6
-Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-932/derrickstolee/sparse-index/status-and-add-v6
+ 10 files changed, 607 insertions(+), 24 deletions(-)
+
+
+base-commit: f723f370c89ad61f4f40aabfd3540b1ce19c00e5
+Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-932%2Fderrickstolee%2Fsparse-index%2Fstatus-and-add-v5
+Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-932/derrickstolee/sparse-index/status-and-add-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/932
-Range-diff vs v5:
-
- 1: 5a2ed3d1d70 = 1: 2a4a7256304 sparse-index: skip indexes with unmerged entries
- 2: 8aa41e74947 ! 2: f5bae86014d sparse-index: include EXTENDED flag when expanding
- @@ Commit message
-
- When creating a full index from a sparse one, we create cache entries
- for every blob within a given sparse directory entry. These are
- - correctly marked with the CE_SKIP_WORKTREE flag, but they must also be
- - marked with the CE_EXTENDED flag to ensure that the skip-worktree bit is
- - correctly written to disk in the case that the index is not converted
- - back down to a sparse-index.
- + correctly marked with the CE_SKIP_WORKTREE flag, but the CE_EXTENDED
- + flag is not included. The CE_EXTENDED flag would exist if we loaded a
- + full index from disk with these entries marked with CE_SKIP_WORKTREE, so
- + we can add the flag here to be consistent. This allows us to directly
- + compare the flags present in cache entries when testing the sparse-index
- + feature, but has no significance to its correctness in the user-facing
- + functionality.
+Range-diff vs v4:
+
+ 1: 5a2ed3d1d701 = 1: 5a2ed3d1d701 sparse-index: skip indexes with unmerged entries
+ 2: 8aa41e749471 = 2: 8aa41e749471 sparse-index: include EXTENDED flag when expanding
+ -: ------------ > 3: b99371c7dd61 t1092: replace incorrect 'echo' with 'cat'
+ 3: 70971b1f9261 ! 4: f4dddac1859e t1092: expand repository data shape
+ @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'setup' '
+ + 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 &&
+ +@@ t/t1092-sparse-checkout-compatibility.sh: 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 . &&
+ +@@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'setup' '
+ +
+ + 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 . &&
+ +@@ t/t1092-sparse-checkout-compatibility.sh: 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 -- .&&
+ ++ 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
+ 4: a80b5a41153f = 5: 856346b72f79 t1092: add tests for status/add and sparse files
+ 5: 07a45b661c4a = 6: f3f6223e955f unpack-trees: preserve cache_bottom
+ 6: cc4a526e7947 = 7: 45ae96adf285 unpack-trees: compare sparse directories correctly
+ 7: e28df7f9395d ! 8: 724194eef9f6 unpack-trees: be careful around sparse directory entries
+ @@ Metadata
+ Author: Derrick Stolee <dstolee@microsoft.com>
+
+ ## Commit message ##
+ - unpack-trees: be careful around sparse directory entries
+ + unpack-trees: unpack sparse directory entries
+
+ - The methods traverse_by_cache_tree() and unpack_nondirectories() have
+ - similar behavior in trying to demonstrate the difference between and
+ - index and a tree, with some differences about how they walk the index.
+ + During unpack_callback(), index entries are compared against tree
+ + entries. These are matched according to names and types. One goal is to
+ + decide if we should recurse into subtrees or simply operate on one index
+ + entry.
+
+ - Each of these is expecting every cache entry to correspond to a file
+ - path. We need to skip over the sparse directory entries in the case of a
+ - sparse-index. Those entries are discovered in the portion that looks for
+ - subtrees among the cache entries by scanning the paths for slashes.
+ + In the case of a sparse-directory entry, we do not want to recurse into
+ + that subtree and instead simply compare the trees. In some cases, we
+ + might want to perform a merge operation on the entry, such as during
+ + 'git checkout <commit>' which wants to replace a sparse tree entry with
+ + the tree for that path at the target commit. We extend the logic within
+ + unpack_nondirectories() to create a sparse-directory entry in this case,
+ + and then that is sent to call_unpack_fn().
+
+ - Skipping these sparse directory entries will have a measurable effect
+ - when we relax 'git status' to work with sparse-indexes: without this
+ - change these methods would call call_unpack_fn() which in turn calls
+ - oneway_diff() and then shows these sparse directory entries as added or
+ - modified files.
+ + There are some subtleties in this process. For instance, we need to
+ + update find_cache_entry() to allow finding a sparse-directory entry that
+ + exactly matches a given path.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
- 3: b99371c7dd6 ! 3: d965669c766 t1092: replace incorrect 'echo' with 'cat'
- @@ Commit message
- t1092: replace incorrect 'echo' with 'cat'
-
- This fixes the test data shape to be as expected, allowing rename
- - detection to work properly now that the 'larger-conent' file actually
- + detection to work properly now that the 'larger-content' file actually
- has meaningful lines.
-
- Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
- 4: f4dddac1859 = 4: 44a940211b2 t1092: expand repository data shape
- 5: 856346b72f7 = 5: 701ac0e8ff6 t1092: add tests for status/add and sparse files
- 6: f3f6223e955 = 6: 587333f7c61 unpack-trees: preserve cache_bottom
- 7: 45ae96adf28 = 7: 6fc898ac23e unpack-trees: compare sparse directories correctly
- 8: 724194eef9f ! 8: b676ef4925b unpack-trees: unpack sparse directory entries
- @@ Commit message
- 'git checkout <commit>' which wants to replace a sparse tree entry with
- the tree for that path at the target commit. We extend the logic within
- unpack_nondirectories() to create a sparse-directory entry in this case,
- - and then that is sent to call_unpack_fn().
- + and then that is sent to call_unpack_fn(). Since the name becomes
- + confusing by handling directories, rename it to unpack_single_entry()
- + since it handles a blob entry or a sparse directory entry without using
- + traverse_trees_recursive().
-
- There are some subtleties in this process. For instance, we need to
- update find_cache_entry() to allow finding a sparse-directory entry that
- - exactly matches a given path.
- + exactly matches a given path. Use the new helper method
- + sparse_dir_matches_path() for this.
-
- Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
-
- @@ unpack-trees.c: static struct cache_entry *create_ce_entry(const struct traverse
- + size_t alloc_len = is_sparse_directory ? len + 1 : len;
- struct cache_entry *ce =
- is_transient ?
- -- make_empty_transient_cache_entry(len) :
- +- make_empty_transient_cache_entry(len, NULL) :
- - make_empty_cache_entry(istate, len);
- -+ make_empty_transient_cache_entry(alloc_len) :
- ++ make_empty_transient_cache_entry(alloc_len, NULL) :
- + make_empty_cache_entry(istate, alloc_len);
+ ## unpack-trees.c ##
+ -@@ unpack-trees.c: static int traverse_by_cache_tree(int pos, int nr_entries, int nr_names,
+ +@@ unpack-trees.c: static struct cache_entry *create_ce_entry(const struct traverse_info *info,
+ + const struct name_entry *n,
+ + int stage,
+ + struct index_state *istate,
+ +- int is_transient)
+ ++ int is_transient,
+ ++ int is_sparse_directory)
+ + {
+ + size_t len = traverse_path_len(info, tree_entry_len(n));
+ ++ size_t alloc_len = is_sparse_directory ? len + 1 : len;
+ + struct cache_entry *ce =
+ + is_transient ?
+ +- make_empty_transient_cache_entry(len) :
+ +- make_empty_cache_entry(istate, len);
+ ++ make_empty_transient_cache_entry(alloc_len) :
+ ++ make_empty_cache_entry(istate, alloc_len);
- ce->ce_mode = create_ce_mode(n->mode);
- @@ unpack-trees.c: static struct cache_entry *create_ce_entry(const struct traverse
+ - src[0] = o->src_index->cache[pos + i];
+ + ce->ce_mode = create_ce_mode(n->mode);
+ + ce->ce_flags = create_ce_flags(stage);
+ +@@ unpack-trees.c: static struct cache_entry *create_ce_entry(const struct traverse_info *info,
+ + /* len+1 because the cache_entry allocates space for NUL */
+ + make_traverse_path(ce->name, len + 1, info, n->path, n->pathlen);
- + if (is_sparse_directory) {
- + ce->name[len] = '/';
- -+ ce->name[len + 1] = 0;
- ++ ce->name[len + 1] = '\0';
- + ce->ce_namelen++;
- + ce->ce_flags |= CE_SKIP_WORKTREE;
- + }
- @@ unpack-trees.c: static struct cache_entry *create_ce_entry(const struct traverse
- return ce;
- }
+ -+ if (S_ISSPARSEDIR(src[0]->ce_mode))
+ -+ continue;
+ ++ if (is_sparse_directory) {
+ ++ ce->name[len] = '/';
+ ++ ce->name[len + 1] = 0;
+ ++ ce->ce_namelen++;
+ ++ ce->ce_flags |= CE_SKIP_WORKTREE;
+ ++ }
+ +
+ - len = ce_namelen(src[0]);
+ - new_ce_len = cache_entry_size(len);
+ + return ce;
+ + }
- -@@ unpack-trees.c: static int unpack_nondirectories(int n, unsigned long mask,
- - unsigned long dirmask,
- - struct cache_entry **src,
- - const struct name_entry *names,
- +@@ unpack-trees.c: static struct cache_entry *create_ce_entry(const struct traverse_info *info,
- + * without actually calling it. If you change the logic here you may need to
- + * check and change there as well.
- + */
- +-static int unpack_nondirectories(int n, unsigned long mask,
- +- unsigned long dirmask,
- +- struct cache_entry **src,
- +- const struct name_entry *names,
- - const struct traverse_info *info)
- -+ const struct traverse_info *info,
- -+ int sparse_directory)
- ++static int unpack_single_entry(int n, unsigned long mask,
- ++ unsigned long dirmask,
- ++ struct cache_entry **src,
- ++ const struct name_entry *names,
- ++ const struct traverse_info *info,
- ++ int sparse_directory)
- {
- int i;
- struct unpack_trees_options *o = info->data;
- @@ unpack-trees.c: static int unpack_nondirectories(int n, unsigned long mask,
+ @@ unpack-trees.c: static int unpack_nondirectories(int n, unsigned long mask,
+ + unsigned long dirmask,
+ + struct cache_entry **src,
+ + const struct name_entry *names,
+ +- const struct traverse_info *info)
+ ++ const struct traverse_info *info,
+ ++ int sparse_directory)
+ + {
+ + int i;
+ + struct unpack_trees_options *o = info->data;
+ + unsigned long conflicts = info->df_conflicts | dirmask;
+ +
+ +- /* Do we have *only* directories? Nothing to do */
if (mask == dirmask && !src[0])
return 0;
- -+ /* no-op if our cache entry doesn't match the expectations. */
- ++ /* defer work if our cache entry doesn't match the expectations. */
- + if (sparse_directory) {
- + if (src[0] && !S_ISSPARSEDIR(src[0]->ce_mode))
- + BUG("expected sparse directory entry");
- @@ unpack-trees.c: static int unpack_nondirectories(int n, unsigned long mask,
-
- if (o->merge) {
- @@ unpack-trees.c: static int find_cache_pos(struct traverse_info *info,
- + return -1;
- + }
- +
- ++/*
- ++ * Given a sparse directory entry 'ce', compare ce->name to
- ++ * info->name + '/' + p->path + '/' if info->name 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.
- ++ */
- ++static int sparse_dir_matches_path(const struct cache_entry *ce,
- ++ struct traverse_info *info,
- ++ const struct name_entry *p)
- ++{
- ++ 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);
- ++ return ce->ce_namelen == p->pathlen + 1 &&
- ++ !strncmp(ce->name, p->path, p->pathlen);
- ++}
- ++
- static struct cache_entry *find_cache_entry(struct traverse_info *info,
- const struct name_entry *p)
- {
- @@ unpack-trees.c: static int find_cache_pos(struct traverse_info *info,
- + pos = -pos - 2;
- +
- + if (pos < 0 || pos >= o->src_index->cache_nr)
- -+ return NULL;
- + return NULL;
- +
- + ce = o->src_index->cache[pos];
- +
- + if (!S_ISSPARSEDIR(ce->ce_mode))
- - return NULL;
- ++ return NULL;
- +
- -+ /*
- -+ * Compare ce->name to info->name + '/' + p->path + '/'
- -+ * if info->name is non-empty. Compare ce->name to
- -+ * p-.path + '/' otherwise.
- -+ */
- -+ if (info->namelen) {
- -+ if (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))
- -+ return ce;
- -+ } else if (ce->ce_namelen == p->pathlen + 1 &&
- -+ !strncmp(ce->name, p->path, p->pathlen))
- ++ if (sparse_dir_matches_path(ce, info, p))
- + return ce;
- ++
- + return NULL;
- }
-
- @@ unpack-trees.c: static void debug_unpack_callback(int n,
- + * sparse-directory entry that matches the given name_entry
- + * from the tree walk at the given traverse_info.
- + */
- -+static int is_sparse_directory_entry(struct cache_entry *ce, struct name_entry *name, struct traverse_info *info)
- ++static int is_sparse_directory_entry(struct cache_entry *ce,
- ++ struct name_entry *name,
- ++ struct traverse_info *info)
- +{
- -+ size_t expected_len, name_start;
- -+
- + if (!ce || !name || !S_ISSPARSEDIR(ce->ce_mode))
+ -+ if (src[0] && S_ISSPARSEDIR(src[0]->ce_mode))
+ ++ /* no-op if our cache entry doesn't match the expectations. */
+ ++ if (sparse_directory) {
+ ++ if (src[0] && !S_ISSPARSEDIR(src[0]->ce_mode))
+ ++ BUG("expected sparse directory entry");
+ ++ } else if (src[0] && S_ISSPARSEDIR(src[0]->ce_mode)) {
+ return 0;
- +
- -+ if (info->namelen)
- -+ name_start = info->namelen + 1;
- -+ else
- -+ name_start = 0;
- -+ expected_len = name->pathlen + 1 + name_start;
- -+
- -+ if (ce->ce_namelen != expected_len ||
- -+ strncmp(ce->name, info->name, info->namelen) ||
- -+ strncmp(ce->name + name_start, name->path, name->pathlen))
- -+ return 0;
- -+
- -+ return 1;
- ++ return sparse_dir_matches_path(ce, info, name);
- +}
- +
- /*
- @@ unpack-trees.c: static int unpack_callback(int n, unsigned long mask, unsigned l
- }
-
- - if (unpack_nondirectories(n, mask, dirmask, src, names, info) < 0)
- -+ if (unpack_nondirectories(n, mask, dirmask, src, names, info, 0) < 0)
- ++ if (unpack_single_entry(n, mask, dirmask, src, names, info, 0) < 0)
- return -1;
-
- if (o->merge && src[0]) {
- @@ unpack-trees.c: static int unpack_callback(int n, unsigned long mask, unsigned l
- - if (traverse_trees_recursive(n, dirmask, mask & ~dirmask,
- - names, info) < 0)
- + if (is_sparse_directory_entry(src[0], names, info)) {
- -+ if (unpack_nondirectories(n, dirmask, mask & ~dirmask, src, names, info, 1) < 0)
- ++ if (unpack_single_entry(n, dirmask, mask & ~dirmask, src, names, info, 1) < 0)
- + return -1;
- + } else if (traverse_trees_recursive(n, dirmask, mask & ~dirmask,
- + names, info) < 0) {
- 9: b8ff179f43e = 9: d693f00d9a2 dir.c: accept a directory as part of cone-mode patterns
- 10: b9b97e01129 ! 10: ed11cfc791f diff-lib: handle index diffs with sparse dirs
- @@ Commit message
- identical to before: the lack of a cache entry is the same with a sparse
- index.
-
- - In the case where a tree is modified, we need to expand the tree
- - recursively, and start comparing each contained entry as either an
- - addition, deletion, or modification. This causes an interesting
- - recursion that did not exist before.
- + Use diff_tree_oid() appropriately to appropriately compute the diff.
-
- Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
-
- ## diff-lib.c ##
- -@@ diff-lib.c: static int get_stat_data(const struct cache_entry *ce,
- - return 0;
- - }
- -
- -+struct show_new_tree_context {
- -+ struct rev_info *revs;
- -+ unsigned added:1;
- -+};
- -+
- -+static int show_new_file_from_tree(const struct object_id *oid,
- -+ struct strbuf *base, const char *path,
- -+ unsigned int mode, void *context)
- -+{
- -+ struct show_new_tree_context *ctx = context;
- -+ struct cache_entry *new_file = make_transient_cache_entry(mode, oid, path, /* stage */ 0);
- -+
- -+ diff_index_show_file(ctx->revs, ctx->added ? "+" : "-", new_file, oid, !is_null_oid(oid), mode, 0);
- -+ discard_cache_entry(new_file);
- -+ return 0;
- -+}
- -+
- -+static void show_directory(struct rev_info *revs,
- -+ const struct cache_entry *new_dir,
- -+ int added)
- -+{
- -+ /*
- -+ * new_dir is a sparse directory entry, so we want to collect all
- -+ * of the new files within the tree. This requires recursively
- -+ * expanding the trees.
- -+ */
- -+ struct show_new_tree_context ctx = { revs, added };
- -+ struct repository *r = revs->repo;
- -+ struct strbuf base = STRBUF_INIT;
- -+ struct pathspec ps;
- -+ struct tree *tree = lookup_tree(r, &new_dir->oid);
- -+
- -+ memset(&ps, 0, sizeof(ps));
- -+ ps.recursive = 1;
- -+ ps.has_wildcard = 1;
- -+ ps.max_depth = -1;
- -+
- -+ strbuf_add(&base, new_dir->name, new_dir->ce_namelen);
- -+ read_tree_at(r, tree, &base, &ps,
- -+ show_new_file_from_tree, &ctx);
- -+}
- -+
- - static void show_new_file(struct rev_info *revs,
- - const struct cache_entry *new_file,
- - int cached, int match_missing)
- @@ diff-lib.c: static void show_new_file(struct rev_info *revs,
- - unsigned int mode;
- unsigned dirty_submodule = 0;
- + struct index_state *istate = revs->diffopt.repo->index;
-
- + if (new_file && S_ISSPARSEDIR(new_file->ce_mode)) {
- -+ show_directory(revs, new_file, /*added */ 1);
- ++ diff_tree_oid(NULL, &new_file->oid, new_file->name, &revs->diffopt);
- + return;
- + }
+ ++ }
+
/*
- * New file in the index: it might actually be different in
- * the working tree.
- -@@ diff-lib.c: static void show_new_file(struct rev_info *revs,
- - diff_index_show_file(revs, "+", new_file, oid, !is_null_oid(oid), mode, dirty_submodule);
- - }
- -
- -+static int show_modified(struct rev_info *revs,
- -+ const struct cache_entry *old_entry,
- -+ const struct cache_entry *new_entry,
- -+ int report_missing,
- -+ int cached, int match_missing);
- -+
- -+static int compare_within_sparse_dir(int n, unsigned long mask,
- -+ unsigned long dirmask, struct name_entry *entry,
- -+ struct traverse_info *info)
- -+{
- -+ struct rev_info *revs = info->data;
- -+ struct object_id *oid0 = &entry[0].oid;
- -+ struct object_id *oid1 = &entry[1].oid;
- -+
- -+ if (oideq(oid0, oid1))
- -+ return mask;
- -+
- -+ /* Directory/file conflicts are handled earlier. */
- -+ if (S_ISDIR(entry[0].mode) && S_ISDIR(entry[1].mode)) {
- -+ struct tree_desc t[2];
- -+ void *buf[2];
- -+ struct traverse_info info_r = { NULL, };
- -+
- -+ info_r.name = xstrfmt("%s%s", info->traverse_path, entry[0].path);
- -+ info_r.namelen = strlen(info_r.name);
- -+ info_r.traverse_path = xstrfmt("%s/", info_r.name);
- -+ info_r.fn = compare_within_sparse_dir;
- -+ info_r.prev = info;
- -+ info_r.mode = entry[0].mode;
- -+ info_r.pathlen = entry[0].pathlen;
- -+ info_r.df_conflicts = 0;
- -+ info_r.data = revs;
- -+
- -+ buf[0] = fill_tree_descriptor(revs->repo, &t[0], oid0);
- -+ buf[1] = fill_tree_descriptor(revs->repo, &t[1], oid1);
- -+
- -+ traverse_trees(NULL, 2, t, &info_r);
- -+
- -+ free((char *)info_r.name);
- -+ free((char *)info_r.traverse_path);
- -+ free(buf[0]);
- -+ free(buf[1]);
- -+ } else {
- -+ char *old_path = NULL, *new_path = NULL;
- -+ struct cache_entry *old_entry = NULL, *new_entry = NULL;
- -+
- -+ if (entry[0].path) {
- -+ old_path = xstrfmt("%s%s", info->traverse_path, entry[0].path);
- -+ old_entry = make_transient_cache_entry(
- -+ entry[0].mode, &entry[0].oid,
- -+ old_path, /* stage */ 0);
- -+ old_entry->ce_flags |= CE_SKIP_WORKTREE;
- -+ }
- -+ if (entry[1].path) {
- -+ new_path = xstrfmt("%s%s", info->traverse_path, entry[1].path);
- -+ new_entry = make_transient_cache_entry(
- -+ entry[1].mode, &entry[1].oid,
- -+ new_path, /* stage */ 0);
- -+ new_entry->ce_flags |= CE_SKIP_WORKTREE;
- -+ }
- -+
- -+ if (entry[0].path && entry[1].path)
- -+ show_modified(revs, old_entry, new_entry, 0, 1, 0);
- -+ else if (entry[0].path)
- -+ diff_index_show_file(revs, revs->prefix,
- -+ old_entry, &entry[0].oid,
- -+ 0, entry[0].mode, 0);
- -+ else if (entry[1].path)
- -+ show_new_file(revs, new_entry, 1, 0);
- -+
- -+ discard_cache_entry(old_entry);
- -+ discard_cache_entry(new_entry);
- -+ free(old_path);
- -+ free(new_path);
- -+ }
- -+
- -+ return mask;
- -+}
- -+
- -+static void show_modified_sparse_directory(struct rev_info *revs,
- -+ const struct cache_entry *old_entry,
- -+ const struct cache_entry *new_entry,
- -+ int report_missing,
- -+ int cached, int match_missing)
- -+{
- -+ struct tree_desc t[2];
- -+ void *buf[2];
- -+ struct traverse_info info = { NULL };
- -+ struct strbuf name = STRBUF_INIT;
- -+ struct strbuf parent_path = STRBUF_INIT;
- -+ char *last_dir_sep;
- -+
- -+ if (oideq(&old_entry->oid, &new_entry->oid))
- -+ return;
- -+
- -+ info.fn = compare_within_sparse_dir;
- -+ info.prev = &info;
- -+
- -+ strbuf_add(&name, new_entry->name, new_entry->ce_namelen - 1);
- -+ info.name = name.buf;
- -+ info.namelen = name.len;
- -+
- -+ strbuf_add(&parent_path, new_entry->name, new_entry->ce_namelen - 1);
- -+ if ((last_dir_sep = find_last_dir_sep(parent_path.buf)) > parent_path.buf)
- -+ strbuf_setlen(&parent_path, (last_dir_sep - parent_path.buf) - 1);
- -+ else
- -+ strbuf_setlen(&parent_path, 0);
- -+
- -+ info.pathlen = parent_path.len;
- -+
- -+ if (parent_path.len)
- -+ info.traverse_path = parent_path.buf;
- -+ else
- -+ info.traverse_path = "";
- -+
- -+ info.mode = new_entry->ce_mode;
- -+ info.df_conflicts = 0;
- -+ info.data = revs;
- -+
- -+ buf[0] = fill_tree_descriptor(revs->repo, &t[0], &old_entry->oid);
- -+ buf[1] = fill_tree_descriptor(revs->repo, &t[1], &new_entry->oid);
- -+
- -+ traverse_trees(NULL, 2, t, &info);
- -+
- -+ free(buf[0]);
- -+ free(buf[1]);
- -+ strbuf_release(&name);
- -+ strbuf_release(&parent_path);
- -+}
- -+
- - static int show_modified(struct rev_info *revs,
- - const struct cache_entry *old_entry,
- - const struct cache_entry *new_entry,
- @@ diff-lib.c: static int show_modified(struct rev_info *revs,
- - const struct object_id *oid;
- unsigned dirty_submodule = 0;
- + struct index_state *istate = revs->diffopt.repo->index;
-
- + /*
- + * If both are sparse directory entries, then expand the
- @@ diff-lib.c: static int show_modified(struct rev_info *revs,
- + if (old_entry && new_entry &&
- + S_ISSPARSEDIR(old_entry->ce_mode) &&
- + S_ISSPARSEDIR(new_entry->ce_mode)) {
- -+ show_modified_sparse_directory(revs, old_entry, new_entry, report_missing, cached, match_missing);
- ++ diff_tree_oid(&old_entry->oid, &new_entry->oid, new_entry->name, &revs->diffopt);
- + return 0;
- + }
+ * Ok, we've filled in up to any potential index entry in src[0],
+ * now do the rest.
+ +@@ unpack-trees.c: static int unpack_nondirectories(int n, unsigned long mask,
+ + * not stored in the index. otherwise construct the
+ + * cache entry from the index aware logic.
+ + */
+ +- src[i + o->merge] = create_ce_entry(info, names + i, stage, &o->result, o->merge);
+ ++ src[i + o->merge] = create_ce_entry(info, names + i, stage,
+ ++ &o->result, o->merge,
+ ++ sparse_directory);
+ + }
+ +
+ + if (o->merge) {
+ +@@ unpack-trees.c: static int find_cache_pos(struct traverse_info *info,
+ + static struct cache_entry *find_cache_entry(struct traverse_info *info,
+ + const struct name_entry *p)
+ + {
+ ++ struct cache_entry *ce;
+ + int pos = find_cache_pos(info, p->path, p->pathlen);
+ + struct unpack_trees_options *o = info->data;
+ +
+ + if (0 <= pos)
+ + return o->src_index->cache[pos];
+ +- else
+ ++
+ ++ /*
+ ++ * Check for a sparse-directory entry named "path/".
+ ++ * Due to the input p->path not having a trailing
+ ++ * slash, the negative 'pos' value overshoots the
+ ++ * expected position by one, hence "-2" here.
+ ++ */
+ ++ pos = -pos - 2;
+ ++
+ ++ if (pos < 0 || pos >= o->src_index->cache_nr)
+ ++ return NULL;
+ ++
+ ++ ce = o->src_index->cache[pos];
+ ++
+ ++ if (!S_ISSPARSEDIR(ce->ce_mode))
+ + return NULL;
+ ++
+ ++ /*
+ ++ * Compare ce->name to info->name + '/' + p->path + '/'
+ ++ * if info->name is non-empty. Compare ce->name to
+ ++ * p-.path + '/' otherwise.
+ ++ */
+ ++ if (info->namelen) {
+ ++ if (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))
+ ++ return ce;
+ ++ } else if (ce->ce_namelen == p->pathlen + 1 &&
+ ++ !strncmp(ce->name, p->path, p->pathlen))
+ ++ return ce;
+ ++ return NULL;
+ + }
+ +
+ + static void debug_path(struct traverse_info *info)
+ +@@ unpack-trees.c: static void debug_unpack_callback(int n,
+ + debug_name_entry(i, names + i);
+ + }
+ +
+ ++/*
+ ++ * Returns true if and only if the given cache_entry is a
+ ++ * sparse-directory entry that matches the given name_entry
+ ++ * from the tree walk at the given traverse_info.
+ ++ */
+ ++static int is_sparse_directory_entry(struct cache_entry *ce, struct name_entry *name, struct traverse_info *info)
+ ++{
+ ++ size_t expected_len, name_start;
+ ++
+ ++ if (!ce || !name || !S_ISSPARSEDIR(ce->ce_mode))
+ ++ return 0;
+ ++
+ ++ if (info->namelen)
+ ++ name_start = info->namelen + 1;
+ ++ else
+ ++ name_start = 0;
+ ++ expected_len = name->pathlen + 1 + name_start;
+ ++
+ ++ if (ce->ce_namelen != expected_len ||
+ ++ strncmp(ce->name, info->name, info->namelen) ||
+ ++ strncmp(ce->name + name_start, name->path, name->pathlen))
+ ++ return 0;
+ ++
+ ++ return 1;
+ ++}
+ ++
+ + /*
+ + * Note that traverse_by_cache_tree() duplicates some logic in this function
+ + * without actually calling it. If you change the logic here you may need to
+ +@@ unpack-trees.c: static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
+ + }
+ + }
+ +
+ +- if (unpack_nondirectories(n, mask, dirmask, src, names, info) < 0)
+ ++ if (unpack_nondirectories(n, mask, dirmask, src, names, info, 0) < 0)
+ + return -1;
+ +
+ + if (o->merge && src[0]) {
+ +@@ unpack-trees.c: static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
+ + }
+ + }
+ +
+ +- if (traverse_trees_recursive(n, dirmask, mask & ~dirmask,
+ +- names, info) < 0)
+ ++ if (is_sparse_directory_entry(src[0], names, info)) {
+ ++ if (unpack_nondirectories(n, dirmask, mask & ~dirmask, src, names, info, 1) < 0)
+ ++ return -1;
+ ++ } else if (traverse_trees_recursive(n, dirmask, mask & ~dirmask,
+ ++ names, info) < 0) {
+ + return -1;
+ ++ }
+ ++
+ + return mask;
+ + }
+ +
+ 8: 2cc3a93d4434 = 9: b8ff179f43e3 dir.c: accept a directory as part of cone-mode patterns
+ -: ------------ > 10: b9b97e011293 diff-lib: handle index diffs with sparse dirs
+ 9: 5011feb1aa04 = 11: 611b9f61fb2c status: skip sparse-checkout percentage with sparse-index
+ 10: 9f2ce5301dc9 = 12: 0c0a765dde80 status: use sparse-index throughout
+ 11: 24417e095243 ! 13: 02f2c7b63982 wt-status: expand added sparse directory entries
+ @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n
+ + init_repos &&
+
- - if (get_stat_data(new_entry, &oid, &mode, cached, match_missing,
- + if (get_stat_data(istate, new_entry, &oid, &mode, cached, match_missing,
- &dirty_submodule, &revs->diffopt) < 0) {
- if (report_missing)
- 11: 611b9f61fb2 = 11: 48fd25aacbe status: skip sparse-checkout percentage with sparse-index
- 12: 0c0a765dde8 = 12: 3499105eb67 status: use sparse-index throughout
- 13: 02f2c7b6398 ! 13: 08225483d69 wt-status: expand added sparse directory entries
- @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n
- + test_sparse_match git reset --mixed HEAD~1 &&
+ + test_all_match git checkout rename-out-to-in &&
+ -+ test_all_match git reset --mixed HEAD~1 &&
+ ++
+ ++ # Sparse checkouts do not agree with full checkouts about
+ ++ # how to report a directory/file conflict during a reset.
+ ++ # This command would fail with test_all_match because the
+ ++ # full checkout reports "T folder1/0/1" while a sparse
+ ++ # checkout reports "D folder1/0/1". This matches because
+ ++ # the sparse checkouts skip "adding" the other side of
+ ++ # the conflict.
+ ++ test_sparse_match git reset --mixed HEAD~1 &&
+ test_sparse_match test-tool read-cache --table --expand &&
- + test_sparse_match git status --porcelain=v2 &&
- -+ test_sparse_match git status --porcelain=v2 &&
+ -+ test_all_match git status --porcelain=v2 &&
+ -+ test_all_match git status --porcelain=v2 &&
+ ++ test_sparse_match git status --porcelain=v2 &&
+ ++ test_sparse_match git status --porcelain=v2 &&
+
+ # At this point, sparse-checkouts behave differently
+ # from the full-checkout.
- + test_sparse_match git checkout --orphan new-branch &&
- + test_sparse_match test-tool read-cache --table --expand &&
- -+ test_sparse_match git status --porcelain=v2 &&
- + test_sparse_match git status --porcelain=v2
- +'
- +
- @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n
- +
- + run_on_all touch deep/deeper1/x &&
- + test_all_match git add . &&
- -+ test_all_match git status --porcelain=v2 &&
- + test_all_match git status --porcelain=v2
- +'
- +
- @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n
-
- ## wt-status.c ##
- @@ wt-status.c: static void wt_status_collect_changes_index(struct wt_status *s)
- - run_diff_index(&rev, 1);
- + clear_pathspec(&rev.prune_data);
- }
-
- +static int add_file_to_list(const struct object_id *oid,
- @@ wt-status.c: static void wt_status_collect_changes_initial(struct wt_status *s)
- + * tree and marking them with DIFF_STATUS_ADDED.
- + */
- + struct strbuf base = STRBUF_INIT;
- -+ struct pathspec ps;
- ++ struct pathspec ps = { 0 };
- + struct tree *tree = lookup_tree(istate->repo, &ce->oid);
- +
- -+ memset(&ps, 0, sizeof(ps));
- + ps.recursive = 1;
- + ps.has_wildcard = 1;
- + ps.max_depth = -1;
- 14: 46ca150c354 = 14: 711e403a63a fsmonitor: integrate with sparse index
+ 12: 584d4b559a91 = 14: 46ca150c3548 fsmonitor: integrate with sparse index
--
gitgitgadget