[PATCH 1 of 1] commands.remove: don't use workingctx.remove(list, unlink=True)

Adrian Buehlmann adrian at cadifra.com
Wed May 25 07:45:08 CDT 2011

# HG changeset patch
# User Adrian Buehlmann <adrian at cadifra.com>
# Date 1306313693 -7200
# Node ID db3623327c9c02f44b6bd9374dfa8594b326820d
# Parent  a6b543e053058c39b52e2b7c1e9a4b7c14c66a56
commands.remove: don't use workingctx.remove(list, unlink=True)

workingctx.remove(list, unlink=True) is unsuited here, because it does too
much: it also unlinks added files.

But the command 'hg remove' is specified to *never* unlink added files.

Instead, we now unlink the files at the commands.remove level (if --after was
not specified), explicitly and carefully excluding the added files, and then
make a bare workingctx.remove(list) call (implying unlink=False, using the
default for the unlink parameter of workingctx.remove).

This spares us from having to resort to using workingctx.forget *just* because
we don't want to have the added files deleted, making the implementation
of commands.remove more explicit, straightforward to read and understand.

As an added bonus, this happens to eliminate a wlock acquire/release pair,
since the previous implementation caused

   acquire wlock
   release wlock
   acquire wlock
   release wlock

where the first pair of acquire/release was caused by the workingctx.forget
call, and the second by the workingctx.remove call.

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -8,7 +8,7 @@
 from node import hex, bin, nullid, nullrev, short
 from lock import release
 from i18n import _, gettext
-import os, re, sys, difflib, time, tempfile
+import os, re, sys, difflib, time, tempfile, errno
 import hg, scmutil, util, revlog, extensions, copies, error, bookmarks
 import patch, help, url, encoding, templatekw, discovery
 import archival, changegroup, cmdutil, sshserver, hbisect, hgweb, hgweb.server
@@ -3918,15 +3918,15 @@
             ret = 1
     if force:
-        remove, forget = modified + deleted + clean, added
+        list = modified + deleted + clean + added
     elif after:
-        remove, forget = deleted, []
+        list = deleted
         for f in modified + added + clean:
             ui.warn(_('not removing %s: file still exists (use -f'
                       ' to force removal)\n') % m.rel(f))
             ret = 1
-        remove, forget = deleted + clean, []
+        list = deleted + clean
         for f in modified:
             ui.warn(_('not removing %s: file is modified (use -f'
                       ' to force removal)\n') % m.rel(f))
@@ -3936,12 +3936,25 @@
                       ' to force removal)\n') % m.rel(f))
             ret = 1
-    for f in sorted(remove + forget):
+    for f in sorted(list):
         if ui.verbose or not m.exact(f):
             ui.status(_('removing %s\n') % m.rel(f))
-    repo[None].forget(forget)
-    repo[None].remove(remove, unlink=not after)
+    wlock = repo.wlock()
+    try:
+        if not after:
+            for f in list:
+                if f in added:
+                    continue # we never unlink added files on remove
+                try:
+                    util.unlinkpath(repo.wjoin(f))
+                except OSError, inst:
+                    if inst.errno != errno.ENOENT:
+                        raise
+        repo[None].remove(list)
+    finally:
+        wlock.release()
     return ret

More information about the Mercurial-devel mailing list