Re: [PATCH v4 2/3] apply: make i-t-a entries never match worktree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Raymond E. Pasco" <ray@xxxxxxxxxxxx> writes:

> By definition, an intent-to-add index entry can never match the
> worktree, because worktrees have no concept of intent-to-add entries.
> Therefore, "apply --index" should always fail on intent-to-add paths.
>
> Because check_preimage() calls verify_index_match(), it already fails
> for patches other than creation patches, which check_preimage() ignores.
> This patch adds a check to check_preimage()'s rough equivalent for
> creation patches, check_to_create().
>
> Helped-by: Junio C Hamano <gitster@xxxxxxxxx>
> Signed-off-by: Raymond E. Pasco <ray@xxxxxxxxxxxx>
> ---
>  apply.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)

At first glance, it feels somewhat sad that this check is not done
in check_preimage(); after all, the caller of check_preimage() feeds
it to all kind of patches, without excluding path creation, so the
helper should be allowed to say "heh, you are trying to create path
F with this patch, but there already is F in the index", "you are
renaming into F but there is F already", etc.

But ensuring the state of the path to be patched is done by
check_to_create() for paths that are to be created, even before this
patch, because it simply needs to do different checks from what is
done on modification patch, and also because we need to resolve
patch->is_new bit for non-git patches before being able to perform
the checks done in check_to_create().

So I agree with the location of this additonal logic.

It is somewhat unsatisfactory that we need to do the same
index_name_pos probing twice.  I wonder if we somehow can
consolidate them?

Perhaps something along this line, instead of this patch?

 apply.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/apply.c b/apply.c
index 4cba4ce71a..c5ecb64102 100644
--- a/apply.c
+++ b/apply.c
@@ -3740,6 +3740,7 @@ static int check_preimage(struct apply_state *state,
 
 #define EXISTS_IN_INDEX 1
 #define EXISTS_IN_WORKTREE 2
+#define EXISTS_IN_INDEX_AS_ITA 3
 
 static int check_to_create(struct apply_state *state,
 			   const char *new_name,
@@ -3747,11 +3748,21 @@ static int check_to_create(struct apply_state *state,
 {
 	struct stat nst;
 
-	if (state->check_index && !ok_if_exists) {
-		int pos = index_name_pos(state->repo->index, new_name, strlen(new_name));
-		if (pos >= 0 &&
-		    !(state->repo->index->cache[pos]->ce_flags & CE_INTENT_TO_ADD))
-			return EXISTS_IN_INDEX;
+	if (state->check_index && (!ok_if_exists || !state->cached)) {
+		int pos;
+
+		pos = index_name_pos(state->repo->index, new_name, strlen(new_name));
+		if (pos >= 0) {
+			struct cache_entry *ce = state->repo->index->cache[pos];
+
+			/* allow ITA, as they do not yet exist in the index */
+			if (!ok_if_exists && !(ce->ce_flags & CE_INTENT_TO_ADD))
+				return EXISTS_IN_INDEX;
+
+			/* ITA entries can never match working tree files */
+			if (!state->cached && (ce->ce_flags & CE_INTENT_TO_ADD))
+				return EXISTS_IN_INDEX_AS_ITA;
+		}
 	}
 
 	if (state->cached)
@@ -3938,6 +3949,9 @@ static int check_patch(struct apply_state *state, struct patch *patch)
 		case EXISTS_IN_INDEX:
 			return error(_("%s: already exists in index"), new_name);
 			break;
+		case EXISTS_IN_INDEX_AS_ITA:
+			return error(_("%s: does not match index"), new_name);
+			break;
 		case EXISTS_IN_WORKTREE:
 			return error(_("%s: already exists in working directory"),
 				     new_name);



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux