patchstripping [PATCH] without losing later tags from mailed patches (Re: [ 02/12] Remove COMPAT_IA32 support)

>>> The subject/first line should include '[IA64]', as in the original
>>> commit.  It looks like this has been automatically stripped.
>>
>> Yeah, munging patches to and from quilt and git will cause that to
>> happen at times, it's quite common :(
>
> Indeed, and I've even changed my patch formats in haproxy to avoid brackets
> due to this issue. The cause is that many patches are sent with a [PATCH]
> prefix and that with Git, either you keep the subject line intact or you
> remove everything that is between brackets. There's the -b option to only
> remove remove tags looking like [PATCH], but my general experience with it
> was not satisfying (I don't remind why).

> > Indeed, and I've even changed my patch formats in haproxy to avoid brackets
> > due to this issue. The cause is that many patches are sent with a [PATCH]
> > prefix and that with Git, either you keep the subject line intact or you
> > remove everything that is between brackets. There's the -b option to only
> > remove remove tags looking like [PATCH], but my general experience with it
> > was not satisfying (I don't remind why).
> 
> Maybe the problem was as simple as "git am" not knowing about "-b".

> Two relevant patches:
> 
>   f7e5ea17 (am: learn passing -b to mailinfo, 2012-01-16)
>   ee2d1cb4 (mailinfo: with -b, keep space after [foo], 2012-01-16)
> 
> are in "master" and 1.7.10-rc0 and were not part of any earlier release.
>
> Kudos to Thomas for writing them.

> Hi Jonathan,
> 
> On Mon, Mar 12, 2012 at 01:48:55AM -0500, Jonathan Nieder wrote:
> > > Indeed, and I've even changed my patch formats in haproxy to avoid brackets
> > > due to this issue. The cause is that many patches are sent with a [PATCH]
> > > prefix and that with Git, either you keep the subject line intact or you
> > > remove everything that is between brackets. There's the -b option to only
> > > remove remove tags looking like [PATCH], but my general experience with it
> > > was not satisfying (I don't remind why).
> > 
> > Maybe the problem was as simple as "git am" not knowing about "-b".
> 
> I think you're quite right.
> 
> > Two relevant patches:
> > 
> >   f7e5ea17 (am: learn passing -b to mailinfo, 2012-01-16)
> >   ee2d1cb4 (mailinfo: with -b, keep space after [foo], 2012-01-16)
> > 
> > are in "master" and 1.7.10-rc0 and were not part of any earlier release.
> >
> > Kudos to Thomas for writing them.
> 
> Ah, thank you very much for this useful info, I'll update my version !

> > > Two relevant patches:
> > > 
> > >   f7e5ea17 (am: learn passing -b to mailinfo, 2012-01-16)
> > >   ee2d1cb4 (mailinfo: with -b, keep space after [foo], 2012-01-16)
> > > 
> > > are in "master" and 1.7.10-rc0 and were not part of any earlier release.
> > >
> > > Kudos to Thomas for writing them.
> > 
> > Ah, thank you very much for this useful info, I'll update my version !
> 
> I don't see a -b option to 'git am' in the manpage, am I missing
> something here?

> On Mon, Mar 12, 2012 at 08:20:04AM -0700, Greg KH wrote:
>> 
>> I don't see a -b option to 'git am' in the manpage, am I missing
>> something here?
>
> It's in the master tree only right now, and the option is "--keep-non-patch"
> (could have been shorter). Currently rebuilding to test it :-)

> [+cc Junio because of backwards-compat issues]
> 
> Willy Tarreau <w@1wt.eu> writes:
> 
> > On Mon, Mar 12, 2012 at 08:20:04AM -0700, Greg KH wrote:
> >> 
> >> I don't see a -b option to 'git am' in the manpage, am I missing
> >> something here?
> >
> > It's in the master tree only right now, and the option is "--keep-non-patch"
> > (could have been shorter). Currently rebuilding to test it :-)
> 
> Exactly.
> 
> The problem with -b is that it's a backwards-compatibility shorthand for
> --binary, which used to pass --allow-binary-replacement (or --binary) to
> git-apply.  However, that option was obsoleted in 2b6eef9 (Make apply
> --binary a no-op., 2006-09-06) and has been a no-op for over 5 years.
> It has also not been documented since cb3a160 (git-am: ignore --binary
> option, 2008-08-09).
> 
> So perhaps we can safely claim -b for --keep-non-patch, like so:

> The problem with -b is that it's a backwards-compatibility shorthand for
> --binary, which used to pass --allow-binary-replacement (or --binary) to
> git-apply.  However, that option was obsoleted in 2b6eef9 (Make apply
> --binary a no-op., 2006-09-06) and has been a no-op for over 5 years.
> It has also not been documented since cb3a160 (git-am: ignore --binary
> option, 2008-08-09).
>
> So perhaps we can safely claim -b for --keep-non-patch, like so:

> Thomas Rast wrote:
>
>> The problem with -b is that it's a backwards-compatibility shorthand for
>> --binary, which used to pass --allow-binary-replacement (or --binary) to
>> git-apply.  However, that option was obsoleted in 2b6eef9 (Make apply
>> --binary a no-op., 2006-09-06) and has been a no-op for over 5 years.
>> It has also not been documented since cb3a160 (git-am: ignore --binary
>> option, 2008-08-09).
>>
>> So perhaps we can safely claim -b for --keep-non-patch, like so:

> >> So perhaps we can safely claim -b for --keep-non-patch, like so:
> 
> We can delete "git am -b" (as it was deprecated long time ago), wait
> for a cycle or two, and then repurpose it.  I do not mind starting
> the first step (delete, but do not say anything about repurposing)
> before 1.7.10-rc1 happens.

> From my user experience and what I see on a number of coworkers, users
> tend to make big jumps when they need a new feature, so in practice,
> not offering the option in a version or two would probably not affect
> most users, especially the ones still relying on the old behaviour. So
> I don't see much benefit in waiting for repurposing the option.

> Willy Tarreau wrote:
> 
> > From my user experience and what I see on a number of coworkers, users
> > tend to make big jumps when they need a new feature, so in practice,
> > not offering the option in a version or two would probably not affect
> > most users, especially the ones still relying on the old behaviour. So
> > I don't see much benefit in waiting for repurposing the option.
> 
> The benefit is that if it does turn out to be a regression, early
> adopters will complain to us because their scripts have stopped
> working and we get a chance to back out the change without having to
> worry about others who have started to rely on the option.

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Thomas Rast wrote:
>>
>>> The problem with -b is that it's a backwards-compatibility shorthand for
>>> --binary, which used to pass --allow-binary-replacement (or --binary) to
>>> git-apply.  However, that option was obsoleted in 2b6eef9 (Make apply
>>> --binary a no-op., 2006-09-06) and has been a no-op for over 5 years.
>>> It has also not been documented since cb3a160 (git-am: ignore --binary
>>> option, 2008-08-09).
>>>
>>> So perhaps we can safely claim -b for --keep-non-patch, like so:
>
> We can delete "git am -b" (as it was deprecated long time ago), wait
> for a cycle or two, and then repurpose it.  I do not mind starting
> the first step (delete, but do not say anything about repurposing)
> before 1.7.10-rc1 happens.

> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -380,7 +380,9 @@ do
>  	-i|--interactive)
>  		interactive=t ;;
>  	-b|--binary)
> -		: ;;
> +		echo >&2 "The -b/--binary option was deprecated in 1.6.0 and removed in 1.7.10."
> +		die "Please adjust your scripts."
> +		;;

