Fix symlink handling and add more smoke tests.

Fix get_native_path_case() on OSX to work with symlinks. This requires involved
handling. Symlinks need to be carefully processed and they are neecessary to
make browser_tests work on OSX.

Improve Results.File.strip_root() to look for symlinks when stripping root. This
happens when running an executable from the temp directory on OSX, causing some
files to be lost otherwise. Have the smoke test verify this condition.

TBR=cmp@chromium.org
NOTRY=true
BUG=
TEST=Improved testing.


Review URL: https://chromiumcodereview.appspot.com/10698101

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@145493 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 219 insertions, 65 deletions

diff --git a/tools/isolate/isolate.py b/tools/isolate/isolate.py
index 32aa29a..e799c96 100755
--- a/tools/isolate/isolate.py
+++ b/tools/isolate/isolate.py
@@ -52,23 +52,6 @@ def normpath(path):
   return out
 
 
-def split_at_symlink(indir, relfile):
-  """Scans each component of relfile and cut the string at the symlink if there
-  is any.
-
-  Returns a tuple (symlink, rest), with rest == None if not symlink was found.
-  """
-  parts = relfile.rstrip(os.path.sep).split(os.path.sep)
-  for i in range(len(parts)):
-    if os.path.islink(os.path.join(indir, *parts[:i])):
-      # A symlink! Keep the trailing os.path.sep if there was any.
-      out = os.path.join(*parts[:i])
-      rest = os.path.sep.join(parts[i:])
-      logging.debug('split_at_symlink(%s) -> (%s, %s)' % (relfile, out, rest))
-      return out, rest
-  return relfile, None
-
-
 def expand_directory_and_symlink(indir, relfile, blacklist):
   """Expands a single input. It can result in multiple outputs.
 
@@ -88,20 +71,21 @@ def expand_directory_and_symlink(indir, relfile, blacklist):
         'Can\'t map file %s outside %s' % (infile, indir))
 
   # Look if any item in relfile is a symlink.
-  symlink_relfile, rest = split_at_symlink(indir, relfile)
-  if rest is not None:
+  base, symlink, rest = trace_inputs.split_at_symlink(indir, relfile)
+  if symlink:
     # Append everything pointed by the symlink. If the symlink is recursive,
     # this code blows up.
-    pointed = os.readlink(os.path.join(indir, symlink_relfile))
-    # Override infile with the new destination.
+    symlink_relfile = os.path.join(base, symlink)
+    symlink_path = os.path.join(indir, symlink_relfile)
+    pointed = os.readlink(symlink_path)
     dest_infile = normpath(
-        os.path.join(indir, os.path.dirname(symlink_relfile), pointed, rest))
-    if os.path.isdir(dest_infile):
-      dest_infile += os.path.sep
+        trace_inputs.safe_join(
+            os.path.join(os.path.dirname(symlink_path), pointed),
+            rest))
     if not dest_infile.startswith(indir):
       raise run_test_from_archive.MappingError(
-          'Can\'t map symlink reference %s->%s outside of %s' %
-          (symlink_relfile, dest_infile, indir))
+          'Can\'t map symlink reference %s (from %s) ->%s outside of %s' %
+          (symlink_relfile, relfile, dest_infile, indir))
     if infile.startswith(dest_infile):
       raise run_test_from_archive.MappingError(
           'Can\'t map recursive symlink reference %s->%s' %
diff --git a/tools/isolate/trace_inputs.py b/tools/isolate/trace_inputs.py
index e7e2408..80442bf 100755
--- a/tools/isolate/trace_inputs.py
+++ b/tools/isolate/trace_inputs.py
@@ -237,8 +237,47 @@ elif sys.platform == 'darwin':
   isabs = os.path.isabs
 
 
+  def _find_item_native_case(root_path, item):
+    """Gets the native path case of a single item based at root_path.
+
+    There is no API to get the native path case of symlinks on OSX. So it
+    needs to be done the slow way.
+    """
+    item = item.lower()
+    for element in os.listdir(root_path):
+      if element.lower() == item:
+        return element
+
+
+  def _native_case(p):
+    """Gets the native path case. Warning: this function resolves symlinks."""
+    logging.debug('native_case(%s)' % p)
+    try:
+      rel_ref, _ = Carbon.File.FSPathMakeRef(p)
+      return rel_ref.FSRefMakePath()
+    except MacOS.Error, e:
+      raise OSError(
+          e.args[0], 'Failed to get native path for %s' % p, p, e.args[1])
+
+
+  def _split_at_symlink_native(base_path, rest):
+    """Returns the native path for a symlink."""
+    base, symlink, rest = split_at_symlink(base_path, rest)
+    if symlink:
+      if not base_path:
+        base_path = base
+      else:
+        base_path = safe_join(base_path, base)
+      symlink = _find_item_native_case(base_path, symlink)
+    return base, symlink, rest
+
+
   def get_native_path_case(path):
-    """Returns the native path case for an existing file."""
+    """Returns the native path case for an existing file.
+
+    Technically, it's only HFS+ on OSX that is case preserving and
+    insensitive. It's the default setting on HFS+ but can be changed.
+    """
     if not isabs(path):
       raise ValueError(
           'Can\'t get native path case for a non-absolute path: %s' % path,
@@ -246,14 +285,30 @@ elif sys.platform == 'darwin':
     if path.startswith('/dev'):
       # /dev is not visible from Carbon, causing an exception.
       return path
-    # Technically, it's only HFS+ on OSX that is case insensitive. It's the
-    # default setting on HFS+ but can be changed.
-    try:
-      rel_ref, _ = Carbon.File.FSPathMakeRef(path)
-      return rel_ref.FSRefMakePath()
-    except MacOS.Error, e:
-      raise OSError(
-          e.args[0], 'Failed to get native path for %s' % path, path, e.args[1])
+
+    # Starts assuming there is no symlink along the path.
+    resolved = _native_case(path)
+    if resolved.lower() == path.lower():
+      # This code path is incredibly faster.
+      return resolved
+
+    # There was a symlink, process it.
+    base, symlink, rest = _split_at_symlink_native(None, path)
+    assert symlink, (path, base, symlink, rest)
+    prev = base
+    base = safe_join(_native_case(base), symlink)
+    assert len(base) > len(prev)
+    while rest:
+      prev = base
+      relbase, symlink, rest = _split_at_symlink_native(base, rest)
+      base = safe_join(base, relbase)
+      assert len(base) > len(prev), (prev, base, symlink)
+      if symlink:
+        base = safe_join(base, symlink)
+      assert len(base) > len(prev), (prev, base, symlink)
+    # Make sure no symlink was resolved.
+    assert base.lower() == path.lower(), (base, path)
+    return base
 
 
 else:  # OSes other than Windows and OSX.
@@ -282,6 +337,72 @@ else:  # OSes other than Windows and OSX.
     return os.path.normpath(path)
 
 
+if sys.platform != 'win32':  # All non-Windows OSes.
+
+
+  def safe_join(*args):
+    """Joins path elements like os.path.join() but doesn't abort on absolute
+    path.
+
+    os.path.join('foo', '/bar') == '/bar'
+    but safe_join('foo', '/bar') == 'foo/bar'.
+    """
+    out = ''
+    for element in args:
+      if element.startswith(os.path.sep):
+        if out.endswith(os.path.sep):
+          out += element[1:]
+        else:
+          out += element
+      else:
+        if out.endswith(os.path.sep):
+          out += element
+        else:
+          out += os.path.sep + element
+    return out
+
+
+  def split_at_symlink(base_dir, relfile):
+    """Scans each component of relfile and cut the string at the symlink if
+    there is any.
+
+    Returns a tuple (base_path, symlink, rest), with symlink == rest == None if
+    not symlink was found.
+    """
+    if base_dir:
+      assert relfile
+      assert os.path.isabs(base_dir)
+      index = 0
+    else:
+      assert os.path.isabs(relfile)
+      index = 1
+
+    def at_root(rest):
+      if base_dir:
+        return safe_join(base_dir, rest)
+      return rest
+
+    while True:
+      try:
+        index = relfile.index(os.path.sep, index)
+      except ValueError:
+        index = len(relfile)
+      full = at_root(relfile[:index])
+      if os.path.islink(full):
+        # A symlink!
+        base = os.path.dirname(relfile[:index])
+        symlink = os.path.basename(relfile[:index])
+        rest = relfile[index:]
+        logging.debug(
+            'split_at_symlink(%s, %s) -> (%s, %s, %s)' %
+            (base_dir, relfile, base, symlink, rest))
+        return base, symlink, rest
+      if index == len(relfile):
+        break
+      index += 1
+    return relfile, None, None
+
+
 def fix_python_path(cmd):
   """Returns the fixed command line to call the right python executable."""
   out = cmd[:]
@@ -445,6 +566,7 @@ class Results(object):
       self.path = path
 
       self.tainted = tainted
+      self._real_path = None
       self._size = None
       # For compatibility with Directory object interface.
       # Shouldn't be used normally, only exists to simplify algorithms.
@@ -477,6 +599,13 @@ class Results(object):
         return os.path.join(self.root, self.path)
       return self.path
 
+    @property
+    def real_path(self):
+      """Returns the path with symlinks resolved."""
+      if not self._real_path:
+        self._real_path = os.path.realpath(self.full_path)
+      return self._real_path
+
     def flatten(self):
       return {
         'path': self.path,
@@ -488,8 +617,14 @@ class Results(object):
       # Check internal consistency.
       assert self.tainted or (isabs(root) and root.endswith(os.path.sep)), root
       if not self.full_path.startswith(root):
-        return None
-      return self._clone(root, self.full_path[len(root):], self.tainted)
+        # Now try to resolve the symlinks to see if it can be reached this way.
+        # Only try *after* trying without resolving symlink.
+        if not self.real_path.startswith(root):
+          return None
+        path = self.real_path
+      else:
+        path = self.full_path
+      return self._clone(root, path[len(root):], self.tainted)
 
     def replace_variables(self, variables):
       """Replaces the root of this File with one of the variables if it matches.
