Patchwork [V4] remove: add dry-run functionality

login
register
mail settings
Submitter sushil khanchi
Date March 28, 2018, 4:52 p.m.
Message ID <d4a3c5969df336e55646.1522255934@workspace>
Download mbox | patch
Permalink /patch/29921/
State Superseded
Headers show

Comments

sushil khanchi - March 28, 2018, 4:52 p.m.
# HG changeset patch
# User Sushil khanchi <sushilkhanchi97@gmail.com>
# Date 1521655571 -19800
#      Wed Mar 21 23:36:11 2018 +0530
# Node ID d4a3c5969df336e556463a1b0b49132dc907f536
# Parent  b6a4881cec1937a8d9cd2e9bbbdf5ca31cfa73dd
remove: add dry-run functionality
Yuya Nishihara - March 29, 2018, 11:41 a.m.
On Wed, 28 Mar 2018 22:22:14 +0530, Sushil khanchi wrote:
> # HG changeset patch
> # User Sushil khanchi <sushilkhanchi97@gmail.com>
> # Date 1521655571 -19800
> #      Wed Mar 21 23:36:11 2018 +0530
> # Node ID d4a3c5969df336e556463a1b0b49132dc907f536
> # Parent  b6a4881cec1937a8d9cd2e9bbbdf5ca31cfa73dd
> remove: add dry-run functionality

> -def removelargefiles(ui, repo, isaddremove, matcher, **opts):
> +def removelargefiles(ui, repo, isaddremove, matcher, dryrun, **opts):
>      after = opts.get(r'after')
>      m = composelargefilematcher(matcher, repo[None].manifest())
>      try:
> @@ -223,11 +223,11 @@
>                      name = m.rel(f)
>                  ui.status(_('removing %s\n') % name)
>  
> -            if not opts.get(r'dry_run'):
> +            if not dryrun:

[...]

>  def overridestatusfn(orig, repo, rev2, **opts):
> @@ -1238,7 +1240,7 @@
>          matchfn = m.matchfn
>          m.matchfn = lambda f: f in s.deleted and matchfn(f)
>  
> -        removelargefiles(repo.ui, repo, True, m, **pycompat.strkwargs(opts))
> +        removelargefiles(repo.ui, repo, True, m, dry_run, **pycompat.strkwargs(opts))

Well, dry_run does not always equal to opts.get('dry_run').

Perhaps opts should be removed from scmutil.addremove(), but that's out
of the scope of this patch.
sushil khanchi - March 29, 2018, 12:29 p.m.
Shall I send a new patch which will remove opts from scmutil.addremove() ?

Perhaps opts should be removed from scmutil.addremove(), but that's out
> of the scope of this patch.
>
Yuya Nishihara - March 29, 2018, 12:50 p.m.
On Thu, 29 Mar 2018 17:59:48 +0530, sushil khanchi wrote:
> Shall I send a new patch which will remove opts from scmutil.addremove() ?

Yes, please. (And make sure all tests pass before sending out the patch.)

Patch

diff -r b6a4881cec19 -r d4a3c5969df3 hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py	Sun Mar 18 15:32:49 2018 -0400
+++ b/hgext/largefiles/overrides.py	Wed Mar 21 23:36:11 2018 +0530
@@ -178,7 +178,7 @@ 
         added = [f for f in lfnames if f not in bad]
     return added, bad
 
-def removelargefiles(ui, repo, isaddremove, matcher, **opts):
+def removelargefiles(ui, repo, isaddremove, matcher, dryrun, **opts):
     after = opts.get(r'after')
     m = composelargefilematcher(matcher, repo[None].manifest())
     try:
@@ -223,11 +223,11 @@ 
                     name = m.rel(f)
                 ui.status(_('removing %s\n') % name)
 
-            if not opts.get(r'dry_run'):
+            if not dryrun:
                 if not after:
                     repo.wvfs.unlinkpath(f, ignoremissing=True)
 
-        if opts.get(r'dry_run'):
+        if dryrun:
             return result
 
         remove = [lfutil.standin(f) for f in remove]
@@ -271,10 +271,12 @@ 
     bad.extend(f for f in lbad)
     return bad
 
