[PATCH 1/2] ie_match_stat(): do not ignore skip-worktree bit with CE_MATCH_IGNORE_VALID

[PATCH 1/2] ie_match_stat(): do not ignore skip-worktree bit with CE_MATCH_IGNORE_VALID

[Nguyễn Thái Ngọc Duy]

Previously CE_MATCH_IGNORE_VALID flag is used by both valid and
skip-worktree bits. While the two bits have similar behaviour, sharing
this flag means "git update-index --really-refresh" will ignore
skip-worktree while it should not. Instead another flag is
introduced to ignore skip-worktree bit, CE_MATCH_IGNORE_VALID only
applies to valid bit.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin-apply.c |    2 +-
 cache.h         |    4 +++-
 entry.c         |    2 +-
 read-cache.c    |   21 ++++++++++++++++++---
 unpack-trees.c  |    6 +++---
 5 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index 36e2f9d..541493e 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2666,7 +2666,7 @@ static int verify_index_match(struct cache_entry *ce, struct stat *st)
 			return -1;
 		return 0;
 	}
-	return ce_match_stat(ce, st, CE_MATCH_IGNORE_VALID);
+	return ce_match_stat(ce, st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
 }
 
 static int check_preimage(struct patch *patch, struct cache_entry **ce, struct stat *st)
diff --git a/cache.h b/cache.h
index 0e69384..1341475 100644
--- a/cache.h
+++ b/cache.h
@@ -469,7 +469,9 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID		01
 /* do not check the contents but report dirty on racily-clean entries */
-#define CE_MATCH_RACY_IS_DIRTY	02
+#define CE_MATCH_RACY_IS_DIRTY		02
+/* do stat comparison even if CE_SKIP_WORKTREE is true */
+#define CE_MATCH_IGNORE_SKIP_WORKTREE	04
 extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 
