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 <phigg...@google.com>
Reported-by: Bjørnar Snoksrud <snoks...@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to