-def cmdutilremove(orig, ui, repo, matcher, prefix, after, force, subrepos):
+def cmdutilremove(orig, ui, repo, matcher, prefix, after, force, subrepos,
+                  dryrun):
     normalmatcher = composenormalfilematcher(matcher, repo[None].manifest())
-    result = orig(ui, repo, normalmatcher, prefix, after, force, subrepos)
-    return removelargefiles(ui, repo, False, matcher, after=after,
+    result = orig(ui, repo, normalmatcher, prefix, after, force, subrepos,
+                  dryrun)
+    return removelargefiles(ui, repo, False, matcher, dryrun, after=after,
                             force=force) or result
 
 def overridestatusfn(orig, repo, rev2, **opts):
@@ -1238,7 +1240,7 @@ 
         matchfn = m.matchfn
         m.matchfn = lambda f: f in s.deleted and matchfn(f)
 
-        removelargefiles(repo.ui, repo, True, m, **pycompat.strkwargs(opts))
+        removelargefiles(repo.ui, repo, True, m, dry_run, **pycompat.strkwargs(opts))
     # Call into the normal add code, and any files that *should* be added as
     # largefiles will be
     added, bad = addlargefiles(repo.ui, repo, True, matcher,
diff -r b6a4881cec19 -r d4a3c5969df3 mercurial/cmdutil.py
--- a/mercurial/cmdutil.py	Sun Mar 18 15:32:49 2018 -0400
+++ b/mercurial/cmdutil.py	Wed Mar 21 23:36:11 2018 +0530
@@ -2076,7 +2076,7 @@ 
 
     return ret
 
-def remove(ui, repo, m, prefix, after, force, subrepos, warnings=None):
+def remove(ui, repo, m, prefix, after, force, subrepos, dryrun, warnings=None):
     join = lambda f: os.path.join(prefix, f)
     ret = 0
     s = repo.status(match=m, clean=True)
@@ -2101,7 +2101,7 @@ 
             sub = wctx.sub(subpath)
             try:
                 if sub.removefiles(submatch, prefix, after, force, subrepos,
-                                   warnings):
+                                   dryrun, warnings):
                     ret = 1
             except error.LookupError:
                 warnings.append(_("skipping missing subrepository: %s\n")
@@ -2181,13 +2181,14 @@ 
             ui.status(_('removing %s\n') % m.rel(f))
     ui.progress(_('deleting'), None)
 
-    with repo.wlock():
-        if not after:
-            for f in list:
-                if f in added:
-                    continue # we never unlink added files on remove
-                repo.wvfs.unlinkpath(f, ignoremissing=True)
-        repo[None].forget(list)
+    if not dryrun:
+        with repo.wlock():
+            if not after:
+                for f in list:
+                    if f in added:
+                        continue # we never unlink added files on remove
+                    repo.wvfs.unlinkpath(f, ignoremissing=True)
+            repo[None].forget(list)
 
     if warn:
         for warning in warnings:
diff -r b6a4881cec19 -r d4a3c5969df3 mercurial/commands.py
--- a/mercurial/commands.py	Sun Mar 18 15:32:49 2018 -0400
+++ b/mercurial/commands.py	Wed Mar 21 23:36:11 2018 +0530
@@ -4207,7 +4207,7 @@ 
     [('A', 'after', None, _('record delete for missing files')),
     ('f', 'force', None,
      _('forget added files, delete modified files')),
-    ] + subrepoopts + walkopts,
+    ] + subrepoopts + walkopts + dryrunopts,
     _('[OPTION]... FILE...'),
     inferrepo=True)
 def remove(ui, repo, *pats, **opts):
@@ -4251,12 +4251,14 @@ 
 
     opts = pycompat.byteskwargs(opts)
     after, force = opts.get('after'), opts.get('force')
+    dryrun = opts.get('dry_run')
     if not pats and not after:
         raise error.Abort(_('no files specified'))
 
     m = scmutil.match(repo[None], pats, opts)
     subrepos = opts.get('subrepos')
