[PATCH stable] subrepo: use standard handling of \ in subpath replacement strings (BC)

[Matt Mackall]

On Mon, 2012-07-30 at 14:35 +0200, Mads Kiilerich wrote:
> On 30/07/12 03:55, Matt Mackall wrote:
> > On Mon, 2012-07-30 at 03:18 +0200, Mads Kiilerich wrote:
> >> Matt Mackall wrote, On 07/27/2012 09:07 PM:
> >>> On Thu, 2012-07-26 at 02:57 +0200, Mads Kiilerich wrote:
> >>>> # HG changeset patch
> >>>> # User Mads Kiilerich <mads at kiilerich.com>
> >>>> # Date 1343264252 -7200
> >>>> # Branch stable
> >>>> # Node ID 1de046d28995fb6c5af554882d2c5ec14b364ece
> >>>> # Parent  1675ef8a2bf2e32e9f42aa3a74b48c0f44ff9b80
> >>>> subrepo: use standard handling of \ in subpath replacement strings (BC)
> >>> So if I've checked in a .hgsub file which contains replacement patterns
> >>> using backslashes, I won't be able to check it out after upgrading?
> >>>
> >>> That seems a rather big deal.
> >> Negative. All sane setups will work as before.
> > Yes... but where does that leave Windows users? </rimshot>
> >
> > Really, even as bad as the current state is, if there's a non-trivial
> > risk it's going to mean people can't check out what they checked in
> > after upgrading (definitely a cardinal sin for an SCM), I'm not going to
> > buy it. Given how many Windows users have reported using "\" in their
> > subrepo config and the fact that this misfeature was clearly introduced
> > _for the benefit of Windows users_ (probably for one of Martin's
> > clients!), I have basically no confidence that Windows users aren't
> > dependent on this.
> 
> Yes, some Windows users might depend on the misfeature.

As I said before, this misfeature was introduced on purpose, probably
for a customer of Martin's. Given that the fundamental mandate of a VCS
is "give me access to my old code"[1], _knowingly_ putting that customer
and anyone else who might be using subpaths "wrong" in a position where
they can't check out their old code is, in my view, just about the worst
thing the developers of a VCS could ever do. It's a more severe breach
of contract than even a radical change like renaming the 'merge'
command.

But that's not to say we shouldn't try to improve the situation here. We
just have to tread VERY carefully with close attention to all the abuses
users are likely to have committed.

> Ok. The problem and the opinions on it is now quite well understood. It 
> is your call whether you want this corner to suck forever or if you want 
> to have one upgrade that might suck for a few users.

Let me be clear about my philosophy for future reference: sucking
forever is massively preferable for tools like VCSes and programming
languages if it's synonymous with "my old stuff keeps working". If I
want to check out the version of the project I wrote 10 years ago, and
the VCS developers have on 10 different occasions decided "sucking once
is preferable to sucking forever", then I am 10 layers deep in
accumulated layers of "suck once" that I'm encountering all at once and
the code is effectively lost. Here, any user would be justified in
declaring the VCS unfit for purpose.

> A compromise could be to check if if the replacement string contains any 
> single \ without a following digit and in that case keep the old 
> behaviour and issue a warning. Users can fix their .hgsub configuration 
> in tip and will only see the warning when they update back to old revisions.

That seems closer to acceptable, but not all the way there.

It might be easier to discuss this with a set of test cases that ought
to continue working and things that ought to change. So here's a sample
set:

 c:\foo\bar -> needs "\" to be preserved for backward compatibility
 c:\foo\1 -> ambiguous (currently \1 is replaced)
 c:\\foo\\1 -> needs to become c:\foo\1?
 c:\foo\\1 -> needs to become c:\foo\1?
 c:/foo\1 -> needs to replace \1
 c:\foo/bar -> undefined (should continue working unless 
 http://foo/\1 -> needs "\1" to be a replacement
 http://foo/\\1 -> needs "\1" to be literal
 http:\\foo\1 -> probably don't care
 http://foo\bar -> probably don't care

I think we can pull heuristics like "we don't need to worry about
escaping backslashes if forward slashes are present" out of the above
set.

Also note that a replacement pattern like this:

 foo = c:\foo\1994\

doesn't actually work, as we'll get an "invalid group reference" error.
So we can disambiguate a bit here too.

[1] Lots of people think the fundamental mandate is "give me easy access
to the latest version of code"; they are wrong.

-- 
Mathematics is the supreme nostalgia of our time.