[PATCH] apply: fix adding new files on i-t-a entries

["Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>]

Since d95d728 (diff-lib.c: adjust position of i-t-a entries in diff -
2015-03-16), a normal "git diff" on i-t-a entries would produce a diff
that _adds_ those files, not just adding content to existing and empty
files like before.

This is correct. Unfortunately, applying such a patch on the same
repository that has the same i-t-a entries will fail with message
"already exists in index" because git-apply checks, sees those i-t-a
entries and aborts. git-apply does not realize those are for
bookkeeping only, they do not really exist in the index.

This patch tightens the "exists in index" check, ignoring i-t-a
entries. For fixing the above problem, only the change in
check_to_create() is needed. For other changes,

 - load_current(), reporting "not exists in index" is better than "does
   not match index"

 - check_preimage(), similar to load_current(), but it may also use
   ce_mode from i-t-a entry which is always zero

 - get_current_sha1(), or actually build_fake_ancestor(), we should not
   add i-t-a entries to the temporary index, at least not without also
   adding i-t-a flag back

Since "git add -p" and "git am" use "git apply" underneath, they are
broken too before this patch and fixed now.

Reported-by: Patrick Higgins <phiggins@google.com>
Reported-by: Bjørnar Snoksrud <snoksrud@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I think I'm opening a can of worms with d95d728. There's nothing
 wrong with that patch per se, but with this issue popping up, I need
 to go over all {cache,index}_name_pos call sites and see what would be
 the sensible behavior when i-t-a entries are involved.
 
 So far blame, rm and checkout-entry and "checkout <paths>" are on my
 to-think-or-fix list. But this patch can get in early to fix a real
 regression instead of waiting for one big series. A lot more
 discussions will be had before that series gets in good shape.

 builtin/apply.c       |  8 ++++----
 cache.h               |  2 ++
 read-cache.c          | 12 ++++++++++++
 t/t2203-add-intent.sh | 10 ++++++++++
 4 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 146be97..4f813ac 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3344,7 +3344,7 @@ static int load_current(struct image *image, struct patch *patch)
 	if (!patch->is_new)
 		die("BUG: patch to %s is not a creation", patch->old_name);
 
-	pos = cache_name_pos(name, strlen(name));
+	pos = exists_in_cache(name, strlen(name));
 	if (pos < 0)
 		return error(_("%s: does not exist in index"), name);
 	ce = active_cache[pos];
@@ -3497,7 +3497,7 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s
 	}
 
 	if (check_index && !previous) {
-		int pos = cache_name_pos(old_name, strlen(old_name));
+		int pos = exists_in_cache(old_name, strlen(old_name));
 		if (pos < 0) {
 			if (patch->is_new < 0)
 				goto is_new;
@@ -3551,7 +3551,7 @@ static int check_to_create(const char *new_name, int ok_if_exists)
 	struct stat nst;
 
 	if (check_index &&
-	    cache_name_pos(new_name, strlen(new_name)) >= 0 &&
+	    exists_in_cache(new_name, strlen(new_name)) >= 0 &&
 	    !ok_if_exists)
 		return EXISTS_IN_INDEX;
 	if (cached)
@@ -3824,7 +3824,7 @@ static int get_current_sha1(const char *path, unsigned char *sha1)
 
 	if (read_cache() < 0)
 		return -1;
-	pos = cache_name_pos(path, strlen(path));
+	pos = exists_in_cache(path, strlen(path));
 	if (pos < 0)
 		return -1;
 	hashcpy(sha1, active_cache[pos]->sha1);
diff --git a/cache.h b/cache.h
index 571c98f..6a44cb6 100644
--- a/cache.h
+++ b/cache.h
@@ -341,6 +341,7 @@ extern void free_name_hash(struct index_state *istate);
 #define discard_cache() discard_index(&the_index)
 #define unmerged_cache() unmerged_index(&the_index)
 #define cache_name_pos(name, namelen) index_name_pos(&the_index,(name),(namelen))
+#define exists_in_cache(name, namelen) exists_in_index(&the_index,(name),(namelen))
 #define add_cache_entry(ce, option) add_index_entry(&the_index, (ce), (option))
 #define rename_cache_entry_at(pos, new_name) rename_index_entry_at(&the_index, (pos), (new_name))
 #define remove_cache_entry_at(pos) remove_index_entry_at(&the_index, (pos))
@@ -512,6 +513,7 @@ extern int verify_path(const char *path);
 extern struct cache_entry *index_dir_exists(struct index_state *istate, const char *name, int namelen);
 extern struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
 extern int index_name_pos(const struct index_state *, const char *name, int namelen);
+extern int exists_in_index(const struct index_state *, const char *name, int namelen);
 #define ADD_CACHE_OK_TO_ADD 1		/* Ok to add */
 #define ADD_CACHE_OK_TO_REPLACE 2	/* Ok to replace file/directory */
 #define ADD_CACHE_SKIP_DFCHECK 4	/* Ok to skip DF conflict checks */
diff --git a/read-cache.c b/read-cache.c
index 5dee4e2..d3b50cb 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -506,6 +506,18 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel
 	return index_name_stage_pos(istate, name, namelen, 0);
 }
 
+/* This is the same as index_name_pos, except that i-t-a entries are invisible */
+int exists_in_index(const struct index_state *istate, const char *name, int namelen)
+{
+	int pos = index_name_stage_pos(istate, name, namelen, 0);
+
+	if (pos < 0)
+		return pos;
+	if (istate->cache[pos]->ce_flags & CE_INTENT_TO_ADD)
+		return -pos-1;
+	return pos;
+}
+
 /* Remove entry, return true if there are more entries to go.. */
 int remove_index_entry_at(struct index_state *istate, int pos)
 {
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 7c641bf..a10a96a 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -97,5 +97,15 @@ test_expect_success 'cache-tree invalidates i-t-a paths' '
 	test_cmp expect actual
 '
 
+test_expect_success 'apply on i-t-a entries' '
+	git init apply &&
+	(
+		cd apply &&
+		echo content >file &&
+		git add -N file &&
+		git diff | git apply --cached
+	)
+'
+
 test_done
 
-- 
2.3.0.rc1.137.g477eb31