Commit 65ccf613 authored by Ben Pastene's avatar Ben Pastene Committed by Commit Bot

mb: Add a top-level comment to GN args when using the Simple Chrome SDK.

This will add a comment to the gn_arg log link on chromium bots:
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8930669115526655984/+/steps/lookup_GN_args/0/logs/gn_args/0

This will hopefully help avoid confusion when a non-CrOS chromium dev
gets a CL blocked by a simplechrome bot. (See the linked bug where
someone wasn't familiar with it and tried copy-pasting the arg dump
outside the SDK.)

I looked into adding the comment further upstream in chromite's
cros_chrome_sdk libs, but it makes heavy use of gn_helpers, and adding
comment support to that is non-trivial.

This can't land until https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1320309
and gets rolled into chromium.

Bug: 901471
Change-Id: I9ad646a9b6ab6fd29d4adb7acfd842b26c3825ae
Reviewed-on: https://chromium-review.googlesource.com/c/1318179Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606257}
parent 8c11f123
...@@ -1047,6 +1047,17 @@ class MetaBuildWrapper(object): ...@@ -1047,6 +1047,17 @@ class MetaBuildWrapper(object):
# the last instance of each arg is listed. # the last instance of each arg is listed.
gn_args = gn_helpers.ToGNString(gn_helpers.FromGNArgs(gn_args)) gn_args = gn_helpers.ToGNString(gn_helpers.FromGNArgs(gn_args))
# If we're using the Simple Chrome SDK, add a comment at the top that
# points to the doc. This must happen after the gn_helpers.ToGNString()
# call above since gn_helpers strips comments.
if vals['cros_passthrough']:
simplechrome_comment = [
'# These args are generated via the Simple Chrome SDK. See the link',
'# below for more details:',
'# https://chromium.googlesource.com/chromiumos/docs/+/master/simple_chrome_workflow.md', # pylint: disable=line-too-long
]
gn_args = '%s\n%s' % ('\n'.join(simplechrome_comment), gn_args)
args_file = vals.get('args_file', None) args_file = vals.get('args_file', None)
if args_file: if args_file:
gn_args = ('import("%s")\n' % vals['args_file']) + gn_args gn_args = ('import("%s")\n' % vals['args_file']) + gn_args
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
"""Tests for mb.py.""" """Tests for mb.py."""
import json import json
import os
import StringIO import StringIO
import sys import sys
import unittest import unittest
...@@ -110,6 +111,7 @@ TEST_CONFIG = """\ ...@@ -110,6 +111,7 @@ TEST_CONFIG = """\
'fake_master': { 'fake_master': {
'fake_builder': 'rel_bot', 'fake_builder': 'rel_bot',
'fake_debug_builder': 'debug_goma', 'fake_debug_builder': 'debug_goma',
'fake_simplechrome_builder': 'cros_chrome_sdk',
'fake_args_bot': '//build/args/bots/fake_master/fake_args_bot.gn', 'fake_args_bot': '//build/args/bots/fake_master/fake_args_bot.gn',
'fake_multi_phase': { 'phase_1': 'phase_1', 'phase_2': 'phase_2'}, 'fake_multi_phase': { 'phase_1': 'phase_1', 'phase_2': 'phase_2'},
'fake_args_file': 'args_file_goma', 'fake_args_file': 'args_file_goma',
...@@ -119,12 +121,16 @@ TEST_CONFIG = """\ ...@@ -119,12 +121,16 @@ TEST_CONFIG = """\
'configs': { 'configs': {
'args_file_goma': ['args_file', 'goma'], 'args_file_goma': ['args_file', 'goma'],
'args_file_twice': ['args_file', 'args_file'], 'args_file_twice': ['args_file', 'args_file'],
'cros_chrome_sdk': ['cros_chrome_sdk'],
'rel_bot': ['rel', 'goma', 'fake_feature1'], 'rel_bot': ['rel', 'goma', 'fake_feature1'],
'debug_goma': ['debug', 'goma'], 'debug_goma': ['debug', 'goma'],
'phase_1': ['phase_1'], 'phase_1': ['phase_1'],
'phase_2': ['phase_2'], 'phase_2': ['phase_2'],
}, },
'mixins': { 'mixins': {
'cros_chrome_sdk': {
'cros_passthrough': True,
},
'fake_feature1': { 'fake_feature1': {
'gn_args': 'enable_doom_melon=true', 'gn_args': 'enable_doom_melon=true',
}, },
...@@ -217,11 +223,17 @@ class UnitTest(unittest.TestCase): ...@@ -217,11 +223,17 @@ class UnitTest(unittest.TestCase):
mbw.files[path] = contents mbw.files[path] = contents
return mbw return mbw
def check(self, args, mbw=None, files=None, out=None, err=None, ret=None): def check(self, args, mbw=None, files=None, out=None, err=None, ret=None,
env=None):
if not mbw: if not mbw:
mbw = self.fake_mbw(files) mbw = self.fake_mbw(files)
actual_ret = mbw.Main(args) try:
prev_env = os.environ.copy()
os.environ = env if env else prev_env
actual_ret = mbw.Main(args)
finally:
os.environ = prev_env
self.assertEqual(actual_ret, ret) self.assertEqual(actual_ret, ret)
if out is not None: if out is not None:
...@@ -583,6 +595,12 @@ class UnitTest(unittest.TestCase): ...@@ -583,6 +595,12 @@ class UnitTest(unittest.TestCase):
'""" to _path_/args.gn.\n\n' '""" to _path_/args.gn.\n\n'
'/fake_src/buildtools/linux64/gn gen _path_\n')) '/fake_src/buildtools/linux64/gn gen _path_\n'))
def test_lookup_simplechrome(self):
simplechrome_env = {
'GN_ARGS': 'is_chromeos=1 target_os="chromeos"',
}
self.check(['lookup', '-c', 'cros_chrome_sdk'], ret=0, env=simplechrome_env)
def test_help(self): def test_help(self):
orig_stdout = sys.stdout orig_stdout = sys.stdout
try: try:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment