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

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

[Jonathan Nieder]

Hi,

(adding git list and Thomas to cc)
Willy Tarreau wrote:
> On Sun, Mar 11, 2012 at 07:49:48PM -0700, Greg KH wrote:
>> On Mon, Mar 12, 2012 at 01:07:26AM +0000, Ben Hutchings wrote:

>>> 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).

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.

Jonathan

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

[Willy Tarreau]

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 !

Cheers,
Willy

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

[Greg KH]

On Mon, Mar 12, 2012 at 09:58:20AM +0100, Willy Tarreau wrote:
> 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 !

I don't see a -b option to 'git am' in the manpage, am I missing
something here?

greg k-h

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

[Willy Tarreau]

On Mon, Mar 12, 2012 at 08:20:04AM -0700, Greg KH wrote:
> > > 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?

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 :-)

Willy

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

[Junio C Hamano]

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').

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

[Thomas Rast]

[+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:

diff --git i/Documentation/git-am.txt w/Documentation/git-am.txt
index ee6cca2..9ec9313 100644
--- i/Documentation/git-am.txt
+++ w/Documentation/git-am.txt
@@ -40,6 +40,7 @@ OPTIONS
 --keep::
 	Pass `-k` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
 
+-b::
 --keep-non-patch::
 	Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
 
diff --git i/git-am.sh w/git-am.sh
index 0bd290b..4b071de 100755
--- i/git-am.sh
+++ w/git-am.sh
@@ -9,13 +9,13 @@ git am [options] [(<mbox>|<Maildir>)...]
 git am [options] (--resolved | --skip | --abort)
 --
 i,interactive   run interactively
-b,binary*       (historical option -- no-op)
+binary*       (historical option -- no-op)
 3,3way          allow fall back on 3way merging if needed
 q,quiet         be quiet
 s,signoff       add a Signed-off-by line to the commit message
 u,utf8          recode into utf8 (default)
 k,keep          pass -k flag to git-mailinfo
-keep-non-patch  pass -b flag to git-mailinfo
+b,keep-non-patch pass -b flag to git-mailinfo
 keep-cr         pass --keep-cr flag to git-mailsplit for mbox format
 no-keep-cr      do not pass --keep-cr flag to git-mailsplit independent of am.keepcr
 c,scissors      strip everything before a scissors line
@@ -379,7 +379,7 @@ do
 	case "$1" in
 	-i|--interactive)
 		interactive=t ;;
-	-b|--binary)
+	--binary)
 		: ;;
 	-3|--3way)
 		threeway=t ;;
@@ -391,7 +391,7 @@ do
 		utf8= ;;
 	-k|--keep)
 		keep=t ;;
-	--keep-non-patch)
+	-b|--keep-non-patch)
 		keep=b ;;
 	-c|--scissors)
 		scissors=t ;;

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

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

[Willy Tarreau]

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.

Thanks!
Willy

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

[Willy Tarreau]

On Mon, Mar 12, 2012 at 05:41:49PM +0100, Thomas Rast wrote:
> [+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:

Yes I do think so, especially since 5 years ago, git commands were
called hyphenated like "git-am" instead of "git am". So I don't think
there's any risk in reusing the option.

Regards,
Willy

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

[Jonathan Nieder]

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:

Thanks.

It we want to be extra friendly to people who have been using
"format-patch --binary" with "am -b" in their scripts, we could have a
transitional period during which -b is treated as a usage error.

Luckily, a quick Google code search does not reveal any users for "am
-b", so I am not too worried and would not mind your patch that just
switches over right away, though.  After all, the failure modes are:

 - if my current script using "am -b" gets run using ancient git, it
   will accept binary patches and will strip out too many brackets
   in the subject line

 - if my ancient script using "am -b" gets run using current git, it
   will helpefully keep [IA64] brackets in the subject line

Neither seems terribly painful.

The manual would need to mention that this once meant --binary to
avoid confusion when that happens.

Hope that helps,
Jonathan

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

[Greg KH]

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').

Nice, I'll go do that now.

Does the flag propagate from 'git quiltimport'?  That's how I import
patches that would need this option most of the time.

thanks,

greg k-h

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

[Junio C Hamano]

Willy Tarreau <w@1wt.eu> writes:

> 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.

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.

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

[Junio C Hamano]

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.

A tested patch from quilt stakeholders is very much welcomed.

Thanks for bringing the script to my attention.

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

[Junio C Hamano]

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.

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

[Willy Tarreau]

On Mon, Mar 12, 2012 at 10:57:36AM -0700, Junio C Hamano wrote:
> > 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.

I know, but you see I was running on 1.7.2.3. Generally if one version
works for me, I don't upgrade it for a year or two. I've been hit a few
times in the past by some quite annoying bugs (as is to be expected from
any software in the development branch), and lost a lot of time on this.
Rest assured that I don't feel comfortable on 1.7.9 either, that's a big
jump for me but I know that most often it works quite well :-)

Willy

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

[Willy Tarreau]

On Mon, Mar 12, 2012 at 11:04:23AM -0700, Junio C Hamano wrote:
> >> 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.

Just my 2 cents,
Willy

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

[Jonathan Nieder]

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.

Regards,
Jonathan

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

[Willy Tarreau]

On Mon, Mar 12, 2012 at 01:54:40PM -0500, Jonathan Nieder wrote:
> 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.

Indeed you're right. I forgot the case of the old script relying
on the silently ignored parameter!

Regards,
Willy

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

[Greg KH]

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.

thanks,

greg k-h

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

[Junio C Hamano]

Willy Tarreau <w@1wt.eu> writes:

> Rest assured that I don't feel comfortable on 1.7.9 either, that's a big
> jump for me but I know that most often it works quite well :-)

It being a big jump is not a problem I can solve, as it is up to you
to deliberately stay behind until you decide to make a big jump.

I can indirectly solve it by not backporting as many fixes to the
maintenance tracks, but I am too nice to be playing that nasty ;-).

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

[Junio C Hamano]

Greg KH <greg@kroah.com> writes:

> 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.

Surely, and thanks.

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

[Willy Tarreau]

On Mon, Mar 12, 2012 at 12:26:27PM -0700, Greg KH wrote:
> 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 :)

Well, we're two precisely :-)

> > 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.

That would be great, this tool is really handy.

Cheers,
Willy

Re: Removing unmaintained scripts

[Jonathan Nieder]

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

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

[Thomas Rast]

Junio C Hamano <gitster@pobox.com> writes:

> 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.

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)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] git-am: error out when seeing -b/--binary

[Jonathan Nieder]

Thomas Rast wrote:

> 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?

I don't mind either way.

[...]
> --- 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.

Jonathan

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

[Junio C Hamano]

Thomas Rast <trast@inf.ethz.ch> writes:

>> 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?

Surely.  It is not "Am I at least allowed to say" at all; we really
_should_ make it clear why it is no longer supported.

What I meant was that "it will start meaning something different" is
not a relevant thing to see when somebody sees his old script that
uses "-b" breaks.

Let's apply your patch so that we do not have to wait one cycle too
long.

Thanks.

>
> ----- 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)

Re: [PATCH] git-am: error out when seeing -b/--binary

[Thomas Rast]

Jonathan Nieder <jrnieder@gmail.com> writes:

> 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).

By the time I knew what the problem was, I figured posting anywhere was
useless since the change was already in the wild, and thus needed
working around on our end; and all I could possibly post was an angry
letter saying how unhappy I was about their work.

I didn't, and worked around it.  But that was after a lot of frustrated
investigation.  So I'd rather not do the same to our unlucky users.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] git-am: error out when seeing -b/--binary

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> 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 do not mind keeping --binary working intact, but I think an
approach to say that "-b" no longer works and is finally removed
very firmly is a very sane one.  We would be getting a slightly
better feel of how stale the people's script could be with your
approach, but at the same time, we would be annoying far more people
who do not even know that there are some people passionately trying
to make the users' Git life better, or where these people are
hanging out.

I do not think removal of a no-op "-b" is something subject to
voting at this point, so the only thing you would get from the
better feel of the user universe is when we can repurpose the option
safely.

Re: [PATCH] git-am: error out when seeing -b/--binary

[Jonathan Nieder]

Thomas Rast wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> 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).

Oh, now that I think about it that way, you're definitely right.

So, how about something like this?

	--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".

Jonathan

Re: Removing unmaintained scripts

[Sergio]

Jonathan Nieder <jrnieder <at> gmail.com> writes:

> 
> 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
> 


Hi,

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.

Sergio

Re: Removing unmaintained scripts

