| Submitter | Katsunori FUJIWARA |
|---|---|
| Date | May 15, 2013, 3:44 p.m. |
| Message ID | <185f02ec640199455dab.1368632690@juju> |
| Download | mbox | patch |
| Permalink | /patch/1651/ |
| State | Superseded, archived |
| Commit | 9bfa86746c9c1f6ab51deb8f174ffc482417d09f |
| Headers | show |
Comments
On 05/15/2013 08:44 AM, FUJIWARA Katsunori wrote: > # HG changeset patch > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > # Date 1368632246 -32400 > # Thu May 16 00:37:26 2013 +0900 > # Node ID 185f02ec640199455dabfb80ebaca407f1249f82 > # Parent 06f5233fe026033740b5f7ee77234bd0a9b02016 > merge: remove directory recursively, if there is no one other than directories I'm uncomfortable with file system modifications happening in _checkunknown, or really at all during the calculate phase. mpm needs to comment here. Also maybe call the new function rmemptydirs or similar?
On Thu, 2013-05-16 at 13:26 -0700, Siddharth Agarwal wrote: > On 05/15/2013 08:44 AM, FUJIWARA Katsunori wrote: > > # HG changeset patch > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > > # Date 1368632246 -32400 > > # Thu May 16 00:37:26 2013 +0900 > > # Node ID 185f02ec640199455dabfb80ebaca407f1249f82 > > # Parent 06f5233fe026033740b5f7ee77234bd0a9b02016 > > merge: remove directory recursively, if there is no one other than directories > > I'm uncomfortable with file system modifications happening in > _checkunknown, or really at all during the calculate phase. mpm needs to > comment here. Yeah, it's a bit of a step in the wrong direction, especially given: http://mercurial.selenic.com/wiki/ConsensusMerge
At Thu, 16 May 2013 15:49:56 -0500, Matt Mackall wrote: > > On Thu, 2013-05-16 at 13:26 -0700, Siddharth Agarwal wrote: > > On 05/15/2013 08:44 AM, FUJIWARA Katsunori wrote: > > > # HG changeset patch > > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > > > # Date 1368632246 -32400 > > > # Thu May 16 00:37:26 2013 +0900 > > > # Node ID 185f02ec640199455dabfb80ebaca407f1249f82 > > > # Parent 06f5233fe026033740b5f7ee77234bd0a9b02016 > > > merge: remove directory recursively, if there is no one other than directories > > > > I'm uncomfortable with file system modifications happening in > > _checkunknown, or really at all during the calculate phase. mpm needs to > > comment here. > > Yeah, it's a bit of a step in the wrong direction, especially given: > > http://mercurial.selenic.com/wiki/ConsensusMerge Should I pass "actions" to "_checkunknown()" and store new action entry to remove empty directory tree (at "applyupate()" phase) into it, instead of removing in "_checkunknown()" immediately ? ---------------------------------------------------------------------- [FUJIWARA Katsunori] foozy@lares.dti.ne.jp
On Fri, 2013-05-17 at 17:54 +0900, FUJIWARA Katsunori wrote: > At Thu, 16 May 2013 15:49:56 -0500, > Matt Mackall wrote: > > > > On Thu, 2013-05-16 at 13:26 -0700, Siddharth Agarwal wrote: > > > On 05/15/2013 08:44 AM, FUJIWARA Katsunori wrote: > > > > # HG changeset patch > > > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> > > > > # Date 1368632246 -32400 > > > > # Thu May 16 00:37:26 2013 +0900 > > > > # Node ID 185f02ec640199455dabfb80ebaca407f1249f82 > > > > # Parent 06f5233fe026033740b5f7ee77234bd0a9b02016 > > > > merge: remove directory recursively, if there is no one other than directories > > > > > > I'm uncomfortable with file system modifications happening in > > > _checkunknown, or really at all during the calculate phase. mpm needs to > > > comment here. > > > > Yeah, it's a bit of a step in the wrong direction, especially given: > > > > http://mercurial.selenic.com/wiki/ConsensusMerge > > Should I pass "actions" to "_checkunknown()" and store new action > entry to remove empty directory tree (at "applyupate()" phase) into > it, instead of removing in "_checkunknown()" immediately ? Seems promising.
Patch
diff --git a/mercurial/merge.py b/mercurial/merge.py --- a/mercurial/merge.py +++ b/mercurial/merge.py @@ -100,7 +100,15 @@ raise return False if stat.S_ISDIR(mode): - return repo.dirstate.normalize(f) not in repo.dirstate.dirs() + if repo.dirstate.normalize(f) in repo.dirstate.dirs(): + return False + try: + repo.wvfs.rmdirtree(f) + return False + except OSError, err: + if err.errno is not errno.ENOTEMPTY: + raise + return True return (not repo.dirstate._ignore(f) and repo.wopener.audit.check(f) diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py --- a/mercurial/scmutil.py +++ b/mercurial/scmutil.py @@ -268,6 +268,24 @@ def readlink(self, path): return os.readlink(self.join(path)) + def rmdirtree(self, path=""): + '''remove specified directory and its sub-directories recursively, + if there is no one other than directories under it. + ''' + dirs = [path] + pending = [(path, n, t) for n, t in self.readdir(path)] + while pending: + dir, name, type = pending.pop() + if type != stat.S_IFDIR: + raise OSError(errno.ENOTEMPTY, + "Directory not empty: '%s'" % self.join(path)) + dir = os.path.join(dir, name) + dirs.append(dir) + pending.extend([(dir, n, t) for n, t in self.readdir(dir)]) + while dirs: + dir = dirs.pop() + os.rmdir(self.join(dir)) + def setflags(self, path, l, x): return util.setflags(self.join(path), l, x) diff --git a/tests/test-merge1.t b/tests/test-merge1.t --- a/tests/test-merge1.t +++ b/tests/test-merge1.t @@ -184,10 +184,13 @@ a b c - $ mkdir c + $ mkdir -p c/d/e/f c/d/g c/d/h/i/j + $ touch c/d/e/k $ hg update -q 3 c: untracked file differs abort: untracked files in working directory differ from files in requested revision [255] + $ rm c/d/e/k + $ hg update -q 3 $ cd ..
# HG changeset patch # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp> # Date 1368632246 -32400 # Thu May 16 00:37:26 2013 +0900 # Node ID 185f02ec640199455dabfb80ebaca407f1249f82 # Parent 06f5233fe026033740b5f7ee77234bd0a9b02016 merge: remove directory recursively, if there is no one other than directories Before this patch, merging/updating is aborted, if there is the directory, which is not related to tracked file in wctx and collides against the file in the target context, even if it is empty. This patch tries to remove colliding directory recursively, if there is no one other than directories under it. "vfs.rmdirtree()", which is introduced by this patch, differs from functions below: - "os.removedirs(dir)" removes specified "dir" as a leaf directory and all empty intermediate ones, but doesn't remove sub-directories of "dir" - "shutil.rmtree(dir)" removes not only specified "dir" and its sub-directories, but also files under it - "util.unlinkpath(f)" expects that "f" is a file (and acts on parent of "f" like "os.removedirs()")