@@ -676,9 +811,13 @@ class ApiBase(object):
             return x
           return get_native_path_case(x)
 
+        files = [
+          f for f in set(map(render_to_string_and_fix_case, self.files))
+          if not os.path.isdir(f)
+        ]
         return Results.Process(
             self.pid,
-            set(map(render_to_string_and_fix_case, self.files)),
+            files,
             render_to_string_and_fix_case(self.executable),
             self.command,
             render_to_string_and_fix_case(self.initial_cwd),
diff --git a/tools/isolate/trace_inputs_smoke_test.py b/tools/isolate/trace_inputs_smoke_test.py
index b1136f7..dde74d4 100755
--- a/tools/isolate/trace_inputs_smoke_test.py
+++ b/tools/isolate/trace_inputs_smoke_test.py
@@ -67,15 +67,6 @@ class TraceInputsBase(unittest.TestCase):
         unicode(self.executable))
     trace_inputs = None
 
-    if sys.platform == 'darwin':
-      # Interestingly, only OSX does resolve the symlink manually before
-      # starting the executable.
-      if os.path.islink(self.real_executable):
-        self.real_executable = os.path.normpath(
-            os.path.join(
-                os.path.dirname(self.real_executable),
-                os.readlink(self.real_executable)))
-
     # self.naked_executable will only be naked on Windows.
     self.naked_executable = unicode(sys.executable)
     if sys.platform == 'win32':
