| hg-reviewers |
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
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| tests/test-removeemptydirs.t | ||
|---|---|---|
| 126 | I'm rather uncertain about this, but there's no generic 'linux' in hghave at the moment; I'm testing on my mac now. | |
Queued, thanks.
+ $ 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 }
changed to [ ... ] since [[ ... ]] requires bash.
| mercurial/configitems.py | ||
|---|---|---|
| 569 | Not a huge deal for experimental options, but be aware of https://www.mercurial-scm.org/wiki/UIGuideline#naming_config_options (which recommends remove-empty-dirs). | |
| Path | Packages | |||
|---|---|---|---|---|
| M | mercurial/cmdutil.py (10 lines) | |||
| M | mercurial/configitems.py (3 lines) | |||
| M | mercurial/context.py (4 lines) | |||
| M | mercurial/patch.py (3 lines) | |||
| M | mercurial/util.py (13 lines) | |||
| M | mercurial/vfs.py (5 lines) | |||
| A | M | tests/test-removeemptydirs.t (242 lines) |
| 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( | ||||
| 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): | ||||
| 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) | ||||
| 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 | ||||
| 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 | ||||
Not a huge deal for experimental options, but be aware of https://www.mercurial-scm.org/wiki/UIGuideline#naming_config_options (which recommends remove-empty-dirs).