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 <jwata@google.com>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
changes/09/5853009/4
Junji Watanabe 8 months ago committed by LUCI CQ
parent 17226d7965
commit 20a0cda9e9

@ -50,12 +50,16 @@ def ParseGNArgs(gn_args):
"""Parse gn_args as json and return config dictionary.""" """Parse gn_args as json and return config dictionary."""
configs = json.loads(gn_args) configs = json.loads(gn_args)
build_configs = {} build_configs = {}
explicit_keys = []
user = getpass.getuser() user = getpass.getuser()
for config in configs: for config in configs:
key = config["name"] key = config["name"]
if "current" in config: if "current" in config:
value = config["current"]["value"] value = config["current"]["value"]
# Record configs specified in args.gn as explicit configs.
if config["current"]["file"] != "//.gn":
explicit_keys.append(key)
else: else:
value = config["default"]["value"] value = config["default"]["value"]
value = value.strip('"') value = value.strip('"')
@ -68,7 +72,7 @@ def ParseGNArgs(gn_args):
]) ])
build_configs[key] = value build_configs[key] = value
return build_configs return build_configs, explicit_keys
def GetBuildTargetFromCommandLine(cmdline): def GetBuildTargetFromCommandLine(cmdline):
@ -132,15 +136,16 @@ def GetMetadata(cmdline, ninjalog, exit_code, build_duration, user):
build_dir = os.path.dirname(ninjalog) build_dir = os.path.dirname(ninjalog)
build_configs = {} build_configs = {}
explicit_keys = []
try: try:
args = ["gn", "args", build_dir, "--list", "--short", "--json"] args = ["gn", "args", build_dir, "--list", "--json"]
if sys.platform == "win32": if sys.platform == "win32":
# gn in PATH is bat file in windows environment (except cygwin). # gn in PATH is bat file in windows environment (except cygwin).
args = ["cmd", "/c"] + args args = ["cmd", "/c"] + args
gn_args = subprocess.check_output(args) gn_args = subprocess.check_output(args)
build_configs = ParseGNArgs(gn_args) build_configs, explicit_keys = ParseGNArgs(gn_args)
except subprocess.CalledProcessError as e: except subprocess.CalledProcessError as e:
logging.error("Failed to call gn %s", e) logging.error("Failed to call gn %s", e)
build_configs = {} build_configs = {}
@ -156,6 +161,7 @@ def GetMetadata(cmdline, ninjalog, exit_code, build_duration, user):
"platform": platform.system(), "platform": platform.system(),
"cpu_core": multiprocessing.cpu_count(), "cpu_core": multiprocessing.cpu_count(),
"build_configs": build_configs, "build_configs": build_configs,
"explicit_build_config_keys": explicit_keys,
"targets": GetBuildTargetFromCommandLine(cmdline), "targets": GetBuildTargetFromCommandLine(cmdline),
} }

@ -18,15 +18,17 @@ import ninjalog_uploader
class NinjalogUploaderTest(unittest.TestCase): class NinjalogUploaderTest(unittest.TestCase):
def test_parse_gn_args(self): 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. # Extract current configs from GN's output json.
self.assertEqual( gn_args, explicit_keys = ninjalog_uploader.ParseGNArgs(
ninjalog_uploader.ParseGNArgs(
json.dumps([ json.dumps([
{ {
'current': { 'current': {
'value': 'true' 'value': 'true',
'file': '//path/to/args.gn',
}, },
'default': { 'default': {
'value': 'false' 'value': 'false'
@ -39,17 +41,20 @@ class NinjalogUploaderTest(unittest.TestCase):
}, },
'name': 'host_cpu' 'name': 'host_cpu'
}, },
])), { ]))
self.assertEqual(gn_args, {
'is_component_build': 'true', 'is_component_build': 'true',
'host_cpu': 'x64', 'host_cpu': 'x64',
}) })
self.assertEqual(explicit_keys, ['is_component_build'])
self.assertEqual( gn_args, explicit_keys = ninjalog_uploader.ParseGNArgs(
ninjalog_uploader.ParseGNArgs(
json.dumps([ json.dumps([
{ {
'current': { 'current': {
'value': 'true' 'value': 'true',
'file': '//.gn',
}, },
'default': { 'default': {
'value': 'false' 'value': 'false'
@ -58,25 +63,29 @@ class NinjalogUploaderTest(unittest.TestCase):
}, },
{ {
'current': { 'current': {
'value': 'false' 'value': 'false',
'file': '//path/to/args.gn',
}, },
'default': { 'default': {
'value': 'false' 'value': 'false'
}, },
'name': 'use_remoteexec' 'name': 'use_remoteexec'
}, },
])), { ]))
self.assertEqual(gn_args, {
'is_component_build': 'true', 'is_component_build': 'true',
'use_remoteexec': 'false' 'use_remoteexec': 'false'
}) })
self.assertEqual(explicit_keys, ['use_remoteexec'])
# Do not include sensitive information. # Do not include sensitive information.
with unittest.mock.patch('getpass.getuser', return_value='bob'): with unittest.mock.patch('getpass.getuser', return_value='bob'):
args = ninjalog_uploader.ParseGNArgs( gn_args, explicit_keys = ninjalog_uploader.ParseGNArgs(
json.dumps([ json.dumps([
{ {
'current': { 'current': {
'value': 'xyz' 'value': 'xyz',
'file': '//path/to/args.gn',
}, },
'default': { 'default': {
'value': '' 'value': ''
@ -85,7 +94,8 @@ class NinjalogUploaderTest(unittest.TestCase):
}, },
{ {
'current': { 'current': {
'value': '/home/bob/bobo' 'value': '/home/bob/bobo',
'file': '//path/to/args.gn',
}, },
'default': { 'default': {
'value': '' 'value': ''
@ -94,7 +104,7 @@ class NinjalogUploaderTest(unittest.TestCase):
}, },
])) ]))
self.assertEqual( self.assertEqual(
args, { gn_args, {
'google_api_key': '<omitted>', 'google_api_key': '<omitted>',
'path_with_homedir': '/home/$USER/bobo', 'path_with_homedir': '/home/$USER/bobo',
}) })

Loading…
Cancel
Save