Patchwork [5,of,8,V3] commit: propagate --addremove to subrepos if -S is specified (issue3759)

login
register
mail settings
Submitter Matt Harbison
Date Dec. 12, 2014, 3:35 a.m.
Message ID <f082b5a5d4eb14f7aaba.1418355317@Envy>
Download mbox | patch
Permalink /patch/7062/
State Accepted
Commit f1b06a8aad4256b771019b1ced70c99f26f079eb
Headers show

Comments

Matt Harbison - Dec. 12, 2014, 3:35 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1416886069 18000
#      Mon Nov 24 22:27:49 2014 -0500
# Node ID f082b5a5d4eb14f7aaba6c24ce48a881fcfb3cc4
# Parent  71bacffdfe45b9b262e8a3c7260375e1b0f0b29c
commit: propagate --addremove to subrepos if -S is specified (issue3759)

The recursive addremove operation occurs completely before the first subrepo is
committed.  Only hg subrepos support the addremove operation at the moment- svn
and git subrepos will warn and abort the commit.
Martin von Zweigbergk - Dec. 12, 2014, 5:59 p.m.
On Thu Dec 11 2014 at 7:36:39 PM Matt Harbison <mharbison72@gmail.com>
wrote:

> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
> --- a/mercurial/scmutil.py
> +++ b/mercurial/scmutil.py
> @@ -713,13 +713,28 @@
>      '''Return a matcher that will efficiently match exactly these
> files.'''
>      return matchmod.exact(repo.root, repo.getcwd(), files)
>
> -def addremove(repo, matcher, opts={}, dry_run=None, similarity=None):
> +def addremove(repo, matcher, prefix, opts={}, dry_run=None,
> similarity=None):
>      m = matcher
>      if dry_run is None:
>          dry_run = opts.get('dry_run')
>      if similarity is None:
>          similarity = float(opts.get('similarity') or 0)
>
> +    ret = 0
> +    join = lambda f: os.path.join(prefix, f)
> +
> +    wctx = repo[None]
> +    for subpath in sorted(wctx.substate):
> +        if opts.get('subrepos'):
> +            sub = wctx.sub(subpath)
> +            try:
> +                submatch = matchmod.narrowmatcher(subpath, m)
> +                if sub.addremove(submatch, prefix, opts, dry_run,
> similarity):
> +                    ret = 1
> +            except error.LookupError:
> +                repo.ui.status(_("skipping missing subrepository: %s\n")
> +                                 % join(subpath))
> +
>

The other pieces of similar code live in cmdutil. Do you know why this
function is not there?
Matt Harbison - Dec. 13, 2014, 1:45 a.m.
On Fri, 12 Dec 2014 12:59:31 -0500, Martin von Zweigbergk  
<martinvonz@google.com> wrote:

> On Thu Dec 11 2014 at 7:36:39 PM Matt Harbison <mharbison72@gmail.com>
> wrote:
>
>> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
>> --- a/mercurial/scmutil.py
>> +++ b/mercurial/scmutil.py
>> @@ -713,13 +713,28 @@
>>      '''Return a matcher that will efficiently match exactly these
>> files.'''
>>      return matchmod.exact(repo.root, repo.getcwd(), files)
>>
>> -def addremove(repo, matcher, opts={}, dry_run=None, similarity=None):
>> +def addremove(repo, matcher, prefix, opts={}, dry_run=None,
>> similarity=None):
>>      m = matcher
>>      if dry_run is None:
>>          dry_run = opts.get('dry_run')
>>      if similarity is None:
>>          similarity = float(opts.get('similarity') or 0)
>>
>> +    ret = 0
>> +    join = lambda f: os.path.join(prefix, f)
>> +
>> +    wctx = repo[None]
>> +    for subpath in sorted(wctx.substate):
>> +        if opts.get('subrepos'):
>> +            sub = wctx.sub(subpath)
>> +            try:
>> +                submatch = matchmod.narrowmatcher(subpath, m)
>> +                if sub.addremove(submatch, prefix, opts, dry_run,
>> similarity):
>> +                    ret = 1
>> +            except error.LookupError:
>> +                repo.ui.status(_("skipping missing subrepository:  
>> %s\n")
>> +                                 % join(subpath))
>> +
>>
>
> The other pieces of similar code live in cmdutil. Do you know why this
> function is not there?

No clue, and I wondered the same thing.  It was convenient having each in  
a separate file when comparing add() and addremove() messages, but I see  
no reason it shouldn't be moved to cmdutil.

--Matt

Patch

diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -95,7 +95,7 @@ 
         oldquiet = repo.ui.quiet
         repo.ui.quiet = True
         matcher = scmutil.match(repo[None])
-        timer(lambda: scmutil.addremove(repo, matcher, dry_run=True))
+        timer(lambda: scmutil.addremove(repo, matcher, "", dry_run=True))
     finally:
         repo.ui.quiet = oldquiet
         fm.end()
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -1084,10 +1084,10 @@ 
     finally:
         repo.lfstatus = False
 
-def scmutiladdremove(orig, repo, matcher, opts={}, dry_run=None,
+def scmutiladdremove(orig, repo, matcher, prefix, opts={}, dry_run=None,
                      similarity=None):
     if not lfutil.islfilesrepo(repo):
-        return orig(repo, matcher, opts, dry_run, similarity)
+        return orig(repo, matcher, prefix, opts, dry_run, similarity)
     # Get the list of missing largefiles so we can remove them
     lfdirstate = lfutil.openlfdirstate(repo.ui, repo)
     unsure, s = lfdirstate.status(match_.always(repo.root, repo.getcwd()), [],
@@ -1107,7 +1107,7 @@ 
     # function to take care of the rest.  Make sure it doesn't do anything with
     # largefiles by passing a matcher that will ignore them.
     matcher = composenormalfilematcher(matcher, repo[None].manifest())
-    result = orig(repo, matcher, opts, dry_run, similarity)
+    result = orig(repo, matcher, prefix, opts, dry_run, similarity)
     return result
 
 # Calling purge with --all will cause the largefiles to be deleted.
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2202,7 +2202,7 @@ 
     # extract addremove carefully -- this function can be called from a command
     # that doesn't support addremove
     if opts.get('addremove'):
-        if scmutil.addremove(repo, matcher, opts) != 0:
+        if scmutil.addremove(repo, matcher, "", opts) != 0:
             raise util.Abort(
                 _("failed to mark all new/missing files as added/removed"))
 
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -236,7 +236,7 @@ 
     if sim < 0 or sim > 100:
         raise util.Abort(_('similarity must be between 0 and 100'))
     matcher = scmutil.match(repo[None], pats, opts)
-    return scmutil.addremove(repo, matcher, opts, similarity=sim / 100.0)
+    return scmutil.addremove(repo, matcher, "", opts, similarity=sim / 100.0)
 
 @command('^annotate|blame',
     [('r', 'rev', '', _('annotate the specified revision'), _('REV')),
diff --git a/mercurial/help/subrepos.txt b/mercurial/help/subrepos.txt
--- a/mercurial/help/subrepos.txt
+++ b/mercurial/help/subrepos.txt
@@ -94,7 +94,9 @@ 
     -S/--subrepos, or setting "ui.commitsubrepos=True" in a
     configuration file (see :hg:`help config`).  After there are no
     longer any modified subrepositories, it records their state and
-    finally commits it in the parent repository.
+    finally commits it in the parent repository.  The --addremove
+    option also honors the -S/--subrepos option.  However, Git and
+    Subversion subrepositories will print a warning and abort.
 
 :diff: diff does not recurse in subrepos unless -S/--subrepos is
     specified. Changes are displayed as usual, on the subrepositories
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -713,13 +713,28 @@ 
     '''Return a matcher that will efficiently match exactly these files.'''
     return matchmod.exact(repo.root, repo.getcwd(), files)
 
-def addremove(repo, matcher, opts={}, dry_run=None, similarity=None):
+def addremove(repo, matcher, prefix, opts={}, dry_run=None, similarity=None):
     m = matcher
     if dry_run is None:
         dry_run = opts.get('dry_run')
     if similarity is None:
         similarity = float(opts.get('similarity') or 0)
 
+    ret = 0
+    join = lambda f: os.path.join(prefix, f)
+
+    wctx = repo[None]
+    for subpath in sorted(wctx.substate):
+        if opts.get('subrepos'):
+            sub = wctx.sub(subpath)
+            try:
+                submatch = matchmod.narrowmatcher(subpath, m)
+                if sub.addremove(submatch, prefix, opts, dry_run, similarity):
+                    ret = 1
+            except error.LookupError:
+                repo.ui.status(_("skipping missing subrepository: %s\n")
+                                 % join(subpath))
+
     rejected = []
     origbad = m.bad
     def badfn(f, msg):
@@ -737,9 +752,9 @@ 
     for abs in sorted(toprint):
         if repo.ui.verbose or not m.exact(abs):
             if abs in unknownset:
-                status = _('adding %s\n') % m.uipath(abs)
+                status = _('adding %s\n') % m.uipath(join(abs))
             else:
-                status = _('removing %s\n') % m.uipath(abs)
+                status = _('removing %s\n') % m.uipath(join(abs))
             repo.ui.status(status)
 
     renames = _findrenames(repo, m, added + unknown, removed + deleted,
@@ -751,7 +766,7 @@ 
     for f in rejected:
         if f in m.files():
             return 1
-    return 0
+    return ret
 
 def marktouched(repo, files, similarity=0.0):
     '''Assert that files have somehow been operated upon. files are relative to
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -437,6 +437,10 @@ 
     def add(self, ui, match, dryrun, listsubrepos, prefix, explicitonly):
         return []
 
+    def addremove(self, matcher, prefix, opts, dry_run, similarity):
+        self._ui.warn("%s: %s" % (prefix, _("addremove is not supported")))
+        return 1
+
     def cat(self, ui, match, prefix, **opts):
         return 1
 
@@ -619,6 +623,11 @@ 
         return cmdutil.add(ui, self._repo, match, dryrun, listsubrepos,
                            os.path.join(prefix, self._path), explicitonly)
 
+    def addremove(self, m, prefix, opts, dry_run, similarity):
+        return scmutil.addremove(self._repo, m,
+                                 os.path.join(prefix, self._path), opts,
+                                 dry_run, similarity)
+
     @annotatesubrepoerror
     def cat(self, ui, match, prefix, **opts):
         rev = self._state[1]
diff --git a/tests/test-subrepo-deep-nested-change.t b/tests/test-subrepo-deep-nested-change.t
--- a/tests/test-subrepo-deep-nested-change.t
+++ b/tests/test-subrepo-deep-nested-change.t
@@ -106,8 +106,8 @@ 
   $ hg --config extensions.largefiles=! add sub1/sub2/test.txt
   $ mkdir sub1/sub2/folder
   $ echo 'subfolder' > sub1/sub2/folder/test.txt
-  $ hg --config extensions.largefiles=! add sub1/sub2/folder/test.txt
-  $ hg ci -Sm "add test.txt"
+  $ hg ci -ASm "add test.txt"
+  adding sub1/sub2/folder/test.txt (glob)
   committing subrepository sub1
   committing subrepository sub1/sub2 (glob)
 
@@ -128,6 +128,15 @@ 
   R sub1/.hgsubstate
   R sub1/sub2/folder/test.txt
   $ hg update -Cq
+  $ rm sub1/sub2/folder/test.txt
+  $ rm sub1/sub2/test.txt
+  $ hg ci -ASm "remove test.txt"
+  removing sub1/sub2/folder/test.txt (glob)
+  removing sub1/sub2/test.txt (glob)
+  committing subrepository sub1
+  committing subrepository sub1/sub2 (glob)
+  $ hg rollback -q
+  $ hg up -Cq
 
   $ hg --config extensions.largefiles=! archive -S ../archive_all
   $ find ../archive_all | sort