get_native_path_case() now preserves the trailing os.sep.path and work with non-existing path.

This simplifies a few code paths.
Add unit test to make sure the behavior is consistent accross platforms.

Fixes test regressions.

R=cmp@chromium.org
NOTRY=true
BUG=
TEST=


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

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

4 files changed, 86 insertions, 28 deletions

diff --git a/tools/isolate/isolate_smoke_test.py b/tools/isolate/isolate_smoke_test.py
index e199f91..294f31c 100755
--- a/tools/isolate/isolate_smoke_test.py
+++ b/tools/isolate/isolate_smoke_test.py
@@ -606,7 +606,8 @@ class Isolate_trace_read_merge(IsolateModeBase):
   LEVEL = isolate.STATS_ONLY
 
   def _check_merge(self, filename):
-    filepath = os.path.join(ROOT_DIR, 'data', 'isolate', filename)
+    filepath = isolate.trace_inputs.get_native_path_case(
+        os.path.join(ROOT_DIR, 'data', 'isolate', filename))
     expected = 'Updating %s\n' % filepath
     with open(filepath, 'rb') as f:
       old_content = f.read()
@@ -883,8 +884,8 @@ class IsolateNoOutdir(IsolateBase):
     self.assertEquals(self._wrap_in_condition(expected), output)
 
     output = self._execute('merge', [], True)
-    expected = 'Updating %s\n' % os.path.join(
-        self.root, 'data', 'isolate', 'touch_root.isolate')
+    expected = 'Updating %s\n' % isolate.trace_inputs.get_native_path_case(
+        os.path.join(self.root, 'data', 'isolate', 'touch_root.isolate'))
     self.assertEquals(expected, output)
     # In theory the file is going to be updated but in practice its content
     # won't change.
diff --git a/tools/isolate/run_test_from_archive_smoke_test.py b/tools/isolate/run_test_from_archive_smoke_test.py
index 686249e..7c2d9e1 100755
--- a/tools/isolate/run_test_from_archive_smoke_test.py
+++ b/tools/isolate/run_test_from_archive_smoke_test.py
@@ -104,7 +104,10 @@ class RunTestFromArchive(unittest.TestCase):
       '--cache', self.cache,
       '--remote', self.table,
     ]
-    proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    proc = subprocess.Popen(
+        cmd,
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+        universal_newlines=True)
     out, err = proc.communicate()
     self.assertEquals(1070, len(out))
     self.assertEquals('', err)
@@ -132,7 +135,10 @@ class RunTestFromArchive(unittest.TestCase):
       '--cache', self.cache,
       '--remote', self.table,
     ]
-    proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    proc = subprocess.Popen(
+        cmd,
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+        universal_newlines=True)
     out, err = proc.communicate()
     self.assertEquals(1070, len(out))
     self.assertEquals('', err)
@@ -146,7 +152,10 @@ class RunTestFromArchive(unittest.TestCase):
       '--cache', self.cache,
       '--remote', self.table,
     ]
-    proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    proc = subprocess.Popen(
+        cmd,
+        stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+        universal_newlines=True)
     out, err = proc.communicate()
     self.assertEquals('', out)
     self.assertEquals('No file to map\n', err)
diff --git a/tools/isolate/trace_inputs.py b/tools/isolate/trace_inputs.py
index e7573a8..e04d94a 100755
--- a/tools/isolate/trace_inputs.py
+++ b/tools/isolate/trace_inputs.py
@@ -194,29 +194,32 @@ if sys.platform == 'win32':
     return os.path.isabs(path) or len(path) == 2 and path[1] == ':'
 
 
-  def get_native_path_case(path):
+  def get_native_path_case(p):
     """Returns the native path case for an existing file.
 
     On Windows, removes any leading '\\?\'.
     """
-    if not isabs(path):
+    if not isabs(p):
       raise ValueError(
-          'Can\'t get native path case for a non-absolute path: %s' % path,
-          path)
+          'Can\'t get native path case for a non-absolute path: %s' % p,
+          p)
     # Windows used to have an option to turn on case sensitivity on non Win32
     # subsystem but that's out of scope here and isn't supported anymore.
     # Go figure why GetShortPathName() is needed.
     try:
-      path = GetLongPathName(GetShortPathName(path))
-    except OSError:
-      # This is wrong to silently eat the exception but there's nothing that can
-      # be done about it.
-      logging.info('No access to %s' % path)
-    if path.startswith('\\\\?\\'):
-      path = path[4:]
+      out = GetLongPathName(GetShortPathName(p))
+    except OSError, e:
+      if e.args[0] in (2, 3):
+        # The path does not exist. Try to recurse and reconstruct the path.
+        base = os.path.dirname(p)
+        rest = os.path.basename(p)
+        return os.path.join(get_native_path_case(base), rest)
+      raise
+    if out.startswith('\\\\?\\'):
+      out = out[4:]
     # Always upper case the first letter since GetLongPathName() will return the
     # drive letter in the case it was given.
-    return path[0].upper() + path[1:]
+    return out[0].upper() + out[1:]
 
 
   def CommandLineToArgvW(command_line):
@@ -254,8 +257,16 @@ elif sys.platform == 'darwin':
     logging.debug('native_case(%s)' % p)
     try:
       rel_ref, _ = Carbon.File.FSPathMakeRef(p)
-      return rel_ref.FSRefMakePath()
+      out = rel_ref.FSRefMakePath()
+      if p.endswith(os.path.sep) and not out.endswith(os.path.sep):
+        return out + os.path.sep
+      return out
     except MacOS.Error, e:
+      if e.args[0] == -43:
+        # The path does not exist. Try to recurse and reconstruct the path.
+        base = os.path.dirname(p)
+        rest = os.path.basename(p)
+        return os.path.join(_native_case(base), rest)
       raise OSError(
           e.args[0], 'Failed to get native path for %s' % p, p, e.args[1])
 
@@ -294,7 +305,7 @@ elif sys.platform == 'darwin':
 
     # There was a symlink, process it.
     base, symlink, rest = _split_at_symlink_native(None, path)
-    assert symlink, (path, base, symlink, rest)
+    assert symlink, (path, base, symlink, rest, resolved)
     prev = base
     base = safe_join(_native_case(base), symlink)
     assert len(base) > len(prev)
@@ -334,7 +345,10 @@ else:  # OSes other than Windows and OSX.
     # Linux traces tends to not be normalized so use this occasion to normalize
     # it. This function implementation already normalizes the path on the other
     # OS so this needs to be done here to be coherent between OSes.
-    return os.path.normpath(path)
+    out = os.path.normpath(path)
+    if path.endswith(os.path.sep) and not out.endswith(os.path.sep):
+      return out + os.path.sep
+    return out
 
 
 if sys.platform != 'win32':  # All non-Windows OSes.
@@ -569,8 +583,7 @@ class Results(object):
       assert tainted or bool(root) != bool(isabs(path)), (root, path)
       assert tainted or (
           not os.path.exists(self.full_path) or
-          (self.full_path.rstrip(os.path.sep) ==
-            get_native_path_case(self.full_path))), (
+          (self.full_path == get_native_path_case(self.full_path))), (
               tainted, self.full_path, get_native_path_case(self.full_path))
 
     @property
diff --git a/tools/isolate/trace_inputs_test.py b/tools/isolate/trace_inputs_test.py
index 79d7da1..3301b27 100755
--- a/tools/isolate/trace_inputs_test.py
+++ b/tools/isolate/trace_inputs_test.py
@@ -64,12 +64,47 @@ class TraceInputs(unittest.TestCase):
     self.assertEquals(os.path.join('/usr', '$FOO/bar'), actual.full_path)
     self.assertEquals(True, actual.tainted)
 
-  if sys.platform == 'win32':
-    def test_native_case_windows(self):
-      windows_path = os.environ['SystemRoot']
+  def test_native_case_end_with_os_path_sep(self):
+    # Make sure the trailing os.path.sep is kept.
+    path = trace_inputs.get_native_path_case(ROOT_DIR) + os.path.sep
+    self.assertEquals(trace_inputs.get_native_path_case(path), path)
+
+  def test_native_case_non_existing(self):
+    # Make sure it doesn't throw on non-existing files.
+    non_existing = 'trace_input_test_this_file_should_not_exist'
+    path = os.path.expanduser('~/' + non_existing)
+    self.assertFalse(os.path.exists(path))
+    path = trace_inputs.get_native_path_case(ROOT_DIR) + os.path.sep
+    self.assertEquals(trace_inputs.get_native_path_case(path), path)
+
+  if sys.platform in ('darwin', 'win32'):
+    def test_native_case_not_sensitive(self):
+      # The home directory is almost guaranteed to have mixed upper/lower case
+      # letters on both Windows and OSX.
+      # This test also ensures that the output is independent on the input
+      # string case.
+      path = os.path.expanduser('~')
+      self.assertTrue(os.path.isdir(path))
+      # This test assumes the variable is in the native path case on disk, this
+      # should be the case. Verify this assumption:
+      self.assertEquals(path, trace_inputs.get_native_path_case(path))
       self.assertEquals(
-          trace_inputs.get_native_path_case(windows_path.lower()),
-          trace_inputs.get_native_path_case(windows_path.upper()))
+          trace_inputs.get_native_path_case(path.lower()),
+          trace_inputs.get_native_path_case(path.upper()))
+
+    def test_native_case_not_sensitive_non_existent(self):
+      # This test also ensures that the output is independent on the input
+      # string case.
+      non_existing = os.path.join(
+          'trace_input_test_this_dir_should_not_exist', 'really not', '')
+      path = os.path.expanduser(os.path.join('~', non_existing))
+      self.assertFalse(os.path.exists(path))
+      lower = trace_inputs.get_native_path_case(path.lower())
+      upper = trace_inputs.get_native_path_case(path.upper())
+      # Make sure non-existing element is not modified:
+      self.assertTrue(lower.endswith(non_existing.lower()))
+      self.assertTrue(upper.endswith(non_existing.upper()))
+      self.assertEquals(lower[:-len(non_existing)], upper[:-len(non_existing)])
 
   if sys.platform != 'win32':
     def test_symlink(self):