[PATCH 2 of 3] revert: rename working directory matcher 'm' -> 'wmatch'

[Matt Mackall]

On Fri, 2015-03-27 at 21:45 -0400, Matt Harbison wrote:
> On Fri, 27 Mar 2015 14:15:09 -0400, Matt Mackall <mpm at selenic.com> wrote:
> 
> > On Thu, 2015-03-26 at 21:06 -0400, Matt Harbison wrote:
> >> On Thu, 26 Mar 2015 17:09:13 -0400, Matt Mackall <mpm at selenic.com>  
> >> wrote:
> >>
> >> > On Thu, 2015-03-26 at 00:20 -0400, Matt Harbison wrote:
> >> >> # HG changeset patch
> >> >> # User Matt Harbison <matt_harbison at yahoo.com>
> >> >> # Date 1427333998 14400
> >> >> #      Wed Mar 25 21:39:58 2015 -0400
> >> >> # Node ID 781d27e7fcd3a438d02bf2e1a9ba0a85537da6b9
> >> >> # Parent  6e1b26088b8eca13825a91e4e1e3547c2e89f6a0
> >> >> revert: rename working directory matcher 'm' -> 'wmatch'
> >> >>
> >> >> Further down in the function, 'm' is reassigned to a matcher for a  
> >> list
> >> >> of
> >> >> files.  At the bottom of the function, another matcher is created to
> >> >> figure out
> >> >> what subrepos need to be reverted.  For consistency with  
> >> 5b85a5bc5bbb,
> >> >> this will
> >> >> be changed to a working directory matcher.
> >> >
> >> > Not sure about this one. The first use of 'm' here is the standard  
> >> "make
> >> > a matcher from the supplied args" pattern and the second definition
> >> > appears to have very limited use.
> >> >
> >> > It seems the shadowing here is kind of intentional: we're making a  
> >> more
> >> > narrower version of the initial match that should be used for the rest
> >> > of the function. So perhaps we should instead be moving the subrepo  
> >> set
> >> > calculation higher up instead of doing any renaming?
> >> >
> >> > But really, your patch 3 would actually be _simpler_ as a patch 2 that
> >> > just did s/ctx/wctx/ locally, right? I'll just do that for now. Patch  
> >> 1
> >> > and 3' queued, thanks.
> >>
> >> That looks like it works for this series.  The next thing to do though  
> >> is
> >> to pass a matcher to the subrepo, that is a narrowed form of the  
> >> original
> >> passed to the function.  The original matcher needs to make it to the
> >> subrepo section, but moving the whole subrepo section to the top looks
> >> like it would work too.
> >>
> >>
> >> I guess now is a good time to ask before doing the work- subrepo revert
> >> works by essentially doing a clean update of the subrepo, after doing a
> >> cmdutil.revert() on that layer to generate backup files.  That
> >> functionality would seem to preclude the path/into/subrepo form that is
> >> being supported on more commands, and would seem to violate the  
> >> documented
> >> help for revert:
> >>
> >> 	 "Because revert does not change the working directory parents..."
> >>
> >> Granted, it isn't changing the parents of the outer repo, but the  
> >> subrepo
> >> working directory seems like an extension of the outer repo's working
> >> directory.  Backout doesn't support subrepos, and maybe it isn't  
> >> possible
> >> to do so.  But it certainly can't if subrepos are being updated like  
> >> this
> >> by revert.  And because of the recursive nature, a subrepo N subrepos  
> >> deep
> >> will be clean updated N times.
> >
> > Oh man, that's a hard question. My internal model of subrepo semantics
> > says that yes, we should get a clean checkout of subrepos under the top
> > level, but I can believe that would surprise people. I'm not sure if the
> > alternative is particularly logically consistent though.
> 
> I'm not sure I understand the logical consistency part.  If I want to  
> revert a directory in a flat repo, that's spelled 'hg revert dir/subdir'.   
> The result may or may not be consistent codewise- other code depending on  
> subdir may now be missing a dependency, but that's what I asked for, and  
> the tool doesn't get in the way.
> 
> Given that a subdirectory can be reverted (and because other commands  
> support path/into/subrepo syntax), I think from a command line POV, it is  
> more consistent to be able to do 'hg revert subrepo/dir' or 'hg revert  
> subrepo/file'.  A subrepo clean update prevents that (or at the very least  
> is inconsistent with that, if we keep the current behavior as well when no  
> path or only the root of the subrepo is given.)
> 
> Any other top level repo using the subrepo is still in a consistent state,  
> until it updates the subrepo to a different revision.  (And then it is  
> free to add more commits to the subrepo to "fix" it before committing a  
> specific subrepo rev).
> 
> The reason I brought up backout is because in a flat repo, a one line  
> commit even hundreds of revs back can be surgically nuked.  But a 'backout  
> -S' using the current revert would kill *all* of the subsequent changes,  
> and I think that would be very surprising.
> 
> I remember the threads a few years back about 'should commit recurse into  
> subrepos or abort?'.  You argued consistency then, and my take away was  
> that letting a user accidentally commit something not identical to the  
> filesystem would be a nasty (and permanent, without history editing)  
> surprise.  I don't see how that principle applies here either though-  
> revert isn't recording history.

Consider that "hg revert -a" is basically the same as "hg update -C .":
change the working copy to match the parent. So if I do "hg revert
subrepo", I'd expect that to give me the state of the subrepo in the
parent... which is not the same as "hg --cwd subrepo revert:.

Another way to look at it is this. We can think of the working directory
as a tuple:

 < parent | [files...] >

(such that if hash(files) = parent, we're clean)

The most obvious way to think about subrepos is:

 < parent | [files..., <subparent | subfiles>...] >

So when we talk about revert reverting "the working copy".. well
subparent is a property of that. In particular, it's included in the
hash above.

-- 
Mathematics is the supreme nostalgia of our time.