Re: [PATCH 0/2] Reinstate the helpful message when `git pull --rebase` fails
Me again,
On 2015-10-09 11:50, Johannes Schindelin wrote:
Show 25 quoted lines
>
> On 2015-10-09 03:40, Paul Tan wrote:
>> On Fri, Oct 9, 2015 at 8:52 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>>
>>>> Brendan Forster noticed that we no longer see the helpful message after
>>>> a failed `git pull --rebase`. It turns out that the builtin `am` calls
>>>> the recursive merge function directly, not via a separate process.
>>>>
>>>> But that function was not really safe to be called that way, as it
>>>> die()s pretty liberally.
>>
>> I'm not too familiar with the merge-recursive.c code, but I was under
>> the impression that it only called die() under fatal conditions. In
>> common use cases, such as merge conflicts, it just errors out and the
>> helpful error message does get printed. Is there a reproduction recipe
>> for this?
>
> Yes. Sorry, I should have added that as part of the patch series.
> Actually, I should have written it *before* making those patches.
> Because it revealed that the underlying problem is completely
> different: *Normally* you are correct, if `pull --rebase` fails with a
> merge conflict, the advice is shown.
>
> The problem occurs with CR/LF.
I finally have that test case working, took way longer than I wanted to:
-- snip --
Subject: [PATCH 3/2] Verify that `git pull --rebase` shows the helpful advice when failing
Author: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri Oct 9 11:15:30 2015 +0200
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a0013ee..bce332f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -237,6 +237,18 @@ test_expect_success '--rebase' '
test new = "$(git show HEAD:file2)"
'
+test_expect_success 'failed --rebase shows advice' '
+ git checkout -b diverging &&
+ test_commit attributes .gitattributes "* text=auto" attrs &&
+ sha1="$(printf "1\\r\\n" | git hash-object -w --stdin)" &&
+ git update-index --cacheinfo 0644 $sha1 file &&
+ git commit -m v1-with-cr &&
+ git checkout -f -b fails-to-rebase HEAD^ &&
+ test_commit v2-without-cr file "2" file2-lf &&
+ test_must_fail git pull --rebase . diverging 2>err >out &&
+ grep "When you have resolved this problem" out
+'
+
test_expect_success '--rebase fails with multiple branches' '
git reset --hard before-rebase &&
test_must_fail git pull --rebase . copy master 2>err &&
--
So the reason is that `unpack_trees()` fails with
error: Your local changes to the following files would be overwritten by merge:
file
Please, commit your changes or stash them before you can merge.
then returns -1 to its caller, `git_merge_trees()`, which still returns -1 in turn to *its* caller, `merge_trees()`, which then gives up by die()ing:
Aborting
I think there is more than one fix necessary to truly address the issue: the underlying problem is that Git handles *committed* CR/LF really badly when the corresponding `.gitattributes` label the file as `text=auto`. In fact, those files are labeled as modified in `git status`. If you change the line endings of them, they are labeled as modified in `git status`. And after a `git reset --hard`, they are *still* labeled as modified in `git status`.
I will try to make some time to continue to work on this later today, but in the meantime I would be relatively happy if we could introduce that gentle flag. It is really a very gentle patch, after all, much gentler than reverting to the heavy-handed spawning of `merge-recursive`.
Ciao,
Dscho
Johannes Schindelin <johannes.schindelin@gmx.de>