From 4099daa97b38b2ddb95e34d9fc3e2d37f58df069 Mon Sep 17 00:00:00 2001
From: John Budorick <jbudorick@chromium.org>
Date: Thu, 21 Jun 2018 19:22:10 +0000
Subject: [PATCH] gclient: Use posixpath-style separators for cipd subdirs on
 all platforms.

Bug: 854219
Change-Id: Ibd83135dcb96979f8ab989a248e3e2cf59b9dd43
Reviewed-on: https://chromium-review.googlesource.com/1106698
Commit-Queue: John Budorick <jbudorick@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
---
 gclient.py                  |  8 ++++++--
 tests/gclient_test.py       | 32 ++++++++++++++++++++++++++++++++
 tests/presubmit_unittest.py |  4 ++--
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/gclient.py b/gclient.py
index 4d963c60d..2ebadb023 100755
--- a/gclient.py
+++ b/gclient.py
@@ -1810,8 +1810,10 @@ class CipdDependency(Dependency):
           'Relative CIPD dependencies are not currently supported.')
     self._cipd_package = None
     self._cipd_root = cipd_root
-    self._cipd_subdir = os.path.relpath(
+    # CIPD wants /-separated paths, even on Windows.
+    native_subdir_path = os.path.relpath(
         os.path.join(self.root.root_dir, name), cipd_root.root_dir)
+    self._cipd_subdir = posixpath.join(*native_subdir_path.split(os.sep))
     self._package_name = package
     self._package_version = version
 
@@ -1869,7 +1871,9 @@ class CipdDependency(Dependency):
           '  "%s": {' % (self.name.split(':')[0],),
           '    "packages": [',
       ])
-      for p in self._cipd_root.packages(self._cipd_subdir):
+      for p in sorted(
+          self._cipd_root.packages(self._cipd_subdir),
+          cmp=lambda x, y: cmp(x.name, y.name)):
         s.extend([
             '      {',
             '        "package": "%s",' % p.name,
diff --git a/tests/gclient_test.py b/tests/gclient_test.py
index 98b83a33f..a772a3613 100755
--- a/tests/gclient_test.py
+++ b/tests/gclient_test.py
@@ -11,6 +11,7 @@ See gclient_smoketest.py for integration tests.
 import Queue
 import copy
 import logging
+import ntpath
 import os
 import sys
 import unittest
@@ -21,6 +22,7 @@ import gclient
 import gclient_utils
 import gclient_scm
 from testing_support import trial_dir
+from third_party import mock
 
 
 def write(filename, content):
@@ -1078,6 +1080,36 @@ class GclientTest(trial_dir.TestCase):
     self.assertEquals('https://example.com/foo_package@foo_version', dep0.url)
     self.assertEquals('https://example.com/bar_package@bar_version', dep1.url)
 
+  def _testPosixpathImpl(self):
+    parser = gclient.OptionParser()
+    options, _ = parser.parse_args([])
+    obj = gclient.GClient('src', options)
+    cipd_root = obj.GetCipdRoot()
+
+    cipd_dep = gclient.CipdDependency(
+        parent=obj,
+        name='src/foo/bar/baz',
+        dep_value={
+          'package': 'baz_package',
+          'version': 'baz_version',
+        },
+        cipd_root=cipd_root,
+        custom_vars=None,
+        should_process=True,
+        relative=False,
+        condition=None)
+    self.assertEquals(cipd_dep._cipd_subdir, 'src/foo/bar/baz')
+
+  def testPosixpathCipdSubdir(self):
+    self._testPosixpathImpl()
+
+  # CIPD wants posixpath separators for subdirs, even on windows.
+  # See crbug.com/854219.
+  def testPosixpathCipdSubdirOnWindows(self):
+    with mock.patch('os.path', new=ntpath), (
+         mock.patch('os.sep', new=ntpath.sep)):
+      self._testPosixpathImpl()
+
   def testFuzzyMatchUrlByURL(self):
     write(
         '.gclient',
diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py
index 710e73b43..9673c48b9 100755
--- a/tests/presubmit_unittest.py
+++ b/tests/presubmit_unittest.py
@@ -1979,7 +1979,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
     input_api = self.MockInputApi(change1, False)
     affected_files = (affected_file1, affected_file2)
 
-    input_api.AffectedFiles = lambda: affected_files
+    input_api.AffectedFiles = lambda **_: affected_files
 
     self.mox.ReplayAll()
 
@@ -2389,7 +2389,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
     affected_file.Action = lambda: 'M'
 
     change = self.mox.CreateMock(presubmit.Change)
-    change.AffectedFiles = lambda: [affected_file]
+    change.AffectedFiles = lambda **_: [affected_file]
 
     input_api = self.MockInputApi(None, False)
     input_api.change = change