From a92b961190ff4a2fefda10a89bd95c07d5d42c7a Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Tue, 3 Jul 2018 02:34:32 +0000 Subject: [PATCH] gclient setdep: Add support for CIPD versions gclient setdep used to assume that the version of a CIPD package was always of the form 'version:'. However, the following are all valid: * SHA1 hashes that specify a single instance * tags (tagname:tagvalue) * References (e.g. 'latest') that are dynamically resolved by the CIPD server. This change adds support for all of them: * GetCIPD simply returns whatever the 'version' entry is, without assuming it begins with 'version:'. * SetCIPD sets 'version' ta whatever it is passed, without prepending 'version:' at the beginning. Bug: 858978 Change-Id: I53669c5df7fb51cde42e0af271139b5719a47622 Reviewed-on: https://chromium-review.googlesource.com/1120943 Reviewed-by: Aaron Gable Commit-Queue: Edward Lesmes --- gclient_eval.py | 3 +- tests/gclient_eval_unittest.py | 73 ++++++++++++++++++++++++++++++---- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/gclient_eval.py b/gclient_eval.py index dd39823fd1..2a9b4bf22b 100644 --- a/gclient_eval.py +++ b/gclient_eval.py @@ -725,7 +725,6 @@ def SetCIPD(gclient_dict, dep_name, package_name, new_version): "The deps entry for %s:%s has no formatting information." % (dep_name, package_name)) - new_version = 'version:' + new_version _UpdateAstString(tokens, node, new_version) packages[0].SetNode('version', new_version, node) @@ -808,7 +807,7 @@ def GetCIPD(gclient_dict, dep_name, package_name): "There must be exactly one package with the given name (%s), " "%s were found." % (package_name, len(packages))) - return packages[0]['version'][len('version:'):] + return packages[0]['version'] def GetRevision(gclient_dict, dep_name): diff --git a/tests/gclient_eval_unittest.py b/tests/gclient_eval_unittest.py index 613fa71d6c..e34879b32b 100755 --- a/tests/gclient_eval_unittest.py +++ b/tests/gclient_eval_unittest.py @@ -382,7 +382,7 @@ class CipdTest(unittest.TestCase): ' "packages": [', ' {', ' "package": "some/cipd/package",', - ' "version": "version:1234",', + ' "version": "deadbeef",', ' },', ' {', ' "package": "another/cipd/package",', @@ -395,12 +395,20 @@ class CipdTest(unittest.TestCase): '}', ])) - result = gclient_eval.GetCIPD( - local_scope, 'src/cipd/package', 'another/cipd/package') - self.assertEqual(result, '5678') + self.assertEqual( + gclient_eval.GetCIPD( + local_scope, 'src/cipd/package', 'some/cipd/package'), + 'deadbeef') + + self.assertEqual( + gclient_eval.GetCIPD( + local_scope, 'src/cipd/package', 'another/cipd/package'), + 'version:5678') gclient_eval.SetCIPD( - local_scope, 'src/cipd/package', 'another/cipd/package', '6.789') + local_scope, 'src/cipd/package', 'another/cipd/package', 'version:6789') + gclient_eval.SetCIPD( + local_scope, 'src/cipd/package', 'some/cipd/package', 'foobar') result = gclient_eval.RenderDEPSFile(local_scope) self.assertEqual(result, '\n'.join([ @@ -409,11 +417,11 @@ class CipdTest(unittest.TestCase): ' "packages": [', ' {', ' "package": "some/cipd/package",', - ' "version": "version:1234",', + ' "version": "foobar",', ' },', ' {', ' "package": "another/cipd/package",', - ' "version": "version:6.789",', + ' "version": "version:6789",', ' },', ' ],', ' "condition": "checkout_android",', @@ -422,6 +430,39 @@ class CipdTest(unittest.TestCase): '}', ])) + def test_preserves_escaped_vars(self): + local_scope = gclient_eval.Exec('\n'.join([ + 'deps = {', + ' "src/cipd/package": {', + ' "packages": [', + ' {', + ' "package": "package/${{platform}}",', + ' "version": "version:abcd",', + ' },', + ' ],', + ' "dep_type": "cipd",', + ' },', + '}', + ])) + + gclient_eval.SetCIPD( + local_scope, 'src/cipd/package', 'package/${platform}', 'version:dcba') + result = gclient_eval.RenderDEPSFile(local_scope) + + self.assertEqual(result, '\n'.join([ + 'deps = {', + ' "src/cipd/package": {', + ' "packages": [', + ' {', + ' "package": "package/${{platform}}",', + ' "version": "version:dcba",', + ' },', + ' ],', + ' "dep_type": "cipd",', + ' },', + '}', + ])) + class RevisionTest(unittest.TestCase): def assert_gets_and_sets_revision(self, before, after, rev_before='deadbeef'): @@ -552,6 +593,24 @@ class RevisionTest(unittest.TestCase): ] self.assert_gets_and_sets_revision(before, after, rev_before=None) + def test_preserves_variables(self): + before = [ + 'vars = {', + ' "src_root": "src"', + '}', + 'deps = {', + ' "{src_root}/dep": "https://example.com/dep.git@deadbeef",', + '}', + ] + after = [ + 'vars = {', + ' "src_root": "src"', + '}', + 'deps = {', + ' "{src_root}/dep": "https://example.com/dep.git@deadfeed",', + '}', + ] + self.assert_gets_and_sets_revision(before, after) def test_preserves_formatting(self): before = [