unlinkpath: make empty directory removal optional (
issue5901) (
issue5826)
There are known cases where performing operations such as rebase from a
directory that is newly created can fail or at least lead to being in a
directory handle that no longer exists.
This is even reproducible by just doing something as simple as:
cd foo; hg rm *
The behavior is different if you use `hg addremove`, the directory is not
removed until we attempt to go back to the node after committing it:
cd foo; rm *; hg addremove; hg ci -m'bye foo'; hg co .^; hg co tip
Differential Revision: https://phab.mercurial-scm.org/D3859
--- a/mercurial/cmdutil.py Thu Jun 28 21:24:47 2018 +0530
+++ b/mercurial/cmdutil.py Thu Jun 28 18:07:22 2018 -0700
@@ -1242,7 +1242,8 @@
dryrun=dryrun, cwd=cwd)
if rename and not dryrun:
if not after and srcexists and not samefile:
- repo.wvfs.unlinkpath(abssrc)
+ rmdir = repo.ui.configbool('experimental', 'removeemptydirs')
+ repo.wvfs.unlinkpath(abssrc, rmdir=rmdir)
wctx.forget([abssrc])
# pat: ossep
@@ -2269,7 +2270,9 @@
for f in list:
if f in added:
continue # we never unlink added files on remove
- repo.wvfs.unlinkpath(f, ignoremissing=True)
+ rmdir = repo.ui.configbool('experimental',
+ 'removeemptydirs')
+ repo.wvfs.unlinkpath(f, ignoremissing=True, rmdir=rmdir)
repo[None].forget(list)
if warn:
@@ -3029,7 +3032,8 @@
def doremove(f):
try:
- repo.wvfs.unlinkpath(f)
+ rmdir = repo.ui.configbool('experimental', 'removeemptydirs')
+ repo.wvfs.unlinkpath(f, rmdir=rmdir)
except OSError:
pass
repo.dirstate.remove(f)
--- a/mercurial/configitems.py Thu Jun 28 21:24:47 2018 +0530
+++ b/mercurial/configitems.py Thu Jun 28 18:07:22 2018 -0700
@@ -566,6 +566,9 @@
coreconfigitem('experimental', 'remotenames',
default=False,
)
+coreconfigitem('experimental', 'removeemptydirs',
+ default=True,
+)
coreconfigitem('experimental', 'revlogv2',
default=None,
)
--- a/mercurial/context.py Thu Jun 28 21:24:47 2018 +0530
+++ b/mercurial/context.py Thu Jun 28 18:07:22 2018 -0700
@@ -1707,7 +1707,9 @@
def remove(self, ignoremissing=False):
"""wraps unlink for a repo's working directory"""
- self._repo.wvfs.unlinkpath(self._path, ignoremissing=ignoremissing)
+ rmdir = self._repo.ui.configbool('experimental', 'removeemptydirs')
+ self._repo.wvfs.unlinkpath(self._path, ignoremissing=ignoremissing,
+ rmdir=rmdir)
def write(self, data, flags, backgroundclose=False, **kwargs):
"""wraps repo.wwrite"""
--- a/mercurial/patch.py Thu Jun 28 21:24:47 2018 +0530
+++ b/mercurial/patch.py Thu Jun 28 18:07:22 2018 -0700
@@ -497,7 +497,8 @@
self.opener.setflags(fname, False, True)
def unlink(self, fname):
- self.opener.unlinkpath(fname, ignoremissing=True)
+ rmdir = self.ui.configbool('experimental', 'removeemptydirs')
+ self.opener.unlinkpath(fname, ignoremissing=True, rmdir=rmdir)
def writerej(self, fname, failed, total, lines):
fname = fname + ".rej"
--- a/mercurial/util.py Thu Jun 28 21:24:47 2018 +0530
+++ b/mercurial/util.py Thu Jun 28 18:07:22 2018 -0700
@@ -2139,17 +2139,18 @@
else:
self.close()
-def unlinkpath(f, ignoremissing=False):
+def unlinkpath(f, ignoremissing=False, rmdir=True):
"""unlink and remove the directory if it is empty"""
if ignoremissing:
tryunlink(f)
else:
unlink(f)
- # try removing directories that might now be empty
- try:
- removedirs(os.path.dirname(f))
- except OSError:
- pass
+ if rmdir:
+ # try removing directories that might now be empty
+ try:
+ removedirs(os.path.dirname(f))
+ except OSError:
+ pass
def tryunlink(f):
"""Attempt to remove a file, ignoring ENOENT errors."""
--- a/mercurial/vfs.py Thu Jun 28 21:24:47 2018 +0530
+++ b/mercurial/vfs.py Thu Jun 28 18:07:22 2018 -0700
@@ -246,8 +246,9 @@
"""Attempt to remove a file, ignoring missing file errors."""
util.tryunlink(self.join(path))
- def unlinkpath(self, path=None, ignoremissing=False):
- return util.unlinkpath(self.join(path), ignoremissing=ignoremissing)
+ def unlinkpath(self, path=None, ignoremissing=False, rmdir=True):
+ return util.unlinkpath(self.join(path), ignoremissing=ignoremissing,
+ rmdir=rmdir)
def utime(self, path=None, t=None):
return os.utime(self.join(path), t)
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/test-removeemptydirs.t Thu Jun 28 18:07:22 2018 -0700
@@ -0,0 +1,242 @@
+Tests for experimental.removeemptydirs
+
+ $ NO_RM=--config=experimental.removeemptydirs=0
+ $ isdir() { if [ -d $1 ]; then echo yes; else echo no; fi }
+ $ isfile() { if [ -f $1 ]; then echo yes; else echo no; fi }
+
+`hg rm` of the last file in a directory:
+ $ hg init hgrm
+ $ cd hgrm
+ $ mkdir somedir
+ $ echo hi > somedir/foo
+ $ hg ci -qAm foo
+ $ isdir somedir
+ yes
+ $ hg rm somedir/foo
+ $ isdir somedir
+ no
+ $ hg revert -qa
+ $ isdir somedir
+ yes
+ $ hg $NO_RM rm somedir/foo
+ $ isdir somedir
+ yes
+ $ ls somedir
+ $ cd $TESTTMP
+
+`hg mv` of the last file in a directory:
+ $ hg init hgmv
+ $ cd hgmv
+ $ mkdir somedir
+ $ mkdir destdir
+ $ echo hi > somedir/foo
+ $ hg ci -qAm foo
+ $ isdir somedir
+ yes
+ $ hg mv somedir/foo destdir/foo
+ $ isdir somedir
+ no
+ $ hg revert -qa
+(revert doesn't get rid of destdir/foo?)
+ $ rm destdir/foo
+ $ isdir somedir
+ yes
+ $ hg $NO_RM mv somedir/foo destdir/foo
+ $ isdir somedir
+ yes
+ $ ls somedir
+ $ cd $TESTTMP
+
+Updating to a commit that doesn't have the directory:
+ $ hg init hgupdate
+ $ cd hgupdate
+ $ echo hi > r0
+ $ hg ci -qAm r0
+ $ mkdir somedir
+ $ echo hi > somedir/foo
+ $ hg ci -qAm r1
+ $ isdir somedir
+ yes
+ $ hg co -q -r ".^"
+ $ isdir somedir
+ no
+ $ hg co -q tip
+ $ isdir somedir
+ yes
+ $ hg $NO_RM co -q -r ".^"
+ $ isdir somedir
+ yes
+ $ ls somedir
+ $ cd $TESTTMP
+
+Rebasing across a commit that doesn't have the directory, from inside the
+directory:
+ $ hg init hgrebase
+ $ cd hgrebase
+ $ echo hi > r0
+ $ hg ci -qAm r0
+ $ mkdir somedir
+ $ echo hi > somedir/foo
+ $ hg ci -qAm first_rebase_source
+ $ hg $NO_RM co -q -r ".^"
+ $ echo hi > somedir/bar
+ $ hg ci -qAm first_rebase_dest
+ $ hg $NO_RM co -q -r ".^"
+ $ echo hi > somedir/baz
+ $ hg ci -qAm second_rebase_dest
+ $ hg co -qr 'desc(first_rebase_source)'
+ $ cd $TESTTMP/hgrebase/somedir
+ $ hg --config extensions.rebase= rebase -qr . -d 'desc(first_rebase_dest)'
+ current directory was removed
+ (consider changing to repo root: $TESTTMP/hgrebase)
+ $ cd $TESTTMP/hgrebase/somedir
+(The current node is the rebased first_rebase_source on top of
+first_rebase_dest)
+This should not output anything about current directory being removed:
+ $ hg $NO_RM --config extensions.rebase= rebase -qr . -d 'desc(second_rebase_dest)'
+ $ cd $TESTTMP
+
+Histediting across a commit that doesn't have the directory, from inside the
+directory (reordering nodes):
+ $ hg init hghistedit
+ $ cd hghistedit
+ $ echo hi > r0
+ $ hg ci -qAm r0
+ $ echo hi > r1
+ $ hg ci -qAm r1
+ $ echo hi > r2
+ $ hg ci -qAm r2
+ $ mkdir somedir
+ $ echo hi > somedir/foo
+ $ hg ci -qAm migrating_revision
+ $ cat > histedit_commands <<EOF
+ > pick 89079fab8aee 0 r0
+ > pick e6d271df3142 1 r1
+ > pick 89e25aa83f0f 3 migrating_revision
+ > pick b550aa12d873 2 r2
+ > EOF
+ $ cd $TESTTMP/hghistedit/somedir
+ $ hg --config extensions.histedit= histedit -q --commands ../histedit_commands
+
+histedit doesn't output anything when the current diretory is removed. We rely
+on the tests being commonly run on machines where the current directory
+disappearing from underneath us actually has an observable effect, such as an
+error or no files listed
+#if linuxormacos
+ $ isfile foo
+ no
+#endif
+ $ cd $TESTTMP/hghistedit/somedir
+ $ isfile foo
+ yes
+
+ $ cd $TESTTMP/hghistedit
+ $ cat > histedit_commands <<EOF
+ > pick 89079fab8aee 0 r0
+ > pick 7c7a22c6009f 3 migrating_revision
+ > pick e6d271df3142 1 r1
+ > pick 40a53c2d4276 2 r2
+ > EOF
+ $ cd $TESTTMP/hghistedit/somedir
+ $ hg $NO_RM --config extensions.histedit= histedit -q --commands ../histedit_commands
+Regardless of system, we should always get a 'yes' here.
+ $ isfile foo
+ yes
+ $ cd $TESTTMP
+
+This is essentially the exact test from issue5826, just cleaned up a little:
+
+ $ hg init issue5826_withrm
+ $ cd issue5826_withrm
+
+ $ cat >> $HGRCPATH <<EOF
+ > [extensions]
+ > histedit =
+ > EOF
+Commit three revisions that each create a directory:
+
+ $ mkdir foo
+ $ touch foo/bar
+ $ hg commit -qAm "add foo"
+
+ $ mkdir bar
+ $ touch bar/bar
+ $ hg commit -qAm "add bar"
+
+ $ mkdir baz
+ $ touch baz/bar
+ $ hg commit -qAm "add baz"
+
+Enter the first directory:
+
+ $ cd foo
+
+Histedit doing 'pick, pick, fold':
+
+ $ hg histedit --commands /dev/stdin <<EOF
+ > pick 6274c77c93c3 1 add bar
+ > pick ff70a87b588f 0 add foo
+ > fold 9992bb0ac0db 2 add baz
+ > EOF
+ abort: $ENOENT$
+ [255]
+
+Go back to the repo root after losing it as part of that operation:
+ $ cd $TESTTMP/issue5826_withrm
+
+Note the lack of a non-zero exit code from this function - it exits
+successfully, but doesn't really do anything.
+ $ hg histedit --continue
+ 9992bb0ac0db: cannot fold - working copy is not a descendant of previous commit 5c806432464a
+ saved backup bundle to $TESTTMP/issue5826_withrm/.hg/strip-backup/ff70a87b588f-e94f9789-histedit.hg
+
+ $ hg log -T '{rev}:{node|short} {desc}\n'
+ 2:94e3f9fae1d6 fold-temp-revision 9992bb0ac0db
+ 1:5c806432464a add foo
+ 0:d17db4b0303a add bar
+
+Now test that again with experimental.removeemptydirs=false:
+ $ hg init issue5826_norm
+ $ cd issue5826_norm
+
+ $ cat >> $HGRCPATH <<EOF
+ > [extensions]
+ > histedit =
+ > [experimental]
+ > removeemptydirs = false
+ > EOF
+Commit three revisions that each create a directory:
+
+ $ mkdir foo
+ $ touch foo/bar
+ $ hg commit -qAm "add foo"
+
+ $ mkdir bar
+ $ touch bar/bar
+ $ hg commit -qAm "add bar"
+
+ $ mkdir baz
+ $ touch baz/bar
+ $ hg commit -qAm "add baz"
+
+Enter the first directory:
+
+ $ cd foo
+
+Histedit doing 'pick, pick, fold':
+
+ $ hg histedit --commands /dev/stdin <<EOF
+ > pick 6274c77c93c3 1 add bar
+ > pick ff70a87b588f 0 add foo
+ > fold 9992bb0ac0db 2 add baz
+ > EOF
+ saved backup bundle to $TESTTMP/issue5826_withrm/issue5826_norm/.hg/strip-backup/5c806432464a-cd4c8d86-histedit.hg
+
+Note the lack of a 'cd' being necessary here, and we don't need to 'histedit
+--continue'
+
+ $ hg log -T '{rev}:{node|short} {desc}\n'
+ 1:b9eddaa97cbc add foo
+ ***
+ add baz
+ 0:d17db4b0303a add bar