Cross validate mb_config_buckets.pyl

Make mb validate checks for internal consistency
between mb_config.pyl and mb_config_bucket.pyl

Also regenerate mb_config_buckets.pyl using
https://drive.google.com/a/google.com/file/d/1rypZZRxxMdyPZ8zAYt0GanfRkAHgtsYe/view?usp=sharing

Bug: 1062141
Change-Id: I865a2ca64b26dc7a6c7415b4a6f87a439b0e5241
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2108793
Commit-Queue: Gregory Guterman <guterman@google.com>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755584}

tools/mb/lib/validation.py

@@ -3,7 +3,9 @@
 # found in the LICENSE file.
 """Validation functions for the Meta-Build config file"""
 
+import ast
 import collections
+import json
 
 
 def GetAllConfigsMaster(masters):
@@ -132,6 +134,83 @@ def EnsureNoProprietaryMixinsMaster(errs, default_config, config_file, masters,
                   'responsible for public build artifacts.')
 
 
+def _GetConfigsByBuilder(masters):
+  """Builds a mapping from buildername -> [config]
+
+    Args
+      masters: the master's dict from mb_config.pyl
+    """
+
+  result = collections.defaultdict(list)
+  for master in masters.values():
+    for buildername, builder in master.items():
+      result[buildername].append(builder)
+
+  return result
+
+
+def CheckMasterBucketConsistency(errs, masters_file, buckets_file):
+  """Checks that mb_config_buckets.pyl is consistent with mb_config.pyl
+
+    mb_config_buckets.pyl is a subset of mb_config.pyl.
+    Make sure all configs that do exist are consistent.
+    Populates errs with any errors
+
+    Args:
+      errs: an accumulator for errors
+      masters_file: string form of mb_config.pyl
+      bucket_file: string form of mb_config_buckets.pyl
+    """
+  master_contents = ast.literal_eval(masters_file)
+  bucket_contents = ast.literal_eval(buckets_file)
+
+  def check_missing(bucket_dict, master_dict):
+    return [name for name in bucket_dict.keys() if name not in master_dict]
+
+  # Cross check builders
+  configs_by_builder = _GetConfigsByBuilder(master_contents['masters'])
+  for builders in bucket_contents['buckets'].values():
+    missing = check_missing(builders, configs_by_builder)
+    errs.extend('Builder "%s" from mb_config_buckets.pyl '
+                'not found in mb_config.pyl' % builder for builder in missing)
+
+    for buildername, config in builders.items():
+      if config not in configs_by_builder[buildername]:
+        errs.append('Builder "%s" from mb_config_buckets.pyl '
+                    'doesn\'t match mb_config.pyl' % buildername)
+
+  def check_mismatch(bucket_dict, master_dict):
+    mismatched = []
+    for configname, config in bucket_dict.items():
+      if configname in master_dict and config != master_dict[configname]:
+        mismatched.append(configname)
+
+    return mismatched
+
+  # Cross check configs
+  missing = check_missing(bucket_contents['configs'],
+                          master_contents['configs'])
+  errs.extend('Config "%s" from mb_config_buckets.pyl '
+              'not found in mb_config.pyl' % config for config in missing)
+
+  mismatched = check_mismatch(bucket_contents['configs'],
+                              master_contents['configs'])
+  errs.extend(
+      'Config "%s" from mb_config_buckets.pyl doesn\'t match mb_config.pyl' %
+      config for config in mismatched)
+
+  # Cross check mixins
+  missing = check_missing(bucket_contents['mixins'], master_contents['mixins'])
+  errs.extend('Mixin "%s" from mb_config_buckets.pyl '
+              'not found in mb_config.pyl' % mixin for mixin in missing)
+
+  mismatched = check_mismatch(bucket_contents['mixins'],
+                              master_contents['mixins'])
+  errs.extend(
+      'Mixin "%s" from mb_config_buckets.pyl doesn\'t match mb_config.pyl' %
+      mixin for mixin in mismatched)
+
+
 def CheckDuplicateConfigs(errs, config_pool, mixin_pool, grouping,
                           flatten_config):
   """Check for duplicate configs.

tools/mb/mb.py

@@ -383,7 +383,7 @@ class MetaBuildWrapper(object):
     return self.RunGNAnalyze(vals)
 
   def CmdExportBucket(self):
-    self.ReadConfigFile()
+    self.ReadConfigFile(self.args.config_file)
     obj = {}
     for bucket, builders in self.buckets.items():
       obj[bucket] = {}
@@ -418,7 +418,7 @@ class MetaBuildWrapper(object):
     if self.group_by_bucket:
       return self.CmdExportBucket()
 
-    self.ReadConfigFile()
+    self.ReadConfigFile(self.args.config_file)
     obj = {}
     for master, builders in self.masters.items():
       obj[master] = {}
@@ -795,11 +795,14 @@ class MetaBuildWrapper(object):
                                      self.buckets, FlattenConfig)
 
     if errs:
-      raise MBErr(('mb config file %s has problems:' % self.args.config_file) +
-                  '\n  ' + '\n  '.join(errs))
+      raise MBErr(('mb config file %s has problems:' %
+                   (self.args.config_file if self.args.config_file else self.
+                    default_config_bucket)) + '\n  ' + '\n  '.join(errs))
 
     if print_ok:
-      self.Print('mb config file %s looks ok.' % self.args.config_file)
+      self.Print('mb config file %s looks ok.' %
+                 (self.args.config_file
+                  if self.args.config_file else self.default_config_bucket))
     return 0
 
   def CmdValidate(self, print_ok=True):
@@ -808,15 +811,20 @@ class MetaBuildWrapper(object):
     # Validate both bucket and master configs if
     # a specific one isn't specified
     if getattr(self.args, 'config_file', None) is None:
+      # Cross reference bucket and master configs
+      # Bucket configs should be a subset of master configs, but consistent for
+      # any builders present in both.
+      validation.CheckMasterBucketConsistency(
+          errs, self.ReadFile(self.default_config_master),
+          self.ReadFile(self.default_config_bucket))
+
       # Read the file to make sure it parses.
-      self.args.config_file = self.default_config_bucket
-      self.ReadConfigFile()
+      self.ReadConfigFile(self.default_config_bucket)
       self.CmdValidateBucket()
 
-      self.args.config_file = self.default_config_master
-      self.ReadConfigFile()
+      self.ReadConfigFile(self.default_config_master)
     else:
-      self.ReadConfigFile()
+      self.ReadConfigFile(self.args.config_file)
       if self.group_by_bucket:
         return self.CmdValidateBucket()
 
@@ -845,11 +853,14 @@ class MetaBuildWrapper(object):
                                      self.masters, FlattenConfig)
 
     if errs:
-      raise MBErr(('mb config file %s has problems:' % self.args.config_file) +
-                    '\n  ' + '\n  '.join(errs))
+      raise MBErr(('mb config file %s has problems:' %
+                   (self.args.config_file if self.args.config_file else self.
+                    default_config_master)) + '\n  ' + '\n  '.join(errs))
 
     if print_ok:
-      self.Print('mb config file %s looks ok.' % self.args.config_file)
+      self.Print('mb config file %s looks ok.' %
+                 (self.args.config_file
+                  if self.args.config_file else self.default_config_master))
     return 0
 
   def GetConfig(self):
@@ -890,7 +901,7 @@ class MetaBuildWrapper(object):
     return ' '.join(gn_args)
 
   def Lookup(self):
-    self.ReadConfigFile()
+    self.ReadConfigFile(self.args.config_file)
     try:
       if self.group_by_bucket:
         config = self.ConfigFromArgsBucket()
@@ -946,15 +957,14 @@ class MetaBuildWrapper(object):
     vals['gn_args'] = gn_args
     return vals
 
-  def ReadConfigFile(self):
-    if not self.Exists(self.args.config_file):
-      raise MBErr('config file not found at %s' % self.args.config_file)
+  def ReadConfigFile(self, config_file):
+    if not self.Exists(config_file):
+      raise MBErr('config file not found at %s' % config_file)
 
     try:
-      contents = ast.literal_eval(self.ReadFile(self.args.config_file))
+      contents = ast.literal_eval(self.ReadFile(config_file))
     except SyntaxError as e:
-      raise MBErr('Failed to parse config file "%s": %s' %
-                 (self.args.config_file, e))
+      raise MBErr('Failed to parse config file "%s": %s' % (config_file, e))
 
     self.configs = contents['configs']
     self.mixins = contents['mixins']

tools/mb/mb_unittest.py

@@ -236,6 +236,58 @@ TEST_CONFIG_BUCKET = """\
 }
 """
 
+# Same as previous config but with fake_debug_builder
+# and fake_simplechrome_builder configs swapped
+TEST_CONFIG_BUCKET_2 = """\
+{
+  'public_artifact_builders': {},
+  'buckets': {
+    'ci': {
+      'fake_builder': 'rel_bot',
+      'fake_debug_builder': 'cros_chrome_sdk',
+      'fake_simplechrome_builder': 'debug_goma',
+      'fake_args_bot': '//build/args/bots/fake_master/fake_args_bot.gn',
+      'fake_multi_phase': { 'phase_1': 'phase_1', 'phase_2': 'phase_2'},
+      'fake_args_file': 'args_file_goma',
+    }
+  },
+  'configs': {
+    'args_file_goma': ['args_file', 'goma'],
+    'cros_chrome_sdk': ['cros_chrome_sdk'],
+    'rel_bot': ['rel', 'goma', 'fake_feature1'],
+    'debug_goma': ['debug', 'goma'],
+    'phase_1': ['phase_1'],
+    'phase_2': ['phase_2'],
+  },
+  'mixins': {
+    'cros_chrome_sdk': {
+      'cros_passthrough': True,
+    },
+    'fake_feature1': {
+      'gn_args': 'enable_doom_melon=true',
+    },
+    'goma': {
+      'gn_args': 'use_goma=true',
+    },
+    'args_file': {
+      'args_file': '//build/args/fake.gn',
+    },
+    'phase_1': {
+      'gn_args': 'phase=1',
+    },
+    'phase_2': {
+      'gn_args': 'phase=2',
+    },
+    'rel': {
+      'gn_args': 'is_debug=false',
+    },
+    'debug': {
+      'gn_args': 'is_debug=true',
+    },
+  },
+}
+"""
+
 TEST_BAD_CONFIG = """\
 {
   'configs': {
@@ -956,6 +1008,12 @@ class UnitTest(unittest.TestCase):
     mbw = self.fake_mbw()
     self.check(['validate'], mbw=mbw, ret=0)
 
+  def test_validate_inconsistent(self):
+    mbw = self.fake_mbw()
+    mbw.files[mbw.default_config_bucket] = TEST_CONFIG_BUCKET_2
+    self.check(['validate'], mbw=mbw, ret=1)
+    self.assertIn('mb_config_buckets.pyl doesn\'t match', mbw.out)
+
   def test_bad_validate(self):
     mbw = self.fake_mbw()
     mbw.files[mbw.default_config_master] = TEST_BAD_CONFIG