-    return cmdutil.remove(ui, repo, m, "", after, force, subrepos)
+    return cmdutil.remove(ui, repo, m, "", after, force, subrepos,
+                          dryrun=dryrun)
 
 @command('rename|move|mv',
     [('A', 'after', None, _('record a rename that has already occurred')),
diff -r b6a4881cec19 -r d4a3c5969df3 mercurial/subrepo.py
--- a/mercurial/subrepo.py	Sun Mar 18 15:32:49 2018 -0400
+++ b/mercurial/subrepo.py	Wed Mar 21 23:36:11 2018 +0530
@@ -351,7 +351,8 @@ 
     def forget(self, match, prefix, dryrun):
         return ([], [])
 
-    def removefiles(self, matcher, prefix, after, force, subrepos, warnings):
+    def removefiles(self, matcher, prefix, after, force, subrepos,
+                    dryrun, warnings):
         """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.
@@ -817,10 +818,11 @@ 
                               True, dryrun=dryrun)
 
     @annotatesubrepoerror
-    def removefiles(self, matcher, prefix, after, force, subrepos, warnings):
+    def removefiles(self, matcher, prefix, after, force, subrepos,
+                    dryrun, warnings):
         return cmdutil.remove(self.ui, self._repo, matcher,
                               self.wvfs.reljoin(prefix, self._path),
-                              after, force, subrepos)
+                              after, force, subrepos, dryrun)
 
     @annotatesubrepoerror
     def revert(self, substate, *pats, **opts):
diff -r b6a4881cec19 -r d4a3c5969df3 tests/test-completion.t
--- a/tests/test-completion.t	Sun Mar 18 15:32:49 2018 -0400
+++ b/tests/test-completion.t	Wed Mar 21 23:36:11 2018 +0530
@@ -238,7 +238,7 @@ 
   merge: force, rev, preview, abort, tool
   pull: update, force, rev, bookmark, branch, ssh, remotecmd, insecure
   push: force, rev, bookmark, branch, new-branch, pushvars, ssh, remotecmd, insecure
-  remove: after, force, subrepos, include, exclude
+  remove: after, force, subrepos, include, exclude, dry-run
   serve: accesslog, daemon, daemon-postexec, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate, subrepos
   status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, terse, copies, print0, rev, change, include, exclude, subrepos, template
   summary: remote
diff -r b6a4881cec19 -r d4a3c5969df3 tests/test-help.t
--- a/tests/test-help.t	Sun Mar 18 15:32:49 2018 -0400
+++ b/tests/test-help.t	Wed Mar 21 23:36:11 2018 +0530
@@ -2845,6 +2845,9 @@ 
   <tr><td>-X</td>
   <td>--exclude PATTERN [+]</td>
   <td>exclude names matching the given patterns</td></tr>
+  <tr><td>-n</td>
+  <td>--dry-run</td>
+  <td>do not perform actions, just print output</td></tr>
   </table>
   <p>
   global options ([+] can be repeated):
diff -r b6a4881cec19 -r d4a3c5969df3 tests/test-remove.t
--- a/tests/test-remove.t	Sun Mar 18 15:32:49 2018 -0400
+++ b/tests/test-remove.t	Wed Mar 21 23:36:11 2018 +0530
@@ -505,3 +505,32 @@ 
   deleting [===========================================>] 1/1\r (no-eol) (esc)
                                                               \r (no-eol) (esc)
   [1]
+
+test dry-run for remove
+
+  $ hg init testdryrun
+  $ cd testdryrun
+  $ echo a>a
+  $ hg ci -qAm1
+  $ hg remove a -nv
+  \r (no-eol) (esc)
+  deleting [===========================================>] 1/1\r (no-eol) (esc)
+                                                              \r (no-eol) (esc)
+  \r (no-eol) (esc)
+  deleting [===========================================>] 1/1\r (no-eol) (esc)
+                                                              \r (no-eol) (esc)
+  removing a
+  $ hg diff
+
+  $ cat >> .hg/hgrc <<EOF
+  > [extensions]
+  > largefiles=
+  > EOF
+  $ echo 'B as largefile' > B
+  $ hg add --large B
+  $ hg ci -m "B"
+  $ hg remove B -nv
+  removing B
+  $ hg st
+
+  $ cd ..