threads / patch / 33511

patchrebase-am: fix regression with new cover-letter config

Subject: [PATCH] rebase-am: fix regression with new cover-letter config

## tl;dr

5 messages between Apr 14, 2013 and Apr 15, 2013. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Felipe Contreras· Apr 14, 2013, 22:27 UTC · lore

If you have a cover-letter configuration set to anything other than 'false', 'git format-patch' might generate a cover letter, and 'git am' doesn't seem to like that.

Ideally 'git am' should skip the cover-letter, but for now lets just fix the regression.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-rebase--am.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to git-rebase--am.sh +2 −2
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 97f31dc..f84854f 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -31,8 +31,8 @@ else
 	rm -f "$GIT_DIR/rebased-patches"
 
 	git format-patch -k --stdout --full-index --ignore-if-in-upstream \
-		--src-prefix=a/ --dst-prefix=b/ \
-		--no-renames $root_flag "$revisions" >"$GIT_DIR/rebased-patches"
+		--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
+		$root_flag "$revisions" >"$GIT_DIR/rebased-patches"
 	ret=$?
 
 	if test 0 != $ret

-- 
1.8.2.1.643.ge3cc75d
Junio C Hamano· Apr 15, 2013, 01:48 UTC · re: Felipe Contreras · lore

Re: [PATCH] rebase-am: fix regression with new cover-letter config

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 9 quoted lines
> If you have a cover-letter configuration set to anything other than
> 'false', 'git format-patch' might generate a cover letter, and 'git am'
> doesn't seem to like that.
>
> Ideally 'git am' should skip the cover-letter, but for now lets just fix
> the regression.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
This is a follow-up to the fc/send-email-annotate topic, right?

You are mean to say "fix regression", which gave me a small heart attack ;-). I was worried if we had already merged it to 'next'.

As I already wrote in the recent "What's cooking", the series looks good to me. I am inclined to say that we should merge it (together with this update) to 'next'.

Thanks for catching this before breaking the real world users.
Show 18 quoted lines
>  git-rebase--am.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index 97f31dc..f84854f 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -31,8 +31,8 @@ else
>  	rm -f "$GIT_DIR/rebased-patches"
>  
>  	git format-patch -k --stdout --full-index --ignore-if-in-upstream \
> -		--src-prefix=a/ --dst-prefix=b/ \
> -		--no-renames $root_flag "$revisions" >"$GIT_DIR/rebased-patches"
> +		--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
> +		$root_flag "$revisions" >"$GIT_DIR/rebased-patches"
>  	ret=$?
>  
>  	if test 0 != $ret
Felipe Contreras· Apr 15, 2013, 02:31 UTC · re: Junio C Hamano · lore

Re: [PATCH] rebase-am: fix regression with new cover-letter config

On Sun, Apr 14, 2013 at 8:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> If you have a cover-letter configuration set to anything other than
>> 'false', 'git format-patch' might generate a cover letter, and 'git am'
>> doesn't seem to like that.
>>
>> Ideally 'git am' should skip the cover-letter, but for now lets just fix
>> the regression.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>
> This is a follow-up to the fc/send-email-annotate topic, right?
Right.
Show 6 quoted lines
> You are mean to say "fix regression", which gave me a small heart
> attack ;-).  I was worried if we had already merged it to 'next'.
>
> As I already wrote in the recent "What's cooking", the series looks
> good to me.  I am inclined to say that we should merge it (together
> with this update) to 'next'.

Yeah, after thinking about it, it's not even really a regression, because people don't have format.coverletter=auto in their configs (yet), so the bug can't can possibly affect them.

> Thanks for catching this before breaking the real world users.

I'm running 'next' as you suggested we should do, along with all my patches, so that helps :)

Cheers.
-- 
Felipe Contreras
Junio C Hamano· Apr 15, 2013, 02:51 UTC · re: Felipe Contreras · lore

Re: [PATCH] rebase-am: fix regression with new cover-letter config

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 10 quoted lines
>> You are mean to say "fix regression", which gave me a small heart
>> attack ;-).  I was worried if we had already merged it to 'next'.
>>
>> As I already wrote in the recent "What's cooking", the series looks
>> good to me.  I am inclined to say that we should merge it (together
>> with this update) to 'next'.
>
> Yeah, after thinking about it, it's not even really a regression,
> because people don't have format.coverletter=auto in their configs
> (yet), so the bug can't can possibly affect them.
Here is what I tentatively queued.
    rebase-am: explicitly disable cover-letter
    
    If the user has a cover-letter configuration set to anything
    other than 'false', 'git format-patch' may generate a cover
    letter, which has no place in "format-patch | am" pipeline.
    
    The internal invocatoin of format-patch must explicitly override
    the configuration from the command line, just like --src-prefix
    and other options already do.
    
    Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine· Apr 15, 2013, 02:57 UTC · re: Junio C Hamano · lore

Re: [PATCH] rebase-am: fix regression with new cover-letter config

On Sun, Apr 14, 2013 at 10:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Here is what I tentatively queued.
>
>     rebase-am: explicitly disable cover-letter
>
>     If the user has a cover-letter configuration set to anything
>     other than 'false', 'git format-patch' may generate a cover
>     letter, which has no place in "format-patch | am" pipeline.
>
>     The internal invocatoin of format-patch must explicitly override
s/invocatoin/invocation/
Show 5 quoted lines
>     the configuration from the command line, just like --src-prefix
>     and other options already do.
>
>     Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>

← back to recent threads