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

Martin von Zweigbergk martinvonz at google.com
Wed Mar 25 19:10:21 CDT 2015


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.)


>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150326/8fe513bd/attachment.html>


More information about the Mercurial-devel mailing list