From 6b84fbfb201f8fd99c3d049328615943a7df3593 Mon Sep 17 00:00:00 2001 From: Michael Savigny Date: Wed, 28 Feb 2024 14:56:56 +0000 Subject: [PATCH] Remove ability to use_goma in autoninja (chromeos excepted) Won't run a build if use_goma=true and target_os!="chromeos". Bug: b/277197166 Test: Manual tests and updates to unit tests. Change-Id: I753618e6ad11cb623d9926a4af00a07339c02c43 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5329167 Reviewed-by: Takuto Ikuta Commit-Queue: Michael Savigny --- autoninja.py | 72 +++++++++++++++++++---------------------- tests/autoninja_test.py | 53 ++++++++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 41 deletions(-) diff --git a/autoninja.py b/autoninja.py index d00b28358..312328a95 100755 --- a/autoninja.py +++ b/autoninja.py @@ -228,10 +228,13 @@ def main(args): use_goma = False use_remoteexec = False use_siso = False + target_chromeos = False # Attempt to auto-detect remote build acceleration. We support gn-based # builds, where we look for args.gn in the build tree, and cmake-based # builds where we look for rules.ninja. + # Attempt to determine if targetos is chromeos to prevent use_goma starting + # a build for other targets. if os.path.exists(os.path.join(output_dir, "args.gn")): for line in _gn_lines(output_dir, os.path.join(output_dir, "args.gn")): # use_goma, or use_remoteexec will activate build @@ -257,6 +260,10 @@ def main(args): line_without_comment): use_siso = True continue + if re.search(r"(^|\s)(target_os)\s*=\s*\"chromeos\"($|\s)", + line_without_comment): + target_chromeos = True + continue if use_remoteexec: if _is_google_corp_machine_using_external_account(): @@ -332,6 +339,33 @@ def main(args): use_goma = False if use_goma: + # Only allow use_goma for builds targeting chromeos now. + if not target_chromeos: + print( + "The gn arg use_goma=true is no longer supported, please switch " + "to use_remoteexec=true instead.", + file=sys.stderr) + if sys.platform.startswith("win"): + print( + "See https://chromium.googlesource.com/chromium/src/+/main/docs/" + "windows_build_instructions.md#use-reclient " + "for setup instructions.", + file=sys.stderr, + ) + elif sys.platform == "darwin": + print( + "If you are a googler see http://go/building-chrome-mac" + "#using-remote-execution for setup instructions.", + file=sys.stderr, + ) + else: + print( + "See https://chromium.googlesource.com/chromium/src/+/main/docs/" + "linux/build_instructions.md#use-reclient for setup instructions.", + file=sys.stderr, + ) + sys.exit(1) + gomacc_file = ("gomacc.exe" if sys.platform.startswith("win") else "gomacc") goma_dir = os.environ.get("GOMA_DIR", @@ -361,44 +395,6 @@ def main(args): # script. print("false") sys.exit(1) - # Display a warning that goma is being deprecated, every time a build - # is executed with 'use_goma. - # Further changes to encourage switching may follow. - if sys.platform.startswith("win"): - print( - "The gn arg use_goma=true will be deprecated by EOY 2023. " - "Please use `use_remoteexec=true` instead. See " - "https://chromium.googlesource.com/chromium/src/+/main/docs/" - "windows_build_instructions.md#use-reclient " - "for setup instructions.", - file=sys.stderr, - ) - elif sys.platform == "darwin": - print( - "The gn arg use_goma=true will be removed on Feb 7th 2024. " - "Please use `use_remoteexec=true` instead. " - "If you are a googler see http://go/building-chrome-mac" - "#using-remote-execution for setup instructions. ", - file=sys.stderr, - ) - else: - print( - "The gn arg use_goma=true will be removed on Feb 7th 2024. " - "Please use `use_remoteexec=true` instead. See " - "https://chromium.googlesource.com/chromium/src/+/main/docs/" - "linux/build_instructions.md#use-reclient for setup instructions.", - file=sys.stderr, - ) - if not sys.platform.startswith("win"): - # Artificial build delay is for linux/mac for now. - t = 5 - while t > 0: - print( - f"The build will start in {t} seconds.", - file=sys.stderr, - ) - time.sleep(1) - t = t - 1 # A large build (with or without goma) tends to hog all system resources. diff --git a/tests/autoninja_test.py b/tests/autoninja_test.py index d562b4bfd..fc61968cc 100755 --- a/tests/autoninja_test.py +++ b/tests/autoninja_test.py @@ -42,6 +42,28 @@ class AutoninjaTest(trial_dir.TestCase): os.chdir(self.previous_dir) super(AutoninjaTest, self).tearDown() + def autoninja_goma_not_supported(self): + """ + Test that when specifying use_goma=true and on windows, the + message that goma is not supported is displayed. + """ + goma_dir = os.path.join(self.root_dir, 'goma_dir') + with mock.patch.dict(os.environ, {"GOMA_DIR": goma_dir}): + out_dir = os.path.join('out', 'dir') + write(os.path.join(out_dir, 'args.gn'), 'use_goma=true') + write( + os.path.join( + 'goma_dir', 'gomacc.exe' + if sys.platform.startswith('win') else 'gomacc'), 'content') + with contextlib.redirect_stderr(io.StringIO()) as f: + with self.assertRaises(SystemExit): + self.assertEqual( + autoninja.main(['autoninja.py', '-C', out_dir]), 1) + self.maxDiff = None + print(f.getvalue()) + self.assertIn("The gn arg use_goma=true is no longer supported", + f.getvalue()) + def test_autoninja(self): """Test that by default (= no GN args) autoninja delegates to ninja.""" with mock.patch('ninja.main', return_value=0) as ninja_main: @@ -73,17 +95,42 @@ class AutoninjaTest(trial_dir.TestCase): self.assertEqual(args[args.index('-C') + 1], out_dir) self.assertIn('base', args) - def test_autoninja_goma(self): + @mock.patch('sys.platform', 'linux') + def test_autoninja_goma_not_supported_linux(self): + """ + Test that when specifying use_goma=true and on linux, the + message that goma is not supported is displayed. + """ + self.autoninja_goma_not_supported() + + @mock.patch('sys.platform', 'win') + def test_autoninja_goma_not_supported_windows(self): + """ + Test that when specifying use_goma=true and on windows, the + message that goma is not supported is displayed. + """ + self.autoninja_goma_not_supported() + + @mock.patch('sys.platform', 'darwin') + def test_autoninja_goma_not_supported_mac(self): + """ + Test that when specifying use_goma=true and on mac, the + message that goma is not supported is displayed. + """ + self.autoninja_goma_not_supported() + + def test_autoninja_goma_supported_chromeos(self): """ Test that when specifying use_goma=true, autoninja verifies that Goma - is running and then delegates to ninja. + is running and then delegates to ninja when the target_os is chromeos """ goma_dir = os.path.join(self.root_dir, 'goma_dir') with mock.patch('subprocess.call', return_value=0), \ mock.patch('ninja.main', return_value=0) as ninja_main, \ mock.patch.dict(os.environ, {"GOMA_DIR": goma_dir}): out_dir = os.path.join('out', 'dir') - write(os.path.join(out_dir, 'args.gn'), 'use_goma=true') + write(os.path.join(out_dir, 'args.gn'), + 'use_goma=true\ntarget_os=\"chromeos\"') write( os.path.join( 'goma_dir', 'gomacc.exe'