Patchwork [1,of,2] revert: move targetsubs calculation down to its use

login
register
mail settings
Submitter Durham Goode
Date Sept. 20, 2014, 1:53 a.m.
Message ID <b74b7d5f46d5d8ad13ca.1411177990@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/5898/
State Accepted
Headers show

Comments

Durham Goode - Sept. 20, 2014, 1:53 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1411177239 25200
#      Fri Sep 19 18:40:39 2014 -0700
# Node ID b74b7d5f46d5d8ad13ca74e6b740970129090529
# Parent  273bf648227540a7ef87c4794e96b8a9d915d5b7
revert: move targetsubs calculation down to its use

A future patch will be reorganizing this section of the code into two paths, and
targetsubs complicates this by existing in the middle of one path, but not the
other. We fix that by moving it 200 lines down, to the only place it's used.
Augie Fackler - Sept. 23, 2014, 8:32 p.m.
On Fri, Sep 19, 2014 at 06:53:10PM -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1411177239 25200
> #      Fri Sep 19 18:40:39 2014 -0700
> # Node ID b74b7d5f46d5d8ad13ca74e6b740970129090529
> # Parent  273bf648227540a7ef87c4794e96b8a9d915d5b7
> revert: move targetsubs calculation down to its use

This one is queued. The next one needs me to think more about it, so
I'm leaving it flagged for later (but do consider a resend if you
don't see feedback on it after a day or two...)

>
> A future patch will be reorganizing this section of the code into two paths, and
> targetsubs complicates this by existing in the middle of one path, but not the
> other. We fix that by moving it 200 lines down, to the only place it's used.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2503,9 +2503,6 @@
>              if abs not in names:
>                  names[abs] = m.rel(abs), m.exact(abs)
>
> -        # get the list of subrepos that must be reverted
> -        targetsubs = sorted(s for s in ctx.substate if m(s))
> -
>          # Find status of all file in `names`.
>          m = scmutil.matchfiles(repo, names)
>
> @@ -2692,6 +2689,10 @@
>          if not opts.get('dry_run'):
>              _performrevert(repo, parents, ctx, actions)
>
> +            # get the list of subrepos that must be reverted
> +            subrepomatch = scmutil.match(ctx, pats, opts)
> +            targetsubs = sorted(s for s in ctx.substate if subrepomatch(s))
> +
>              if targetsubs:
>                  # Revert the subrepos on the revert list
>                  for sub in targetsubs:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2503,9 +2503,6 @@ 
             if abs not in names:
                 names[abs] = m.rel(abs), m.exact(abs)
 
-        # get the list of subrepos that must be reverted
-        targetsubs = sorted(s for s in ctx.substate if m(s))
-
         # Find status of all file in `names`.
         m = scmutil.matchfiles(repo, names)
 
@@ -2692,6 +2689,10 @@ 
         if not opts.get('dry_run'):
             _performrevert(repo, parents, ctx, actions)
 
+            # get the list of subrepos that must be reverted
+            subrepomatch = scmutil.match(ctx, pats, opts)
+            targetsubs = sorted(s for s in ctx.substate if subrepomatch(s))
+
             if targetsubs:
                 # Revert the subrepos on the revert list
                 for sub in targetsubs: