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

Matt Harbison mharbison72 at gmail.com
Thu Mar 26 20:06:24 CDT 2015


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.

It's been like this for 3 years now, but revert subrepo support is totally  
undocumented AFAICT- nothing in 'hg help revert' or in 'hg help  
subrepos'.  Would you consider this a bug (and accept patches to fix  
this), or are we stuck with this?

--Matt


More information about the Mercurial-devel mailing list