[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