{"thread":{"id":"41124","subject":"[PATCH] Add a test for subtree rebase that loses commits","startedAt":"2016-01-05T04:40:04Z","lastAt":"2016-06-28T11:30:12Z","messageCount":24,"participants":["David Greene","Torsten Bögershausen","Dennis Kaarsemaker","Eric Sunshine","David A. Greene","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"275348","messageId":"1451968805-6948-1-git-send-email-greened@obbligato.org","threadId":"41124","inReplyTo":null,"subject":"[PATCH] Test rebase -Xsubtree","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-05T04:40:04Z","receivedAt":"2016-01-05T04:40:04Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Here is a test that finds a bug in rebase -Xsubtree.  With\n--preserve-merges, commits are lost.\n\n                    -David\n"},{"id":"275347","messageId":"1451968805-6948-2-git-send-email-greened@obbligato.org","threadId":"41124","inReplyTo":"1451968805-6948-1-git-send-email-greened@obbligato.org","subject":"[PATCH] Add a test for subtree rebase that loses commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-05T04:40:05Z","receivedAt":"2016-01-05T04:40:05Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: \"David A. Greene\" <greened@obbligato.org>\n\nThis test merges an external tree in as a subtree, makes some commits\non top of it and splits it back out.  In the process the added commits\nare lost.  This is marked to expect failure so that we don't forget to\nfix it.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n t/t3427-rebase-subtree.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 68 insertions(+)\n create mode 100755 t/t3427-rebase-subtree.sh\n\ndiff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\nnew file mode 100755\nindex 0000000..7eb28ab\n--- /dev/null\n+++ b/t/t3427-rebase-subtree.sh\n@@ -0,0 +1,68 @@\n+#!/bin/sh\n+\n+test_description='git rebase tests for -Xsubtree\n+\n+This test runs git rebase and tests the subtree strategy.\n+'\n+. ./test-lib.sh\n+\n+addfile() {\n+    name=$1\n+    echo $(basename ${name}) > ${name}\n+    ${git} add ${name}\n+    ${git} commit -m \"Add $(basename ${name})\"\n+}\n+\n+check_equal()\n+{\n+\ttest_debug 'echo'\n+\ttest_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n+\ttest_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n+\tif [ \"$1\" = \"$2\" ]; then\n+\t\treturn 0\n+\telse\n+\t\treturn 1\n+\tfi\n+}\n+\n+last_commit_message()\n+{\n+\tgit log --pretty=format:%s -1\n+}\n+\n+test_expect_success 'setup' '\n+\ttest_commit README &&\n+\tmkdir files &&\n+\tcd files &&\n+\tgit init &&\n+\ttest_commit master1 &&\n+\ttest_commit master2 &&\n+\ttest_commit master3 &&\n+\tcd .. &&\n+\ttest_debug \"echo Add project master to master\" &&\n+\tgit fetch files master &&\n+\tgit branch files-master FETCH_HEAD &&\n+\ttest_debug \"echo Add subtree master to master via subtree\" &&\n+\tgit read-tree --prefix=files_subtree files-master &&\n+\tgit checkout -- files_subtree &&\n+\ttree=$(git write-tree) &&\n+\thead=$(git rev-parse HEAD) &&\n+\trev=$(git rev-parse --verify files-master^0) &&\n+\tcommit=$(git commit-tree -p ${head} -p ${rev} -m \"Add subproject master\" ${tree}) &&\n+\tgit reset ${commit} &&\n+\tcd files_subtree &&\n+\ttest_commit master4 &&\n+\tcd .. &&\n+\ttest_commit files_subtree/master5\n+'\n+\n+# Does not preserve master4 and master5.\n+test_expect_failure 'Rebase default' '\n+\tgit checkout -b rebase-default master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree  --preserve-merges --onto files-master master &&\n+\tcheck_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n+'\n+\n+test_done\n-- \n2.6.1\n"},{"id":"275366","messageId":"568B833B.4060001@web.de","threadId":"41124","inReplyTo":"1451968805-6948-2-git-send-email-greened@obbligato.org","subject":"Re: [PATCH] Add a test for subtree rebase that loses commits","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-01-05T08:47:55Z","receivedAt":"2016-01-05T08:47:55Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Need to drop\nDavid Greene <greened@obbligato.org>\nfrom List, no MX record\n\nOn 2016-01-05 05.40, David Greene wrote:\n> From: \"David A. Greene\" <greened@obbligato.org>\n> \n> This test merges an external tree in as a subtree, makes some commits\n> on top of it and splits it back out.  In the process the added commits\n> are lost.  This is marked to expect failure so that we don't forget to\n> fix it.\n> \n> Signed-off-by: David A. Greene <greened@obbligato.org>\n> ---\n>  t/t3427-rebase-subtree.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 68 insertions(+)\n>  create mode 100755 t/t3427-rebase-subtree.sh\n> \n> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\n> new file mode 100755\n> index 0000000..7eb28ab\n> --- /dev/null\n> +++ b/t/t3427-rebase-subtree.sh\n> @@ -0,0 +1,68 @@\n> +#!/bin/sh\n> +\n> +test_description='git rebase tests for -Xsubtree\n> +\n> +This test runs git rebase and tests the subtree strategy.\n> +'\n> +. ./test-lib.sh\n> +\n> +addfile() {\n> +    name=$1\n> +    echo $(basename ${name}) > ${name}\n> +    ${git} add ${name}\n> +    ${git} commit -m \"Add $(basename ${name})\"\n> +}\n> +\n> +check_equal()\n> +{\n> +\ttest_debug 'echo'\n> +\ttest_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n> +\ttest_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n> +\tif [ \"$1\" = \"$2\" ]; then\n> +\t\treturn 0\n> +\telse\n> +\t\treturn 1\n> +\tfi\n> +}\n> +\n> +last_commit_message()\n> +{\n> +\tgit log --pretty=format:%s -1\n> +}\n> +\n> +test_expect_success 'setup' '\n> +\ttest_commit README &&\n> +\tmkdir files &&\nWhen cd'ing into a directory,\nwe need to do it in a sub-shell:\n> +\tcd files &&\n> +\tgit init &&\n> +\ttest_commit master1 &&\n> +\ttest_commit master2 &&\n> +\ttest_commit master3 &&\n> +\tcd .. &&\n\tmkdir files &&\n\t(\n\tcd files &&\n\tgit init &&\n\ttest_commit master1 &&\n\ttest_commit master2 &&\n\ttest_commit master3\n\t)\n\n\n(And similar below)\n> +\ttest_debug \"echo Add project master to master\" &&\n> +\tgit fetch files master &&\n> +\tgit branch files-master FETCH_HEAD &&\n> +\ttest_debug \"echo Add subtree master to master via subtree\" &&\n> +\tgit read-tree --prefix=files_subtree files-master &&\n> +\tgit checkout -- files_subtree &&\n> +\ttree=$(git write-tree) &&\n> +\thead=$(git rev-parse HEAD) &&\n> +\trev=$(git rev-parse --verify files-master^0) &&\n> +\tcommit=$(git commit-tree -p ${head} -p ${rev} -m \"Add subproject master\" ${tree}) &&\n> +\tgit reset ${commit} &&\n> +\tcd files_subtree &&\n> +\ttest_commit master4 &&\n> +\tcd .. &&\n> +\ttest_commit files_subtree/master5\n> +'\n> +\n> +# Does not preserve master4 and master5.\n> +test_expect_failure 'Rebase default' '\n> +\tgit checkout -b rebase-default master &&\n> +\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +\tgit commit -m \"Empty commit\" --allow-empty &&\n> +\tgit rebase -Xsubtree=files_subtree  --preserve-merges --onto files-master master &&\n> +\tcheck_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n> +'\n> +\n> +test_done\n> \n"},{"id":"275367","messageId":"1451987857.2668.5.camel@kaarsemaker.net","threadId":"41124","inReplyTo":"568B833B.4060001@web.de","subject":"Re: [PATCH] Add a test for subtree rebase that loses commits","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-01-05T09:57:37Z","receivedAt":"2016-01-05T09:57:37Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On di, 2016-01-05 at 09:47 +0100, Torsten Bögershausen wrote:\n> Need to drop\n> David Greene <greened@obbligato.org>\n> from List, no MX record\n\nseahawk:~$ dig MX obbligato.org\nobbligato.org.\t\t1800\tIN\tMX\t10\nmail.obbligato.org.\nseahawk:~$ dig mail.obbligato.org\nmail.obbligato.org.\t1800\tIN\tCNAME\tobbligato\n.org.\nobbligato.org.\t\t1800\tIN\tA\t173.255.19\n9.253\n\nSo it has an MX record, it's just incorrect: MX records must not point\nto things that are CNAMEs.\n\n-- \nDennis Kaarsemaker\nhttp://www.kaarsemaker.net\n"},{"id":"275370","messageId":"568BA670.6070104@web.de","threadId":"41124","inReplyTo":"1451987857.2668.5.camel@kaarsemaker.net","subject":"Re: [PATCH] Add a test for subtree rebase that loses commits","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-01-05T11:18:08Z","receivedAt":"2016-01-05T11:18:08Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\nOn 2016-01-05 10.57, Dennis Kaarsemaker wrote:\n> On di, 2016-01-05 at 09:47 +0100, Torsten Bögershausen wrote:\n>> Need to drop\n>> David Greene <greened@obbligato.org>\n>> from List, no MX record\n> \n> seahawk:~$ dig MX obbligato.org\n> obbligato.org.\t\t1800\tIN\tMX\t10\n> mail.obbligato.org.\n> seahawk:~$ dig mail.obbligato.org\n> mail.obbligato.org.\t1800\tIN\tCNAME\tobbligato\n> .org.\n> obbligato.org.\t\t1800\tIN\tA\t173.255.19\n> 9.253\n> \n> So it has an MX record, it's just incorrect: MX records must not point\n> to things that are CNAMEs.\n> \nThis may be a problem from web.de:\n\nAn error occurred while sending mail. The mail server responded:\nRequested action not taken: mailbox unavailable\ninvalid DNS MX or A/AAAA resource record.\nPlease check the message recipient \"greened@obbligato.org\" and try again.\n"},{"id":"275396","messageId":"CAPig+cSOzwGdp-FACM2=wK78KSjvEZoB6VKiEtBLnBX0G1L4QQ@mail.gmail.com","threadId":"41124","inReplyTo":"1451968805-6948-2-git-send-email-greened@obbligato.org","subject":"Re: [PATCH] Add a test for subtree rebase that loses commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-05T20:34:04Z","receivedAt":"2016-01-05T20:34:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 4, 2016 at 11:40 PM, David Greene <greened@obbligato.org> wrote:\n> This test merges an external tree in as a subtree, makes some commits\n> on top of it and splits it back out.  In the process the added commits\n> are lost.  This is marked to expect failure so that we don't forget to\n> fix it.\n>\n> Signed-off-by: David A. Greene <greened@obbligato.org>\n> ---\n> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\n> @@ -0,0 +1,68 @@\n> +#!/bin/sh\n> +\n> +test_description='git rebase tests for -Xsubtree\n> +\n> +This test runs git rebase and tests the subtree strategy.\n> +'\n> +. ./test-lib.sh\n> +\n> +addfile() {\n> +    name=$1\n> +    echo $(basename ${name}) > ${name}\n> +    ${git} add ${name}\n> +    ${git} commit -m \"Add $(basename ${name})\"\n> +}\n\nWhat is this function for? It doesn't seem to be used at all by this script.\n\n> +check_equal()\n> +{\n\nStyle: Place brace on the same line as the function declaration.\n\n> +       test_debug 'echo'\n> +       test_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n> +       test_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n> +       if [ \"$1\" = \"$2\" ]; then\n\nStyle: Use 'test' rather than '[', drop semi-colon, and place 'then'\non its own line.\n\n> +               return 0\n> +       else\n> +               return 1\n> +       fi\n\nThis entire if/else/fi can be rephrased as just a single line at the\nend of the function:\n\n    test \"$1\" = \"$2\"\n\nthe result of which will be 0 if the strings are equal, else 1, thus\nthere's no need for if/else/fi.\n\n> +}\n\nIsn't check_equal() pretty much a (less generic) re-invention of\nt/test-lib-functions.sh:verbose()?\n\n> +last_commit_message()\n> +{\n> +       git log --pretty=format:%s -1\n> +}\n\nAre there plans to re-use this function by more than the current\nsingle call site? If not, it might be just as clear to assign the\nresult of the expression to an aptly named variable directly in the\ncaller:\n\n   last_commit_msg=$(git log --pretty=format:%s -1)\n\nor something.\n\n> +test_expect_success 'setup' '\n> +       test_commit README &&\n> +       mkdir files &&\n> +       cd files &&\n> +       git init &&\n> +       test_commit master1 &&\n> +       test_commit master2 &&\n> +       test_commit master3 &&\n> +       cd .. &&\n\nMentioned by Torsten: If any command before \"cd ..\" fails, then \"cd\n..\" won't be invoked, and subsequent tests will be executed in the\nwrong directory. Use a subshell to overcome this problem since the\ncurrent directory of the parent shell is not impacted by the subshell\n(thus you can drop the \"cd ..\" altogether):\n\n    mkdir files &&\n    (\n        cd files &&\n        git init &&\n        ...\n    ) &&\n    ...\n\n> +       test_debug \"echo Add project master to master\" &&\n> +       git fetch files master &&\n> +       git branch files-master FETCH_HEAD &&\n> +       test_debug \"echo Add subtree master to master via subtree\" &&\n> +       git read-tree --prefix=files_subtree files-master &&\n> +       git checkout -- files_subtree &&\n> +       tree=$(git write-tree) &&\n> +       head=$(git rev-parse HEAD) &&\n> +       rev=$(git rev-parse --verify files-master^0) &&\n> +       commit=$(git commit-tree -p ${head} -p ${rev} -m \"Add subproject master\" ${tree}) &&\n\nNit: This could be less syntactically noisy by dropping the\nunnecessary braces: ${head} -> $head\n\n> +       git reset ${commit} &&\n> +       cd files_subtree &&\n> +       test_commit master4 &&\n> +       cd .. &&\n> +       test_commit files_subtree/master5\n> +'\n> +\n> +# Does not preserve master4 and master5.\n> +test_expect_failure 'Rebase default' '\n> +       git checkout -b rebase-default master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree  --preserve-merges --onto files-master master &&\n\nStyle: Too many spaces before --preserve-merges.\n\n> +       check_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n\nHmm, is checking the commit message the best way to determine if the\nexpected commit was there? Why not check the commit ID instead or\nsomething?\n\n> +'\n> +\n> +test_done\n> --\n> 2.6.1\n"},{"id":"275401","messageId":"87si2bwwyd.fsf@waller.obbligato.org","threadId":"41124","inReplyTo":"CAPig+cSOzwGdp-FACM2=wK78KSjvEZoB6VKiEtBLnBX0G1L4QQ@mail.gmail.com","subject":"Re: [PATCH] Add a test for subtree rebase that loses commits","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-05T21:14:18Z","receivedAt":"2016-01-05T21:14:18Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Mon, Jan 4, 2016 at 11:40 PM, David Greene <greened@obbligato.org> wrote:\n>> This test merges an external tree in as a subtree, makes some commits\n>> on top of it and splits it back out.  In the process the added commits\n>> are lost.  This is marked to expect failure so that we don't forget to\n>> fix it.\n>>\n>> Signed-off-by: David A. Greene <greened@obbligato.org>\n>> ---\n>> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\n>> @@ -0,0 +1,68 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='git rebase tests for -Xsubtree\n>> +\n>> +This test runs git rebase and tests the subtree strategy.\n>> +'\n>> +. ./test-lib.sh\n>> +\n>> +addfile() {\n>> +    name=$1\n>> +    echo $(basename ${name}) > ${name}\n>> +    ${git} add ${name}\n>> +    ${git} commit -m \"Add $(basename ${name})\"\n>> +}\n>\n> What is this function for? It doesn't seem to be used at all by this script.\n\nNothing.  I had sent a mail saying not to apply the patch but it\nbounced.  :)\n\nWill fix.\n\n>> +check_equal()\n>> +{\n>\n> Style: Place brace on the same line as the function declaration.\n>\n>> +       test_debug 'echo'\n>> +       test_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n>> +       test_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n>> +       if [ \"$1\" = \"$2\" ]; then\n>\n> Style: Use 'test' rather than '[', drop semi-colon, and place 'then'\n> on its own line.\n\nOk.\n\n>> +               return 0\n>> +       else\n>> +               return 1\n>> +       fi\n>\n> This entire if/else/fi can be rephrased as just a single line at the\n> end of the function:\n>\n>     test \"$1\" = \"$2\"\n>\n> the result of which will be 0 if the strings are equal, else 1, thus\n> there's no need for if/else/fi.\n\nOk.\n\n>> +}\n>\n> Isn't check_equal() pretty much a (less generic) re-invention of\n> t/test-lib-functions.sh:verbose()?\n\nDunno.  I'll have to look.\n\n>> +last_commit_message()\n>> +{\n>> +       git log --pretty=format:%s -1\n>> +}\n>\n> Are there plans to re-use this function by more than the current\n> single call site? If not, it might be just as clear to assign the\n> result of the expression to an aptly named variable directly in the\n> caller:\n>\n>    last_commit_msg=$(git log --pretty=format:%s -1)\n>\n> or something.\n\nThe intent is to add more tests later.  In fact I have at least a couple\nmore to add.\n\n>> +test_expect_success 'setup' '\n>> +       test_commit README &&\n>> +       mkdir files &&\n>> +       cd files &&\n>> +       git init &&\n>> +       test_commit master1 &&\n>> +       test_commit master2 &&\n>> +       test_commit master3 &&\n>> +       cd .. &&\n>\n> Mentioned by Torsten: If any command before \"cd ..\" fails, then \"cd\n> ..\" won't be invoked, and subsequent tests will be executed in the\n> wrong directory. Use a subshell to overcome this problem since the\n> current directory of the parent shell is not impacted by the subshell\n> (thus you can drop the \"cd ..\" altogether):\n>\n>     mkdir files &&\n>     (\n>         cd files &&\n>         git init &&\n>         ...\n>     ) &&\n>     ...\n\nYep.  Thanks.\n\n>> +       test_debug \"echo Add project master to master\" &&\n>> +       git fetch files master &&\n>> +       git branch files-master FETCH_HEAD &&\n>> +       test_debug \"echo Add subtree master to master via subtree\" &&\n>> +       git read-tree --prefix=files_subtree files-master &&\n>> +       git checkout -- files_subtree &&\n>> +       tree=$(git write-tree) &&\n>> +       head=$(git rev-parse HEAD) &&\n>> +       rev=$(git rev-parse --verify files-master^0) &&\n>> +       commit=$(git commit-tree -p ${head} -p ${rev} -m \"Add subproject master\" ${tree}) &&\n>\n> Nit: This could be less syntactically noisy by dropping the\n> unnecessary braces: ${head} -> $head\n\nOk.  It's the style I usually use but I'll go with the git convention.\n\n>> +       git reset ${commit} &&\n>> +       cd files_subtree &&\n>> +       test_commit master4 &&\n>> +       cd .. &&\n>> +       test_commit files_subtree/master5\n>> +'\n>> +\n>> +# Does not preserve master4 and master5.\n>> +test_expect_failure 'Rebase default' '\n>> +       git checkout -b rebase-default master &&\n>> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n>> +       git commit -m \"Empty commit\" --allow-empty &&\n>> +       git rebase -Xsubtree=files_subtree  --preserve-merges --onto files-master master &&\n>\n> Style: Too many spaces before --preserve-merges.\n\nThanks.\n\n>> +       check_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n>\n> Hmm, is checking the commit message the best way to determine if the\n> expected commit was there? Why not check the commit ID instead or\n> something?\n\nI'll look into that.\n\nThanks for the good feedback!\n\n                          -David\n"},{"id":"275616","messageId":"1452467297-16868-1-git-send-email-greened@obbligato.org","threadId":"41124","inReplyTo":"1451968805-6948-2-git-send-email-greened@obbligato.org","subject":"[PATCH v2] Test rebase -Xsubtree","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-10T23:08:16Z","receivedAt":"2016-01-10T23:08:16Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"\nVersion 2 of the patch to test and expose problems with rebase -Xsubtree.\nI've included an additional failure mode John reproduced and responded\nto the comments from Eric.  The tests still check the last commit message\nfor correctness because I'm not sure what would work better.  I'm open\nto learning new tricks.  :)\n\n                       -David\n"},{"id":"275617","messageId":"1452467297-16868-2-git-send-email-greened@obbligato.org","threadId":"41124","inReplyTo":"1452467297-16868-1-git-send-email-greened@obbligato.org","subject":"[PATCH] Add a test for subtree rebase that loses commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-10T23:08:17Z","receivedAt":"2016-01-10T23:08:17Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: \"David A. Greene\" <greened@obbligato.org>\n\nThis test merges an external tree in as a subtree, makes some commits\non top of it and splits it back out.  In the process the added commits\nare lost or the rebase aborts with an internal error.  The tests are\nmarked to expect failure so that we don't forget to fix it.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n t/t3427-rebase-subtree.sh | 79 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 79 insertions(+)\n create mode 100755 t/t3427-rebase-subtree.sh\n\ndiff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\nnew file mode 100755\nindex 0000000..add3b79\n--- /dev/null\n+++ b/t/t3427-rebase-subtree.sh\n@@ -0,0 +1,79 @@\n+#!/bin/sh\n+\n+test_description='git rebase tests for -Xsubtree\n+\n+This test runs git rebase and tests the subtree strategy.\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n+check_equal() {\n+\ttest_debug 'echo'\n+\ttest_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n+\ttest_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n+\ttest \"$1\" = \"$2\"\n+}\n+\n+last_commit_message() {\n+\tgit log --pretty=format:%s -1\n+}\n+\n+test_expect_success 'setup' '\n+\ttest_commit README &&\n+\tmkdir files &&\n+\t(\n+\t\tcd files &&\n+\t\tgit init &&\n+\t\ttest_commit master1 &&\n+\t\ttest_commit master2 &&\n+\t\ttest_commit master3\n+\t) &&\n+\ttest_debug \"echo Add project master to master\" &&\n+\tgit fetch files master &&\n+\tgit branch files-master FETCH_HEAD &&\n+\ttest_debug \"echo Add subtree master to master via subtree\" &&\n+\tgit read-tree --prefix=files_subtree files-master &&\n+\tgit checkout -- files_subtree &&\n+\ttree=$(git write-tree) &&\n+\thead=$(git rev-parse HEAD) &&\n+\trev=$(git rev-parse --verify files-master^0) &&\n+\tcommit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n+\tgit reset $commit &&\n+\t(\n+\t\tcd files_subtree &&\n+\t\ttest_commit master4\n+\t) &&\n+\ttest_commit files_subtree/master5\n+'\n+\n+# Does not preserve master4 and master5.\n+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-preserve-merges master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n+\tcheck_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n+'\n+\n+# Does not preserve master4, master5 and empty.\n+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-keep-empty master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n+\tcheck_equal \"$(last_commit_message)\" \"Empty commit\"\n+'\n+\n+# fatal: Could not parse object\n+test_expect_failure 'Rebase -Xsubtree --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-onto master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --onto files-master master &&\n+\tcheck_equal \"$(last_commit_message)\" \"Empty commit\"\n+'\n+\n+test_done\n-- \n2.6.1\n"},{"id":"276117","messageId":"CAPig+cQ6Dfvc4dkQVZ6BqzD76nZ4mCcqkO4eAecrMENKWtgWEg@mail.gmail.com","threadId":"41124","inReplyTo":"1452467297-16868-2-git-send-email-greened@obbligato.org","subject":"Re: [PATCH] Add a test for subtree rebase that loses commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-15T01:19:17Z","receivedAt":"2016-01-15T01:19:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 10, 2016 at 6:08 PM, David Greene <greened@obbligato.org> wrote:\n> From: \"David A. Greene\" <greened@obbligato.org>\n>\n> This test merges an external tree in as a subtree, makes some commits\n> on top of it and splits it back out.  In the process the added commits\n> are lost or the rebase aborts with an internal error.  The tests are\n> marked to expect failure so that we don't forget to fix it.\n\nThis version looks better. A few minor comments below (not necessarily\ndeserving a re-roll)...\n\n> Signed-off-by: David A. Greene <greened@obbligato.org>\n> ---\n> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\n> @@ -0,0 +1,79 @@\n> +#!/bin/sh\n> +\n> +test_description='git rebase tests for -Xsubtree\n> +\n> +This test runs git rebase and tests the subtree strategy.\n> +'\n> +. ./test-lib.sh\n> +. \"$TEST_DIRECTORY\"/lib-rebase.sh\n> +\n> +check_equal() {\n> +       test_debug 'echo'\n> +       test_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n> +       test_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n> +       test \"$1\" = \"$2\"\n> +}\n\nI'm still curious as to why check_equal() is preferred over\ntest-lib-functions.sh:verbose().\n\n> +last_commit_message() {\n> +       git log --pretty=format:%s -1\n> +}\n> +\n> +test_expect_success 'setup' '\n> +       test_commit README &&\n> +       mkdir files &&\n> +       (\n> +               cd files &&\n> +               git init &&\n> +               test_commit master1 &&\n> +               test_commit master2 &&\n> +               test_commit master3\n> +       ) &&\n> +       test_debug \"echo Add project master to master\" &&\n\nAre these test_debug invocations still useful now that the test has\nbeen fully developed?\n\n> +       git fetch files master &&\n> +       git branch files-master FETCH_HEAD &&\n> +       test_debug \"echo Add subtree master to master via subtree\" &&\n> +       git read-tree --prefix=files_subtree files-master &&\n> +       git checkout -- files_subtree &&\n> +       tree=$(git write-tree) &&\n> +       head=$(git rev-parse HEAD) &&\n> +       rev=$(git rev-parse --verify files-master^0) &&\n> +       commit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n> +       git reset $commit &&\n> +       (\n> +               cd files_subtree &&\n> +               test_commit master4\n> +       ) &&\n> +       test_commit files_subtree/master5\n> +'\n> +\n> +# Does not preserve master4 and master5.\n\nThis comment is explaining why the test is marked \"failure\", right?\nWhen someone gets around to fixing the breakage and toggling this to\n\"success\", there is a reasonably good chance that the comment will be\noverlooked and thus become stale. Perhaps prefixing the comment with a\nbold \"FAILURE:\" would serve as a reminder that the comment should be\ndropped when the problem is fixed?\n\n> +test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-preserve-merges master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n> +       check_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n> +'\n> +\n> +# Does not preserve master4, master5 and empty.\n> +test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-keep-empty master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n> +       check_equal \"$(last_commit_message)\" \"Empty commit\"\n> +'\n> +\n> +# fatal: Could not parse object\n> +test_expect_failure 'Rebase -Xsubtree --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-onto master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --onto files-master master &&\n> +       check_equal \"$(last_commit_message)\" \"Empty commit\"\n> +'\n> +\n> +test_done\n> --\n> 2.6.1\n"},{"id":"276256","messageId":"87y4bnaki9.fsf@waller.obbligato.org","threadId":"41124","inReplyTo":"CAPig+cQ6Dfvc4dkQVZ6BqzD76nZ4mCcqkO4eAecrMENKWtgWEg@mail.gmail.com","subject":"Re: [PATCH] Add a test for subtree rebase that loses commits","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-17T22:50:22Z","receivedAt":"2016-01-17T22:50:22Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Sun, Jan 10, 2016 at 6:08 PM, David Greene <greened@obbligato.org> wrote:\n>> From: \"David A. Greene\" <greened@obbligato.org>\n>>\n>> This test merges an external tree in as a subtree, makes some commits\n>> on top of it and splits it back out.  In the process the added commits\n>> are lost or the rebase aborts with an internal error.  The tests are\n>> marked to expect failure so that we don't forget to fix it.\n>\n> This version looks better. A few minor comments below (not necessarily\n> deserving a re-roll)...\n\nI'll re-roll because I think your comments make sense.\n\n>> Signed-off-by: David A. Greene <greened@obbligato.org>\n>> ---\n>> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\n>> @@ -0,0 +1,79 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='git rebase tests for -Xsubtree\n>> +\n>> +This test runs git rebase and tests the subtree strategy.\n>> +'\n>> +. ./test-lib.sh\n>> +. \"$TEST_DIRECTORY\"/lib-rebase.sh\n>> +\n>> +check_equal() {\n>> +       test_debug 'echo'\n>> +       test_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n>> +       test_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n>> +       test \"$1\" = \"$2\"\n>> +}\n>\n> I'm still curious as to why check_equal() is preferred over\n> test-lib-functions.sh:verbose().\n\nI can change it.  Better to use standard tools when available.  I like\nthe output from test_debug when I want to look at it but that's a\nrelatively minor thing.\n\n>> +last_commit_message() {\n>> +       git log --pretty=format:%s -1\n>> +}\n>> +\n>> +test_expect_success 'setup' '\n>> +       test_commit README &&\n>> +       mkdir files &&\n>> +       (\n>> +               cd files &&\n>> +               git init &&\n>> +               test_commit master1 &&\n>> +               test_commit master2 &&\n>> +               test_commit master3\n>> +       ) &&\n>> +       test_debug \"echo Add project master to master\" &&\n>\n> Are these test_debug invocations still useful now that the test has\n> been fully developed?\n\nYeah, I'll remove these.\n\n>> +       git fetch files master &&\n>> +       git branch files-master FETCH_HEAD &&\n>> +       test_debug \"echo Add subtree master to master via subtree\" &&\n>> +       git read-tree --prefix=files_subtree files-master &&\n>> +       git checkout -- files_subtree &&\n>> +       tree=$(git write-tree) &&\n>> +       head=$(git rev-parse HEAD) &&\n>> +       rev=$(git rev-parse --verify files-master^0) &&\n>> +       commit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n>> +       git reset $commit &&\n>> +       (\n>> +               cd files_subtree &&\n>> +               test_commit master4\n>> +       ) &&\n>> +       test_commit files_subtree/master5\n>> +'\n>> +\n>> +# Does not preserve master4 and master5.\n>\n> This comment is explaining why the test is marked \"failure\", right?\n\nRight.\n\n> When someone gets around to fixing the breakage and toggling this to\n> \"success\", there is a reasonably good chance that the comment will be\n> overlooked and thus become stale. Perhaps prefixing the comment with a\n> bold \"FAILURE:\" would serve as a reminder that the comment should be\n> dropped when the problem is fixed?\n\nGood idea.\n\n                        -David\n"},{"id":"276257","messageId":"ec1decfc5fd463f1e78a5aa2636c24fb11e80a62.1453072387.git.greened@obbligato.org","threadId":"41124","inReplyTo":"CAPig+cQ6Dfvc4dkQVZ6BqzD76nZ4mCcqkO4eAecrMENKWtgWEg@mail.gmail.com","subject":"[PATCH v3 contrib/subtree 1/1] Add a test for subtree rebase that loses commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-17T23:13:28Z","receivedAt":"2016-01-17T23:13:28Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: David A. Greene <greened@obbligato.org>\n\nThis test merges an external tree in as a subtree, makes some commits\non top of it and splits it back out.  In the process the added commits\nare lost or the rebase aborts with an internal error.  The tests are\nmarked to expect failure so that we don't forget to fix it.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n\nNotes:\n    Change History:\n    \n    v1 - Initial version\n    v2 - Additional tests and code cleanup\n    v3 - Remove check_equal, mark comments on failure and remove\n         test_debug statements\n\n t/t3427-rebase-subtree.sh | 79 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 79 insertions(+)\n\ndiff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\nnew file mode 100755\nindex 0000000..add3b79\n--- /dev/null\n+++ b/t/t3427-rebase-subtree.sh\n@@ -0,0 +1,79 @@\n+#!/bin/sh\n+\n+test_description='git rebase tests for -Xsubtree\n+\n+This test runs git rebase and tests the subtree strategy.\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n+check_equal() {\n+\ttest_debug 'echo'\n+\ttest_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n+\ttest_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n+\ttest \"$1\" = \"$2\"\n+}\n+\n+last_commit_message() {\n+\tgit log --pretty=format:%s -1\n+}\n+\n+test_expect_success 'setup' '\n+\ttest_commit README &&\n+\tmkdir files &&\n+\t(\n+\t\tcd files &&\n+\t\tgit init &&\n+\t\ttest_commit master1 &&\n+\t\ttest_commit master2 &&\n+\t\ttest_commit master3\n+\t) &&\n+\ttest_debug \"echo Add project master to master\" &&\n+\tgit fetch files master &&\n+\tgit branch files-master FETCH_HEAD &&\n+\ttest_debug \"echo Add subtree master to master via subtree\" &&\n+\tgit read-tree --prefix=files_subtree files-master &&\n+\tgit checkout -- files_subtree &&\n+\ttree=$(git write-tree) &&\n+\thead=$(git rev-parse HEAD) &&\n+\trev=$(git rev-parse --verify files-master^0) &&\n+\tcommit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n+\tgit reset $commit &&\n+\t(\n+\t\tcd files_subtree &&\n+\t\ttest_commit master4\n+\t) &&\n+\ttest_commit files_subtree/master5\n+'\n+\n+# Does not preserve master4 and master5.\n+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-preserve-merges master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n+\tcheck_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n+'\n+\n+# Does not preserve master4, master5 and empty.\n+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-keep-empty master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n+\tcheck_equal \"$(last_commit_message)\" \"Empty commit\"\n+'\n+\n+# fatal: Could not parse object\n+test_expect_failure 'Rebase -Xsubtree --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-onto master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --onto files-master master &&\n+\tcheck_equal \"$(last_commit_message)\" \"Empty commit\"\n+'\n+\n+test_done\n-- \n2.6.1\n"},{"id":"276260","messageId":"CAPig+cTMAnbyT3-FFN4juUooosiydOCX-ETwTghpnKoOeXcMpQ@mail.gmail.com","threadId":"41124","inReplyTo":"ec1decfc5fd463f1e78a5aa2636c24fb11e80a62.1453072387.git.greened@obbligato.org","subject":"Re: [PATCH v3 contrib/subtree 1/1] Add a test for subtree rebase that loses commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-17T23:32:44Z","receivedAt":"2016-01-17T23:32:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 17, 2016 at 6:13 PM, David Greene <greened@obbligato.org> wrote:\n> From: David A. Greene <greened@obbligato.org>\n>\n> This test merges an external tree in as a subtree, makes some commits\n> on top of it and splits it back out.  In the process the added commits\n> are lost or the rebase aborts with an internal error.  The tests are\n> marked to expect failure so that we don't forget to fix it.\n>\n> Signed-off-by: David A. Greene <greened@obbligato.org>\n> ---\n>\n> Notes:\n>     Change History:\n>\n>     v1 - Initial version\n>     v2 - Additional tests and code cleanup\n>     v3 - Remove check_equal, mark comments on failure and remove\n>          test_debug statements\n\nHmm, the v3 changes described here don't appear in this version. In\nfact, v2 and v3 are identical.\n\n>  t/t3427-rebase-subtree.sh | 79 +++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 79 insertions(+)\n>\n> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\n> new file mode 100755\n> index 0000000..add3b79\n> --- /dev/null\n> +++ b/t/t3427-rebase-subtree.sh\n> @@ -0,0 +1,79 @@\n> +#!/bin/sh\n> +\n> +test_description='git rebase tests for -Xsubtree\n> +\n> +This test runs git rebase and tests the subtree strategy.\n> +'\n> +. ./test-lib.sh\n> +. \"$TEST_DIRECTORY\"/lib-rebase.sh\n> +\n> +check_equal() {\n> +       test_debug 'echo'\n> +       test_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n> +       test_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n> +       test \"$1\" = \"$2\"\n> +}\n> +\n> +last_commit_message() {\n> +       git log --pretty=format:%s -1\n> +}\n> +\n> +test_expect_success 'setup' '\n> +       test_commit README &&\n> +       mkdir files &&\n> +       (\n> +               cd files &&\n> +               git init &&\n> +               test_commit master1 &&\n> +               test_commit master2 &&\n> +               test_commit master3\n> +       ) &&\n> +       test_debug \"echo Add project master to master\" &&\n> +       git fetch files master &&\n> +       git branch files-master FETCH_HEAD &&\n> +       test_debug \"echo Add subtree master to master via subtree\" &&\n> +       git read-tree --prefix=files_subtree files-master &&\n> +       git checkout -- files_subtree &&\n> +       tree=$(git write-tree) &&\n> +       head=$(git rev-parse HEAD) &&\n> +       rev=$(git rev-parse --verify files-master^0) &&\n> +       commit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n> +       git reset $commit &&\n> +       (\n> +               cd files_subtree &&\n> +               test_commit master4\n> +       ) &&\n> +       test_commit files_subtree/master5\n> +'\n> +\n> +# Does not preserve master4 and master5.\n> +test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-preserve-merges master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n> +       check_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n> +'\n> +\n> +# Does not preserve master4, master5 and empty.\n> +test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-keep-empty master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n> +       check_equal \"$(last_commit_message)\" \"Empty commit\"\n> +'\n> +\n> +# fatal: Could not parse object\n> +test_expect_failure 'Rebase -Xsubtree --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-onto master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --onto files-master master &&\n> +       check_equal \"$(last_commit_message)\" \"Empty commit\"\n> +'\n> +\n> +test_done\n> --\n> 2.6.1\n"},{"id":"276261","messageId":"87h9ibaido.fsf@waller.obbligato.org","threadId":"41124","inReplyTo":"CAPig+cTMAnbyT3-FFN4juUooosiydOCX-ETwTghpnKoOeXcMpQ@mail.gmail.com","subject":"Re: [PATCH v3 contrib/subtree 1/1] Add a test for subtree rebase that loses commits","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-17T23:36:19Z","receivedAt":"2016-01-17T23:36:19Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Sun, Jan 17, 2016 at 6:13 PM, David Greene <greened@obbligato.org> wrote:\n>> From: David A. Greene <greened@obbligato.org>\n>>\n>> This test merges an external tree in as a subtree, makes some commits\n>> on top of it and splits it back out.  In the process the added commits\n>> are lost or the rebase aborts with an internal error.  The tests are\n>> marked to expect failure so that we don't forget to fix it.\n>>\n>> Signed-off-by: David A. Greene <greened@obbligato.org>\n>> ---\n>>\n>> Notes:\n>>     Change History:\n>>\n>>     v1 - Initial version\n>>     v2 - Additional tests and code cleanup\n>>     v3 - Remove check_equal, mark comments on failure and remove\n>>          test_debug statements\n>\n> Hmm, the v3 changes described here don't appear in this version. In\n> fact, v2 and v3 are identical.\n\nDang, you caught me before I could reply.  :)\n\nYes, I botched this one.  Sending v4... ;)\n\n                          -David\n"},{"id":"276262","messageId":"047e625a28954b8fd79225b55cab7620cb5f3b1f.1453074191.git.greened@obbligato.org","threadId":"41124","inReplyTo":"CAPig+cTMAnbyT3-FFN4juUooosiydOCX-ETwTghpnKoOeXcMpQ@mail.gmail.com","subject":"[PATCH v4 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-17T23:43:21Z","receivedAt":"2016-01-17T23:43:21Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: David A. Greene <greened@obbligato.org>\n\nThis test merges an external tree in as a subtree, makes some commits\non top of it and splits it back out.  In the process the added commits\nare lost or the rebase aborts with an internal error.  The tests are\nmarked to expect failure so that we don't forget to fix it.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n\nNotes:\n    Change History:\n    \n    v1 - Initial version\n    v2 - Additional tests and code cleanup\n    v3 - Remove check_equal, mark comments on failure and remove\n         test_debug statements\n    v4 - Send correct v3 test (botched v3)\n\n t/t3427-rebase-subtree.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 70 insertions(+)\n\ndiff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\nnew file mode 100755\nindex 0000000..4d47f77\n--- /dev/null\n+++ b/t/t3427-rebase-subtree.sh\n@@ -0,0 +1,70 @@\n+#!/bin/sh\n+\n+test_description='git rebase tests for -Xsubtree\n+\n+This test runs git rebase and tests the subtree strategy.\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n+last_commit_message() {\n+\tgit log --pretty=format:%s -1\n+}\n+\n+test_expect_success 'setup' '\n+\ttest_commit README &&\n+\tmkdir files &&\n+\t(\n+\t\tcd files &&\n+\t\tgit init &&\n+\t\ttest_commit master1 &&\n+\t\ttest_commit master2 &&\n+\t\ttest_commit master3\n+\t) &&\n+\tgit fetch files master &&\n+\tgit branch files-master FETCH_HEAD &&\n+\tgit read-tree --prefix=files_subtree files-master &&\n+\tgit checkout -- files_subtree &&\n+\ttree=$(git write-tree) &&\n+\thead=$(git rev-parse HEAD) &&\n+\trev=$(git rev-parse --verify files-master^0) &&\n+\tcommit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n+\tgit reset $commit &&\n+\t(\n+\t\tcd files_subtree &&\n+\t\ttest_commit master4\n+\t) &&\n+\ttest_commit files_subtree/master5\n+'\n+\n+# FAILURE: Does not preserve master4 and master5.\n+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-preserve-merges master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n+\tverbose \"$(last_commit_message)\" \"files_subtree/master5\"\n+'\n+\n+# FAILURE: Does not preserve master4, master5 and empty.\n+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-keep-empty master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n+\tverbose \"$(last_commit_message)\" \"Empty commit\"\n+'\n+\n+# FAILURE: fatal: Could not parse object\n+test_expect_failure 'Rebase -Xsubtree --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-onto master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --onto files-master master &&\n+\tverbose \"$(last_commit_message)\" \"Empty commit\"\n+'\n+\n+test_done\n-- \n2.6.1\n"},{"id":"276287","messageId":"CAPig+cS6ouc+kdJaz10M2AApPoDODDcgDX9Azz8ih=4zxxD2zg@mail.gmail.com","threadId":"41124","inReplyTo":"047e625a28954b8fd79225b55cab7620cb5f3b1f.1453074191.git.greened@obbligato.org","subject":"Re: [PATCH v4 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-18T18:10:36Z","receivedAt":"2016-01-18T18:10:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 17, 2016 at 6:43 PM, David Greene <greened@obbligato.org> wrote:\n> This test merges an external tree in as a subtree, makes some commits\n> on top of it and splits it back out.  In the process the added commits\n> are lost or the rebase aborts with an internal error.  The tests are\n> marked to expect failure so that we don't forget to fix it.\n>\n> Signed-off-by: David A. Greene <greened@obbligato.org>\n> ---\n> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\n> @@ -0,0 +1,70 @@\n> +# FAILURE: Does not preserve master4 and master5.\n> +test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-preserve-merges master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n> +       verbose \"$(last_commit_message)\" \"files_subtree/master5\"\n\nHmm, does this test succeed? If it does, it's only by accident.\nverbose() is just a helper for printing the the expression being\ntested upon failure, but you still need to supply a proper expression\nfor testing. It is intended to be used like this:\n\n    verbose test \"$(last_commit_message)\" = files_subtree/master5\n\nSame comment applies to the remaining tests.\n\n> +'\n> +\n> +# FAILURE: Does not preserve master4, master5 and empty.\n> +test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-keep-empty master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n> +       verbose \"$(last_commit_message)\" \"Empty commit\"\n> +'\n> +\n> +# FAILURE: fatal: Could not parse object\n> +test_expect_failure 'Rebase -Xsubtree --onto' '\n> +       reset_rebase &&\n> +       git checkout -b rebase-onto master &&\n> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> +       git commit -m \"Empty commit\" --allow-empty &&\n> +       git rebase -Xsubtree=files_subtree --onto files-master master &&\n> +       verbose \"$(last_commit_message)\" \"Empty commit\"\n> +'\n> +\n> +test_done\n> --\n> 2.6.1\n"},{"id":"276328","messageId":"87wpr6l1os.fsf@waller.obbligato.org","threadId":"41124","inReplyTo":"CAPig+cS6ouc+kdJaz10M2AApPoDODDcgDX9Azz8ih=4zxxD2zg@mail.gmail.com","subject":"Re: [PATCH v4 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-19T02:53:39Z","receivedAt":"2016-01-19T02:53:39Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Sun, Jan 17, 2016 at 6:43 PM, David Greene <greened@obbligato.org> wrote:\n>> This test merges an external tree in as a subtree, makes some commits\n>> on top of it and splits it back out.  In the process the added commits\n>> are lost or the rebase aborts with an internal error.  The tests are\n>> marked to expect failure so that we don't forget to fix it.\n>>\n>> Signed-off-by: David A. Greene <greened@obbligato.org>\n>> ---\n>> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\n>> @@ -0,0 +1,70 @@\n>> +# FAILURE: Does not preserve master4 and master5.\n>> +test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto' '\n>> +       reset_rebase &&\n>> +       git checkout -b rebase-preserve-merges master &&\n>> +       git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n>> +       git commit -m \"Empty commit\" --allow-empty &&\n>> +       git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n>> +       verbose \"$(last_commit_message)\" \"files_subtree/master5\"\n>\n> Hmm, does this test succeed? If it does, it's only by accident.\n> verbose() is just a helper for printing the the expression being\n> tested upon failure, but you still need to supply a proper expression\n> for testing. It is intended to be used like this:\n>\n>     verbose test \"$(last_commit_message)\" = files_subtree/master5\n>\n> Same comment applies to the remaining tests.\n\nBlast.  Yes, it did pass (expect failure) but it's definitely wrong.  On\nto v5!\n\n                           -David\n"},{"id":"276329","messageId":"3eb25268597083cdb10303e3d5790302e719a803.1453172369.git.greened@obbligato.org","threadId":"41124","inReplyTo":"CAPig+cS6ouc+kdJaz10M2AApPoDODDcgDX9Azz8ih=4zxxD2zg@mail.gmail.com","subject":"[PATCH v5 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-19T02:59:38Z","receivedAt":"2016-01-19T02:59:38Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: David A. Greene <greened@obbligato.org>\n\nThis test merges an external tree in as a subtree, makes some commits\non top of it and splits it back out.  In the process the added commits\nare lost or the rebase aborts with an internal error.  The tests are\nmarked to expect failure so that we don't forget to fix it.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n\nNotes:\n    Change History:\n    \n    v1 - Initial version\n    v2 - Additional tests and code cleanup\n    v3 - Remove check_equal, mark comments on failure and remove\n         test_debug statements\n    v4 - Send correct v3 test (botched v3)\n    v5 - Fix use of verbose\n\n t/t3427-rebase-subtree.sh | 70 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 70 insertions(+)\n\ndiff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\nnew file mode 100755\nindex 0000000..a68a1b1\n--- /dev/null\n+++ b/t/t3427-rebase-subtree.sh\n@@ -0,0 +1,70 @@\n+#!/bin/sh\n+\n+test_description='git rebase tests for -Xsubtree\n+\n+This test runs git rebase and tests the subtree strategy.\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n+last_commit_message() {\n+\tgit log --pretty=format:%s -1\n+}\n+\n+test_expect_success 'setup' '\n+\ttest_commit README &&\n+\tmkdir files &&\n+\t(\n+\t\tcd files &&\n+\t\tgit init &&\n+\t\ttest_commit master1 &&\n+\t\ttest_commit master2 &&\n+\t\ttest_commit master3\n+\t) &&\n+\tgit fetch files master &&\n+\tgit branch files-master FETCH_HEAD &&\n+\tgit read-tree --prefix=files_subtree files-master &&\n+\tgit checkout -- files_subtree &&\n+\ttree=$(git write-tree) &&\n+\thead=$(git rev-parse HEAD) &&\n+\trev=$(git rev-parse --verify files-master^0) &&\n+\tcommit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n+\tgit reset $commit &&\n+\t(\n+\t\tcd files_subtree &&\n+\t\ttest_commit master4\n+\t) &&\n+\ttest_commit files_subtree/master5\n+'\n+\n+# FAILURE: Does not preserve master4 and master5.\n+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-preserve-merges master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n+\tverbose test \"$(last_commit_message)\" = \"files_subtree/master5\"\n+'\n+\n+# FAILURE: Does not preserve master4, master5 and empty.\n+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-keep-empty master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n+\tverbose test \"$(last_commit_message)\" = \"Empty commit\"\n+'\n+\n+# FAILURE: fatal: Could not parse object\n+test_expect_failure 'Rebase -Xsubtree --onto' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-onto master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --onto files-master master &&\n+\tverbose test \"$(last_commit_message)\" = \"Empty commit\"\n+'\n+\n+test_done\n-- \n2.6.1\n"},{"id":"276332","messageId":"CAPig+cQyn-TGocV8Z6UTCJtBZMT8-HCtV7HqqJ+yinaczGUPvg@mail.gmail.com","threadId":"41124","inReplyTo":"3eb25268597083cdb10303e3d5790302e719a803.1453172369.git.greened@obbligato.org","subject":"Re: [PATCH v5 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-19T04:21:59Z","receivedAt":"2016-01-19T04:21:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 18, 2016 at 9:59 PM, David Greene <greened@obbligato.org> wrote:\n> This test merges an external tree in as a subtree, makes some commits\n> on top of it and splits it back out.  In the process the added commits\n> are lost or the rebase aborts with an internal error.  The tests are\n> marked to expect failure so that we don't forget to fix it.\n>\n> Signed-off-by: David A. Greene <greened@obbligato.org>\n> ---\n> Notes:\n>     Change History:\n>\n>     v1 - Initial version\n>     v2 - Additional tests and code cleanup\n>     v3 - Remove check_equal, mark comments on failure and remove\n>          test_debug statements\n>     v4 - Send correct v3 test (botched v3)\n>     v5 - Fix use of verbose\n\nThanks, I think this re-roll addresses all the (relatively\nsuperficial) issues raised by my reviews of previous versions.\n"},{"id":"276347","messageId":"xmqqsi1tbh68.fsf@gitster.mtv.corp.google.com","threadId":"41124","inReplyTo":"3eb25268597083cdb10303e3d5790302e719a803.1453172369.git.greened@obbligato.org","subject":"Re: [PATCH v5 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-19T17:41:35Z","receivedAt":"2016-01-19T17:41:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Greene <greened@obbligato.org> writes:\n\n> From: David A. Greene <greened@obbligato.org>\n>\n> This test merges an external tree in as a subtree, makes some commits\n> on top of it and splits it back out.  In the process the added commits\n> are lost or the rebase aborts with an internal error.  The tests are\n> marked to expect failure so that we don't forget to fix it.\n>\n> Signed-off-by: David A. Greene <greened@obbligato.org>\n> ---\n>\n> Notes:\n>     Change History:\n>     \n>     v1 - Initial version\n>     v2 - Additional tests and code cleanup\n>     v3 - Remove check_equal, mark comments on failure and remove\n>          test_debug statements\n>     v4 - Send correct v3 test (botched v3)\n>     v5 - Fix use of verbose\n\nThanks, both.  Will queue.\n\nI have a couple of questions and comments, though.\n\n> +test_expect_success 'setup' '\n> +\ttest_commit README &&\n> +...\n> +\ttree=$(git write-tree) &&\n> +\thead=$(git rev-parse HEAD) &&\n> +\trev=$(git rev-parse --verify files-master^0) &&\n> +\tcommit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n\nI think at this point, your index and working tree states match that\nof $commit.  So the next command ...\n\n> +\tgit reset $commit &&\n\n... made me wonder what its significance was.  I think you are doing\nthis solely to move the HEAD pointer to point at $commit, but then\nit would be much better and more readable to use update-ref, i.e.\nmaking this line to:\n\n\tgit update-ref HEAD $commit &&\n\ninstead, as \"write-tree && commit-tree && update-ref\" is a familiar\npattern to reimplement \"git commit\" using the plumbing.  Ending that\nthree-command sequence with \"reset\" breaks the pattern.\n\n> +\t(\n> +\t\tcd files_subtree &&\n> +\t\ttest_commit master4\n> +\t) &&\n> +\ttest_commit files_subtree/master5\n\nI understand that you are creating these two commits both in the\ntop-level repository (the one the history initially created in\n\"files\" repository gets merged into), but you are creating them\nslightly differently.  Is that significant?  I am not complaining\nabout the style of writing tests, but I am wondering if having these\ntwo commits created differently has any effect on the bug you\nobserved, which may be a good starting point for anybody who wants\nto fix it to start digging from.  IOW, would the resulting history\ndifferent if you did this instead?\n\n\ttest_commit files_subtree/master4 &&\n\ttest_commit files_subtree/master5\n\nI also notice that files_subtree/master4 does not appear in any of\nthe verification in the three tests that use the history being\nprepared here, i.e. if master4 is silently dropped while master5 is\nkept, such a bug won't be caught by them.\n"},{"id":"276412","messageId":"87wpr4ubzo.fsf@waller.obbligato.org","threadId":"41124","inReplyTo":"xmqqsi1tbh68.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v5 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-20T04:10:51Z","receivedAt":"2016-01-20T04:10:51Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Thanks, both.  Will queue.\n>\n> I have a couple of questions and comments, though.\n>\n>> +test_expect_success 'setup' '\n>> +\ttest_commit README &&\n>> +...\n>> +\ttree=$(git write-tree) &&\n>> +\thead=$(git rev-parse HEAD) &&\n>> +\trev=$(git rev-parse --verify files-master^0) &&\n>> +\tcommit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n>\n> I think at this point, your index and working tree states match that\n> of $commit.  So the next command ...\n>\n>> +\tgit reset $commit &&\n>\n> ... made me wonder what its significance was.  I think you are doing\n> this solely to move the HEAD pointer to point at $commit, but then\n> it would be much better and more readable to use update-ref, i.e.\n> making this line to:\n>\n> \tgit update-ref HEAD $commit &&\n>\n> instead, as \"write-tree && commit-tree && update-ref\" is a familiar\n> pattern to reimplement \"git commit\" using the plumbing.  Ending that\n> three-command sequence with \"reset\" breaks the pattern.\n\nOk, that makes sense.\n\n>> +\t(\n>> +\t\tcd files_subtree &&\n>> +\t\ttest_commit master4\n>> +\t) &&\n>> +\ttest_commit files_subtree/master5\n>\n> I understand that you are creating these two commits both in the\n> top-level repository (the one the history initially created in\n> \"files\" repository gets merged into), but you are creating them\n> slightly differently.  Is that significant?  I am not complaining\n> about the style of writing tests, but I am wondering if having these\n> two commits created differently has any effect on the bug you\n> observed, which may be a good starting point for anybody who wants\n> to fix it to start digging from.  IOW, would the resulting history\n> different if you did this instead?\n>\n> \ttest_commit files_subtree/master4 &&\n> \ttest_commit files_subtree/master5\n\nThat is a good question.  I originally created the test to see if making\nthese two commits differently would cause any problems with\ngit-subtree's split command.  Obviously, I didn't get that far.  :)\n\nSo I think it makes sense for me to at least test and see what happens.\nI will add another test to this set if it makes a difference.\n\n> I also notice that files_subtree/master4 does not appear in any of\n> the verification in the three tests that use the history being\n> prepared here, i.e. if master4 is silently dropped while master5 is\n> kept, such a bug won't be caught by them.\n\nAh, good catch.  I should add a test for that.\n\nLet me do a re-roll of this since I think you bring up some excellent\npoints.  Might be a few days due to work obbligations.\n\n                        -David\n"},{"id":"283249","messageId":"xmqqpotummqn.fsf@gitster.mtv.corp.google.com","threadId":"41124","inReplyTo":"87wpr4ubzo.fsf@waller.obbligato.org","subject":"Re: [PATCH v5 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-12T23:27:28Z","receivedAt":"2016-04-12T23:27:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"greened@obbligato.org (David A. Greene) writes:\n\n>> I also notice that files_subtree/master4 does not appear in any of\n>> the verification in the three tests that use the history being\n>> prepared here, i.e. if master4 is silently dropped while master5 is\n>> kept, such a bug won't be caught by them.\n>\n> Ah, good catch.  I should add a test for that.\n>\n> Let me do a re-roll of this since I think you bring up some excellent\n> points.  Might be a few days due to work obbligations.\n\nA friendly ping to see if I missed anything that happened after this\nmessage...\n"},{"id":"290363","messageId":"834520c69d67b6dbe804ff67b4be00a9ac17d556.1467111148.git.greened@obbligato.org","threadId":"41124","inReplyTo":"xmqqsi1tbh68.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v6 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-06-28T10:54:02Z","receivedAt":"2016-06-28T11:29:46Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: David A. Greene <greened@obbligato.org>\n\nThis test merges an external tree in as a subtree, makes some commits\non top of it and splits it back out.  In the process the added commits\nare lost or the rebase aborts with an internal error.  The tests are\nmarked to expect failure so that we don't forget to fix it.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n\nNotes:\n    Change History:\n    \n    v1 - Initial version\n    v2 - Additional tests and code cleanup\n    v3 - Remove check_equal, mark comments on failure and remove\n         test_debug statements\n    v4 - Send correct v3 test (botched v3)\n    v5 - Fix use of verbose\n    v6 - Add individual tests for each potentially dropped commit\n\n t/t3427-rebase-subtree.sh | 119 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 119 insertions(+)\n\ndiff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\nnew file mode 100755\nindex 0000000..3780877\n--- /dev/null\n+++ b/t/t3427-rebase-subtree.sh\n@@ -0,0 +1,119 @@\n+#!/bin/sh\n+\n+test_description='git rebase tests for -Xsubtree\n+\n+This test runs git rebase and tests the subtree strategy.\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n+commit_message() {\n+\tgit log --pretty=format:%s -1 \"$1\"\n+}\n+\n+test_expect_success 'setup' '\n+\ttest_commit README &&\n+\tmkdir files &&\n+\t(\n+\t\tcd files &&\n+\t\tgit init &&\n+\t\ttest_commit master1 &&\n+\t\ttest_commit master2 &&\n+\t\ttest_commit master3\n+\t) &&\n+\tgit fetch files master &&\n+\tgit branch files-master FETCH_HEAD &&\n+\tgit read-tree --prefix=files_subtree files-master &&\n+\tgit checkout -- files_subtree &&\n+\ttree=$(git write-tree) &&\n+\thead=$(git rev-parse HEAD) &&\n+\trev=$(git rev-parse --verify files-master^0) &&\n+\tcommit=$(git commit-tree -p $head -p $rev -m \"Add subproject master\" $tree) &&\n+\tgit update-ref HEAD $commit &&\n+\t(\n+\t\tcd files_subtree &&\n+\t\ttest_commit master4\n+\t) &&\n+\ttest_commit files_subtree/master5\n+'\n+\n+# FAILURE: Does not preserve master4.\n+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-preserve-merges-4 master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n+\tverbose test \"$(commit_message HEAD~)\" = \"files_subtree/master4\"\n+'\n+\n+# FAILURE: Does not preserve master5.\n+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-preserve-merges-5 master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&\n+\tverbose test \"$(commit_message HEAD)\" = \"files_subtree/master5\"\n+'\n+\n+# FAILURE: Does not preserve master4.\n+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-keep-empty-4 master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n+\tverbose test \"$(commit_message HEAD~2)\" = \"files_subtree/master4\"\n+'\n+\n+# FAILURE: Does not preserve master5.\n+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-keep-empty-5 master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n+\tverbose test \"$(commit_message HEAD~)\" = \"files_subtree/master5\"\n+'\n+\n+# FAILURE: Does not preserve Empty.\n+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-keep-empty-empty master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n+\tverbose test \"$(commit_message HEAD)\" = \"Empty commit\"\n+'\n+\n+# FAILURE: fatal: Could not parse object\n+test_expect_failure 'Rebase -Xsubtree --onto commit 4' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-onto-4 master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --onto files-master master &&\n+\tverbose test \"$(commit_message HEAD~2)\" = \"files_subtree/master4\"\n+'\n+\n+# FAILURE: fatal: Could not parse object\n+test_expect_failure 'Rebase -Xsubtree --onto commit 5' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-onto-5 master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --onto files-master master &&\n+\tverbose test \"$(commit_message HEAD~)\" = \"files_subtree/master5\"\n+'\n+# FAILURE: fatal: Could not parse object\n+test_expect_failure 'Rebase -Xsubtree --onto empty commit' '\n+\treset_rebase &&\n+\tgit checkout -b rebase-onto-empty master &&\n+\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n+\tgit commit -m \"Empty commit\" --allow-empty &&\n+\tgit rebase -Xsubtree=files_subtree --onto files-master master &&\n+\tverbose test \"$(commit_message HEAD)\" = \"Empty commit\"\n+'\n+\n+test_done\n-- \n2.8.1\n\n"},{"id":"290364","messageId":"871t3heg65.fsf@waller.obbligato.org","threadId":"41124","inReplyTo":"xmqqpotummqn.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v5 1/1] contrib/subtree: Add a test for subtree rebase that loses commits","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-06-28T10:55:30Z","receivedAt":"2016-06-28T11:30:12Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> greened@obbligato.org (David A. Greene) writes:\n>\n>>> I also notice that files_subtree/master4 does not appear in any of\n>>> the verification in the three tests that use the history being\n>>> prepared here, i.e. if master4 is silently dropped while master5 is\n>>> kept, such a bug won't be caught by them.\n>>\n>> Ah, good catch.  I should add a test for that.\n>>\n>> Let me do a re-roll of this since I think you bring up some excellent\n>> points.  Might be a few days due to work obbligations.\n>\n> A friendly ping to see if I missed anything that happened after this\n> message...\n\nJust sent it.  I guess it took more than a few days... :)\n\n                   -David\n"}]}