> Hm, on second thought, if people are seeing this message, I would
> prefer if they write to the mailing list so we can find out about it.
> So I really would rather see this say
>
> 	--binary)
> 		: ;;
>
> and have "-b" completely unrecognized, without any words in our
> defense except for a note in the release notes mentioning the option's
> removal and that it has been an unadvertised backward-compatibility
> no-op since 1.6.0.

>> Hm, on second thought, if people are seeing this message, I would
>> prefer if they write to the mailing list so we can find out about it.
>> So I really would rather see this say
>>
>> 	--binary)
>> 		: ;;
>>
>> and have "-b" completely unrecognized, without any words in our
>> defense except for a note in the release notes mentioning the option's
>> removal and that it has been an unadvertised backward-compatibility
>> no-op since 1.6.0.
>
> I'd hate doing that, mostly because other projects got me really angry
> about similar issues, e.g., 71c020c (Disable asciidoc 8.4.1+ semantics
> for `{plus}` and friends, 2009-07-25).

> 	--binary)
> 		: ;;
> 	-b)
> 		gettextln >&2 "The -b option (a no-op short for --binary) was removed in 1.7.10."
> 		die "$(gettext "Please adjust your scripts.")"
> 		;;
>
> Mentioning deprecation in 1.6.0 in the message left me uneasy because
> we never actually did anything to actively deprecate the option; it
> just has not been needed since 1.4.3 and we stopped advertising it in
> the manpage in 1.6.0.  So I don't like the implication of "this is all
> right because we told you so" --- on the contrary, it is "in practice
> nobody seems to be using this option and we hope nobody will notice
> when we take it away".

