{"thread":{"id":"49899","subject":"[PATCH 0/1] Fix git rebase --stat -i ","startedAt":"2018-11-27T11:13:21Z","lastAt":"2018-11-30T01:40:55Z","messageCount":7,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"364125","messageId":"pull.88.git.gitgitgadget@gmail.com","threadId":"49899","inReplyTo":null,"subject":"[PATCH 0/1] Fix git rebase --stat -i ","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-27T11:13:17Z","receivedAt":"2018-11-27T11:13:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"We're really entering obscure territory here, I would say.\n\nTo trigger the bug, two things have to come together: the user must have\nasked for a diffstat afterwards, and the commits need to have been rebased\nonto a completely unrelated commit history (i.e. there must exist no merge\nbase between the pre-rebase HEAD and the post-rebase HEAD).\n\nPlease note that this bug existed already in the scripted rebase, but it was\nnever detected because the scripted version would not even perform any error\nchecking.\n\nIt will make Junio very happy that I squashed the regression test in to the\npatch that fixes it. The reason, however, was not to make Junio happy (I\nhope to make him happy by fixing bugs), but simply that an earlier iteration\nof test would only fail with the built-in rebase, but not with the scripted\nversion. The current version would fail with the scripted version, but I'll\nsave the time to split the patch again now.\n\nJohannes Schindelin (1):\n rebase --stat: fix when rebasing to an unrelated history\n\n builtin/rebase.c | 8 +++++---\n git-legacy-rebase.sh | 6 ++++--\n t/t3406-rebase-message.sh | 10 ++++++++++\n 3 files changed, 19 insertions(+), 5 deletions(-)\n\n\nbase-commit: a1598010f775d82b5adf12c29d0f5bc9b41434c6\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-88%2Fdscho%2Frebase-stat-fix-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-88/dscho/rebase-stat-fix-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/88\n-- \ngitgitgadget\n"},{"id":"364126","messageId":"680385e4bd5c34a5fcf9651a776c52db61557652.1543317195.git.gitgitgadget@gmail.com","threadId":"49899","inReplyTo":"pull.88.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] rebase --stat: fix when rebasing to an unrelated history","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-27T11:13:19Z","receivedAt":"2018-11-27T11:13:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen rebasing to a commit history that has no common commits with the\ncurrent branch, there is no merge base. The scripted version of the `git\nrebase` command was not prepared for that and spewed out\n\n\tfatal: ambiguous argument '': unknown revision or path not in\n\tthe working tree.\n\nbut then continued (due to lack of error checking).\n\nThe built-in version of the `git rebase` command blindly translated that\nshell script code, assuming that there is no need to test whether there\n*was* a merge base, and due to its better error checking, exited with a\nfatal error (because it tried to read the object with hash 00000000...\nas a tree).\n\nFix both scripted and built-in versions to output something sensibly,\nand add a regression test to keep this working in all eternity.\n\nReported-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Johannes Schindelin \n---\n builtin/rebase.c | 8 +++++---\n git-legacy-rebase.sh | 6 ++++--\n t/t3406-rebase-message.sh | 10 ++++++++++\n 3 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 5b3e5baec8..9e4b0b564f 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1483,7 +1483,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \n \t\tif (options.flags & REBASE_VERBOSE)\n \t\t\tprintf(_(\"Changes from %s to %s:\\n\"),\n-\t\t\t\toid_to_hex(&merge_base),\n+\t\t\t\tis_null_oid(&merge_base) ?\n+\t\t\t\t\"(empty)\" : oid_to_hex(&merge_base),\n \t\t\t\toid_to_hex(&options.onto->object.oid));\n \n \t\t/* We want color (if set), but no pager */\n@@ -1494,8 +1495,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\tDIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;\n \t\topts.detect_rename = DIFF_DETECT_RENAME;\n \t\tdiff_setup_done(&opts);\n-\t\tdiff_tree_oid(&merge_base, &options.onto->object.oid,\n-\t\t\t \"\", &opts);\n+\t\tdiff_tree_oid(is_null_oid(&merge_base) ?\n+\t\t\t the_hash_algo->empty_tree : &merge_base,\n+\t\t\t &options.onto->object.oid, \"\", &opts);\n \t\tdiffcore_std(&opts);\n \t\tdiff_flush(&opts);\n \t}\ndiff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh\nindex b97ffdc9dd..be3b241676 100755\n--- a/git-legacy-rebase.sh\n+++ b/git-legacy-rebase.sh\n@@ -718,10 +718,12 @@ if test -n \"$diffstat\"\n then\n \tif test -n \"$verbose\"\n \tthen\n-\t\techo \"$(eval_gettext \"Changes from \\$mb to \\$onto:\")\"\n+\t\tmb_display=\"${mb:-(empty)}\"\n+\t\techo \"$(eval_gettext \"Changes from \\$mb_display to \\$onto:\")\"\n \tfi\n+\tmb_tree=\"${mb:-$(git hash-object -t tree /dev/null)}\"\n \t# We want color (if set), but no pager\n-\tGIT_PAGER='' git diff --stat --summary \"$mb\" \"$onto\"\n+\tGIT_PAGER='' git diff --stat --summary \"$mb_tree\" \"$onto\"\n fi\n \n test -n \"$interactive_rebase\" && run_specific_rebase\ndiff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh\nindex 38bd876cab..a1ee912118 100755\n--- a/t/t3406-rebase-message.sh\n+++ b/t/t3406-rebase-message.sh\n@@ -91,4 +91,14 @@ test_expect_success 'error out early upon -C or --whitespace=' '\n \ttest_i18ngrep \"Invalid whitespace option\" err\n '\n \n+test_expect_success 'rebase -i onto unrelated history' '\n+\tgit init unrelated &&\n+\ttest_commit -C unrelated 1 &&\n+\tgit -C unrelated remote add -f origin \"$PWD\" &&\n+\tgit -C unrelated branch --set-upstream-to=origin/master &&\n+\tgit -C unrelated -c core.editor=true rebase -i -v --stat >actual &&\n+\ttest_i18ngrep \"Changes from (empty)\" actual &&\n+\ttest_i18ngrep \"5 files changed\" actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"364286","messageId":"xmqqlg5cph1n.fsf@gitster-ct.c.googlers.com","threadId":"49899","inReplyTo":"680385e4bd5c34a5fcf9651a776c52db61557652.1543317195.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] rebase --stat: fix when rebasing to an unrelated history","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-29T05:51:48Z","receivedAt":"2018-11-29T05:51:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> The built-in version of the `git rebase` command blindly translated that\n> shell script code, assuming that there is no need to test whether there\n> *was* a merge base, and due to its better error checking, exited with a\n> fatal error (because it tried to read the object with hash 00000000...\n> as a tree).\n\nAnd the scripted version produced a wrong result because it did not\ncheck the lack of merge base and fed an empty string (presumably, as\nthat is what you would get from mb=$(merge-base A B) when A and B\nare unrelated) to later steps? If that is the case, then it *is* a\nvery significant thing to record here. As it was not merely \"failed\nto stop due to lack of error checking\", but a lot worse. It was\nproducing a wrong result.\n\nA more faithful reimplementation would not have exited with fatal\nerror, but would have produced the same wrong result, so \"a better\nerror checking caused the reimplementation die where the original\ndid not die\" may be correct but is much less important than the fact\nthat \"the logic used in the original to produce diffstat forgot that\nthere may not be a merge base and would not have worked correctly in\nsuch a case, and that is what we are correcting\" is more important.\n\nPlease descibe the issue as such, if that is the case (I cannot read\nfrom the description in the proposed log message if that is the\ncase---but I came to the conclusion that it is what you wanted to\nfix reading the code).\n\n> \t\tif (options.flags & REBASE_VERBOSE)\n> \t\t\tprintf(_(\"Changes from %s to %s:\\n\"),\n> -\t\t\t\toid_to_hex(&merge_base),\n> +\t\t\t\tis_null_oid(&merge_base) ?\n> +\t\t\t\t\"(empty)\" : oid_to_hex(&merge_base),\n\nI am not sure (empty) is a good idea for two reasons. It is going\nto be inserted into an translated string without translation. Also\nit is too similar to the fallback string used by some printf\nimplementations when NULL was given to %s by mistake.\n\nIf there weren't i18n issues, I would suggest to use \"empty merge\nbase\" or \"empty tree\" instead, but the old one would have shown\n0{40}, so perhaps empty_tree_oid_hex() is a good substitute?\n\n> @@ -1494,8 +1495,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t\t\tDIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;\n> \t\topts.detect_rename = DIFF_DETECT_RENAME;\n> \t\tdiff_setup_done(&opts);\n> -\t\tdiff_tree_oid(&merge_base, &options.onto->object.oid,\n> -\t\t\t \"\", &opts);\n> +\t\tdiff_tree_oid(is_null_oid(&merge_base) ?\n> +\t\t\t the_hash_algo->empty_tree : &merge_base,\n> +\t\t\t &options.onto->object.oid, \"\", &opts);\n\nThe original would have run \"git diff '' $onto\", and died with an\nerror message, so both the original and the reimplementation were\nfailing. Just in different ways ;-)\n\n> diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh\n> index b97ffdc9dd..be3b241676 100755\n> --- a/git-legacy-rebase.sh\n> +++ b/git-legacy-rebase.sh\n> @@ -718,10 +718,12 @@ if test -n \"$diffstat\"\n> then\n> \tif test -n \"$verbose\"\n> \tthen\n> -\t\techo \"$(eval_gettext \"Changes from \\$mb to \\$onto:\")\"\n> +\t\tmb_display=\"${mb:-(empty)}\"\n> +\t\techo \"$(eval_gettext \"Changes from \\$mb_display to \\$onto:\")\"\n> \tfi\n> +\tmb_tree=\"${mb:-$(git hash-object -t tree /dev/null)}\"\n> \t# We want color (if set), but no pager\n> -\tGIT_PAGER='' git diff --stat --summary \"$mb\" \"$onto\"\n> +\tGIT_PAGER='' git diff --stat --summary \"$mb_tree\" \"$onto\"\n\nCode fix for diff-tree invocation, both in the builtin/ version and\nthis one, looks correct.\n"},{"id":"364308","messageId":"nycvar.QRO.7.76.6.1811291318090.41@tvgsbejvaqbjf.bet","threadId":"49899","inReplyTo":"xmqqlg5cph1n.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] rebase --stat: fix when rebasing to an unrelated history","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-29T12:51:38Z","receivedAt":"2018-11-29T12:51:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 29 Nov 2018, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > The built-in version of the `git rebase` command blindly translated that\n> > shell script code, assuming that there is no need to test whether there\n> > *was* a merge base, and due to its better error checking, exited with a\n> > fatal error (because it tried to read the object with hash 00000000...\n> > as a tree).\n> \n> And the scripted version produced a wrong result because it did not\n> check the lack of merge base and fed an empty string (presumably, as\n> that is what you would get from mb=$(merge-base A B) when A and B\n> are unrelated) to later steps? If that is the case, then it *is* a\n> very significant thing to record here. As it was not merely \"failed\n> to stop due to lack of error checking\", but a lot worse. It was\n> producing a wrong result.\n\nIndeed. But it was only in the `--stat` reporting, so it did not produce\nan incorrect rebased history.\n\n> A more faithful reimplementation would not have exited with fatal\n> error, but would have produced the same wrong result, so \"a better\n> error checking caused the reimplementation die where the original\n> did not die\" may be correct but is much less important than the fact\n> that \"the logic used in the original to produce diffstat forgot that\n> there may not be a merge base and would not have worked correctly in\n> such a case, and that is what we are correcting\" is more important.\n\nTrue.\n\n> Please descibe the issue as such, if that is the case (I cannot read\n> from the description in the proposed log message if that is the\n> case---but I came to the conclusion that it is what you wanted to\n> fix reading the code).\n\nIndeed, my commit message is a bit too close to what I fixed, and not\nenough about what needed to be fixed.\n\n> > \t\tif (options.flags & REBASE_VERBOSE)\n> > \t\t\tprintf(_(\"Changes from %s to %s:\\n\"),\n> > -\t\t\t\toid_to_hex(&merge_base),\n> > +\t\t\t\tis_null_oid(&merge_base) ?\n> > +\t\t\t\t\"(empty)\" : oid_to_hex(&merge_base),\n> \n> I am not sure (empty) is a good idea for two reasons. It is going\n> to be inserted into an translated string without translation.\n\nOooops.\n\n> Also it is too similar to the fallback string used by some printf\n> implementations when NULL was given to %s by mistake.\n\nYou mean `(null)`? That was actually intentional, I just thought that\n`(empty)` would be different enough...\n\n> If there weren't i18n issues, I would suggest to use \"empty merge\n> base\" or \"empty tree\" instead, but the old one would have shown\n> 0{40}, so perhaps empty_tree_oid_hex() is a good substitute?\n\nAs this is a user-facing message, I'd rather avoid something like\n`empty_tree_oid_hex()`, which to every Git user who does not happen to be\na Git developer, too, must sound very cryptic.\n\nBut I guess that I should not be so lazy and really use two different\nmessages here:\n\n\tChanges from to \n\nand if there is no merge base,\n\n\tChanges in \n\nWill fix.\n\n> > @@ -1494,8 +1495,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> > \t\t\tDIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;\n> > \t\topts.detect_rename = DIFF_DETECT_RENAME;\n> > \t\tdiff_setup_done(&opts);\n> > -\t\tdiff_tree_oid(&merge_base, &options.onto->object.oid,\n> > -\t\t\t \"\", &opts);\n> > +\t\tdiff_tree_oid(is_null_oid(&merge_base) ?\n> > +\t\t\t the_hash_algo->empty_tree : &merge_base,\n> > +\t\t\t &options.onto->object.oid, \"\", &opts);\n> \n> The original would have run \"git diff '' $onto\", and died with an\n> error message, so both the original and the reimplementation were\n> failing. Just in different ways ;-)\n\nRight.\n\n> > diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh\n> > index b97ffdc9dd..be3b241676 100755\n> > --- a/git-legacy-rebase.sh\n> > +++ b/git-legacy-rebase.sh\n> > @@ -718,10 +718,12 @@ if test -n \"$diffstat\"\n> > then\n> > \tif test -n \"$verbose\"\n> > \tthen\n> > -\t\techo \"$(eval_gettext \"Changes from \\$mb to \\$onto:\")\"\n> > +\t\tmb_display=\"${mb:-(empty)}\"\n> > +\t\techo \"$(eval_gettext \"Changes from \\$mb_display to \\$onto:\")\"\n> > \tfi\n> > +\tmb_tree=\"${mb:-$(git hash-object -t tree /dev/null)}\"\n> > \t# We want color (if set), but no pager\n> > -\tGIT_PAGER='' git diff --stat --summary \"$mb\" \"$onto\"\n> > +\tGIT_PAGER='' git diff --stat --summary \"$mb_tree\" \"$onto\"\n> \n> Code fix for diff-tree invocation, both in the builtin/ version and\n> this one, looks correct.\n\nOkay. I fixed the two things you pointed out, just waiting for the Linux\nphase to finish (I don't think there is anything OS-specific in this\npatch, so I trust macOS and Windows to pass if Linux passes):\nhttps://git-for-windows.visualstudio.com/git/_build/results?buildId=26116\n\nCiao,\nDscho\n"},{"id":"364310","messageId":"pull.88.v2.git.gitgitgadget@gmail.com","threadId":"49899","inReplyTo":"pull.88.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/1] Fix git rebase --stat -i ","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-29T13:01:52Z","receivedAt":"2018-11-29T13:01:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"We're really entering obscure territory here, I would say.\n\nTo trigger the bug, two things have to come together: the user must have\nasked for a diffstat afterwards, and the commits need to have been rebased\nonto a completely unrelated commit history (i.e. there must exist no merge\nbase between the pre-rebase HEAD and the post-rebase HEAD).\n\nPlease note that this bug existed already in the scripted rebase, but it was\nnever detected because the scripted version would not even perform any error\nchecking.\n\nIt will make Junio very happy that I squashed the regression test in to the\npatch that fixes it. The reason, however, was not to make Junio happy (I\nhope to make him happy by fixing bugs), but simply that an earlier iteration\nof test would only fail with the built-in rebase, but not with the scripted\nversion. The current version would fail with the scripted version, but I'll\nsave the time to split the patch again now.\n\nChanges since v1:\n\n * The commit message now talks more about what we should do in case that\n there is no merge base, rather than stressing the differences between the\n way scripted vs built-in rebase handled it (both buggy, and fixed by this\n patch).\n * In case that there is no merge base, it no longer reports \"Changes from\n (empty) to ...\" but \"Changes to ...\", which should be a lot less\n controversial.\n\nJohannes Schindelin (1):\n rebase --stat: fix when rebasing to an unrelated history\n\n builtin/rebase.c | 18 ++++++++++++------\n git-legacy-rebase.sh | 10 ++++++++--\n t/t3406-rebase-message.sh | 10 ++++++++++\n 3 files changed, 30 insertions(+), 8 deletions(-)\n\n\nbase-commit: a1598010f775d82b5adf12c29d0f5bc9b41434c6\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-88%2Fdscho%2Frebase-stat-fix-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-88/dscho/rebase-stat-fix-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/88\n\nRange-diff vs v1:\n\n 1: 680385e4bd ! 1: 190c7856ad rebase --stat: fix when rebasing to an unrelated history\n @@ -3,22 +3,21 @@\n rebase --stat: fix when rebasing to an unrelated history\n \n When rebasing to a commit history that has no common commits with the\n - current branch, there is no merge base. The scripted version of the `git\n - rebase` command was not prepared for that and spewed out\n + current branch, there is no merge base. In diffstat mode, this means\n + that we cannot compare to the merge base, but we have to compare to the\n + empty tree instead.\n \n - fatal: ambiguous argument '': unknown revision or path not in\n - the working tree.\n + Also, if running in verbose diffstat mode, we should not output\n \n - but then continued (due to lack of error checking).\n + Changes from to \n \n - The built-in version of the `git rebase` command blindly translated that\n - shell script code, assuming that there is no need to test whether there\n - *was* a merge base, and due to its better error checking, exited with a\n - fatal error (because it tried to read the object with hash 00000000...\n - as a tree).\n + as that does not make sense without any merge base.\n \n - Fix both scripted and built-in versions to output something sensibly,\n - and add a regression test to keep this working in all eternity.\n + Note: neither scripted nor built-in versoin of `git rebase` were\n + prepared for this situation well. We use this opportunity not only to\n + fix the bug(s), but also to make both versions' output consistent in\n + this instance. And add a regression test to keep this working in all\n + eternity.\n \n Reported-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Johannes Schindelin \n @@ -27,15 +26,25 @@\n --- a/builtin/rebase.c\n +++ b/builtin/rebase.c\n @@\n + \tif (options.flags & REBASE_DIFFSTAT) {\n + \t\tstruct diff_options opts;\n \n - \t\tif (options.flags & REBASE_VERBOSE)\n - \t\t\tprintf(_(\"Changes from %s to %s:\\n\"),\n +-\t\tif (options.flags & REBASE_VERBOSE)\n +-\t\t\tprintf(_(\"Changes from %s to %s:\\n\"),\n -\t\t\t\toid_to_hex(&merge_base),\n -+\t\t\t\tis_null_oid(&merge_base) ?\n -+\t\t\t\t\"(empty)\" : oid_to_hex(&merge_base),\n - \t\t\t\toid_to_hex(&options.onto->object.oid));\n +-\t\t\t\toid_to_hex(&options.onto->object.oid));\n ++\t\tif (options.flags & REBASE_VERBOSE) {\n ++\t\t\tif (is_null_oid(&merge_base))\n ++\t\t\t\tprintf(_(\"Changes to %s:\\n\"),\n ++\t\t\t\t oid_to_hex(&options.onto->object.oid));\n ++\t\t\telse\n ++\t\t\t\tprintf(_(\"Changes from %s to %s:\\n\"),\n ++\t\t\t\t oid_to_hex(&merge_base),\n ++\t\t\t\t oid_to_hex(&options.onto->object.oid));\n ++\t\t}\n \n \t\t/* We want color (if set), but no pager */\n + \t\tdiff_setup(&opts);\n @@\n \t\t\tDIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;\n \t\topts.detect_rename = DIFF_DETECT_RENAME;\n @@ -57,8 +66,12 @@\n \tif test -n \"$verbose\"\n \tthen\n -\t\techo \"$(eval_gettext \"Changes from \\$mb to \\$onto:\")\"\n -+\t\tmb_display=\"${mb:-(empty)}\"\n -+\t\techo \"$(eval_gettext \"Changes from \\$mb_display to \\$onto:\")\"\n ++\t\tif test -z \"$mb\"\n ++\t\tthen\n ++\t\t\techo \"$(eval_gettext \"Changes to \\$onto:\")\"\n ++\t\telse\n ++\t\t\techo \"$(eval_gettext \"Changes from \\$mb to \\$onto:\")\"\n ++\t\tfi\n \tfi\n +\tmb_tree=\"${mb:-$(git hash-object -t tree /dev/null)}\"\n \t# We want color (if set), but no pager\n @@ -81,7 +94,7 @@\n +\tgit -C unrelated remote add -f origin \"$PWD\" &&\n +\tgit -C unrelated branch --set-upstream-to=origin/master &&\n +\tgit -C unrelated -c core.editor=true rebase -i -v --stat >actual &&\n -+\ttest_i18ngrep \"Changes from (empty)\" actual &&\n ++\ttest_i18ngrep \"Changes to \" actual &&\n +\ttest_i18ngrep \"5 files changed\" actual\n +'\n +\n\n-- \ngitgitgadget\n"},{"id":"364311","messageId":"190c7856ad0c500107d0e063572890c151e4eae7.1543496511.git.gitgitgadget@gmail.com","threadId":"49899","inReplyTo":"pull.88.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/1] rebase --stat: fix when rebasing to an unrelated history","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-29T13:01:54Z","receivedAt":"2018-11-29T13:01:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen rebasing to a commit history that has no common commits with the\ncurrent branch, there is no merge base. In diffstat mode, this means\nthat we cannot compare to the merge base, but we have to compare to the\nempty tree instead.\n\nAlso, if running in verbose diffstat mode, we should not output\n\n\tChanges from to \n\nas that does not make sense without any merge base.\n\nNote: neither scripted nor built-in versoin of `git rebase` were\nprepared for this situation well. We use this opportunity not only to\nfix the bug(s), but also to make both versions' output consistent in\nthis instance. And add a regression test to keep this working in all\neternity.\n\nReported-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Johannes Schindelin \n---\n builtin/rebase.c | 18 ++++++++++++------\n git-legacy-rebase.sh | 10 ++++++++--\n t/t3406-rebase-message.sh | 10 ++++++++++\n 3 files changed, 30 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 5b3e5baec8..1c6f817f4b 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1481,10 +1481,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (options.flags & REBASE_DIFFSTAT) {\n \t\tstruct diff_options opts;\n \n-\t\tif (options.flags & REBASE_VERBOSE)\n-\t\t\tprintf(_(\"Changes from %s to %s:\\n\"),\n-\t\t\t\toid_to_hex(&merge_base),\n-\t\t\t\toid_to_hex(&options.onto->object.oid));\n+\t\tif (options.flags & REBASE_VERBOSE) {\n+\t\t\tif (is_null_oid(&merge_base))\n+\t\t\t\tprintf(_(\"Changes to %s:\\n\"),\n+\t\t\t\t oid_to_hex(&options.onto->object.oid));\n+\t\t\telse\n+\t\t\t\tprintf(_(\"Changes from %s to %s:\\n\"),\n+\t\t\t\t oid_to_hex(&merge_base),\n+\t\t\t\t oid_to_hex(&options.onto->object.oid));\n+\t\t}\n \n \t\t/* We want color (if set), but no pager */\n \t\tdiff_setup(&opts);\n@@ -1494,8 +1499,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\tDIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;\n \t\topts.detect_rename = DIFF_DETECT_RENAME;\n \t\tdiff_setup_done(&opts);\n-\t\tdiff_tree_oid(&merge_base, &options.onto->object.oid,\n-\t\t\t \"\", &opts);\n+\t\tdiff_tree_oid(is_null_oid(&merge_base) ?\n+\t\t\t the_hash_algo->empty_tree : &merge_base,\n+\t\t\t &options.onto->object.oid, \"\", &opts);\n \t\tdiffcore_std(&opts);\n \t\tdiff_flush(&opts);\n \t}\ndiff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh\nindex b97ffdc9dd..b4c7dbfa57 100755\n--- a/git-legacy-rebase.sh\n+++ b/git-legacy-rebase.sh\n@@ -718,10 +718,16 @@ if test -n \"$diffstat\"\n then\n \tif test -n \"$verbose\"\n \tthen\n-\t\techo \"$(eval_gettext \"Changes from \\$mb to \\$onto:\")\"\n+\t\tif test -z \"$mb\"\n+\t\tthen\n+\t\t\techo \"$(eval_gettext \"Changes to \\$onto:\")\"\n+\t\telse\n+\t\t\techo \"$(eval_gettext \"Changes from \\$mb to \\$onto:\")\"\n+\t\tfi\n \tfi\n+\tmb_tree=\"${mb:-$(git hash-object -t tree /dev/null)}\"\n \t# We want color (if set), but no pager\n-\tGIT_PAGER='' git diff --stat --summary \"$mb\" \"$onto\"\n+\tGIT_PAGER='' git diff --stat --summary \"$mb_tree\" \"$onto\"\n fi\n \n test -n \"$interactive_rebase\" && run_specific_rebase\ndiff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh\nindex 38bd876cab..c2c9950568 100755\n--- a/t/t3406-rebase-message.sh\n+++ b/t/t3406-rebase-message.sh\n@@ -91,4 +91,14 @@ test_expect_success 'error out early upon -C or --whitespace=' '\n \ttest_i18ngrep \"Invalid whitespace option\" err\n '\n \n+test_expect_success 'rebase -i onto unrelated history' '\n+\tgit init unrelated &&\n+\ttest_commit -C unrelated 1 &&\n+\tgit -C unrelated remote add -f origin \"$PWD\" &&\n+\tgit -C unrelated branch --set-upstream-to=origin/master &&\n+\tgit -C unrelated -c core.editor=true rebase -i -v --stat >actual &&\n+\ttest_i18ngrep \"Changes to \" actual &&\n+\ttest_i18ngrep \"5 files changed\" actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"364375","messageId":"xmqqo9a7mjfh.fsf@gitster-ct.c.googlers.com","threadId":"49899","inReplyTo":"nycvar.QRO.7.76.6.1811291318090.41@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] rebase --stat: fix when rebasing to an unrelated history","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-30T01:40:50Z","receivedAt":"2018-11-30T01:40:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> But I guess that I should not be so lazy and really use two different\n> messages here:\n>\n> \tChanges from to \n>\n> and if there is no merge base,\n>\n> \tChanges in \n\nAh, that's excellent.\n\nThanks.\n"}]}