{"thread":{"id":"36601","subject":"Re: [PATCH] contrib/subtree bugfix: Crash if FETCH_HEAD is tag","startedAt":"2014-05-07T21:53:39Z","lastAt":"2014-05-08T01:04:29Z","messageCount":2,"participants":["James Denholm"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"240970","messageId":"df8addda-8c59-480a-ac52-a958de9d43c9@email.android.com","threadId":"36601","inReplyTo":"1399185212-17833-1-git-send-email-nod.helm@gmail.com","subject":"Re: [PATCH] contrib/subtree bugfix: Crash if FETCH_HEAD is tag","fromName":"James Denholm","fromEmail":"nod.helm@gmail.com","sentAt":"2014-05-07T21:53:39Z","receivedAt":"2014-05-07T21:53:39Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"On 4 May 2014 16:33:32 GMT+10:00, James Denholm wrote:\n>cmd_add_commit() is passed FETCH_HEAD by cmd_add_repository, which is\n>then rev-parsed into an object ID. However, if the user is fetching a\n>tag rather than a branch HEAD, such as by executing:\n>\n>$ git subtree add -P oldGit https://github.com/git/git.git tags/v1.8.0\n>\n>The object ID is a tag and is never peeled, and the git commit-tree\n>call\n>(line 561) slaps us in the face because it doesn't handle tag IDs.\n>\n>Because peeling a committish ID doesn't do anything if it's already a\n>commit, fix by peeling[1] the object ID before assigning it to $rev, as\n>per the patch.\n>\n>[*1*]: Via peel_committish(), from git:git-sh-setup.sh\n>\n>Reported-by: Kevin Cagle \n>Diagnosed-by: Junio C Hamano \n>Signed-off-by: James Denholm \n>---\n>NB: This bug doesn't surface when using --squash, as $rev is reassigned\n>to the squash commit via new_squash_commit before git commit-tree sees\n>it (though for simplicity, new_squash_commit now also sees the peeled\n>ID).\n>\n>Also doesn't surface when using \"git subtree merge\", as git merge can\n>handle tag objects.\n>\n>On a side note, if merging a tag without --squash, git merge recognises\n>that it's a tag and adds a note to the merge commit body. It may be\n>worth mimicking this when using \"subtree merge --squash\" or\n>\"subtree add\".\n>\n> contrib/subtree/git-subtree.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n>diff --git a/contrib/subtree/git-subtree.sh\n>b/contrib/subtree/git-subtree.sh\n>index dc59a91..9453dae 100755\n>--- a/contrib/subtree/git-subtree.sh\n>+++ b/contrib/subtree/git-subtree.sh\n>@@ -538,7 +538,7 @@ cmd_add_commit()\n> {\n> \trevs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n> \tset -- $revs\n>-\trev=\"$1\"\n>+\trev=$(peel_committish \"$1\")\n> \t\n> \tdebug \"Adding $dir as '$rev'...\"\n> \tgit read-tree --prefix=\"$dir\" $rev || exit $?\n\nI know that subtree isn't exactly the most popular or exciting part of\nthe project at the moment, but given that this is adding a subtree based\non an annotated tag is a reasonably sensible operation and (to me) the\nfix seems reasonably trivial, could I get some eyes on this?\n\nRegards,\nJames Denholm.\n"},{"id":"240981","messageId":"CAHYYfeENvT7Oe3DXSEAXuNpyd+kbKMvKZau0L9YGtaZmTaxeaw@mail.gmail.com","threadId":"36601","inReplyTo":"df8addda-8c59-480a-ac52-a958de9d43c9@email.android.com","subject":"Re: [PATCH] contrib/subtree bugfix: Crash if FETCH_HEAD is tag","fromName":"James Denholm","fromEmail":"nod.helm@gmail.com","sentAt":"2014-05-08T01:04:29Z","receivedAt":"2014-05-08T01:04:29Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"After a closer look, it seems the initial patch wasn't correctly sent\nto the list. Please disregard, I'm re-sending the patch entirely.\n\nRegards,\nJames Denholm.\n\nOn 8 May 2014 07:53, James Denholm wrote:\n> On 4 May 2014 16:33:32 GMT+10:00, James Denholm wrote:\n>>cmd_add_commit() is passed FETCH_HEAD by cmd_add_repository, which is\n>>then rev-parsed into an object ID. However, if the user is fetching a\n>>tag rather than a branch HEAD, such as by executing:\n>>\n>>$ git subtree add -P oldGit https://github.com/git/git.git tags/v1.8.0\n>>\n>>The object ID is a tag and is never peeled, and the git commit-tree\n>>call\n>>(line 561) slaps us in the face because it doesn't handle tag IDs.\n>>\n>>Because peeling a committish ID doesn't do anything if it's already a\n>>commit, fix by peeling[1] the object ID before assigning it to $rev, as\n>>per the patch.\n>>\n>>[*1*]: Via peel_committish(), from git:git-sh-setup.sh\n>>\n>>Reported-by: Kevin Cagle \n>>Diagnosed-by: Junio C Hamano \n>>Signed-off-by: James Denholm \n>>---\n>>NB: This bug doesn't surface when using --squash, as $rev is reassigned\n>>to the squash commit via new_squash_commit before git commit-tree sees\n>>it (though for simplicity, new_squash_commit now also sees the peeled\n>>ID).\n>>\n>>Also doesn't surface when using \"git subtree merge\", as git merge can\n>>handle tag objects.\n>>\n>>On a side note, if merging a tag without --squash, git merge recognises\n>>that it's a tag and adds a note to the merge commit body. It may be\n>>worth mimicking this when using \"subtree merge --squash\" or\n>>\"subtree add\".\n>>\n>> contrib/subtree/git-subtree.sh | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>>diff --git a/contrib/subtree/git-subtree.sh\n>>b/contrib/subtree/git-subtree.sh\n>>index dc59a91..9453dae 100755\n>>--- a/contrib/subtree/git-subtree.sh\n>>+++ b/contrib/subtree/git-subtree.sh\n>>@@ -538,7 +538,7 @@ cmd_add_commit()\n>> {\n>> revs=$(git rev-parse $default --revs-only \"$@\") || exit $?\n>> set -- $revs\n>>- rev=\"$1\"\n>>+ rev=$(peel_committish \"$1\")\n>>\n>> debug \"Adding $dir as '$rev'...\"\n>> git read-tree --prefix=\"$dir\" $rev || exit $?\n>\n> I know that subtree isn't exactly the most popular or exciting part of\n> the project at the moment, but given that this is adding a subtree based\n> on an annotated tag is a reasonably sensible operation and (to me) the\n> fix seems reasonably trivial, could I get some eyes on this?\n>\n> Regards,\n> James Denholm.\n"}]}