Patchwork [2,of,2] remove: support remove with explicit paths in subrepos

login
register
mail settings
Submitter Matt Harbison
Date Nov. 10, 2014, 1:24 a.m.
Message ID <b928e82d0c8a4fa67f10.1415582688@Envy>
Download mbox | patch
Permalink /patch/6657/
State Superseded
Commit f6b8d23492e530fa3b99504cbc2a7b964316a69a
Headers show

Comments

Matt Harbison - Nov. 10, 2014, 1:24 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1415561732 18000
#      Sun Nov 09 14:35:32 2014 -0500
# Node ID b928e82d0c8a4fa67f10f0aa361fb3820337ff18
# Parent  cbb4fe231f72f2e677ca3e90eb018b2134bac39b
remove: support remove with explicit paths in subrepos

Like 'forget', git and svn subrepos are currently not supported.  They won't,
however, cause a non zero exit code unless the explicit paths point into them.
Unfortunately the name 'remove' is already used in the subrepo classes, so we
break the convention of naming the subrepo function after the command.
Matt Harbison - Nov. 12, 2014, 12:55 a.m.
On Sun, 09 Nov 2014 20:24:48 -0500, Matt Harbison wrote:

> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1415561732 18000
> #      Sun Nov 09 14:35:32 2014 -0500
> # Node ID b928e82d0c8a4fa67f10f0aa361fb3820337ff18
> # Parent  cbb4fe231f72f2e677ca3e90eb018b2134bac39b
> remove: support remove with explicit paths in subrepos
> 

After playing with this more, I need to fix this second patch because it 
can be made to recurse into subrepos improperly with -I.  The first patch 
should still be OK.  I'll fix this one and resend both if the first isn't 
queued by the time I resend.

--Matt
Matt Mackall - Nov. 12, 2014, 11:22 p.m.
On Wed, 2014-11-12 at 00:55 +0000, Matt Harbison wrote:
> On Sun, 09 Nov 2014 20:24:48 -0500, Matt Harbison wrote:
> 
> > # HG changeset patch
> > # User Matt Harbison <matt_harbison@yahoo.com>
> > # Date 1415561732 18000
> > #      Sun Nov 09 14:35:32 2014 -0500
> > # Node ID b928e82d0c8a4fa67f10f0aa361fb3820337ff18
> > # Parent  cbb4fe231f72f2e677ca3e90eb018b2134bac39b
> > remove: support remove with explicit paths in subrepos
> > 
> 
> After playing with this more, I need to fix this second patch because it 
> can be made to recurse into subrepos improperly with -I.  The first patch 
> should still be OK.  I'll fix this one and resend both if the first isn't 
> queued by the time I resend.

I've gone ahead and grabbed the first one, thanks.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2052,21 +2052,41 @@ 
     forgot.extend(forget)
     return bad, forgot
 
-def remove(ui, repo, m, after, force):
+def remove(ui, repo, m, prefix, after, force):
+    join = lambda f: os.path.join(prefix, f)
     ret = 0
     s = repo.status(match=m, clean=True)
     modified, added, deleted, clean = s[0], s[1], s[3], s[6]
 
+    wctx = repo[None]
+    for subpath in sorted(wctx.substate):
+        sub = wctx.sub(subpath)
+        try:
+            submatch = matchmod.narrowmatcher(subpath, m)
+            if sub.removefiles(ui, submatch, prefix, after, force):
+                ret = 1
+        except error.LookupError:
+            ui.status(_("skipping missing subrepository: %s\n")
+                           % join(subpath))
+
     # warn about failure to delete explicit files/dirs
-    wctx = repo[None]
     for f in m.files():
-        if f in repo.dirstate or f in wctx.dirs():
+        def insubrepo():
+            for subpath in wctx.substate:
+                if f.startswith(subpath):
+                    return True
+            return False
+
+        if f in repo.dirstate or f in wctx.dirs() or insubrepo():
             continue
-        if os.path.exists(m.rel(f)):
-            if os.path.isdir(m.rel(f)):
-                ui.warn(_('not removing %s: no tracked files\n') % m.rel(f))
+
+        if os.path.exists(m.rel(join(f))):
+            if os.path.isdir(m.rel(join(f))):
+                ui.warn(_('not removing %s: no tracked files\n')
+                        % m.rel(join(f)))
             else:
