Merge branch 'nd/fix-sparse-checkout'

* nd/fix-sparse-checkout:
  unpack-trees: mark new entries skip-worktree appropriately
  unpack-trees: do not check for conflict entries too early
  unpack-trees: let read-tree -u remove index entries outside sparse area
  unpack-trees: only clear CE_UPDATE|CE_REMOVE when skip-worktree is always set
  t1011 (sparse checkout): style nitpicks

cache.h

@@ -179,8 +179,7 @@ struct cache_entry {
 #define CE_UNHASHED  (0x200000)
 #define CE_CONFLICTED (0x800000)
 
-/* Only remove in work directory, not index */
-#define CE_WT_REMOVE (0x400000)
+#define CE_WT_REMOVE (0x400000) /* remove in work directory */
 
 #define CE_UNPACKED  (0x1000000)
 

t/t1011-read-tree-sparse-checkout.sh

@@ -1,14 +1,28 @@
 #!/bin/sh
 
-test_description='sparse checkout tests'
+test_description='sparse checkout tests
+
+* (tag: removed, master) removed
+| D	sub/added
+* (HEAD, tag: top) modified and added
+| M	init.t
+| A	sub/added
+* (tag: init) init
+  A	init.t
+'
 
 . ./test-lib.sh
 
-cat >expected <<EOF
+test_expect_success 'setup' '
+	cat >expected <<-\EOF &&
 	100644 77f0ba1734ed79d12881f81b36ee134de6a3327b 0	init.t
 	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	sub/added
 	EOF
-test_expect_success 'setup' '
+	cat >expected.swt <<-\EOF &&
+	H init.t
+	H sub/added
+	EOF
+
 	test_commit init &&
 	echo modified >>init.t &&
 	mkdir sub &&
@@ -24,10 +38,6 @@ test_expect_success 'setup' '
 	test_cmp expected result
 '
 
-cat >expected.swt <<EOF
-H init.t
-H sub/added
-EOF
 test_expect_success 'read-tree without .git/info/sparse-checkout' '
 	git read-tree -m -u HEAD &&
 	git ls-files --stage >result &&
@@ -67,23 +77,20 @@ test_expect_success 'read-tree with empty .git/info/sparse-checkout' '
 	test -f sub/added
 '
 
-cat >expected.swt <<EOF
+test_expect_success 'match directories with trailing slash' '
+	cat >expected.swt-noinit <<-\EOF &&
 	S init.t
 	H sub/added
 	EOF
-test_expect_success 'match directories with trailing slash' '
+
 	echo sub/ > .git/info/sparse-checkout &&
 	git read-tree -m -u HEAD &&
 	git ls-files -t > result &&
-	test_cmp expected.swt result &&
+	test_cmp expected.swt-noinit result &&
 	test ! -f init.t &&
 	test -f sub/added
 '
 
-cat >expected.swt <<EOF
-H init.t
-H sub/added
-EOF
 test_expect_failure 'match directories without trailing slash' '
 	echo init.t >.git/info/sparse-checkout &&
 	echo sub >>.git/info/sparse-checkout &&
@@ -94,15 +101,16 @@ test_expect_failure 'match directories without trailing slash' '
 	test -f sub/added
 '
 
-cat >expected.swt <<EOF
+test_expect_success 'checkout area changes' '
+	cat >expected.swt-nosub <<-\EOF &&
 	H init.t
 	S sub/added
 	EOF
-test_expect_success 'checkout area changes' '
+
 	echo init.t >.git/info/sparse-checkout &&
 	git read-tree -m -u HEAD &&
 	git ls-files -t >result &&
-	test_cmp expected.swt result &&
+	test_cmp expected.swt-nosub result &&
 	test -f init.t &&
 	test ! -f sub/added
 '
@@ -147,4 +155,25 @@ test_expect_success 'read-tree adds to worktree, dirty case' '
 	grep -q dirty sub/added
 '
 
+test_expect_success 'index removal and worktree narrowing at the same time' '
+	>empty &&
+	echo init.t >.git/info/sparse-checkout &&
+	echo sub/added >>.git/info/sparse-checkout &&
+	git checkout -f top &&
+	echo init.t >.git/info/sparse-checkout &&
+	git checkout removed &&
+	git ls-files sub/added >result &&
+	test ! -f sub/added &&
+	test_cmp empty result
+'
+
+test_expect_success 'read-tree --reset removes outside worktree' '
+	>empty &&
+	echo init.t >.git/info/sparse-checkout &&
+	git checkout -f top &&
+	git reset --hard removed &&
+	git ls-files sub/added >result &&
+	test_cmp empty result
+'
+
 test_done

unpack-trees.c

@@ -58,6 +58,9 @@ 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;
@@ -156,7 +159,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_REMOVE | CE_WT_REMOVE))
+			if (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))
 				total++;
 		}
 
@@ -176,12 +179,6 @@ static int check_updates(struct unpack_trees_options *o)
 				unlink_entry(ce);
 			continue;
 		}
-
-		if (ce->ce_flags & CE_REMOVE) {
-			display_progress(progress, ++cnt);
-			if (o->update)
-				unlink_entry(ce);
-		}
 	}
 	remove_marked_cache_entries(&o->result);
 	remove_scheduled_dirs();
@@ -210,9 +207,6 @@ static int will_have_skip_worktree(const struct cache_entry *ce, struct unpack_t
 {
 	const char *basename;
 
-	if (ce_stage(ce))
-		return 0;
-
 	basename = strrchr(ce->name, '/');
 	basename = basename ? basename+1 : ce->name;
 	return excluded_from_list(ce->name, ce_namelen(ce), basename, NULL, o->el) <= 0;
@@ -222,19 +216,36 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt
 {
 	int was_skip_worktree = ce_skip_worktree(ce);
 
-	if (will_have_skip_worktree(ce, o))
+	if (!ce_stage(ce) && will_have_skip_worktree(ce, o))
 		ce->ce_flags |= CE_SKIP_WORKTREE;
 	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 (!was_skip_worktree && !ce_skip_worktree()) {
+	 *	This is perfectly normal. Move on;
+	 * }
+	 */
+
+	/*
+	 * 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 if they are already
+	 * outside checkout area
+	 */
+	if (was_skip_worktree && ce_skip_worktree(ce)) {
+		ce->ce_flags &= ~CE_UPDATE;
+
+		/*
+		 * By default, when CE_REMOVE is on, CE_WT_REMOVE is also
+		 * on to get that file removed from both index and worktree.
+		 * If that file is already outside worktree area, don't
+		 * bother remove it.
 		 */
 		if (ce->ce_flags & CE_REMOVE)
-		return 0;
+			ce->ce_flags &= ~CE_WT_REMOVE;
+	}
 
 	if (!was_skip_worktree && ce_skip_worktree(ce)) {
 		/*
@@ -876,14 +887,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 				ret = -1;
 				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(). Clear them.
-			 */
-			if (ce_skip_worktree(ce))
-				ce->ce_flags &= ~(CE_UPDATE | CE_REMOVE);
-			else
+			if (!ce_skip_worktree(ce))
 				empty_worktree = 0;
 
 		}
@@ -1182,6 +1186,8 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 	if (!old) {
 		if (verify_absent(merge, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))
 			return -1;
+		if (!o->skip_sparse_checkout && will_have_skip_worktree(merge, o))
+			update |= CE_SKIP_WORKTREE;
 		invalidate_ce_path(merge, o);
 	} else if (!(old->ce_flags & CE_CONFLICTED)) {
 		/*