[Thomas Rast]

Sergio <sergio.callegari@gmail.com> writes:

> 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.

Perhaps it would be a good idea to add a test to ensure that
git-archimport doesn't suddenly break with nobody noticing for months.
Can you contribute a reasonable (small but representative) test repo?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] git-am: error out when seeing -b/--binary

[Thomas Rast]

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 don't really think it matters enough to apply *another*, so I'll leave
it at that.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] git-am: error out when seeing -b/--binary

[Jonathan Nieder]

(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.

Re: [PATCH] git-am: error out when seeing -b/--binary

[Thomas Rast]

Jonathan Nieder <jrnieder@gmail.com> writes:

> (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.

Sure, but it's nothing magic :-)

------ 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 |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/git-am.sh b/git-am.sh
index 0bd290b..e1bdbfc 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -379,8 +379,12 @@ do
 	case "$1" in
 	-i|--interactive)
 		interactive=t ;;
-	-b|--binary)
+	--binary)
 		: ;;
+	-b)
+		gettextln >&2 "The -b option (shorthand for --binary, both no-ops) was removed in 1.7.10."
+		die "$(gettext "Please adjust your scripts.")"
+		;;
 	-3|--3way)
 		threeway=t ;;
 	-s|--signoff)


-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] git-am: error out when seeing -b/--binary

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> 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?

To put it another way, think what your answer would be when somebody
sees the message and says "eh? all of a sudden it was removed?".
Wouldn't you tell him "At 1.6.0 we deprecated it and stopped
advertising it"?  Why not give that answer upfront?

Especially when you think "in practice nobody seems to be using
this" is true?

Re: [PATCH] git-am: error out when seeing -b/--binary

[Jonathan Nieder]

Junio C Hamano wrote:

> 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.

> To put it another way, think what your answer would be when somebody
> sees the message and says "eh? all of a sudden it was removed?".
> Wouldn't you tell him "At 1.6.0 we deprecated it and stopped
> advertising it"?  Why not give that answer upfront?

I would tell her "Since 1.4.3 it has been a compatibility no-op and
our documentation made that clear, and by now based on a search nobody
seems to be using it".

What happened in 1.6.0, then?  Well, before 1.6.0, the git-am(1)
manual said

 -b, --binary
	Pass --allow-binary-replacement flag to git-apply (see
	git-apply(1)).

and the git-apply(1) manual said

 --allow-binary-replacement, --binary
	Historically we did not allow binary patch applied
	without an explicit permission from the user, and this
	flag was the way to do so.  Currently we always allow binary
	patch application, so this is a no-op.

Afterwards, the entry in the git-am(1) manual was removed, saving the
reader a little time.

Jonathan

Re: [PATCH] git-am: error out when seeing -b/--binary

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

>> 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.

Ok, then we should probably instead do these two starting at 1.7.10 ("the
official deprecation date"), and then start erroring out on "-b" three
cycles after that.

[PATCH] git-am: officially deprecate -b/--binary

[Junio C Hamano]

We have had these options as harmless no-op for more than 3 years without
officially deprecating them.  Let's announce the deprecation and start
warning against their use, but without failing the command just not yet,
so that we can later repurpose the option if we want to in the future.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/RelNotes/1.7.10.txt |    4 ++--
 git-am.sh                         |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/RelNotes/1.7.10.txt b/Documentation/RelNotes/1.7.10.txt
index 65df74b..6286485 100644
--- a/Documentation/RelNotes/1.7.10.txt
+++ b/Documentation/RelNotes/1.7.10.txt
@@ -26,8 +26,8 @@ Compatibility Notes
    Git v1.7.8 or newer.
 
  * The "--binary/-b" options to "git am" have been a no-op for quite a
-   while and was deprecated in mid 2008 (v1.6.0).  When you give these
-   options to "git am", it will now fail with an error.
+   while and were deprecated in mid 2008 (v1.6.0).  When you give these
+   options to "git am", it will now warn and ask you not to use them.
 
 
 Updates since v1.7.9
diff --git a/git-am.sh b/git-am.sh
index faae820..db6ade3 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -380,8 +380,8 @@ 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."
+		echo >&2 "The $1 option was deprecated in 1.6.0 and will be removed."
+		echo >&2 "Please do not use it anymore."
 		;;
 	-3|--3way)
 		threeway=t ;;