buck.py more permissive symlinking.
Summary:
f0dedaec09331f73b3970658344cf8f3cb37a83b introduced a symlink aware walk aimed
at preventing recursively symlink loops; however, it was too restrictive in
that it pruned all symlinks that it has visited before, not only those that
would result in recursive symlinks. This breaks legitimate uses of symlinking
to siblings and cousin directories.
This diff adds a second test before pruning the symlink: not only must the
target directory be one already visited, but it must also be a direct ancestor
of the current directory.
Test Plan:
Trigger the traversal code by using recursive `glob` and `buck test --all` on
directories with the following configurations:
Test that siblings are okay (`b` and `c` are both traversed):
a/
b/
c -> b
Test that ancestors are still rejected (`c` is not traversed):
a/
b/
c/ -> ../b
diff --git a/src/com/facebook/buck/parser/buck.py b/src/com/facebook/buck/parser/buck.py
index ef73568..4217007 100644
--- a/src/com/facebook/buck/parser/buck.py
+++ b/src/com/facebook/buck/parser/buck.py
@@ -136,15 +136,18 @@
def symlink_aware_walk(base):
""" Recursive symlink aware version of `os.walk`.
- Will not emit an entry for a directory it has previously visited.
+ Will not traverse a symlink that refers to a previously visited ancestor of
+ the current directory.
"""
visited_dirs = set()
for entry in os.walk(base, topdown=True, followlinks=True):
(root, dirs, _files) = entry
realdirpath = os.path.realpath(root)
if realdirpath in visited_dirs:
- dirs[:] = []
- continue
+ absdirpath = os.path.abspath(root)
+ if absdirpath.startswith(realdirpath):
+ dirs[:] = []
+ continue
visited_dirs.add(realdirpath)
yield entry
raise StopIteration
diff --git a/src/com/facebook/buck/parser/buck_test.py b/src/com/facebook/buck/parser/buck_test.py
index 23814b8..d7785bb 100644
--- a/src/com/facebook/buck/parser/buck_test.py
+++ b/src/com/facebook/buck/parser/buck_test.py
@@ -1,6 +1,7 @@
from buck import glob_pattern_to_regex_string
from buck import LazyBuildEnvPartial
from buck import relpath
+from buck import symlink_aware_walk
import unittest
import re
import os
@@ -81,5 +82,66 @@
os.getcwd = real_getcwd
+ def test_symlink_aware_walk(self):
+ real_walk = os.walk
+ real_realpath = os.path.realpath
+ real_abspath = os.path.abspath
+
+ # a/
+ # b/
+ # c/
+ # file
+ # sibling -> c
+ # ancestor -> ../..
+
+ def mock_walk(base, **kwargs):
+ self.assertEqual('a', base)
+
+ dirs = ['b']
+ yield ('a', dirs, [])
+ self.assertEqual(['b'], dirs)
+
+ dirs = ['c','sibling','ancestor']
+ yield ('a/b', dirs, [])
+ self.assertEqual(['c', 'sibling', 'ancestor'], dirs)
+
+ yield ('a/b/c', [], ['file'])
+ yield ('a/b/sibling', [], ['file'])
+
+ dirs = ['b']
+ yield ('a/b/ancestor', dirs, [])
+ self.assertEqual([], dirs)
+
+ raise StopIteration
+
+ def mock_realpath(path):
+ if path == 'a/b/sibling':
+ return 'a/b/c'
+ if path == 'a/b/ancestor':
+ return 'a'
+ return path
+
+ def mock_abspath(path):
+ return path
+
+ try:
+ os.walk = mock_walk
+ os.path.realpath = mock_realpath
+ os.path.abspath = mock_abspath
+ result = set(root for (root, _, _) in symlink_aware_walk('a'))
+ self.assertEqual(
+ set([
+ 'a',
+ 'a/b',
+ 'a/b/c',
+ 'a/b/sibling',
+ ]),
+ result)
+ finally:
+ os.walk = real_walk
+ os.path.realpath = real_realpath
+ os.path.abspath = real_abspath
+
+
if __name__ == '__main__':
unittest.main()