-                ui.warn(_('not removing %s: file is untracked\n') % m.rel(f))
+                ui.warn(_('not removing %s: file is untracked\n')
+                        % m.rel(join(f)))
         # missing files will generate a warning elsewhere
         ret = 1
 
@@ -2075,22 +2095,22 @@ 
     elif after:
         list = deleted
         for f in modified + added + clean:
-            ui.warn(_('not removing %s: file still exists\n') % m.rel(f))
+            ui.warn(_('not removing %s: file still exists\n') % m.rel(join(f)))
             ret = 1
     else:
         list = deleted + clean
         for f in modified:
             ui.warn(_('not removing %s: file is modified (use -f'
-                      ' to force removal)\n') % m.rel(f))
+                      ' to force removal)\n') % m.rel(join(f)))
             ret = 1
         for f in added:
             ui.warn(_('not removing %s: file has been marked for add'
-                      ' (use forget to undo)\n') % m.rel(f))
+                      ' (use forget to undo)\n') % m.rel(join(f)))
             ret = 1
 
     for f in sorted(list):
         if ui.verbose or not m.exact(f):
-            ui.status(_('removing %s\n') % m.rel(f))
+            ui.status(_('removing %s\n') % m.rel(join(f)))
 
     wlock = repo.wlock()
     try:
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5142,7 +5142,7 @@ 
         raise util.Abort(_('no files specified'))
 
     m = scmutil.match(repo[None], pats, opts)
-    return cmdutil.remove(ui, repo, m, after, force)
+    return cmdutil.remove(ui, repo, m, "", after, force)
 
 @command('rename|move|mv',
     [('A', 'after', None, _('record a rename that has already occurred')),
diff --git a/mercurial/help/subrepos.txt b/mercurial/help/subrepos.txt
--- a/mercurial/help/subrepos.txt
+++ b/mercurial/help/subrepos.txt
@@ -129,6 +129,9 @@ 
     elements. Subversion subrepositories are currently silently
     ignored.
 
+:remove: remove currently only handles exact file matches in subrepos.
+    Git and Subversion subrepositories are currently silently ignored.
+
 :update: update restores the subrepos in the state they were
     originally committed in target changeset. If the recorded
     changeset is not available in the current subrepository, Mercurial
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -501,6 +501,15 @@ 
     def forget(self, ui, match, prefix):
         return ([], [])
 
+    def removefiles(self, ui, matcher, prefix, after, force):
+        """remove the matched files from the subrepository and the filesystem,
+        possibly by force and/or after the file has been removed from the
+        filesystem.  Return 0 on success, 1 on any warning.
+        """
+        if len(matcher.files()) == 0:
+            return 0
+        return 1
+
     def revert(self, ui, substate, *pats, **opts):
         ui.warn('%s: reverting %s subrepos is unsupported\n' \
             % (substate[0], substate[2]))
@@ -854,6 +863,11 @@ 
                               os.path.join(prefix, self._path), True)
 
     @annotatesubrepoerror
+    def removefiles(self, ui, matcher, prefix, after, force):
+        return cmdutil.remove(ui, self._repo, matcher,
+                              os.path.join(prefix, self._path), after, force)
+
+    @annotatesubrepoerror
     def revert(self, ui, substate, *pats, **opts):
         # reverting a subrepo is a 2 step process:
         # 1. if the no_backup is not set, revert all modified
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
@@ -110,6 +110,14 @@ 
   $ hg ci -Sm "add test.txt"
   committing subrepository sub1
   committing subrepository sub1/sub2 (glob)
+
+  $ hg remove sub1/sub2/folder/test.txt
+  $ hg remove sub1/.hgsubstate
+  $ hg status -S
+  R sub1/.hgsubstate
+  R sub1/sub2/folder/test.txt
+  $ hg update -Cq
+
   $ hg --config extensions.largefiles=! archive -S ../archive_all
   $ find ../archive_all | sort
   ../archive_all