Replace all uses of the word 'master' with 'builder_group' in //tools/mb
This removes every reference but the "--master/-m" cmd-line arg and the "masters" mb_config.pyl key, which will be removed in a follow-up once all users of mb.py (ie: recipes) have switched over. "builder_group" is also the term we're using when replacing "master" in recipe code: crbug.com/1109276. So we should conform on using that term going forward. Bug: chromium:1117773 Change-Id: I1de1b8e68bcf2c9d68b00a05f0f5761cf8b4ef9a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/201382 Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org> Reviewed-by: Dirk Pranke <dpranke@google.com> Cr-Commit-Position: refs/heads/master@{#32964}
This commit is contained in:
parent
6e509f9167
commit
8606b9c218
@ -52,8 +52,8 @@ for a particular build directory, and what set of flags (`GYP_DEFINES` or `gn
|
||||
args`) to use.
|
||||
|
||||
A config can either be specified directly (useful for testing) or by specifying
|
||||
the master name and builder name (useful on the bots so that they do not need
|
||||
to specify a config directly and can be hidden from the details).
|
||||
the builder group name and builder name (useful on the bots so that they do not
|
||||
need to specify a config directly and can be hidden from the details).
|
||||
|
||||
See the [user guide](user_guide.md#mb_config.pyl) for details.
|
||||
|
||||
|
||||
@ -27,8 +27,8 @@ a list of files (e.g., the list of files in a patch on a trybot):
|
||||
mb analyze -c chromium_linux_rel //out/Release input.json output.json
|
||||
```
|
||||
|
||||
Either the `-c/--config` flag or the `-m/--master` and `-b/--builder` flags
|
||||
must be specified so that `mb` can figure out which config to use.
|
||||
Either the `-c/--config` flag or the `-m/--builder-group` and `-b/--builder`
|
||||
flags must be specified so that `mb` can figure out which config to use.
|
||||
|
||||
The first positional argument must be a GN-style "source-absolute" path
|
||||
to the build directory.
|
||||
@ -90,15 +90,16 @@ differences can be subtle. We won't even go into how the `targets` and
|
||||
`build_targets` differ from each other or from `compile_targets` and
|
||||
`test_targets`.
|
||||
|
||||
The `-b/--builder`, `-c/--config`, `-f/--config-file`, `-m/--master`,
|
||||
The `-b/--builder`, `-c/--config`, `-f/--config-file`, `-m/--builder-group`,
|
||||
`-q/--quiet`, and `-v/--verbose` flags work as documented for `mb gen`.
|
||||
|
||||
### `mb audit`
|
||||
|
||||
`mb audit` is used to track the progress of the GYP->GN migration. You can
|
||||
use it to check a single master, or all the masters we care about. See
|
||||
`mb help audit` for more details (most people are not expected to care about
|
||||
this).
|
||||
use it to check a single builder group, or all the builder groups we care
|
||||
about.
|
||||
See `mb help audit` for more details (most people are not expected to care
|
||||
about this).
|
||||
|
||||
### `mb gen`
|
||||
|
||||
@ -111,8 +112,8 @@ a directory, then runs GYP or GN as appropriate:
|
||||
% mb gen -c linux_rel_trybot //out/Release
|
||||
```
|
||||
|
||||
Either the `-c/--config` flag or the `-m/--master` and `-b/--builder` flags
|
||||
must be specified so that `mb` can figure out which config to use. The
|
||||
Either the `-c/--config` flag or the `-m/--builder-group` and `-b/--builder`
|
||||
flags must be specified so that `mb` can figure out which config to use. The
|
||||
`--phase` flag must also be used with builders that have multiple
|
||||
build/compile steps (and only with those builders).
|
||||
|
||||
@ -149,7 +150,7 @@ Produces help output on the other subcommands
|
||||
Prints what command will be run by `mb gen` (like `mb gen -n` but does
|
||||
not require you to specify a path).
|
||||
|
||||
The `-b/--builder`, `-c/--config`, `-f/--config-file`, `-m/--master`,
|
||||
The `-b/--builder`, `-c/--config`, `-f/--config-file`, `-m/--builder-group`,
|
||||
`--phase`, `-q/--quiet`, and `-v/--verbose` flags work as documented for
|
||||
`mb gen`.
|
||||
|
||||
@ -198,11 +199,11 @@ listed here, and so by using the configs in this file you can avoid
|
||||
having to juggle long lists of GYP_DEFINES and gn args by hand.
|
||||
|
||||
`mb_config.pyl` is structured as a file containing a single PYthon Literal
|
||||
expression: a dictionary with three main keys, `masters`, `configs` and
|
||||
expression: a dictionary with three main keys, `builder_groups`, `configs` and
|
||||
`mixins`.
|
||||
|
||||
The `masters` key contains a nested series of dicts containing mappings
|
||||
of master -> builder -> config . This allows us to isolate the buildbot
|
||||
The `builder_groups` key contains a nested series of dicts containing mappings
|
||||
of builder group -> builder -> config . This allows us to isolate the buildbot
|
||||
recipes from the actual details of the configs. The config should either
|
||||
be a single string value representing a key in the `configs` dictionary,
|
||||
or a list of strings, each of which is a key in the `configs` dictionary;
|
||||
|
||||
@ -54,7 +54,7 @@ class MetaBuildWrapper(object):
|
||||
self.sep = os.sep
|
||||
self.args = argparse.Namespace()
|
||||
self.configs = {}
|
||||
self.masters = {}
|
||||
self.builder_groups = {}
|
||||
self.mixins = {}
|
||||
self.isolate_exe = 'isolate.exe' if self.platform.startswith(
|
||||
'win') else 'isolate'
|
||||
@ -80,8 +80,10 @@ class MetaBuildWrapper(object):
|
||||
def AddCommonOptions(subp):
|
||||
subp.add_argument('-b', '--builder',
|
||||
help='builder name to look up config from')
|
||||
subp.add_argument('-m', '--master',
|
||||
help='master name to look up config from')
|
||||
subp.add_argument('-m', '--builder-group',
|
||||
# TODO(crbug.com/1117773): Remove the 'master' args.
|
||||
'--master',
|
||||
help='builder group name to look up config from')
|
||||
subp.add_argument('-c', '--config',
|
||||
help='configuration to analyze')
|
||||
subp.add_argument('--phase',
|
||||
@ -252,10 +254,10 @@ class MetaBuildWrapper(object):
|
||||
def CmdExport(self):
|
||||
self.ReadConfigFile()
|
||||
obj = {}
|
||||
for master, builders in self.masters.items():
|
||||
obj[master] = {}
|
||||
for builder_group, builders in self.builder_groups.items():
|
||||
obj[builder_group] = {}
|
||||
for builder in builders:
|
||||
config = self.masters[master][builder]
|
||||
config = self.builder_groups[builder_group][builder]
|
||||
if not config:
|
||||
continue
|
||||
|
||||
@ -269,7 +271,7 @@ class MetaBuildWrapper(object):
|
||||
if 'error' in args:
|
||||
continue
|
||||
|
||||
obj[master][builder] = args
|
||||
obj[builder_group][builder] = args
|
||||
|
||||
# Dump object and trim trailing whitespace.
|
||||
s = '\n'.join(l.rstrip() for l in
|
||||
@ -402,13 +404,13 @@ class MetaBuildWrapper(object):
|
||||
|
||||
# Build a list of all of the configs referenced by builders.
|
||||
all_configs = {}
|
||||
for master in self.masters:
|
||||
for config in self.masters[master].values():
|
||||
for builder_group in self.builder_groups:
|
||||
for config in self.builder_groups[builder_group].values():
|
||||
if isinstance(config, dict):
|
||||
for c in config.values():
|
||||
all_configs[c] = master
|
||||
all_configs[c] = builder_group
|
||||
else:
|
||||
all_configs[config] = master
|
||||
all_configs[config] = builder_group
|
||||
|
||||
# Check that every referenced args file or config actually exists.
|
||||
for config, loc in all_configs.items():
|
||||
@ -459,7 +461,7 @@ class MetaBuildWrapper(object):
|
||||
build_dir = self.args.path[0]
|
||||
|
||||
vals = self.DefaultVals()
|
||||
if self.args.builder or self.args.master or self.args.config:
|
||||
if self.args.builder or self.args.builder_group or self.args.config:
|
||||
vals = self.Lookup()
|
||||
# Re-run gn gen in order to ensure the config is consistent with the
|
||||
# build dir.
|
||||
@ -517,7 +519,7 @@ class MetaBuildWrapper(object):
|
||||
(self.args.config_file, e))
|
||||
|
||||
self.configs = contents['configs']
|
||||
self.masters = contents['masters']
|
||||
self.builder_groups = contents['builder_groups']
|
||||
self.mixins = contents['mixins']
|
||||
|
||||
def ReadIsolateMap(self):
|
||||
@ -532,38 +534,39 @@ class MetaBuildWrapper(object):
|
||||
|
||||
def ConfigFromArgs(self):
|
||||
if self.args.config:
|
||||
if self.args.master or self.args.builder:
|
||||
raise MBErr('Can not specific both -c/--config and -m/--master or '
|
||||
'-b/--builder')
|
||||
if self.args.builder_group or self.args.builder:
|
||||
raise MBErr('Can not specific both -c/--config and -m/--builder-group '
|
||||
'or -b/--builder')
|
||||
|
||||
return self.args.config
|
||||
|
||||
if not self.args.master or not self.args.builder:
|
||||
if not self.args.builder_group or not self.args.builder:
|
||||
raise MBErr('Must specify either -c/--config or '
|
||||
'(-m/--master and -b/--builder)')
|
||||
'(-m/--builder-group and -b/--builder)')
|
||||
|
||||
if not self.args.master in self.masters:
|
||||
if not self.args.builder_group in self.builder_groups:
|
||||
raise MBErr('Master name "%s" not found in "%s"' %
|
||||
(self.args.master, self.args.config_file))
|
||||
(self.args.builder_group, self.args.config_file))
|
||||
|
||||
if not self.args.builder in self.masters[self.args.master]:
|
||||
raise MBErr('Builder name "%s" not found under masters[%s] in "%s"' %
|
||||
(self.args.builder, self.args.master, self.args.config_file))
|
||||
if not self.args.builder in self.builder_groups[self.args.builder_group]:
|
||||
raise MBErr(
|
||||
'Builder name "%s" not found under builder_groups[%s] in "%s"' %
|
||||
(self.args.builder, self.args.builder_group, self.args.config_file))
|
||||
|
||||
config = self.masters[self.args.master][self.args.builder]
|
||||
config = self.builder_groups[self.args.builder_group][self.args.builder]
|
||||
if isinstance(config, dict):
|
||||
if self.args.phase is None:
|
||||
raise MBErr('Must specify a build --phase for %s on %s' %
|
||||
(self.args.builder, self.args.master))
|
||||
(self.args.builder, self.args.builder_group))
|
||||
phase = str(self.args.phase)
|
||||
if phase not in config:
|
||||
raise MBErr('Phase %s doesn\'t exist for %s on %s' %
|
||||
(phase, self.args.builder, self.args.master))
|
||||
(phase, self.args.builder, self.args.builder_group))
|
||||
return config[phase]
|
||||
|
||||
if self.args.phase is not None:
|
||||
raise MBErr('Must not specify a build --phase for %s on %s' %
|
||||
(self.args.builder, self.args.master))
|
||||
(self.args.builder, self.args.builder_group))
|
||||
return config
|
||||
|
||||
def FlattenConfig(self, config):
|
||||
|
||||
@ -12,12 +12,12 @@
|
||||
# easy to try different configurations of GN args in tryjob patches.
|
||||
|
||||
{
|
||||
# This is a map of buildbot master names -> buildbot builder names ->
|
||||
# This is a map of buildbot builder group names -> buildbot builder names ->
|
||||
# config names (where each config name is a key in the 'configs' dict,
|
||||
# above). mb uses this dict to look up which config to use for a given bot.
|
||||
# The builders should be sorted by the order they appear in the /builders
|
||||
# page on the buildbots, *not* alphabetically.
|
||||
'masters': {
|
||||
'builder_groups': {
|
||||
'client.webrtc': {
|
||||
# iOS
|
||||
'iOS32 Debug': 'ios_debug_bot_arm',
|
||||
|
||||
@ -111,12 +111,12 @@ class FakeFile(object):
|
||||
|
||||
TEST_CONFIG = """\
|
||||
{
|
||||
'masters': {
|
||||
'builder_groups': {
|
||||
'chromium': {},
|
||||
'fake_master': {
|
||||
'fake_group': {
|
||||
'fake_builder': 'rel_bot',
|
||||
'fake_debug_builder': 'debug_goma',
|
||||
'fake_args_bot': '//build/args/bots/fake_master/fake_args_bot.gn',
|
||||
'fake_args_bot': '//build/args/bots/fake_group/fake_args_bot.gn',
|
||||
'fake_multi_phase': { 'phase_1': 'phase_1', 'phase_2': 'phase_2'},
|
||||
'fake_android_bot': 'android_bot',
|
||||
},
|
||||
@ -169,7 +169,7 @@ class UnitTest(unittest.TestCase):
|
||||
},
|
||||
}''')
|
||||
mbw.files.setdefault(
|
||||
mbw.ToAbsPath('//build/args/bots/fake_master/fake_args_bot.gn'),
|
||||
mbw.ToAbsPath('//build/args/bots/fake_group/fake_args_bot.gn'),
|
||||
'is_debug = false\n')
|
||||
if files:
|
||||
for path, contents in files.items():
|
||||
@ -238,12 +238,12 @@ class UnitTest(unittest.TestCase):
|
||||
'--check\n', mbw.out)
|
||||
|
||||
mbw = self.fake_mbw()
|
||||
self.check(['gen', '-m', 'fake_master', '-b', 'fake_args_bot',
|
||||
self.check(['gen', '-m', 'fake_group', '-b', 'fake_args_bot',
|
||||
'//out/Debug'],
|
||||
mbw=mbw, ret=0)
|
||||
self.assertEqual(
|
||||
mbw.files['/fake_src/out/Debug/args.gn'],
|
||||
'import("//build/args/bots/fake_master/fake_args_bot.gn")\n\n')
|
||||
'import("//build/args/bots/fake_group/fake_args_bot.gn")\n\n')
|
||||
|
||||
|
||||
def test_gen_fails(self):
|
||||
@ -801,26 +801,26 @@ class UnitTest(unittest.TestCase):
|
||||
|
||||
def test_multiple_phases(self):
|
||||
# Check that not passing a --phase to a multi-phase builder fails.
|
||||
mbw = self.check(['lookup', '-m', 'fake_master', '-b', 'fake_multi_phase'],
|
||||
mbw = self.check(['lookup', '-m', 'fake_group', '-b', 'fake_multi_phase'],
|
||||
ret=1)
|
||||
self.assertIn('Must specify a build --phase', mbw.out)
|
||||
|
||||
# Check that passing a --phase to a single-phase builder fails.
|
||||
mbw = self.check(['lookup', '-m', 'fake_master', '-b', 'fake_builder',
|
||||
mbw = self.check(['lookup', '-m', 'fake_group', '-b', 'fake_builder',
|
||||
'--phase', 'phase_1'], ret=1)
|
||||
self.assertIn('Must not specify a build --phase', mbw.out)
|
||||
|
||||
# Check that passing a wrong phase key to a multi-phase builder fails.
|
||||
mbw = self.check(['lookup', '-m', 'fake_master', '-b', 'fake_multi_phase',
|
||||
mbw = self.check(['lookup', '-m', 'fake_group', '-b', 'fake_multi_phase',
|
||||
'--phase', 'wrong_phase'], ret=1)
|
||||
self.assertIn('Phase wrong_phase doesn\'t exist', mbw.out)
|
||||
|
||||
# Check that passing a correct phase key to a multi-phase builder passes.
|
||||
mbw = self.check(['lookup', '-m', 'fake_master', '-b', 'fake_multi_phase',
|
||||
mbw = self.check(['lookup', '-m', 'fake_group', '-b', 'fake_multi_phase',
|
||||
'--phase', 'phase_1'], ret=0)
|
||||
self.assertIn('phase = 1', mbw.out)
|
||||
|
||||
mbw = self.check(['lookup', '-m', 'fake_master', '-b', 'fake_multi_phase',
|
||||
mbw = self.check(['lookup', '-m', 'fake_group', '-b', 'fake_multi_phase',
|
||||
'--phase', 'phase_2'], ret=0)
|
||||
self.assertIn('phase = 2', mbw.out)
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user