> Hmm, I had an alternate patch ready in the morning, but Junio beat us to
> it and applied the old one to master.
>
> I don't really think it matters enough to apply *another*, so I'll leave
> it at that.

> (resetting cc list)
> Thomas Rast wrote:
>
>> Hmm, I had an alternate patch ready in the morning, but Junio beat us to
>> it and applied the old one to master.
>>
>> I don't really think it matters enough to apply *another*, so I'll leave
>> it at that.
>
> May I have a copy?  If it makes the wording better somehow, I would be
> happy to do the work of making a patch on top of master using it.

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> 	--binary)
>> 		: ;;
>> 	-b)
>> 		gettextln >&2 "The -b option (a no-op short for --binary) was removed in 1.7.10."
>> 		die "$(gettext "Please adjust your scripts.")"
>> 		;;
>>
>> Mentioning deprecation in 1.6.0 in the message left me uneasy because
>> we never actually did anything to actively deprecate the option; it
>> just has not been needed since 1.4.3 and we stopped advertising it in
>> the manpage in 1.6.0.  So I don't like the implication of "this is all
>> right because we told you so" --- on the contrary, it is "in practice
>> nobody seems to be using this option and we hope nobody will notice
>> when we take it away".
>
> Hmm, I had an alternate patch ready in the morning, but Junio beat us to
> it and applied the old one to master.

>> I really don't think it is a good idea to avoid mentioning 1.6.0, at
>> which we *removed* description of the option in our manual pages and
>> from the "git am -h" help message. How much more active deprecation
>> would a user want?
>
> A warning when the option is used or a mention in the release notes.

> Hm, on second thought, if people are seeing this message, I would
> prefer if they write to the mailing list so we can find out about it.
> So I really would rather see this say
>
> 	--binary)
> 		: ;;
>
> and have "-b" completely unrecognized, without any words in our
> defense except for a note in the release notes mentioning the option's
> removal and that it has been an unadvertised backward-compatibility
> no-op since 1.6.0.

>> We can delete "git am -b" (as it was deprecated long time ago), wait
>> for a cycle or two, and then repurpose it.  I do not mind starting
>> the first step (delete, but do not say anything about repurposing)
>> before 1.7.10-rc1 happens.
>
> Ok, but if I don't get to say anything about repurposing, can I at least
> make it an error message instead of just spewing out the "invalid
> option" help?

>
> ----- 8< -----
> Subject: [PATCH] git-am: error out when seeing -b/--binary
>
> The --binary option to git-apply has been a no-op since 2b6eef9 (Make
> apply --binary a no-op., 2006-09-06) and was deprecated in cb3a160
> (git-am: ignore --binary option, 2008-08-09).
>
> We could remove it outright, but let's be nice to people who still
> have scripts saying 'git am -b' (if they exist) and tell them the
> reason for the sudden failure.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>  git-am.sh |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 0bd290b..faae820 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -380,7 +380,9 @@ do
>  	-i|--interactive)
>  		interactive=t ;;
>  	-b|--binary)
> -		: ;;
> +		echo >&2 "The -b/--binary option was deprecated in 1.6.0 and removed in 1.7.10."
> +		die "Please adjust your scripts."
> +		;;
>  	-3|--3way)
>  		threeway=t ;;
>  	-s|--signoff)

> Greg KH <greg@kroah.com> writes:
> 
> > I don't see a -b option to 'git am' in the manpage, am I missing
> > something here?
> 
> As this is a recent enhancement, it is very much appreciated if you
> try out 1.7.10-rc0 (or 'master').

