git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Torsten Bögershausen" <tboegi@web.de>
To: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Paul Tan <pyokagan@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Brendan Forster <shiftkey@github.com>,
	Git List <git@vger.kernel.org>
Subject: Re: [PATCH 0/2] Reinstate the helpful message when `git pull --rebase` fails
Date: Sat, 10 Oct 2015 18:05:52 +0200	[thread overview]
Message-ID: <56193760.5010307@web.de> (raw)
In-Reply-To: <d89d595a223508896db9303c901e7c30@dscho.org>

On 09.10.15 12:11, Johannes Schindelin wrote:
> Me again,
> 
> On 2015-10-09 11:50, Johannes Schindelin wrote:
>>
>> 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
> +'
> +

One other question:
Is it good to mix 2 different things in one test case ?
"shows the helpful advice when failing" is one thing,
and the problematic CRLF handling another.

Does it make sense to simply create "really-modified" file to test the helpful advice ?

And may be another one witch test the CRLF handling, (may be)

  parent reply	other threads:[~2015-10-10 16:06 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-08 20:35 [PATCH 0/2] Reinstate the helpful message when `git pull --rebase` fails Johannes Schindelin
2015-10-08 20:35 ` [PATCH 1/2] merge_recursive_options: introduce the "gentle" flag Johannes Schindelin
2015-10-08 20:35 ` [PATCH 2/2] pull --rebase: reinstate helpful message on abort Johannes Schindelin
2015-10-09 18:36   ` Junio C Hamano
2015-10-12  9:16     ` Johannes Schindelin
2015-10-12 20:33       ` Junio C Hamano
2015-10-09  0:52 ` [PATCH 0/2] Reinstate the helpful message when `git pull --rebase` fails Junio C Hamano
2015-10-09  1:40   ` Paul Tan
2015-10-09  9:50     ` Johannes Schindelin
2015-10-09 10:11       ` Johannes Schindelin
2015-10-09 20:49         ` Junio C Hamano
2015-10-10  4:58         ` Torsten Bögershausen
2015-10-10 16:05         ` Torsten Bögershausen [this message]
2015-10-12 10:45           ` Johannes Schindelin
2015-10-09 18:15     ` Junio C Hamano
2015-10-09 18:40       ` Junio C Hamano
2015-10-09 18:55         ` Junio C Hamano
2015-10-12  9:46           ` Johannes Schindelin
2015-10-09 20:46         ` Junio C Hamano
2015-10-12  9:40         ` Johannes Schindelin
2015-10-12 20:28           ` Junio C Hamano
2015-10-13 11:48             ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56193760.5010307@web.de \
    --to=tboegi@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=pyokagan@gmail.com \
    --cc=shiftkey@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).