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 ..