Patchwork [3,of,7,STABLE] merge: check whether existing directory is related to tracked files or not

login
register
mail settings
Submitter Katsunori FUJIWARA
Date May 6, 2013, 8:35 p.m.
Message ID <d12b0211d5ea3b6e30a7.1367872503@juju>
Download mbox | patch
Permalink /patch/1568/
State Superseded, archived
Commit 9bfa86746c9c1f6ab51deb8f174ffc482417d09f
Headers show

Comments

Katsunori FUJIWARA - May 6, 2013, 8:35 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1367870651 -32400
#      Tue May 07 05:04:11 2013 +0900
# Branch stable
# Node ID d12b0211d5ea3b6e30a732a9d5381314a83862b1
# Parent  1bede396d3f4dfd7763e214befd12fea9df63130
merge: check whether existing directory is related to tracked files or not

Before this patch, updating/merging is aborted and updates working
directory status incompletely, when there is the directory of which
name is as same as one in target context.

This patch checks type of the specified file in the working directory,
and check whether it is related to tracked files if it is directory.

If so, this can be ignored, because collision check in
"_checkcollision()" can recognize this directory. Otherwise, this
directory collides the file in the target context, and
merging/updating should be aborted.

This patch adds "lstat()" to vfs instead of using "os.lstat()" or
"vfs.isdir()", because:

  - "os.lstat()" requires path composition like "repo.wjoin(f)"

  - "vfs.isdir()" (= "os.isdir()") recognizes also symbolic link to
    directory as directory unexpectedly
Matt Mackall - May 7, 2013, 9:44 p.m.
On Tue, 2013-05-07 at 05:35 +0900, FUJIWARA Katsunori wrote:
> #  $ mkdir c
> +  $ hg update -q 3
> +  c: untracked file differs
> +  abort: untracked files in working directory differ from files in requested revision

I'm not sure this is the correct result, given that the directory is
empty. Mercurial is happy to delete/ignore empty directories in other
cases.
Katsunori FUJIWARA - May 8, 2013, 2:37 p.m.
At Tue, 07 May 2013 16:44:25 -0500,
Matt Mackall wrote:
> 
> On Tue, 2013-05-07 at 05:35 +0900, FUJIWARA Katsunori wrote:
> > #  $ mkdir c
> > +  $ hg update -q 3
> > +  c: untracked file differs
> > +  abort: untracked files in working directory differ from files in requested revision
> 
> I'm not sure this is the correct result, given that the directory is
> empty. Mercurial is happy to delete/ignore empty directories in other
> cases.

Thank you for your comment.

I'll re-send the patch which tries to delete empty directories.


BTW, can I assume things below ?

  - it is acceptable that some (empty) directories are deleted, but
    others are not, when merging/updating is aborted halfway

    Or should we follow "all or nothing" rule ?

  - "rmdir(2)" on non empty directory should always fail

    At least in the 20th century (or early 1990s ?), "unlink(2)" on
    directory should always fail, shouldn't it ?

    But according to "unlink(2)" man page on systems conforming
    POSIX.1-2001, "unlink(2)" on directory may be allowed, even though
    it fails on almost all filesystems.

    I'm afraid that I overlook such kind of specification changes on
    "rmdir(2)": it may cause unexpected deleting non-empty directory
    on some filesystems.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -9,7 +9,7 @@ 
 from i18n import _
 from mercurial import obsolete
 import error, util, filemerge, copies, subrepo, worker, dicthelpers, scmutil
-import errno, os, shutil
+import errno, os, shutil, stat
 
 class mergestate(object):
     '''track 3-way merge state of individual files'''
@@ -93,8 +93,15 @@ 
         return r
 
 def _checkunknownfile(repo, wctx, mctx, f):
+    try:
+        mode = repo.wvfs.lstat(f).st_mode
+    except OSError, err:
+        if err.errno not in (errno.ENOENT, errno.ENOTDIR):
+            raise
+        return False
+    if stat.S_ISDIR(mode):
+        return repo.dirstate.normalize(f) not in repo.dirstate.dirs()
     return (not repo.dirstate._ignore(f)
-        and os.path.isfile(repo.wjoin(f))
         and repo.dirstate.normalize(f) not in repo.dirstate
         and mctx[f].cmp(wctx[f]))
 
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -247,6 +247,9 @@ 
     def islink(self, path=None):
         return os.path.islink(self.join(path))
 
+    def lstat(self, path=None):
+        return os.lstat(self.join(path))
+
     def makedir(self, path=None, notindexed=True):
         return util.makedir(self.join(path), notindexed)
 
diff --git a/tests/test-merge1.t b/tests/test-merge1.t
--- a/tests/test-merge1.t
+++ b/tests/test-merge1.t
@@ -196,4 +196,21 @@ 
   abort: collision between file c and other directory
   [255]
 
+test detecting collision between files in target context and unknown
+ones in working directory
+
+  $ hg update -q -C 1
+  $ hg manifest -r 1
+  a
+  b
+  $ hg manifest -r 3
+  a
+  b
+  c
+  $ mkdir c
+  $ hg update -q 3
+  c: untracked file differs
+  abort: untracked files in working directory differ from files in requested revision
+  [255]
+
   $ cd ..