Browse Source

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 <tikuta@chromium.org>
Commit-Queue: Michael Savigny <msavigny@google.com>
Michael Savigny 1 year ago
parent
commit
6b84fbfb20
2 changed files with 84 additions and 41 deletions
  1. 34 38
      autoninja.py
  2. 50 3
      tests/autoninja_test.py

+ 34 - 38
autoninja.py

@@ -228,10 +228,13 @@ def main(args):
     use_goma = False
     use_goma = False
     use_remoteexec = False
     use_remoteexec = False
     use_siso = False
     use_siso = False
+    target_chromeos = False
 
 
     # Attempt to auto-detect remote build acceleration. We support gn-based
     # 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 args.gn in the build tree, and cmake-based
     # builds where we look for rules.ninja.
     # 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")):
     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")):
         for line in _gn_lines(output_dir, os.path.join(output_dir, "args.gn")):
             # use_goma, or use_remoteexec will activate build
             # use_goma, or use_remoteexec will activate build
@@ -257,6 +260,10 @@ def main(args):
                          line_without_comment):
                          line_without_comment):
                 use_siso = True
                 use_siso = True
                 continue
                 continue
+            if re.search(r"(^|\s)(target_os)\s*=\s*\"chromeos\"($|\s)",
+                         line_without_comment):
+                target_chromeos = True
+                continue
 
 
         if use_remoteexec:
         if use_remoteexec:
             if _is_google_corp_machine_using_external_account():
             if _is_google_corp_machine_using_external_account():
@@ -332,6 +339,33 @@ def main(args):
         use_goma = False
         use_goma = False
 
 
     if use_goma:
     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"
         gomacc_file = ("gomacc.exe"
                        if sys.platform.startswith("win") else "gomacc")
                        if sys.platform.startswith("win") else "gomacc")
         goma_dir = os.environ.get("GOMA_DIR",
         goma_dir = os.environ.get("GOMA_DIR",
@@ -361,44 +395,6 @@ def main(args):
                     # script.
                     # script.
                     print("false")
                     print("false")
                 sys.exit(1)
                 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.
     # A large build (with or without goma) tends to hog all system resources.

+ 50 - 3
tests/autoninja_test.py

@@ -42,6 +42,28 @@ class AutoninjaTest(trial_dir.TestCase):
         os.chdir(self.previous_dir)
         os.chdir(self.previous_dir)
         super(AutoninjaTest, self).tearDown()
         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):
     def test_autoninja(self):
         """Test that by default (= no GN args) autoninja delegates to ninja."""
         """Test that by default (= no GN args) autoninja delegates to ninja."""
         with mock.patch('ninja.main', return_value=0) as ninja_main:
         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.assertEqual(args[args.index('-C') + 1], out_dir)
         self.assertIn('base', args)
         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
         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')
         goma_dir = os.path.join(self.root_dir, 'goma_dir')
         with mock.patch('subprocess.call', return_value=0), \
         with mock.patch('subprocess.call', return_value=0), \
              mock.patch('ninja.main', return_value=0) as ninja_main, \
              mock.patch('ninja.main', return_value=0) as ninja_main, \
              mock.patch.dict(os.environ, {"GOMA_DIR": goma_dir}):
              mock.patch.dict(os.environ, {"GOMA_DIR": goma_dir}):
             out_dir = os.path.join('out', '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(
             write(
                 os.path.join(
                 os.path.join(
                     'goma_dir', 'gomacc.exe'
                     'goma_dir', 'gomacc.exe'