{"thread":{"id":"32779","subject":"\"sha1 information is lacking or useless\" when rebasing with a submodule pointer conflict","startedAt":"2013-01-30T18:43:31Z","lastAt":"2013-02-05T19:19:32Z","messageCount":11,"participants":["Michael Sims","Heiko Voigt","Junio C Hamano","Martin von Zweigbergk"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"208289","messageId":"CAFyOhY8YAO4zx6jKQxrEW=-Vbo-TTjU6wJ7UgNVEjA7B2dasng@mail.gmail.com","threadId":"32779","inReplyTo":null,"subject":"\"sha1 information is lacking or useless\" when rebasing with a submodule pointer conflict","fromName":"Michael Sims","fromEmail":"michael.h.sims@gmail.com","sentAt":"2013-01-30T18:43:31Z","receivedAt":"2013-01-30T18:43:31Z","isPatch":false,"sender":{"key":"michael.h.sims@gmail.com","avatar":null},"body":"I'm seeing what might be a bug that was introduced in git 1.7.12 (also\nobserved in 1.8.1.2). If not a bug, it's a changed behavior from\nprevious versions that I don't understand.\n\nHere's the scenario:\n* I have a remote repo containing a pointer to a submodule.\n* Developer A and Developer B clone this repo, and both make a commit\nto first the submodule, and then the parent repo, changing some files\nand also the submodule pointer at the same time.\n* Developer A pushes his changes to both the submodule and the parent\nmodule to the shared remote\n* Developer B either does a \"git pull --rebase\" or a \"git fetch && git\nrebase origin/master\"\n\nResults:\nWhen applying Developer B's changes on top of origin/master, the\nfollowing messages are observed:\n\"fatal: sha1 information is lacking or useless (sub).\nRepository lacks necessary blobs to fall back on 3-way merge.\nCannot fall back to three-way merge.\"\n\nFurthermore, an immediate \"git status\" reports \"all conflicts fixed\",\ndoes not report the submodule pointer as unmerged, and the changes to\nthe other files in the parent module made by Developer B are not\napplied.\n\nExpected Results:\nSimilar to how 1.7.12.1 behaves. No mention of sha1 information or\nblobs being lacking. \"git status\" should report the submodule pointer\nand any other changed files as being unmerged. Conflicting changes\nshould not be lost.\n\nI'm witnessing this behavior on OS X 10.8.2 with git versions 1.7.12,\n1.7.12.1, and 1.8.1.2, installed via homebrew. I do not see this\nbehavior in 1.7.11.5 or any of the 8 previous versions I've tried back\nto 1.7.4.1.\n\nWorkaround:\nUsing an interactive rebase (\"git fetch && git rebase -i\norigin/master\") with no changes made to the commit list seems to\nsidestep this behavior.\n\nThanks in advance for any help with this.\n\nReproduce script:\nExecute the following, then \"cd local-b; git pull --rebase\"\n\n#!/bin/sh\n\nset -e -v\n\nBASEDIR=`pwd`\n\n# Make bare remote repos\nmkdir remotes\npushd remotes\ngit init --bare super.git\ngit init --bare sub.git\npopd\n\n# Initialize the submodule repo\ngit clone --no-hardlinks remotes/sub.git sub\npushd sub\necho subfile > subfile\ngit add subfile\ngit commit -m initial-submodule\ngit push origin master\npopd\nrm -Rf sub\n\n# Clone into local-a and initialize supermodule repo\ngit clone --no-hardlinks remotes/super.git local-a\npushd local-a\ngit submodule add $BASEDIR/remotes/sub.git sub\necho file > file\ngit add sub file\ngit commit -m initial\ngit push origin master\npopd\n\n# Clone into local-b\ngit clone --no-hardlinks remotes/super.git local-b\npushd local-b\ngit submodule init && git submodule update\npopd\n\n# Make a change to supermodule file and submodule pointer from local-a\nand push to remote\npushd local-a\ncd sub\necho subfile changed from local-a > subfile\ngit add subfile\ngit commit -m subfile-changed-from-local-a\ngit push\ncd ..\necho file changed from local-a > file\ngit add sub file\ngit commit -m change-from-local-a\ngit push\npopd\n\n# Make a conflicting change to both supermodule file and submodule\npointer from local-b\npushd local-b\ncd sub\necho subfile changed from local-b > subfile\ngit add subfile\ngit commit -m subfile-changed-from-local-b\ncd ..\necho file changed from local-b > file\ngit add sub file\ngit commit -m change-from-local-b\npopd\n"},{"id":"208318","messageId":"20130130215615.GA1053@book.hvoigt.net","threadId":"32779","inReplyTo":"CAFyOhY8YAO4zx6jKQxrEW=-Vbo-TTjU6wJ7UgNVEjA7B2dasng@mail.gmail.com","subject":"Re: \"sha1 information is lacking or useless\" when rebasing with a submodule pointer conflict","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-01-30T21:56:15Z","receivedAt":"2013-01-30T21:56:15Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Wed, Jan 30, 2013 at 12:43:31PM -0600, Michael Sims wrote:\n> I'm seeing what might be a bug that was introduced in git 1.7.12 (also\n> observed in 1.8.1.2). If not a bug, it's a changed behavior from\n> previous versions that I don't understand.\n> \n> Here's the scenario:\n> * I have a remote repo containing a pointer to a submodule.\n> * Developer A and Developer B clone this repo, and both make a commit\n> to first the submodule, and then the parent repo, changing some files\n> and also the submodule pointer at the same time.\n> * Developer A pushes his changes to both the submodule and the parent\n> module to the shared remote\n> * Developer B either does a \"git pull --rebase\" or a \"git fetch && git\n> rebase origin/master\"\n\nThanks for the detailed bug report and the demo script. I can reproduce\nthe behavior here and will have a look into it. The submodule should be\nmarked as conflict.\n\nCheers Heiko\n"},{"id":"208321","messageId":"20130130224904.GB1053@book.hvoigt.net","threadId":"32779","inReplyTo":"20130130215615.GA1053@book.hvoigt.net","subject":"Re: Re: \"sha1 information is lacking or useless\" when rebasing with a submodule pointer conflict","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-01-30T22:49:04Z","receivedAt":"2013-01-30T22:49:04Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Wed, Jan 30, 2013 at 10:56:15PM +0100, Heiko Voigt wrote:\n> On Wed, Jan 30, 2013 at 12:43:31PM -0600, Michael Sims wrote:\n> > I'm seeing what might be a bug that was introduced in git 1.7.12 (also\n> > observed in 1.8.1.2). If not a bug, it's a changed behavior from\n> > previous versions that I don't understand.\n> > \n> > Here's the scenario:\n> > * I have a remote repo containing a pointer to a submodule.\n> > * Developer A and Developer B clone this repo, and both make a commit\n> > to first the submodule, and then the parent repo, changing some files\n> > and also the submodule pointer at the same time.\n> > * Developer A pushes his changes to both the submodule and the parent\n> > module to the shared remote\n> > * Developer B either does a \"git pull --rebase\" or a \"git fetch && git\n> > rebase origin/master\"\n> \n> Thanks for the detailed bug report and the demo script. I can reproduce\n> the behavior here and will have a look into it. The submodule should be\n> marked as conflict.\n\nBisect identified the following commit:\n\ncommit a230949409f4a650c6a1a9a5879e2a8b993ba695 (HEAD)\nAuthor: Martin von Zweigbergk \nDate: Tue Jun 26 07:51:56 2012 -0700\n\n am --rebasing: get patch body from commit, not from mailbox\n \n Rebasing a commit that contains a diff in the commit message results\n in a failure with output such as\n \n First, rewinding head to replay your work on top of it...\n Applying: My cool patch.\n fatal: sha1 information is lacking or useless\n (app/controllers/settings_controller.rb).\n Repository lacks necessary blobs to fall back on 3-way merge.\n Cannot fall back to three-way merge.\n Patch failed at 0001 My cool patch.\n \n The reason is that 'git rebase' without -p/-i/-m internally calls 'git\n format-patch' and pipes the output to 'git am --rebasing', which has\n no way of knowing what is a real patch and what is a commit message\n that contains a patch.\n \n Make 'git am' while in --rebasing mode get the patch body from the\n commit object instead of extracting it from the mailbox.\n \n Patch by Junio, test case and commit log message by Martin.\n \n Reported-by: anikey \n Helped-by: Junio C Hamano \n Signed-off-by: Martin von Zweigbergk \n Signed-off-by: Junio C Hamano \n\nMaybe Martin or Junio immediately see whats going wrong here? I would\nneed to further dig into the git-am code to find out how to fix it.\n\nCheers Heiko\n"},{"id":"208323","messageId":"7v38xijl5z.fsf@alter.siamese.dyndns.org","threadId":"32779","inReplyTo":"20130130224904.GB1053@book.hvoigt.net","subject":"Re: \"sha1 information is lacking or useless\" when rebasing with a submodule pointer conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T23:39:04Z","receivedAt":"2013-01-30T23:39:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> Maybe Martin or Junio immediately see whats going wrong here? I would\n> need to further dig into the git-am code to find out how to fix it.\n\n\"am -3\" has never worked on a patch that describes changes to any\nnon-blobs; the underlyihng \"apply --fake-ancestor\" is not prepared\nto see anything other than blobs.\n"},{"id":"208437","messageId":"7vboc4d6ni.fsf@alter.siamese.dyndns.org","threadId":"32779","inReplyTo":"20130130224904.GB1053@book.hvoigt.net","subject":"Re: \"sha1 information is lacking or useless\" when rebasing with a submodule pointer conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-01T04:01:21Z","receivedAt":"2013-02-01T04:01:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> Maybe Martin or Junio immediately see whats going wrong here? I would\n> need to further dig into the git-am code to find out how to fix it.\n\nThanks. I think the minimum fix is this, but there are some nits to\npick around this area as well.\n\n\ndiff --git b/git-am.sh a/git-am.sh\nindex b4d95f5..202130f 100755\n--- b/git-am.sh\n+++ a/git-am.sh\n@@ -664,7 +664,7 @@ do\n \t\t\tsed -e '1,/^$/d' >\"$dotest/msg-clean\"\n \t\t\techo \"$commit\" >\"$dotest/original-commit\"\n \t\t\tget_author_ident_from_commit \"$commit\" >\"$dotest/author-script\"\n-\t\t\tgit diff-tree --root --binary \"$commit\" >\"$dotest/patch\"\n+\t\t\tgit diff-tree --root --binary --full-index \"$commit\" >\"$dotest/patch\"\n \t\telse\n \t\t\tgit mailinfo $keep $no_inbody_headers $scissors $utf8 \"$dotest/msg\" \"$dotest/patch\" \\\n \t\t\t\t<\"$dotest/$msgnum\" >\"$dotest/info\" ||\n"},{"id":"208441","messageId":"1359693125-22357-1-git-send-email-gitster@pobox.com","threadId":"32779","inReplyTo":"20130130224904.GB1053@book.hvoigt.net","subject":"[PATCH 0/3] rebasing changes that update submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-01T04:32:02Z","receivedAt":"2013-02-01T04:32:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git rebase\" used to use \"format-patch --full-index\" to generate a\nseries of patches and let \"git am --rebase\" reconstruct a fake\npreimage tree by reading the object names from the \"index\" lines.\n\nWith a230949 (am --rebasing: get patch body from commit, not from\nmailbox, 2012-06-26), we switched to use \"diff-tree\", but forgot\nthat without \"--full-index\", the information generated on the\n\"index\" line is insufficient to recreate a preimage tree that\ncontains a submodule.\n\nThe first one is the real fix to this issue.\n\nThe other two falls into the niceties category; they are to issue a\nbetter error messge when the machinery is fed an abbreviated object\nname on the \"index\" line for a submodule.\n\nJunio C Hamano (3):\n git-am: record full index line in the patch used while rebasing\n apply: simplify build_fake_ancestor()\n apply: diagnose incomplete submodule object name better\n\n builtin/apply.c | 30 ++++++++++++++++--------------\n git-am.sh | 2 +-\n t/t7402-submodule-rebase.sh | 30 ++++++++++++++++++++++++++++--\n 3 files changed, 45 insertions(+), 17 deletions(-)\n\n-- \n1.8.1.2.612.g09f4be5\n"},{"id":"208438","messageId":"1359693125-22357-2-git-send-email-gitster@pobox.com","threadId":"32779","inReplyTo":"1359693125-22357-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] git-am: record full index line in the patch used while rebasing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-01T04:32:03Z","receivedAt":"2013-02-01T04:32:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Earlier, a230949 (am --rebasing: get patch body from commit, not\nfrom mailbox, 2012-06-26) learned to regenerate patch body from the\ncommit object while rebasing, instead of reading from the rebase-am\nfront-end. While doing so, it used \"git diff-tree\" but without\ngiving it the \"--full-index\" option.\n\nThis does not matter for in-repository objects; during rebasing, any\nabbreviated object name should uniquely identify them.\n\nBut we may be rebasing a commit that contains a change to a gitlink,\nin which case we usually should not have the object (it names a\ncommit in the submodule). A full object name is necessary to later\nreconstruct a fake ancestor index for them.\n\nSigned-off-by: Junio C Hamano \n---\n git-am.sh | 2 +-\n t/t7402-submodule-rebase.sh | 30 ++++++++++++++++++++++++++++--\n 2 files changed, 29 insertions(+), 3 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex c682d34..0e0a096 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -664,7 +664,7 @@ do\n \t\t\tsed -e '1,/^$/d' >\"$dotest/msg-clean\"\n \t\t\techo \"$commit\" >\"$dotest/original-commit\"\n \t\t\tget_author_ident_from_commit \"$commit\" >\"$dotest/author-script\"\n-\t\t\tgit diff-tree --root --binary \"$commit\" >\"$dotest/patch\"\n+\t\t\tgit diff-tree --root --binary --full-index \"$commit\" >\"$dotest/patch\"\n \t\telse\n \t\t\tgit mailinfo $keep $no_inbody_headers $scissors $utf8 \"$dotest/msg\" \"$dotest/patch\" \\\n \t\t\t\t<\"$dotest/$msgnum\" >\"$dotest/info\" ||\ndiff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh\nindex f919c8d..8e32f19 100755\n--- a/t/t7402-submodule-rebase.sh\n+++ b/t/t7402-submodule-rebase.sh\n@@ -3,7 +3,7 @@\n # Copyright (c) 2008 Johannes Schindelin\n #\n \n-test_description='Test rebasing and stashing with dirty submodules'\n+test_description='Test rebasing, stashing, etc. with submodules'\n \n . ./test-lib.sh\n \n@@ -20,7 +20,8 @@ test_expect_success setup '\n \techo second line >> file &&\n \t(cd submodule && git pull) &&\n \ttest_tick &&\n-\tgit commit -m file-and-submodule -a\n+\tgit commit -m file-and-submodule -a &&\n+\tgit branch added-submodule\n \n '\n \n@@ -89,4 +90,29 @@ test_expect_success 'stash with a dirty submodule' '\n \n '\n \n+test_expect_success 'rebasing submodule that should conflict' '\n+\tgit reset --hard &&\n+\tgit checkout added-submodule &&\n+\tgit add submodule &&\n+\ttest_tick &&\n+\tgit commit -m third &&\n+\t(\n+\t\tcd submodule &&\n+\t\tgit commit --allow-empty -m extra\n+\t) &&\n+\tgit add submodule &&\n+\ttest_tick &&\n+\tgit commit -m fourth &&\n+\n+\ttest_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 &&\n+\tgit ls-files -s submodule >actual &&\n+\t(\n+\t\tcd submodule &&\n+\t\techo \"160000 $(git rev-parse HEAD^) 1\tsubmodule\" &&\n+\t\techo \"160000 $(git rev-parse HEAD^^) 2\tsubmodule\" &&\n+\t\techo \"160000 $(git rev-parse HEAD) 3\tsubmodule\"\n+\t) >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.8.1.2.612.g09f4be5\n"},{"id":"208439","messageId":"1359693125-22357-3-git-send-email-gitster@pobox.com","threadId":"32779","inReplyTo":"1359693125-22357-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] apply: simplify build_fake_ancestor()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-01T04:32:04Z","receivedAt":"2013-02-01T04:32:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The local variable sha1_ptr in the build_fake_ancestor() function\nused to either point at the null_sha1[] (if the ancestor did not\nhave the path) or at sha1[] (if we read the object name into the\nlocal array), but 7a98869 (apply: get rid of --index-info in favor\nof --build-fake-ancestor, 2007-09-17) made the \"missing in the\nancestor\" case unnecessary, hence sha1_ptr, when used, always points\nat the local array.\n\nGet rid of the unneeded variable, and restructure the if/else\ncascade a bit to make it easier to read. There should be no\nbehaviour change.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 26 ++++++++++++--------------\n 1 file changed, 12 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 156b3ce..a1db7b4 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3598,7 +3598,6 @@ static void build_fake_ancestor(struct patch *list, const char *filename)\n \t * worth showing the new sha1 prefix, but until then...\n \t */\n \tfor (patch = list; patch; patch = patch->next) {\n-\t\tconst unsigned char *sha1_ptr;\n \t\tunsigned char sha1[20];\n \t\tstruct cache_entry *ce;\n \t\tconst char *name;\n@@ -3606,20 +3605,19 @@ static void build_fake_ancestor(struct patch *list, const char *filename)\n \t\tname = patch->old_name ? patch->old_name : patch->new_name;\n \t\tif (0 < patch->is_new)\n \t\t\tcontinue;\n-\t\telse if (get_sha1_blob(patch->old_sha1_prefix, sha1))\n-\t\t\t/* git diff has no index line for mode/type changes */\n-\t\t\tif (!patch->lines_added && !patch->lines_deleted) {\n-\t\t\t\tif (get_current_sha1(patch->old_name, sha1))\n-\t\t\t\t\tdie(\"mode change for %s, which is not \"\n-\t\t\t\t\t\t\"in current HEAD\", name);\n-\t\t\t\tsha1_ptr = sha1;\n-\t\t\t} else\n-\t\t\t\tdie(\"sha1 information is lacking or useless \"\n-\t\t\t\t\t\"(%s).\", name);\n-\t\telse\n-\t\t\tsha1_ptr = sha1;\n \n-\t\tce = make_cache_entry(patch->old_mode, sha1_ptr, name, 0, 0);\n+\t\tif (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {\n+\t\t\t; /* ok */\n+\t\t} else if (!patch->lines_added && !patch->lines_deleted) {\n+\t\t\t/* mode-only change: update the current */\n+\t\t\tif (get_current_sha1(patch->old_name, sha1))\n+\t\t\t\tdie(\"mode change for %s, which is not \"\n+\t\t\t\t \"in current HEAD\", name);\n+\t\t} else\n+\t\t\tdie(\"sha1 information is lacking or useless \"\n+\t\t\t \"(%s).\", name);\n+\n+\t\tce = make_cache_entry(patch->old_mode, sha1, name, 0, 0);\n \t\tif (!ce)\n \t\t\tdie(_(\"make_cache_entry failed for path '%s'\"), name);\n \t\tif (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD))\n-- \n1.8.1.2.612.g09f4be5\n"},{"id":"208440","messageId":"1359693125-22357-4-git-send-email-gitster@pobox.com","threadId":"32779","inReplyTo":"1359693125-22357-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] apply: diagnose incomplete submodule object name better","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-01T04:32:05Z","receivedAt":"2013-02-01T04:32:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git am -3\" uses this function to build a tree that records how the\npreimage the patch was created from would have looked like. An\nabbreviated object name on the index line is ordinarily sufficient\nfor us to figure out the object name the preimage tree would have\ncontained, but a change to a submodule by definition shows an object\nname of a submodule commit which our repository should not have, and\nget_sha1_blob() is not an appropriate way to read it (or get_sha1()\nfor that matter).\n\nUse get_sha1_hex() and complain if we do not find a full object name\nthere.\n\nWe could read from the payload part of the patch to learn the full\nobject name of the commit, but the primary user \"git rebase\" has\nbeen fixed to give us a full object name, so this should suffice\nfor now.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex a1db7b4..1f78e2c 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3606,7 +3606,11 @@ static void build_fake_ancestor(struct patch *list, const char *filename)\n \t\tif (0 < patch->is_new)\n \t\t\tcontinue;\n \n-\t\tif (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {\n+\t\tif (S_ISGITLINK(patch->old_mode)) {\n+\t\t\tif (get_sha1_hex(patch->old_sha1_prefix, sha1))\n+\t\t\t\tdie(\"submoule change for %s without full index name\",\n+\t\t\t\t name);\n+\t\t} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {\n \t\t\t; /* ok */\n \t\t} else if (!patch->lines_added && !patch->lines_deleted) {\n \t\t\t/* mode-only change: update the current */\n-- \n1.8.1.2.612.g09f4be5\n"},{"id":"208447","messageId":"CANiSa6gV=C188ZeaPrhdNdG+iPZ_DuBBbYgM0bT55KNOr03rAA@mail.gmail.com","threadId":"32779","inReplyTo":"1359693125-22357-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 1/3] git-am: record full index line in the patch used while rebasing","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-02-01T06:25:40Z","receivedAt":"2013-02-01T06:25:40Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Thu, Jan 31, 2013 at 8:32 PM, Junio C Hamano wrote:\n> Earlier, a230949 (am --rebasing: get patch body from commit, not\n> from mailbox, 2012-06-26) learned to regenerate patch body from the\n> commit object while rebasing, instead of reading from the rebase-am\n> front-end. While doing so, it used \"git diff-tree\" but without\n> giving it the \"--full-index\" option.\n>\n> This does not matter for in-repository objects; during rebasing, any\n> abbreviated object name should uniquely identify them.\n>\n> But we may be rebasing a commit that contains a change to a gitlink,\n> in which case we usually should not have the object (it names a\n> commit in the submodule). A full object name is necessary to later\n> reconstruct a fake ancestor index for them.\n\n>From what I can understand, this all makes sense. I didn't notice the\nemails about the breakage until now, but I wouldn't have known where\nto even start looking anyway, so thanks a lot for taking care of it.\n"},{"id":"208752","messageId":"7vpq0esh4r.fsf_-_@alter.siamese.dyndns.org","threadId":"32779","inReplyTo":"1359693125-22357-4-git-send-email-gitster@pobox.com","subject":"Re* [PATCH 3/3] apply: diagnose incomplete submodule object name better","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T19:19:32Z","receivedAt":"2013-02-05T19:19:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> We could read from the payload part of the patch to learn the full\n> object name of the commit, but the primary user \"git rebase\" has\n> been fixed to give us a full object name, so this should suffice\n> for now.\n\nAnd the patch on top to do so looks like this. With this patch in\nplace, we could drop 1/3 of this series, but there is no strong\nreason to do so. After all, 1/3 is all about internal implementation\ndetails.\n\n-- >8 --\nSubject: [PATCH 4/3] apply: verify submodule commit object name better\n\nA textual patch also records the submodule commit object name in\nfull. Make the parsing more robust by reading from there and\nverifying the (possibly abbreviated) name on the index line matches.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 40 ++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 1f78e2c..e0f1474 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3587,6 +3587,40 @@ static int get_current_sha1(const char *path, unsigned char *sha1)\n \treturn 0;\n }\n \n+static int preimage_sha1_in_gitlink_patch(struct patch *p, unsigned char sha1[20])\n+{\n+\t/*\n+\t * A usable gitlink patch has only one fragment (hunk) that looks like:\n+\t * @@ -1 +1 @@\n+\t * -Subproject commit \n+\t * +Subproject commit \n+\t * or\n+\t * @@ -1 +0,0 @@\n+\t * -Subproject commit \n+\t * for a removal patch.\n+\t */\n+\tstruct fragment *hunk = p->fragments;\n+\tstatic const char heading[] = \"-Subproject commit \";\n+\tchar *preimage;\n+\n+\tif (/* does the patch have only one hunk? */\n+\t hunk && !hunk->next &&\n+\t /* is its preimage one line? */\n+\t hunk->oldpos == 1 && hunk->oldlines == 1 &&\n+\t /* does preimage begin with the heading? */\n+\t (preimage = memchr(hunk->patch, '\\n', hunk->size)) != NULL &&\n+\t !prefixcmp(++preimage, heading) &&\n+\t /* does it record full SHA-1? */\n+\t !get_sha1_hex(preimage + sizeof(heading) - 1, sha1) &&\n+\t preimage[sizeof(heading) + 40 - 1] == '\\n' &&\n+\t /* does the abbreviated name on the index line agree with it? */\n+\t !prefixcmp(preimage + sizeof(heading) - 1, p->old_sha1_prefix))\n+\t\treturn 0; /* it all looks fine */\n+\n+\t/* we may have full object name on the index line */\n+\treturn get_sha1_hex(p->old_sha1_prefix, sha1);\n+}\n+\n /* Build an index that contains the just the files needed for a 3way merge */\n static void build_fake_ancestor(struct patch *list, const char *filename)\n {\n@@ -3607,8 +3641,10 @@ static void build_fake_ancestor(struct patch *list, const char *filename)\n \t\t\tcontinue;\n \n \t\tif (S_ISGITLINK(patch->old_mode)) {\n-\t\t\tif (get_sha1_hex(patch->old_sha1_prefix, sha1))\n-\t\t\t\tdie(\"submoule change for %s without full index name\",\n+\t\t\tif (!preimage_sha1_in_gitlink_patch(patch, sha1))\n+\t\t\t\t; /* ok, the textual part looks sane */\n+\t\t\telse\n+\t\t\t\tdie(\"sha1 information is lacking or useless for submoule %s\",\n \t\t\t\t name);\n \t\t} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {\n \t\t\t; /* ok */\n-- \n1.8.1.2.639.g9428735\n"}]}