> Hi Junio,
>
> On Mon, Mar 12, 2012 at 09:40:48AM -0700, Junio C Hamano wrote:
>> Greg KH <greg@kroah.com> writes:
>> 
>> > I don't see a -b option to 'git am' in the manpage, am I missing
>> > something here?
>> 
>> As this is a recent enhancement, it is very much appreciated if you
>> try out 1.7.10-rc0 (or 'master').
>
> I've just backported it to 1.7.9.3 (I'm not keen on living on the bleeding
> edge with my everyday tools), and it works nicely as expected.

> > I've just backported it to 1.7.9.3 (I'm not keen on living on the bleeding
> > edge with my everyday tools), and it works nicely as expected.
> 
> This topic will be backported in later versions of 1.7.9.x track,
> but living on the maintenance track does not have much smaller
> chance of breakage than living on the tip of 'master' these days,
> unless you are using distro packaged version. The usual rule of
> thumb if you are compiling from the source is that the tip of
> 'master' is more stable than any tagged version, including the
> maintenance track.  See
> 
> http://thread.gmane.org/gmane.comp.version-control.git/189657/focus=190814
> 
> for details.

> Greg KH <greg@kroah.com> writes:
> 
> > I don't see a -b option to 'git am' in the manpage, am I missing
> > something here?
> 
> As this is a recent enhancement, it is very much appreciated if you
> try out 1.7.10-rc0 (or 'master').

> Greg KH <greg@kroah.com> writes:
> 
> > Does the flag propagate from 'git quiltimport'?
> 
> I didn't even know the script was shipped as part of the main
> Porcelain until I looked.  I do not think anybody is maintaining it;
> the last update was from the 1.6.2 era.

> On Mon, Mar 12, 2012 at 11:01:55AM -0700, Junio C Hamano wrote:
>> Greg KH <greg@kroah.com> writes:
>> 
>> > Does the flag propagate from 'git quiltimport'?
>> 
>> I didn't even know the script was shipped as part of the main
>> Porcelain until I looked.  I do not think anybody is maintaining it;
>> the last update was from the 1.6.2 era.
>
> Ah, so my feeling that I'm the only one using it is true :)
>
>> A tested patch from quilt stakeholders is very much welcomed.
>
> I'll be glad to maintain this if you need me to.  I'll poke around with
> it tomorrow to see what needs to be done here.

> On Mon, Mar 12, 2012 at 11:01:55AM -0700, Junio C Hamano wrote:
> > Greg KH <greg@kroah.com> writes:
> > 
> > > Does the flag propagate from 'git quiltimport'?
> > 
> > I didn't even know the script was shipped as part of the main
> > Porcelain until I looked.  I do not think anybody is maintaining it;
> > the last update was from the 1.6.2 era.
> 
> Ah, so my feeling that I'm the only one using it is true :)

> 
> Jakub Narebski wrote:
> 
> > A'propos, there is git-archimport script, last modified (as an
> > individual script) in 2007.  Is anyone using GNU arch nowadays;
> > isn't arch-fast-export better solution anyway?
> 
> According to [1] (see the green line), it still has some users,
> or at least people who access the file now and then.   As long
> as the script works, why would we want to cause them trouble?
> 
> For comparison, here's the corresponding graph for people using
> GNU arch[2] and participating in the same survey.
> 
> Hope that helps,
> Jonathan
> 
> [1] http://qa.debian.org/popcon-graph.php?packages=git-arch
>     http://qa.debian.org/popcon.php?package=git-core
> [2] http://qa.debian.org/popcon-graph.php?packages=tla
>     http://qa.debian.org/popcon.php?package=tla
> 

> Jonathan Nieder <jrnieder <at> gmail.com> writes:
>
>> > A'propos, there is git-archimport script, last modified (as an
>> > individual script) in 2007.  Is anyone using GNU arch nowadays;
>> > isn't arch-fast-export better solution anyway?
>
> please do not remove the ability to convert arch/tla repos unless it is a real
> cost to maintain the scripts. There may be few people who still use arch, but
> there may be a lot of them who have arch repos of quiescent stuff that one day
> they may need to revitalize.