[PATCH] revert: process subrepos before the local repo
Matt Harbison
mharbison72 at gmail.com
Fri Mar 27 23:09:27 CDT 2015
On Fri, 27 Mar 2015 14:48:04 -0400, Matt Mackall <mpm at selenic.com> wrote:
> On Thu, 2015-03-26 at 23:30 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at 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.
>
> I was thinking of something more like this:
>
> diff -r 95cbc77c0cad mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py Wed Mar 25 15:53:30 2015 -0700
> +++ b/mercurial/cmdutil.py Fri Mar 27 13:37:15 2015 -0500
> @@ -2811,6 +2811,10 @@
> interactive = opts.get('interactive', False)
> wctx = repo[None]
> m = scmutil.match(wctx, pats, opts)
> +
> + # we'll need this later
> + targetsubs = sorted(s for s in wctx.substate if m(s))
> +
> if not m.always():
> m.bad = lambda x, y: False
> for abs in repo.walk(m):
> @@ -3048,10 +3052,6 @@
> _revertprefetch(repo, ctx, *[actions[name][0] for name in
> needdata])
> _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:
>
> I'm not clear if there's something gained in your approach?
My thinking is:
1) Get this, or the rename patch
2) Change the 'pats' parameter to a matcher
3) Drop the scmutil.match() monkey patching in largefiles
4) Narrow the matcher passed from commands.py and pass to sub.revert()
#4 is to support reverting a specific file or directory in the subrepo as
mentioned in the other thread. But in order to narrow the original
matcher, the original matcher either needs to be held in a different
variable to make it to the bottom of the function (the wmatcher patch), or
the entire subrepo processing needs to be brought to the top before it is
overwritten (this patch).
Since I'm not sure what my chances are of getting #4 in, I think I can do
what you suggest here + #2 + #3, in the short term anyway.
The change in what subrepos are processed here surprised me. I think what
happens currently is, if you 'hg revert -r REV . "set:subrepo(s)"', the
.hgsub file is reverted first. So if subrepo 's' doesn't exist in REV, it
is silently *not* reverted.
Your change above will now get it to abort if that is the case, as long as
.hgsub (and .hgsubstate?) is still intact. (For a subrepo 's', it also
warns "s: no such file in..."). If the .hgsub line is deleted, or the
file itself is deleted, the subrepo match list is empty, so the revert is
silently skipped with 'set:subrepo()', but does say 'no such file' if
passed as a simple path.
It's a corner case and the whole thing is pretty confusing. I think the
thing I lost sight of is that the subrepo is "removed" once its reference
is dropped from .hgsub (or the file is trashed), as opposed to on a commit.
>> 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.
>>
>> 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 at selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list