[PATCH 3 of 3] revert: evaluate filesets against working directory

Matt Harbison mharbison72 at gmail.com
Wed Mar 25 20:19:26 CDT 2015


On Wed, 25 Mar 2015 20:10:21 -0400, Martin von Zweigbergk  
<martinvonz at google.com> wrote:

> On Wed, Mar 25, 2015 at 5:03 PM Matt Harbison <mharbison72 at gmail.com>  
> wrote:
>
>> On Tue, 24 Mar 2015 14:16:41 -0400, Martin von Zweigbergk
>> <martinvonz at google.com> wrote:
>>
>> > # HG changeset patch
>> > # User Martin von Zweigbergk <martinvonz at google.com>
>> > # Date 1427177091 25200
>> > #      Mon Mar 23 23:04:51 2015 -0700
>> > # Node ID 23d12cf3fdddf6b57639e6120e96d754e1427a8e
>> > # Parent  a390c931dda089c51b858bb360f12d4e80bddba9
>> > revert: evaluate filesets against working directory
>> >
>> > As the failing revert tests in test-fileset-generated.t show,
>> >
>> > Revert currently creates one matcher for matching files in the working
>> > copy and another matcher for matching files in the target
>> > revision. The two matchers are created with different contexts, which
>> > means filesets are evaluated differently. Then the union of the sets
>> > of files matching the matchers in the two contexts are reverted. It
>> > doesn't seem to make sense to use two different matchers; only the
>> > context they're applied to should be different.
>> >
>> > It seems very likely that the user wants the filesets to be evaluated
>> > against the working directory, which the tests
>> > test-fileset-generated.t also assume, so let's make it so.
>> >
>> > I willingly admit that the largefiles code was modified by trial and
>> > error (according to tests).
>> >
>> ...
>>
>> > diff -r a390c931dda0 -r 23d12cf3fddd mercurial/cmdutil.py
>> > --- a/mercurial/cmdutil.py    Tue Mar 24 10:27:56 2015 -0700
>> > +++ b/mercurial/cmdutil.py    Mon Mar 23 23:04:51 2015 -0700
>> > @@ -2813,7 +2813,6 @@
>> >                          return
>> >                  ui.warn("%s: %s\n" % (m.rel(path), msg))
>> > -            m = scmutil.match(ctx, pats, opts)
>> >              m.bad = badfn
>> >              for abs in ctx.walk(m):
>> >                  if abs not in names:
>>
>> The revert code baffles me, so I'll just ask instead of breaking
>> something... There's a line at the bottom of cmdutil.revert() that  
>> creates
>> this same matcher, in order to figure out subrepos that need reverting.
>> Shouldn't that also use the working directory matcher created at the  
>> top?
>> (i.e. the line at the bottom should be dropped too)
>>
>
> That makes sense to me. I didn't even notice that line. (Note that you
> can't exactly drop the line, since the 'm' matcher gets overwritten above
> it. We should perhaps create a new variable for the matchfiles() call
> instead of overwriting the existing matcher.)

Good, point.  I missed that.  I'll take a run at this (though I'm not sure  
how hard testing it will be).

>>
>> I was going to add path/into/subrepo style reverting, which requires
>> passing a matcher instead of patterns, and I wasn't sure how passing two
>> matchers would be received.  That problem goes away if the line at the
>> bottom gets dropped.
>>
>> I'll see if I can add tests for the largefiles code, since I'm not real
>> sure what those changes do.
>>
>> --Matt


More information about the Mercurial-devel mailing list