[PATCH] dirstate: don't traverse symbolic links in walk (issue1145)

[Maxim Dounin]

Hello!

On Tue, Jul 29, 2008 at 04:47:12PM -0500, Matt Mackall wrote:

> 
> On Wed, 2008-07-30 at 00:58 +0400, Maxim Dounin wrote:
> > Hello!
> > 
> > Here is a patch for issue1145 agains crew.  It basically fixes 
> > status for situation when directory was renamed and symlink was 
> > created instead.
> > 
> > See http://www.selenic.com/mercurial/bts/issue1145 for details 
> > (and the same patch against crew-stable there too).
> > 
> > Maxim Dounin
> > 
> > # HG changeset patch
> > # User Maxim Dounin <mdounin at mdounin.ru>
> > # Date 1217065864 -14400
> > # Node ID 15dc9719bac8295ce6e308790d3fde37e2a13c4f
> > # Parent  1a9577da9d02a386526c6a6791d6d57f56b28126
> > dirstate: don't traverse symbolic links in walk (issue1145)
> 
> A couple notes on the BTS:
> 
> First, folks, please don't write "I run these commands and it fails",
> tell us -what the failure is-. Otherwise, each person who looks at the
> report will have to run those commands to figure out what's going on
> every time they pull up the bug, and most won't bother.

Sure.  I bothered only because djc pinged me personally.  The only 
excuse for this particular bug report is that actual failure is in 
title.

> Second, posting patches in the BTS is a great way to have your patch get
> lost: there's no easy way to search for outstanding patches.

That's why it's here.

> > Normally dirstate.walk() tries to stat files that are in dirstate's map but
> > wasn't reached by normal walk for some reason (e.g. ignored) and returns
> > them as present if stat() succeeded.  Use util.path_auditor() to make sure
> > we don't traverse symbolic links here.
> > 
> > diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> > --- a/mercurial/dirstate.py
> > +++ b/mercurial/dirstate.py
> > @@ -528,9 +528,14 @@ class dirstate(object):
> >                          results[nf] = None
> >  
> >          # step 3: report unseen items in the dmap hash
> > +        audit_path = util.path_auditor(self._root)
> 
> Sigh, path auditor scares me. Is there no better way to do this? stat vs
> lstat?

Not really.  We have to check if path traverses symbolic links 
here, i.e. lstat() all path components from repo root.  Path auditor 
does this with caching and I see no reason to reinvent bicycle.

> >          visit = [f for f in dmap if f not in results and match(f)]
> >          for nf in util.sort(visit):
> >              results[nf] = None
> > +            try:
> > +                audit_path(nf)
> > +            except:
> > +                continue
> 
> Open-ended excepts: are frowned on.

Here is the updated patch with except util.Abort (patch against 
crew-stable as attached to issue doesn't have this problem).

Maxim Dounin

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1217065864 -14400
# Node ID 1cdcfc99a0f5886764e62a283c38fcb42dea7ef3
# Parent  1a9577da9d02a386526c6a6791d6d57f56b28126
dirstate: don't traverse symbolic links in walk (issue1145)

Normally dirstate.walk() tries to stat files that are in dirstate's map but
wasn't reached by normal walk for some reason (e.g. ignored) and returns
them as present if stat() succeeded.  Use util.path_auditor() to make sure
we don't traverse symbolic links here.

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -528,9 +528,14 @@ class dirstate(object):
                         results[nf] = None
 
         # step 3: report unseen items in the dmap hash
+        audit_path = util.path_auditor(self._root)
         visit = [f for f in dmap if f not in results and match(f)]
         for nf in util.sort(visit):
             results[nf] = None
+            try:
+                audit_path(nf)
+            except util.Abort:
+                continue
             try:
                 st = lstat(join(nf))
                 kind = getkind(st.st_mode)
diff --git a/tests/test-symlink-issue1145 b/tests/test-symlink-issue1145
new file mode 100755
--- /dev/null
+++ b/tests/test-symlink-issue1145
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+"$TESTDIR/hghave" symlink || exit 80
+
+hg init a
+cd a
+
+mkdir a
+touch a/f
+hg ci -Ama
+mv a b
+
+echo '% stat without symlink'
+hg stat
+
+ln -s b a
+
+echo '% stat with symlink created'
+hg stat
+
+echo '% trying to removing a'
+hg rm -A a
diff --git a/tests/test-symlink-issue1145.out b/tests/test-symlink-issue1145.out
new file mode 100644
--- /dev/null
+++ b/tests/test-symlink-issue1145.out
@@ -0,0 +1,10 @@
+adding a/f
+% stat without symlink
+! a/f
+? b/f
+% stat with symlink created
+! a/f
+? a
+? b/f
+% trying to removing a
+removing a/f