Re* [PATCH 3/3] apply: diagnose incomplete submodule object name better

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> We could read from the payload part of the patch to learn the full
> object name of the commit, but the primary user "git rebase" has
> been fixed to give us a full object name, so this should suffice
> for now.

And the patch on top to do so looks like this.  With this patch in
place, we could drop 1/3 of this series, but there is no strong
reason to do so. After all, 1/3 is all about internal implementation
details.

-- >8 --
Subject: [PATCH 4/3] apply: verify submodule commit object name better

A textual patch also records the submodule commit object name in
full.  Make the parsing more robust by reading from there and
verifying the (possibly abbreviated) name on the index line matches.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 builtin/apply.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 1f78e2c..e0f1474 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3587,6 +3587,40 @@ static int get_current_sha1(const char *path, unsigned char *sha1)
 	return 0;
 }
 
+static int preimage_sha1_in_gitlink_patch(struct patch *p, unsigned char sha1[20])
+{
+	/*
+	 * A usable gitlink patch has only one fragment (hunk) that looks like:
+	 * @@ -1 +1 @@
+	 * -Subproject commit <old sha1>
+	 * +Subproject commit <new sha1>
+	 * or
+	 * @@ -1 +0,0 @@
+	 * -Subproject commit <old sha1>
+	 * for a removal patch.
+	 */
+	struct fragment *hunk = p->fragments;
+	static const char heading[] = "-Subproject commit ";
+	char *preimage;
+
+	if (/* does the patch have only one hunk? */
+	    hunk && !hunk->next &&
+	    /* is its preimage one line? */
+	    hunk->oldpos == 1 && hunk->oldlines == 1 &&
+	    /* does preimage begin with the heading? */
+	    (preimage = memchr(hunk->patch, '\n', hunk->size)) != NULL &&
+	    !prefixcmp(++preimage, heading) &&
+	    /* does it record full SHA-1? */
+	    !get_sha1_hex(preimage + sizeof(heading) - 1, sha1) &&
+	    preimage[sizeof(heading) + 40 - 1] == '\n' &&
+	    /* does the abbreviated name on the index line agree with it? */
+	    !prefixcmp(preimage + sizeof(heading) - 1, p->old_sha1_prefix))
+		return 0; /* it all looks fine */
+
+	/* we may have full object name on the index line */
+	return get_sha1_hex(p->old_sha1_prefix, sha1);
+}
+
 /* Build an index that contains the just the files needed for a 3way merge */
 static void build_fake_ancestor(struct patch *list, const char *filename)
 {
@@ -3607,8 +3641,10 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
 			continue;
 
 		if (S_ISGITLINK(patch->old_mode)) {
-			if (get_sha1_hex(patch->old_sha1_prefix, sha1))
-				die("submoule change for %s without full index name",
+			if (!preimage_sha1_in_gitlink_patch(patch, sha1))
+				; /* ok, the textual part looks sane */
+			else
+				die("sha1 information is lacking or useless for submoule %s",
 				    name);
 		} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
 			; /* ok */
-- 
1.8.1.2.639.g9428735

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


[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]