diff --git a/entry.c b/entry.c
index 06d24f1..9d5b232 100644
--- a/entry.c
+++ b/entry.c
@@ -206,7 +206,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
 	len += ce_namelen(ce);
 
 	if (!check_path(path, len, &st, state->base_dir_len)) {
-		unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID);
+		unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
 		if (!changed)
 			return 0;
 		if (!state->force) {
diff --git a/read-cache.c b/read-cache.c
index 7a2385b..b5c66f9 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -259,13 +259,18 @@ int ie_match_stat(const struct index_state *istate,
 {
 	unsigned int changed;
 	int ignore_valid = options & CE_MATCH_IGNORE_VALID;
+	int ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE;
 	int assume_racy_is_modified = options & CE_MATCH_RACY_IS_DIRTY;
 
 	/*
 	 * If it's marked as always valid in the index, it's
 	 * valid whatever the checked-out copy says.
+	 *
+	 * skip-worktree has the same effect with higher precedence
 	 */
-	if (!ignore_valid && ((ce->ce_flags & CE_VALID) || ce_skip_worktree(ce)))
+	if (!ignore_skip_worktree && ce_skip_worktree(ce))
+		return 0;
+	if (!ignore_valid && (ce->ce_flags & CE_VALID))
 		return 0;
 
 	/*
@@ -564,7 +569,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	int size, namelen, was_same;
 	mode_t st_mode = st->st_mode;
 	struct cache_entry *ce, *alias;
-	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
+	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE|CE_MATCH_RACY_IS_DIRTY;
 	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
 	int pretend = flags & ADD_CACHE_PRETEND;
 	int intent_only = flags & ADD_CACHE_INTENT;
@@ -1000,11 +1005,21 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
 	struct cache_entry *updated;
 	int changed, size;
 	int ignore_valid = options & CE_MATCH_IGNORE_VALID;
+	int ignore_skip_worktree = options & CE_MATCH_IGNORE_SKIP_WORKTREE;
 
 	if (ce_uptodate(ce))
 		return ce;
 
-	if (!ignore_valid && ((ce->ce_flags & CE_VALID) || ce_skip_worktree(ce))) {
+	/*
+	 * CE_VALID or CE_SKIP_WORKTREE means the user promised us
+	 * that the change to the work tree does not matter and told
+	 * us not to worry.
+	 */
+	if (!ignore_skip_worktree && ce_skip_worktree(ce)) {
+		ce_mark_uptodate(ce);
+		return ce;
+	}
+	if (!ignore_valid && (ce->ce_flags & CE_VALID)) {
 		ce_mark_uptodate(ce);
 		return ce;
 	}
diff --git a/unpack-trees.c b/unpack-trees.c
index ae1de3f..7570475 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -569,7 +569,7 @@ static int verify_uptodate_1(struct cache_entry *ce,
 		return 0;
 
 	if (!lstat(ce->name, &st)) {
-		unsigned changed = ie_match_stat(o->src_index, ce, &st, CE_MATCH_IGNORE_VALID);
+		unsigned changed = ie_match_stat(o->src_index, ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
 		if (!changed)
 			return 0;
 		/*
@@ -701,7 +701,7 @@ static int icase_exists(struct unpack_trees_options *o, struct cache_entry *dst,
 	struct cache_entry *src;
 
 	src = index_name_exists(o->src_index, dst->name, ce_namelen(dst), 1);
-	return src && !ie_match_stat(o->src_index, src, st, CE_MATCH_IGNORE_VALID);
+	return src && !ie_match_stat(o->src_index, src, st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
 }
 
 /*
@@ -1152,7 +1152,7 @@ int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)
 		if (o->reset && !ce_uptodate(old) && !ce_skip_worktree(old)) {
 			struct stat st;
 			if (lstat(old->name, &st) ||
-			    ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID))
+			    ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE))
 				update |= CE_UPDATE;
 		}
 		add_entry(o, old, update, 0);
-- 
1.6.5.2.216.g9c1ec

[PATCH 2/2] commit: correctly respect skip-worktree bit

[Nguyễn Thái Ngọc Duy]

Commit b4d1690 (Teach Git to respect skip-worktree bit (reading part))
fails to make "git commit -- a b c" respect skip-worktree
(i.e. not committing paths that are skip-worktree). This is because
when the index is reset back to HEAD, all skip-worktree information is
gone.

This patch saves skip-worktree information in the string list of
committed paths, then reuse it later on to skip skip-worktree paths.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin-commit.c                 |   11 +++++++----
 t/t7011-skip-worktree-reading.sh |    4 ++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index a11e585..6e368f0 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -180,11 +180,15 @@ static int list_paths(struct string_list *list, const char *with_tree,
 
 	for (i = 0; i < active_nr; i++) {
 		struct cache_entry *ce = active_cache[i];
+		struct string_list_item *item;
+
 		if (ce->ce_flags & CE_UPDATE)
 			continue;
 		if (!match_pathspec(pattern, ce->name, ce_namelen(ce), 0, m))
 			continue;
-		string_list_insert(ce->name, list);
+		item = string_list_insert(ce->name, list);
+		if (ce_skip_worktree(ce))
+			item->util = item; /* better a valid pointer than a fake one */
 	}
 
 	return report_path_error(m, pattern, prefix ? strlen(prefix) : 0);
@@ -196,10 +200,9 @@ static void add_remove_files(struct string_list *list)
 	for (i = 0; i < list->nr; i++) {
 		struct stat st;
 		struct string_list_item *p = &(list->items[i]);
-		int pos = index_name_pos(&the_index, p->string, strlen(p->string));
-		struct cache_entry *ce = pos < 0 ? NULL : active_cache[pos];
 
-		if (ce && ce_skip_worktree(ce))
+		/* p->util is skip-worktree */
+		if (p->util)
 			continue;
 
 		if (!lstat(p->string, &st)) {
diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index e996928..bb4066f 100755
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -148,13 +148,13 @@ test_expect_success 'git-rm succeeds on skip-worktree absent entries' '
 	git rm 1
 '
 
-test_expect_failure 'commit on skip-worktree absent entries' '
+test_expect_success 'commit on skip-worktree absent entries' '
 	git reset &&
 	setup_absent &&
 	test_must_fail git commit -m null 1
 '
 
-test_expect_failure 'commit on skip-worktree dirty entries' '
+test_expect_success 'commit on skip-worktree dirty entries' '
 	git reset &&
 	setup_dirty &&
 	test_must_fail git commit -m null 1
-- 
1.6.5.2.216.g9c1ec