From 20a0cda9e99abf86d37cb42cd98b1be9b727fdf8 Mon Sep 17 00:00:00 2001 From: Junji Watanabe Date: Thu, 12 Sep 2024 03:00:34 +0000 Subject: [PATCH] ninjalog_uploader: Distinguish between explicit and default build configs This CL adds `explicit_build_config_keys` to the metadata for the server to distinguish between explicit and default configs. This CL excludes the configs set in `//.gn` from the list, so that the configs specified by users will be considered as explicit. Bug: 364971744 Change-Id: I6a3d289d8ca6e054dac691051b53c92ad1ec23c5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5853009 Commit-Queue: Junji Watanabe Reviewed-by: Takuto Ikuta --- ninjalog_uploader.py | 12 +++- tests/ninjalog_uploader_test.py | 108 +++++++++++++++++--------------- 2 files changed, 68 insertions(+), 52 deletions(-) diff --git a/ninjalog_uploader.py b/ninjalog_uploader.py index 5cd741339..2231325dd 100755 --- a/ninjalog_uploader.py +++ b/ninjalog_uploader.py @@ -50,12 +50,16 @@ def ParseGNArgs(gn_args): """Parse gn_args as json and return config dictionary.""" configs = json.loads(gn_args) build_configs = {} + explicit_keys = [] user = getpass.getuser() for config in configs: key = config["name"] if "current" in config: value = config["current"]["value"] + # Record configs specified in args.gn as explicit configs. + if config["current"]["file"] != "//.gn": + explicit_keys.append(key) else: value = config["default"]["value"] value = value.strip('"') @@ -68,7 +72,7 @@ def ParseGNArgs(gn_args): ]) build_configs[key] = value - return build_configs + return build_configs, explicit_keys def GetBuildTargetFromCommandLine(cmdline): @@ -132,15 +136,16 @@ def GetMetadata(cmdline, ninjalog, exit_code, build_duration, user): build_dir = os.path.dirname(ninjalog) build_configs = {} + explicit_keys = [] try: - args = ["gn", "args", build_dir, "--list", "--short", "--json"] + args = ["gn", "args", build_dir, "--list", "--json"] if sys.platform == "win32": # gn in PATH is bat file in windows environment (except cygwin). args = ["cmd", "/c"] + args gn_args = subprocess.check_output(args) - build_configs = ParseGNArgs(gn_args) + build_configs, explicit_keys = ParseGNArgs(gn_args) except subprocess.CalledProcessError as e: logging.error("Failed to call gn %s", e) build_configs = {} @@ -156,6 +161,7 @@ def GetMetadata(cmdline, ninjalog, exit_code, build_duration, user): "platform": platform.system(), "cpu_core": multiprocessing.cpu_count(), "build_configs": build_configs, + "explicit_build_config_keys": explicit_keys, "targets": GetBuildTargetFromCommandLine(cmdline), } diff --git a/tests/ninjalog_uploader_test.py b/tests/ninjalog_uploader_test.py index 13b408c7a..f7b54b292 100755 --- a/tests/ninjalog_uploader_test.py +++ b/tests/ninjalog_uploader_test.py @@ -18,65 +18,74 @@ import ninjalog_uploader class NinjalogUploaderTest(unittest.TestCase): def test_parse_gn_args(self): - self.assertEqual(ninjalog_uploader.ParseGNArgs(json.dumps([])), {}) + gn_args, explicit_keys = ninjalog_uploader.ParseGNArgs(json.dumps([])) + self.assertEqual(gn_args, {}) + self.assertEqual(explicit_keys, []) # Extract current configs from GN's output json. - self.assertEqual( - ninjalog_uploader.ParseGNArgs( - json.dumps([ - { - 'current': { - 'value': 'true' - }, - 'default': { - 'value': 'false' - }, - 'name': 'is_component_build' + gn_args, explicit_keys = ninjalog_uploader.ParseGNArgs( + json.dumps([ + { + 'current': { + 'value': 'true', + 'file': '//path/to/args.gn', }, - { - 'default': { - 'value': '"x64"' - }, - 'name': 'host_cpu' + 'default': { + 'value': 'false' }, - ])), { - 'is_component_build': 'true', - 'host_cpu': 'x64', - }) - - self.assertEqual( - ninjalog_uploader.ParseGNArgs( - json.dumps([ - { - 'current': { - 'value': 'true' - }, - 'default': { - 'value': 'false' - }, - 'name': 'is_component_build' + 'name': 'is_component_build' + }, + { + 'default': { + 'value': '"x64"' }, - { - 'current': { - 'value': 'false' - }, - 'default': { - 'value': 'false' - }, - 'name': 'use_remoteexec' + 'name': 'host_cpu' + }, + ])) + + self.assertEqual(gn_args, { + 'is_component_build': 'true', + 'host_cpu': 'x64', + }) + self.assertEqual(explicit_keys, ['is_component_build']) + + gn_args, explicit_keys = ninjalog_uploader.ParseGNArgs( + json.dumps([ + { + 'current': { + 'value': 'true', + 'file': '//.gn', }, - ])), { - 'is_component_build': 'true', - 'use_remoteexec': 'false' - }) + 'default': { + 'value': 'false' + }, + 'name': 'is_component_build' + }, + { + 'current': { + 'value': 'false', + 'file': '//path/to/args.gn', + }, + 'default': { + 'value': 'false' + }, + 'name': 'use_remoteexec' + }, + ])) + self.assertEqual(gn_args, { + 'is_component_build': 'true', + 'use_remoteexec': 'false' + }) + self.assertEqual(explicit_keys, ['use_remoteexec']) # Do not include sensitive information. with unittest.mock.patch('getpass.getuser', return_value='bob'): - args = ninjalog_uploader.ParseGNArgs( + gn_args, explicit_keys = ninjalog_uploader.ParseGNArgs( json.dumps([ { 'current': { - 'value': 'xyz' + 'value': 'xyz', + 'file': '//path/to/args.gn', }, 'default': { 'value': '' @@ -85,7 +94,8 @@ class NinjalogUploaderTest(unittest.TestCase): }, { 'current': { - 'value': '/home/bob/bobo' + 'value': '/home/bob/bobo', + 'file': '//path/to/args.gn', }, 'default': { 'value': '' @@ -94,7 +104,7 @@ class NinjalogUploaderTest(unittest.TestCase): }, ])) self.assertEqual( - args, { + gn_args, { 'google_api_key': '', 'path_with_homedir': '/home/$USER/bobo', })