From 71b606ecff3465d299e0862c006c3670e0e8defe Mon Sep 17 00:00:00 2001 From: sokcevic Date: Thu, 16 Mar 2023 23:28:36 +0000 Subject: [PATCH] Fix use_relative_path on Windows DEPS entries all have slashes as their path separators. If use_relative_path is set to true, .gclient_entries will actually contain backslashes instead slashes. This is problematic when DEPS file transitions from using use_relative_path = False to True. In such case, gclient thinks there are two distinct repositories and it will delete one with slashes. As this operation is the last step of gclient sync, it results in affected repositories being wrongfully removed. This change forces all deps entries to have slashes regardless of operating system. R=gavinmak@google.com Bug: 1422665 Change-Id: Idb73510d911a260bccf63d24a8d3452998b147f7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4348792 Commit-Queue: Josip Sokcevic Commit-Queue: Gavin Mak Reviewed-by: Gavin Mak Auto-Submit: Josip Sokcevic --- gclient.py | 10 +++++--- tests/gclient_test.py | 60 ++++++++++++++++++------------------------- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/gclient.py b/gclient.py index e3ee4e98c..a30df9741 100755 --- a/gclient.py +++ b/gclient.py @@ -680,7 +680,11 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): for d, url in processed_deps.items(): # normpath is required to allow DEPS to use .. in their # dependency local path. - rel_deps[os.path.normpath(os.path.join(rel_prefix, d))] = url + # We are following the same pattern when use_relative_paths = False, + # which uses slashes. + rel_deps[os.path.normpath(os.path.join(rel_prefix, + d)).replace(os.path.sep, + '/')] = url logging.warning('Updating deps by prepending %s.', rel_prefix) return rel_deps @@ -842,8 +846,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): logging.warning('Updating recursedeps by prepending %s.', rel_prefix) rel_deps = {} for depname, options in self.recursedeps.items(): - rel_deps[ - os.path.normpath(os.path.join(rel_prefix, depname))] = options + rel_deps[os.path.normpath(os.path.join(rel_prefix, depname)).replace( + os.path.sep, '/')] = options self.recursedeps = rel_deps # To get gn_args from another DEPS, that DEPS must be recursed into. if self._gn_args_from: diff --git a/tests/gclient_test.py b/tests/gclient_test.py index 7f8b3f534..252014dec 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -440,11 +440,10 @@ class GclientTest(trial_dir.TestCase): write(os.path.join('foo', 'baz', 'fake.txt'), "bogus content") - self.assertEqual([(h.action, h.effective_cwd) for h in self._get_hooks()], - [ - (('tata', 'titi'), self.root_dir), - (('fire', 'lazors'), os.path.join(self.root_dir, "foo", "baz")) - ]) + self.assertEqual( + [(h.action, h.effective_cwd) for h in self._get_hooks()], + [(('tata', 'titi'), self.root_dir), + (('fire', 'lazors'), os.path.join(self.root_dir, 'foo/baz'))]) def testTargetOS(self): """Verifies that specifying a target_os pulls in all relevant dependencies. @@ -847,13 +846,11 @@ class GclientTest(trial_dir.TestCase): options, _ = gclient.OptionParser().parse_args([]) obj = gclient.GClient.LoadCurrentConfig(options) obj.RunOnDeps('None', []) - self.assertEqual( - [ - ('foo', 'svn://example.com/foo'), - (os.path.join('foo', 'bar'), 'svn://example.com/bar'), - (os.path.join('foo', 'baz'), 'svn://example.com/baz'), - ], - self._get_processed()) + self.assertEqual([ + ('foo', 'svn://example.com/foo'), + ('foo/bar', 'svn://example.com/bar'), + ('foo/baz', 'svn://example.com/baz'), + ], self._get_processed()) def testRecursedepsCustomdepsOverride(self): """Verifies gclient overrides deps within recursedeps using custom deps""" @@ -883,15 +880,12 @@ class GclientTest(trial_dir.TestCase): options, _ = gclient.OptionParser().parse_args([]) obj = gclient.GClient.LoadCurrentConfig(options) obj.RunOnDeps('None', []) - six.assertCountEqual( - self, - [ - ('foo', 'svn://example.com/foo'), - (os.path.join('foo', 'bar'), 'svn://example.com/override'), - (os.path.join('foo', 'foo', 'bar'), 'svn://example.com/override'), - (os.path.join('foo', 'baz'), 'svn://example.com/baz'), - ], - self._get_processed()) + six.assertCountEqual(self, [ + ('foo', 'svn://example.com/foo'), + ('foo/bar', 'svn://example.com/override'), + ('foo/foo/bar', 'svn://example.com/override'), + ('foo/baz', 'svn://example.com/baz'), + ], self._get_processed()) def testRelativeRecursion(self): """Verifies that nested use_relative_paths is always respected.""" @@ -922,13 +916,11 @@ class GclientTest(trial_dir.TestCase): options, _ = gclient.OptionParser().parse_args([]) obj = gclient.GClient.LoadCurrentConfig(options) obj.RunOnDeps('None', []) - self.assertEqual( - [ - ('foo', 'svn://example.com/foo'), - (os.path.join('foo', 'bar'), 'svn://example.com/bar'), - (os.path.join('foo', 'bar', 'baz'), 'svn://example.com/baz'), - ], - self._get_processed()) + self.assertEqual([ + ('foo', 'svn://example.com/foo'), + ('foo/bar', 'svn://example.com/bar'), + ('foo/bar/baz', 'svn://example.com/baz'), + ], self._get_processed()) def testRelativeRecursionInNestedDir(self): """Verifies a gotcha of relative recursion where the parent uses relative @@ -961,13 +953,11 @@ class GclientTest(trial_dir.TestCase): options, _ = gclient.OptionParser().parse_args([]) obj = gclient.GClient.LoadCurrentConfig(options) obj.RunOnDeps('None', []) - self.assertEqual( - [ - ('foo', 'svn://example.com/foo'), - (os.path.join('foo', 'third_party', 'bar'), 'svn://example.com/bar'), - (os.path.join('foo', 'third_party', 'baz'), 'svn://example.com/baz'), - ], - self._get_processed()) + self.assertEqual([ + ('foo', 'svn://example.com/foo'), + ('foo/third_party/bar', 'svn://example.com/bar'), + ('foo/third_party/baz', 'svn://example.com/baz'), + ], self._get_processed()) def testRecursedepsAltfile(self): """Verifies gclient respects the |recursedeps| var syntax with overridden