When narrowing the checkout pattern to exclude a file that the merge
was going to delete, read-tree decides two wrongs make a right and
leaves the file behind.
Or that’s what it looks like. Actually, for some reason
apply_sparse_checkout() is not paying proper attention to entries
removed by the merge, so they look like ordinary deletions outside
the sparse area.
While fixing that, clarify the semantics of the unpack-trees flags:
- CE_UPDATE marks an entry for which the worktree might need
to be updated to match. If CE_UPDATE is not set, changes
only apply to the index file.
- CE_REMOVE marks an entry removed by the merge (just like
it did before).
- CE_WT_REMOVE marks an entry that no longer matches the sparse
pattern.
Cc: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
Sorry for the lack of clarity before. This is just to give the
idea; I have no confidence that I am not breaking something,
especially because I have not checked why the "if" I am removing
in apply_sparse_checkout was added.
cache.h | 3 +-
t/t1011-read-tree-sparse-checkout.sh | 21 +++++++++++++++++
unpack-trees.c | 42 +++++++++------------------------
3 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/cache.h b/cache.h
index 3c7fb6f..c9fa3df 100644
--- a/cache.h
+++ b/cache.h
@@ -179,7 +179,8 @@ struct cache_entry {
#define CE_UNHASHED (0x200000)
#define CE_CONFLICTED (0x800000)
-#define CE_WT_REMOVE (0x400000) /* remove in work directory */
+/* Only remove in work directory, not index */
+#define CE_WT_REMOVE (0x400000)
#define CE_UNPACKED (0x1000000)
diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index 2955071..247f295 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -155,6 +155,27 @@ test_expect_failure 'read-tree adds to worktree, dirty case' '
grep -q dirty sub/added
'
+test_expect_success 'narrow checkout while removing file' '
+ echo "H init.t" >expected.swt-after &&
+ printf "%s\n" init.t sub/added | sort >expected.ls-before
+ echo init.t >expected.ls-after &&
+
+ printf "%s\n" init.t sub/added >.git/info/sparse-checkout &&
+ git checkout -f top &&
+ git ls-files -t >index.before &&
+ ls init.t sub/added | sort >worktree.before &&
+
+ echo init.t >.git/info/sparse-checkout &&
+ git read-tree -m -u HEAD^ &&
+ git ls-files -t >index.after &&
+ ls init.t sub/added | sort >worktree.after &&
+
+ test_cmp expected.swt index.before &&
+ test_cmp expected.swt-after index.after &&
+ test_cmp expected.ls-before worktree.before &&
+ test_cmp expected.ls-after worktree.after
+'
+
test_expect_success 'read-tree --reset removes outside worktree' '
>empty &&
echo init.t >.git/info/sparse-checkout &&
diff --git a/unpack-trees.c b/unpack-trees.c
index 523e1de..deabfd9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -53,9 +53,6 @@ static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
clear |= CE_HASHED | CE_UNHASHED;
- if (set & CE_REMOVE)
- set |= CE_WT_REMOVE;
-
memcpy(new, ce, size);
new->next = NULL;
new->ce_flags = (new->ce_flags & ~clear) | set;
@@ -87,7 +84,7 @@ static int check_updates(struct unpack_trees_options *o)
if (o->update && o->verbose_update) {
for (total = cnt = 0; cnt < index->cache_nr; cnt++) {
struct cache_entry *ce = index->cache[cnt];
- if (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))
+ if (ce->ce_flags & CE_UPDATE)
total++;
}
@@ -101,11 +98,12 @@ static int check_updates(struct unpack_trees_options *o)
for (i = 0; i < index->cache_nr; i++) {
struct cache_entry *ce = index->cache[i];
- if (ce->ce_flags & CE_WT_REMOVE) {
+ if (ce->ce_flags & CE_UPDATE &&
+ ce->ce_flags & (CE_REMOVE | CE_WT_REMOVE)) {
display_progress(progress, ++cnt);
+ ce->ce_flags &= ~CE_UPDATE & ~CE_WT_REMOVE;
if (o->update)
unlink_entry(ce);
- continue;
}
}
remove_marked_cache_entries(&o->result);
@@ -152,15 +150,6 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt
else
ce->ce_flags &= ~CE_SKIP_WORKTREE;
- /*
- * We only care about files getting into the checkout area
- * If merge strategies want to remove some, go ahead, this
- * flag will be removed eventually in unpack_trees() if it's
- * outside checkout area.
- */
- if (ce->ce_flags & CE_REMOVE)
- return 0;
-
if (!was_skip_worktree && ce_skip_worktree(ce)) {
/*
* If CE_UPDATE is set, verify_uptodate() must be called already
@@ -169,7 +158,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt
*/
if (!(ce->ce_flags & CE_UPDATE) && verify_uptodate_sparse(ce, o))
return -1;
- ce->ce_flags |= CE_WT_REMOVE;
+ ce->ce_flags |= CE_WT_REMOVE | CE_UPDATE;
}
if (was_skip_worktree && !ce_skip_worktree(ce)) {
if (verify_absent_sparse(ce, "overwritten", o))
@@ -796,20 +785,13 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
goto done;
}
/*
- * Merge strategies may set CE_UPDATE|CE_REMOVE outside checkout
- * area as a result of ce_skip_worktree() shortcuts in
- * verify_absent() and verify_uptodate().
- * Make sure they don't modify worktree.
+ * Do not modify worktree outside of sparse checkout area,
+ * except to make the checkout more narrow.
*/
- if (ce_skip_worktree(ce)) {
- ce->ce_flags &= ~CE_UPDATE;
-
- if (ce->ce_flags & CE_REMOVE)
- ce->ce_flags &= ~CE_WT_REMOVE;
- }
- else
+ if (!ce_skip_worktree(ce))
empty_worktree = 0;
-
+ else if (!(ce->ce_flags & CE_WT_REMOVE))
+ ce->ce_flags &= ~CE_UPDATE;
}
if (o->result.cache_nr && empty_worktree) {
ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");
@@ -968,7 +950,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
if (!ce_stage(ce2)) {
if (verify_uptodate(ce2, o))
return -1;
- add_entry(o, ce2, CE_REMOVE, 0);
+ add_entry(o, ce2, CE_REMOVE|CE_UPDATE, 0);
mark_ce_used(ce2, o);
}
cnt++;
@@ -1138,7 +1120,7 @@ static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,
}
if (!(old->ce_flags & CE_CONFLICTED) && verify_uptodate(old, o))
return -1;
- add_entry(o, ce, CE_REMOVE, 0);
+ add_entry(o, ce, CE_REMOVE|CE_UPDATE, 0);
invalidate_ce_path(ce, o);
return 1;
}
--
1.7.2.1.544.ga752d.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html