Submitter | Matt Harbison |
---|---|
Date | March 27, 2015, 3:30 a.m. |
Message ID | <910503083e4a0f49e71b.1427427020@Envy> |
Download | mbox | patch |
Permalink | /patch/8306/ |
State | Rejected |
Delegated to: | Matt Mackall |
Headers | show |
Comments
On Thu, Mar 26, 2015 at 11:30:20PM -0400, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1427423049 14400 > # Thu Mar 26 22:24:09 2015 -0400 > # Node ID 910503083e4a0f49e71b9b392913990e00b63f3c > # Parent 30ddc3cf76df464f7b4ad38eb49e4d1f49457f01 > revert: process subrepos before the local repo > > The initial match variable 'm', built from the 'pats' passed to the method, is > replaced part way through the method. But when the 'pats' list is replaced by > a matcher in an upcoming patch, that will need to be narrowed and passed to the > subrepo. Rather than breaking naming conventions, move the subrepo processing > to before the replacement, and revert from bottom up. > > The additional '.hgsub not found' message is because the .hgsub file used to be > reverted before the subrepo list was built. So maybe there is merit to > reverting from top down? OTOH, reverting .hgsub to some revision where the > subrepo didn't exist before processing subrepos will prevent that subrepo from > being reverted. If we are reverting based on the working directory, we probably > need to stick with it for better or worse. Interesting. This might make it strictly worse, because when you revert and it reintroduces .hgsub you end up with a partial working copy (you'd have to revert twice), but if you revert and it removes .hgsub the worst you end up with is some untracked non-hg subrepos. Ideally, we could detect that .hgsub was going to disappear or appear and do things in the right order? Other than this, the patch seems reasonable to me, but I'm going to let someone else take it or not, since I'm unclear on what the lesser evil is going to be. > > The other two .hgsub related messages in the area are from: > 1) The repo.walk() under 'not always()' > 2) repo.status(), just below that > > Shouldn't the @propertycache on basectx.substate() cache the result of > subrepo.state(), so that it is only read once from disk, and therefore the > message is only displayed once? > > diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py > --- a/mercurial/cmdutil.py > +++ b/mercurial/cmdutil.py > @@ -2813,6 +2813,20 @@ > > wctx = repo[None] > m = scmutil.match(wctx, pats, opts) > + > + # get the list of subrepos that must be reverted > + subrepomatch = m > + targetsubs = sorted(s for s in wctx.substate if subrepomatch(s)) > + > + if targetsubs: > + # Revert the subrepos on the revert list > + for sub in targetsubs: > + try: > + wctx.sub(sub).revert(ctx.substate[sub], *pats, **opts) > + except KeyError: > + raise util.Abort("subrepository '%s' does not exist in %s!" > + % (sub, short(ctx.node()))) > + > if not m.always(): > m.bad = lambda x, y: False > for abs in repo.walk(m): > @@ -3047,19 +3061,6 @@ > _revertprefetch(repo, ctx, *[actions[name][0] for name in needdata]) > interactive = opts.get('interactive', False) > _performrevert(repo, parents, ctx, actions, interactive) > - > - # get the list of subrepos that must be reverted > - subrepomatch = scmutil.match(wctx, pats, opts) > - targetsubs = sorted(s for s in wctx.substate if subrepomatch(s)) > - > - if targetsubs: > - # Revert the subrepos on the revert list > - for sub in targetsubs: > - try: > - wctx.sub(sub).revert(ctx.substate[sub], *pats, **opts) > - except KeyError: > - raise util.Abort("subrepository '%s' does not exist in %s!" > - % (sub, short(ctx.node()))) > finally: > wlock.release() > > diff --git a/tests/test-largefiles-misc.t b/tests/test-largefiles-misc.t > --- a/tests/test-largefiles-misc.t > +++ b/tests/test-largefiles-misc.t > @@ -1035,9 +1035,9 @@ > M large > M no-largefiles/normal1 > $ hg -R subrepo-root revert --all > - reverting subrepo-root/.hglf/large (glob) > reverting subrepo no-largefiles > reverting subrepo-root/no-largefiles/normal1 (glob) > + reverting subrepo-root/.hglf/large (glob) > > $ cd .. > > diff --git a/tests/test-subrepo-missing.t b/tests/test-subrepo-missing.t > --- a/tests/test-subrepo-missing.t > +++ b/tests/test-subrepo-missing.t > @@ -34,6 +34,7 @@ > $ hg revert .hgsub > warning: subrepo spec file .hgsub not found > warning: subrepo spec file .hgsub not found > + warning: subrepo spec file .hgsub not found > > delete .hgsubstate and revert it > > diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t > --- a/tests/test-subrepo.t > +++ b/tests/test-subrepo.t > @@ -914,8 +914,8 @@ > $ rm repo/s/b > $ touch -t 200001010000 repo/.hgsubstate > $ hg -R repo revert --all > + reverting subrepo s > reverting repo/.hgsubstate (glob) > - reverting subrepo s > $ hg -R repo update > 1 files updated, 0 files merged, 0 files removed, 0 files unresolved > $ cat repo/s/b > _______________________________________________ > 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 @@ -2813,6 +2813,20 @@ wctx = repo[None] m = scmutil.match(wctx, pats, opts) + + # get the list of subrepos that must be reverted + subrepomatch = m + targetsubs = sorted(s for s in wctx.substate if subrepomatch(s)) + + if targetsubs: + # Revert the subrepos on the revert list + for sub in targetsubs: + try: + wctx.sub(sub).revert(ctx.substate[sub], *pats, **opts) + except KeyError: + raise util.Abort("subrepository '%s' does not exist in %s!" + % (sub, short(ctx.node()))) + if not m.always(): m.bad = lambda x, y: False for abs in repo.walk(m): @@ -3047,19 +3061,6 @@ _revertprefetch(repo, ctx, *[actions[name][0] for name in needdata]) interactive = opts.get('interactive', False) _performrevert(repo, parents, ctx, actions, interactive) - - # get the list of subrepos that must be reverted - subrepomatch = scmutil.match(wctx, pats, opts) - targetsubs = sorted(s for s in wctx.substate if subrepomatch(s)) - - if targetsubs: - # Revert the subrepos on the revert list - for sub in targetsubs: - try: - wctx.sub(sub).revert(ctx.substate[sub], *pats, **opts) - except KeyError: - raise util.Abort("subrepository '%s' does not exist in %s!" - % (sub, short(ctx.node()))) finally: wlock.release() diff --git a/tests/test-largefiles-misc.t b/tests/test-largefiles-misc.t --- a/tests/test-largefiles-misc.t +++ b/tests/test-largefiles-misc.t @@ -1035,9 +1035,9 @@ M large M no-largefiles/normal1 $ hg -R subrepo-root revert --all - reverting subrepo-root/.hglf/large (glob) reverting subrepo no-largefiles reverting subrepo-root/no-largefiles/normal1 (glob) + reverting subrepo-root/.hglf/large (glob) $ cd .. diff --git a/tests/test-subrepo-missing.t b/tests/test-subrepo-missing.t --- a/tests/test-subrepo-missing.t +++ b/tests/test-subrepo-missing.t @@ -34,6 +34,7 @@ $ hg revert .hgsub warning: subrepo spec file .hgsub not found warning: subrepo spec file .hgsub not found + warning: subrepo spec file .hgsub not found delete .hgsubstate and revert it diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t --- a/tests/test-subrepo.t +++ b/tests/test-subrepo.t @@ -914,8 +914,8 @@ $ rm repo/s/b $ touch -t 200001010000 repo/.hgsubstate $ hg -R repo revert --all + reverting subrepo s reverting repo/.hgsubstate (glob) - reverting subrepo s $ hg -R repo update 1 files updated, 0 files merged, 0 files removed, 0 files unresolved $ cat repo/s/b