ninja: error if trying to build for use_remoteexec=true

Change-Id: Ia32dd3cba1d1874401c6614f792f212b2cfa60a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5660200
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Junji Watanabe <jwata@google.com>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Auto-Submit: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Joanna Wang <jojwang@chromium.org>

autoninja.py

@@ -30,6 +30,7 @@ import warnings
 import google.auth
 from google.auth.transport.requests import AuthorizedSession
 
+import gn_helper
 import ninja
 import ninja_reclient
 import reclient_helper
@@ -182,30 +183,6 @@ def _print_cmd(cmd):
     print(*[shell_quoter(arg) for arg in cmd], file=sys.stderr)
 
 
-def _gn_lines(output_dir, path):
-    """
-    Generator function that returns args.gn lines one at a time, following
-    import directives as needed.
-    """
-    import_re = re.compile(r'\s*import\("(.*)"\)')
-    with open(path, encoding="utf-8") as f:
-        for line in f:
-            match = import_re.match(line)
-            if match:
-                raw_import_path = match.groups()[0]
-                if raw_import_path[:2] == "//":
-                    import_path = os.path.normpath(
-                        os.path.join(output_dir, "..", "..",
-                                     raw_import_path[2:]))
-                else:
-                    import_path = os.path.normpath(
-                        os.path.join(os.path.dirname(path), raw_import_path))
-                for import_line in _gn_lines(output_dir, import_path):
-                    yield import_line
-            else:
-                yield line
-
-
 def main(args):
     # if user doesn't set PYTHONPYCACHEPREFIX and PYTHONDONTWRITEBYTECODE
     # set PYTHONDONTWRITEBYTECODE=1 not to create many *.pyc in workspace
@@ -259,8 +236,8 @@ def main(args):
     # 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.
-    if os.path.exists(os.path.join(output_dir, "args.gn")):
+    if gn_helper.exists(output_dir):
-        for line in _gn_lines(output_dir, os.path.join(output_dir, "args.gn")):
+        for k, v in gn_helper.args(output_dir):
             # use_remoteexec will activate build acceleration.
             #
             # This test can match multi-argument lines. Examples of this
@@ -268,19 +245,13 @@ def main(args):
             # use_remoteexec=false# use_remoteexec=true This comment is ignored
             #
             # Anything after a comment is not consider a valid argument.
-            line_without_comment = line.split("#")[0]
+            if k == "use_remoteexec" and v == "true":
-            if re.search(
-                    r"(^|\s)(use_remoteexec)\s*=\s*true($|\s)",
-                    line_without_comment,
-            ):
                 use_remoteexec = True
                 continue
-            if re.search(r"(^|\s)(use_siso)\s*=\s*true($|\s)",
+            if k == "use_siso" and v == "true":
-                         line_without_comment):
                 use_siso = True
                 continue
-            if re.search(r"(^|\s)(use_reclient)\s*=\s*false($|\s)",
+            if k == "use_reclient" and v == "false":
-                         line_without_comment):
                 use_reclient = False
                 continue
         if use_reclient is None:

gn_helper.py

@@ -0,0 +1,60 @@
+# Copyright 2024 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+"""This provides an easy way to access args.gn."""
+
+import os
+import re
+
+
+def _gn_lines(output_dir, path):
+    """
+    Generator function that returns args.gn lines one at a time, following
+    import directives as needed.
+    """
+    import_re = re.compile(r'\s*import\("(.*)"\)')
+    with open(path, encoding="utf-8") as f:
+        for line in f:
+            match = import_re.match(line)
+            if match:
+                raw_import_path = match.groups()[0]
+                if raw_import_path[:2] == "//":
+                    import_path = os.path.normpath(
+                        os.path.join(output_dir, "..", "..",
+                                     raw_import_path[2:]))
+                else:
+                    import_path = os.path.normpath(
+                        os.path.join(os.path.dirname(path), raw_import_path))
+                yield from _gn_lines(output_dir, import_path)
+            else:
+                yield line
+
+
+def _path(output_dir):
+    return os.path.join(output_dir, "args.gn")
+
+
+def exists(output_dir):
+    """Checks args.gn exists in output_dir."""
+    return os.path.exists(_path(output_dir))
+
+
+def lines(output_dir):
+    """Generator of args.gn lines. comment is removed."""
+    if not exists(output_dir):
+        return
+    for line in _gn_lines(output_dir, _path(output_dir)):
+        line_without_comment = line.split("#")[0]
+        yield line_without_comment
+
+
+_gn_arg_pattern = re.compile(r"(^|\s)([^=\s]*)\s*=\s*(\S*)\s*$")
+
+
+def args(output_dir):
+    """Generator of args.gn's key,value pair."""
+    for line in lines(output_dir):
+        m = _gn_arg_pattern.match(line)
+        if not m:
+            continue
+        yield (m.group(2), m.group(3))

ninja.py

@@ -12,6 +12,7 @@ import subprocess
 import sys
 
 import gclient_paths
+import gn_helper
 
 
 def findNinjaInPath():
@@ -29,7 +30,6 @@ def findNinjaInPath():
         if os.path.isfile(ninja_path):
             return ninja_path
 
-
 def checkOutdir(ninja_args):
     out_dir = "."
     tool = ""
@@ -54,7 +54,16 @@ def checkOutdir(ninja_args):
               (out_dir, out_dir),
               file=sys.stderr)
         sys.exit(1)
-
+    if os.environ.get("AUTONINJA_BUILD_ID"):
+        # autoninja sets AUTONINJA_BUILD_ID
+        return
+    for k, v in gn_helper.args(out_dir):
+        if k == "use_remoteexec" and v == "true":
+            print(
+                "depot_tools/ninja.py: detect use_remoteexec=true\n"
+                "Use `autoninja` to choose appropriate build tool\n",
+                file=sys.stderr)
+            sys.exit(1)
 
 def fallback(ninja_args):
     # Try to find ninja in PATH.

tests/OWNERS

@@ -1,6 +1,7 @@
 per-file autoninja_test.py=brucedawson@chromium.org
 per-file autoninja_test.py=file://BUILD_OWNERS
 per-file bazel_test.py=file://CROS_OWNERS
+per-file gn_helper_test.py=file://BUILD_OWNERS
 per-file ninjalog_uploader_test.py=tikuta@chromium.org
 per-file ninja_reclient_test.py=file://BUILD_OWNERS
 per-file ninja_reclient_test.py=file://RECLIENT_OWNERS

tests/autoninja_test.py

@@ -177,23 +177,6 @@ class AutoninjaTest(trial_dir.TestCase):
                     autoninja._is_google_corp_machine_using_external_account()),
                 expected)
 
-    def test_gn_lines(self):
-        out_dir = os.path.join('out', 'dir')
-        # Make sure nested import directives work. This is based on the
-        # reclient test.
-        write(os.path.join(out_dir, 'args.gn'), 'import("//out/common.gni")')
-        write(os.path.join('out', 'common.gni'), 'import("common_2.gni")')
-        write(os.path.join('out', 'common_2.gni'), 'use_remoteexec=true')
-
-        lines = list(
-            autoninja._gn_lines(out_dir, os.path.join(out_dir, 'args.gn')))
-
-        # The test will only pass if both imports work and
-        # 'use_remoteexec=true' is seen.
-        self.assertListEqual(lines, [
-            'use_remoteexec=true',
-        ])
-
     @mock.patch('sys.platform', 'win32')
     def test_print_cmd_windows(self):
         args = [

tests/gn_helper_test.py

@@ -0,0 +1,72 @@
+#!/usr/bin/env vpython3
+# Copyright (c) 2024 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import os
+import sys
+import unittest
+
+ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+sys.path.insert(0, ROOT_DIR)
+
+import gn_helper
+from testing_support import trial_dir
+
+
+def write(filename, content):
+    """Writes the content of a file and create the directories as needed."""
+    filename = os.path.abspath(filename)
+    dirname = os.path.dirname(filename)
+    if not os.path.isdir(dirname):
+        os.makedirs(dirname)
+    with open(filename, 'w') as f:
+        f.write(content)
+
+
+class GnHelperTest(trial_dir.TestCase):
+
+    def setUp(self):
+        super().setUp()
+        self.previous_dir = os.getcwd()
+        os.chdir(self.root_dir)
+
+    def tearDown(self):
+        os.chdir(self.previous_dir)
+        super().tearDown()
+
+    def test_lines(self):
+        out_dir = os.path.join('out', 'dir')
+        # Make sure nested import directives work. This is based on the
+        # reclient test.
+        write(os.path.join(out_dir, 'args.gn'), 'import("//out/common.gni")')
+        write(os.path.join('out', 'common.gni'), 'import("common_2.gni")')
+        write(os.path.join('out', 'common_2.gni'), 'use_remoteexec=true')
+
+        lines = list(gn_helper.lines(out_dir))
+
+        # The test will only pass if both imports work and
+        # 'use_remoteexec=true' is seen.
+        self.assertListEqual(lines, [
+            'use_remoteexec=true',
+        ])
+
+    def test_args(self):
+        out_dir = os.path.join('out', 'dir')
+        # Make sure nested import directives work. This is based on the
+        # reclient test.
+        write(os.path.join(out_dir, 'args.gn'), 'import("//out/common.gni")')
+        write(os.path.join('out', 'common.gni'), 'import("common_2.gni")')
+        write(os.path.join('out', 'common_2.gni'), 'use_remoteexec=true')
+
+        lines = list(gn_helper.args(out_dir))
+
+        # The test will only pass if both imports work and
+        # 'use_remoteexec=true' is seen.
+        self.assertListEqual(lines, [
+            ('use_remoteexec', 'true'),
+        ])
+
+
+if __name__ == '__main__':
+    unittest.main()