diff --git a/tools/isolate/trace_inputs_test.py b/tools/isolate/trace_inputs_test.py
index ddb9b2b..d697543 100755
--- a/tools/isolate/trace_inputs_test.py
+++ b/tools/isolate/trace_inputs_test.py
@@ -64,13 +64,63 @@ class TraceInputs(unittest.TestCase):
     self.assertEquals(os.path.join('/usr', '$FOO/bar'), actual.full_path)
     self.assertEquals(True, actual.tainted)
 
-  def test_native_case_windows(self):
-    if sys.platform != 'win32':
-      return
-    windows_path = os.environ['SystemRoot']
-    self.assertEquals(
-        trace_inputs.get_native_path_case(windows_path.lower()),
-        trace_inputs.get_native_path_case(windows_path.upper()))
+  if sys.platform == 'win32':
+    def test_native_case_windows(self):
+      windows_path = os.environ['SystemRoot']
+      self.assertEquals(
+          trace_inputs.get_native_path_case(windows_path.lower()),
+          trace_inputs.get_native_path_case(windows_path.upper()))
+
+  if sys.platform != 'win32':
+    def test_symlink(self):
+      # This test will fail if the checkout is in a symlink.
+      actual = trace_inputs.split_at_symlink(None, ROOT_DIR)
+      expected = (ROOT_DIR, None, None)
+      self.assertEquals(expected, actual)
+
+      actual = trace_inputs.split_at_symlink(
+          None, os.path.join(ROOT_DIR, 'data', 'trace_inputs'))
+      expected = (
+          os.path.join(ROOT_DIR, 'data', 'trace_inputs'), None, None)
+      self.assertEquals(expected, actual)
+
+      actual = trace_inputs.split_at_symlink(
+          None, os.path.join(ROOT_DIR, 'data', 'trace_inputs', 'files2'))
+      expected = (
+          os.path.join(ROOT_DIR, 'data', 'trace_inputs'), 'files2', '')
+      self.assertEquals(expected, actual)
+
+      actual = trace_inputs.split_at_symlink(
+          ROOT_DIR, os.path.join('data', 'trace_inputs', 'files2'))
+      expected = (
+          os.path.join('data', 'trace_inputs'), 'files2', '')
+      self.assertEquals(expected, actual)
+      actual = trace_inputs.split_at_symlink(
+          ROOT_DIR, os.path.join('data', 'trace_inputs', 'files2', 'bar'))
+      expected = (
+          os.path.join('data', 'trace_inputs'), 'files2', '/bar')
+      self.assertEquals(expected, actual)
+
+    def test_native_case_symlink_right_case(self):
+      actual = trace_inputs.get_native_path_case(
+          os.path.join(ROOT_DIR, 'data', 'trace_inputs'))
+      self.assertEquals('trace_inputs', os.path.basename(actual))
+
+      # Make sure the symlink is not resolved.
+      actual = trace_inputs.get_native_path_case(
+          os.path.join(ROOT_DIR, 'data', 'trace_inputs', 'files2'))
+      self.assertEquals('files2', os.path.basename(actual))
+
+  if sys.platform == 'darwin':
+    def test_native_case_symlink_wrong_case(self):
+      actual = trace_inputs.get_native_path_case(
+          os.path.join(ROOT_DIR, 'data', 'trace_inputs'))
+      self.assertEquals('trace_inputs', os.path.basename(actual))
+
+      # Make sure the symlink is not resolved.
+      actual = trace_inputs.get_native_path_case(
+          os.path.join(ROOT_DIR, 'data', 'trace_inputs', 'Files2'))
+      self.assertEquals('files2', os.path.basename(actual))
 
 
 if sys.platform != 'win32':
diff --git a/tools/isolate/trace_test_cases_smoke_test.py b/tools/isolate/trace_test_cases_smoke_test.py
index 57faf23..2dc09be 100755
--- a/tools/isolate/trace_test_cases_smoke_test.py
+++ b/tools/isolate/trace_test_cases_smoke_test.py
@@ -39,17 +39,7 @@ class TraceTestCases(unittest.TestCase):
         # So it'll look like /usr/bin/python2.7
         self.executable += suffix
 
-    self.real_executable = trace_inputs.get_native_path_case(
-        self.executable)
-
-    if sys.platform == 'darwin':
-      # Interestingly, only OSX does resolve the symlink manually before
-      # starting the executable.
-      if os.path.islink(self.real_executable):
-        self.real_executable = os.path.normpath(
-            os.path.join(
-                os.path.dirname(self.real_executable),
-                os.readlink(self.real_executable)))
+    self.real_executable = trace_inputs.get_native_path_case(self.executable)
 
   def tearDown(self):
     if self.temp_file: