From 50de666ba40a4808daf9791fece3d8a43228a1de Mon Sep 17 00:00:00 2001 From: Michael Savigny Date: Wed, 28 Feb 2024 20:29:18 +0000 Subject: [PATCH] Revert "Remove ability to use_goma in autoninja (chromeos excepted)" This reverts commit 6b84fbfb201f8fd99c3d049328615943a7df3593. Reason for revert: Jumped the gun before final resolution for b/316921158 Original change's description: > 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 Bug: b/277197166 Change-Id: Iaf506a10dfdc1c1672b2927636adf5773edc67c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5333176 Reviewed-by: Bruce Dawson Auto-Submit: Michael Savigny Bot-Commit: Rubber Stamper Commit-Queue: Bruce Dawson Reviewed-by: Philipp Wollermann --- autoninja.py | 72 ++++++++++++++++++++++------------------- tests/autoninja_test.py | 53 ++---------------------------- 2 files changed, 41 insertions(+), 84 deletions(-) diff --git a/autoninja.py b/autoninja.py index 312328a95..d00b28358 100755 --- a/autoninja.py +++ b/autoninja.py @@ -228,13 +228,10 @@ 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 @@ -260,10 +257,6 @@ 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(): @@ -339,33 +332,6 @@ 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", @@ -395,6 +361,44 @@ 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 fc61968cc..d562b4bfd 100755 --- a/tests/autoninja_test.py +++ b/tests/autoninja_test.py @@ -42,28 +42,6 @@ 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: @@ -95,42 +73,17 @@ class AutoninjaTest(trial_dir.TestCase): self.assertEqual(args[args.index('-C') + 1], out_dir) self.assertIn('base', args) - @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): + def test_autoninja_goma(self): """ Test that when specifying use_goma=true, autoninja verifies that Goma - is running and then delegates to ninja when the target_os is chromeos + is running and then delegates to ninja. """ 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\ntarget_os=\"chromeos\"') + write(os.path.join(out_dir, 'args.gn'), 'use_goma=true') write( os.path.join( 'goma_dir', 'gomacc.exe'