This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3858

unlinkpath: make empty directory removal optional (issue5901) (issue5826)
AbandonedPublic

Authored by spectral on Jun 28 2018, 9:09 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

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

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

spectral created this revision.Jun 28 2018, 9:09 PM
spectral abandoned this revision.Jun 28 2018, 9:39 PM

(Sorry for the double send)

Revision Contents
Changeset List

Diff 9349

mercurial/cmdutil.py

targets[abstarget] = abssrc targets[abstarget] = abssrc
# fix up dirstate # fix up dirstate
scmutil.dirstatecopy(ui, repo, wctx, abssrc, abstarget, scmutil.dirstatecopy(ui, repo, wctx, abssrc, abstarget,
dryrun=dryrun, cwd=cwd) dryrun=dryrun, cwd=cwd)
if rename and not dryrun: if rename and not dryrun:
if not after and srcexists and not samefile: 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]) wctx.forget([abssrc])
# pat: ossep # pat: ossep
# dest ossep # dest ossep
# srcs: list of (hgsep, hgsep, ossep, bool) # srcs: list of (hgsep, hgsep, ossep, bool)
# return: function that takes hgsep and returns ossep # return: function that takes hgsep and returns ossep
def targetpathfn(pat, dest, srcs): def targetpathfn(pat, dest, srcs):
if os.path.isdir(pat): if os.path.isdir(pat):
progress.complete() progress.complete()
if not dryrun: if not dryrun:
with repo.wlock(): with repo.wlock():
if not after: if not after:
for f in list: for f in list:
if f in added: if f in added:
continue # we never unlink added files on remove 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) repo[None].forget(list)
if warn: if warn:
for warning in warnings: for warning in warnings:
ui.warn(warning) ui.warn(warning)
return ret return ret
excluded_files = [] excluded_files = []
def checkout(f): def checkout(f):
fc = ctx[f] fc = ctx[f]
repo.wwrite(f, fc.data(), fc.flags()) repo.wwrite(f, fc.data(), fc.flags())
def doremove(f): def doremove(f):
try: try:
repo.wvfs.unlinkpath(f) rmdir = repo.ui.configbool('experimental', 'removeemptydirs')
repo.wvfs.unlinkpath(f, rmdir=rmdir)
except OSError: except OSError:
pass pass
repo.dirstate.remove(f) repo.dirstate.remove(f)
audit_path = pathutil.pathauditor(repo.root, cached=True) audit_path = pathutil.pathauditor(repo.root, cached=True)
for f in actions['forget'][0]: for f in actions['forget'][0]:
if interactive: if interactive:
choice = repo.ui.promptchoice( choice = repo.ui.promptchoice(

mercurial/configitems.py

default=None, default=None,
) )
coreconfigitem('experimental', 'obsmarkers-exchange-debug', coreconfigitem('experimental', 'obsmarkers-exchange-debug',
default=False, default=False,
) )
coreconfigitem('experimental', 'remotenames', coreconfigitem('experimental', 'remotenames',
default=False, default=False,
) )
coreconfigitem('experimental', 'removeemptydirs',
default=True,
)
coreconfigitem('experimental', 'revlogv2', coreconfigitem('experimental', 'revlogv2',
default=None, default=None,
) )
coreconfigitem('experimental', 'single-head-per-branch', coreconfigitem('experimental', 'single-head-per-branch',
default=False, default=False,
) )
coreconfigitem('experimental', 'sshserver.support-v2', coreconfigitem('experimental', 'sshserver.support-v2',
default=False, default=False,

mercurial/context.py

returns True if different than fctx. returns True if different than fctx.
""" """
# fctx should be a filectx (not a workingfilectx) # fctx should be a filectx (not a workingfilectx)
# invert comparison to reuse the same code path # invert comparison to reuse the same code path
return fctx.cmp(self) return fctx.cmp(self)
def remove(self, ignoremissing=False): def remove(self, ignoremissing=False):
"""wraps unlink for a repo's working directory""" """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): def write(self, data, flags, backgroundclose=False, **kwargs):
"""wraps repo.wwrite""" """wraps repo.wwrite"""
self._repo.wwrite(self._path, data, flags, self._repo.wwrite(self._path, data, flags,
backgroundclose=backgroundclose, backgroundclose=backgroundclose,
**kwargs) **kwargs)
def markcopied(self, src): def markcopied(self, src):

mercurial/patch.py

if islink: if islink:
self.opener.symlink(data, fname) self.opener.symlink(data, fname)
else: else:
self.opener.write(fname, data) self.opener.write(fname, data)
if isexec: if isexec:
self.opener.setflags(fname, False, True) self.opener.setflags(fname, False, True)
def unlink(self, fname): 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): def writerej(self, fname, failed, total, lines):
fname = fname + ".rej" fname = fname + ".rej"
self.ui.warn( self.ui.warn(
_("%d out of %d hunks FAILED -- saving rejects to file %s\n") % _("%d out of %d hunks FAILED -- saving rejects to file %s\n") %
(failed, total, fname)) (failed, total, fname))
fp = self.opener(fname, 'w') fp = self.opener(fname, 'w')
fp.writelines(lines) fp.writelines(lines)

mercurial/util.py

return self return self
def __exit__(self, exctype, excvalue, traceback): def __exit__(self, exctype, excvalue, traceback):
if exctype is not None: if exctype is not None:
self.discard() self.discard()
else: else:
self.close() self.close()
def unlinkpath(f, ignoremissing=False): def unlinkpath(f, ignoremissing=False, rmdir=True):
"""unlink and remove the directory if it is empty""" """unlink and remove the directory if it is empty"""
if ignoremissing: if ignoremissing:
tryunlink(f) tryunlink(f)
else: else:
unlink(f) unlink(f)
if rmdir:
# try removing directories that might now be empty # try removing directories that might now be empty
try: try:
removedirs(os.path.dirname(f)) removedirs(os.path.dirname(f))
except OSError: except OSError:
pass pass
def tryunlink(f): def tryunlink(f):
"""Attempt to remove a file, ignoring ENOENT errors.""" """Attempt to remove a file, ignoring ENOENT errors."""
try: try:
unlink(f) unlink(f)
except OSError as e: except OSError as e:
if e.errno != errno.ENOENT: if e.errno != errno.ENOENT:
raise raise

mercurial/vfs.py

def unlink(self, path=None): def unlink(self, path=None):
return util.unlink(self.join(path)) return util.unlink(self.join(path))
def tryunlink(self, path=None): def tryunlink(self, path=None):
"""Attempt to remove a file, ignoring missing file errors.""" """Attempt to remove a file, ignoring missing file errors."""
util.tryunlink(self.join(path)) util.tryunlink(self.join(path))
def unlinkpath(self, path=None, ignoremissing=False): def unlinkpath(self, path=None, ignoremissing=False, rmdir=True):
return util.unlinkpath(self.join(path), ignoremissing=ignoremissing) return util.unlinkpath(self.join(path), ignoremissing=ignoremissing,
rmdir=rmdir)
def utime(self, path=None, t=None): def utime(self, path=None, t=None):
return os.utime(self.join(path), t) return os.utime(self.join(path), t)
def walk(self, path=None, onerror=None): def walk(self, path=None, onerror=None):
"""Yield (dirpath, dirs, files) tuple for each directories under path """Yield (dirpath, dirs, files) tuple for each directories under path
``dirpath`` is relative one from the root of this vfs. This ``dirpath`` is relative one from the root of this vfs. This

tests/test-removeemptydirs.t

  • This file was added.
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