[recipes] Provide tokenized extra_config and use it most places.
Change-Id: I7baae5f90b2e510b66443cda449071c7c6ec9ec7
Reviewed-on: https://skia-review.googlesource.com/83520
Commit-Queue: Ben Wagner <benjaminwagner@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
diff --git a/infra/bots/recipe_modules/core/api.py b/infra/bots/recipe_modules/core/api.py
index 46ea4ec..e8f64cd 100644
--- a/infra/bots/recipe_modules/core/api.py
+++ b/infra/bots/recipe_modules/core/api.py
@@ -97,7 +97,7 @@
main.revision = 'origin/master'
main.managed = True
m[main_name] = 'got_flutter_revision'
- if 'Android' in self.m.vars.builder_cfg.get('extra_config', ''):
+ if 'Android' in self.m.vars.extra_tokens:
gclient_cfg.target_os.add('android')
skia_dep_path = 'src/third_party/skia'
diff --git a/infra/bots/recipe_modules/flavor/api.py b/infra/bots/recipe_modules/flavor/api.py
index a2ac1f1..93aa021 100644
--- a/infra/bots/recipe_modules/flavor/api.py
+++ b/infra/bots/recipe_modules/flavor/api.py
@@ -29,53 +29,53 @@
VERSION_NONE = -1
-def is_android(builder_cfg):
- return 'Android' in builder_cfg.get('extra_config', '')
+def is_android(vars_api):
+ return 'Android' in vars_api.extra_tokens
-def is_chromecast(builder_cfg):
- return ('Chromecast' in builder_cfg.get('extra_config', '') or
- 'Chromecast' in builder_cfg.get('os', ''))
+def is_chromecast(vars_api):
+ return ('Chromecast' in vars_api.extra_tokens or
+ 'Chromecast' in vars_api.builder_cfg.get('os', ''))
-def is_chromebook(builder_cfg):
- return ('Chromebook' in builder_cfg.get('extra_config', '') or
- 'ChromeOS' in builder_cfg.get('os', ''))
+def is_chromebook(vars_api):
+ return ('Chromebook' in vars_api.extra_tokens or
+ 'ChromeOS' in vars_api.builder_cfg.get('os', ''))
-def is_flutter(builder_cfg):
- return 'Flutter' in builder_cfg.get('extra_config', '')
+def is_flutter(vars_api):
+ return 'Flutter' in vars_api.extra_tokens
-def is_ios(builder_cfg):
- return ('iOS' in builder_cfg.get('extra_config', '') or
- 'iOS' == builder_cfg.get('os', ''))
+def is_ios(vars_api):
+ return ('iOS' in vars_api.extra_tokens or
+ 'iOS' == vars_api.builder_cfg.get('os', ''))
-def is_pdfium(builder_cfg):
- return 'PDFium' in builder_cfg.get('extra_config', '')
+def is_pdfium(vars_api):
+ return 'PDFium' in vars_api.extra_tokens
-def is_valgrind(builder_cfg):
- return 'Valgrind' in builder_cfg.get('extra_config', '')
+def is_valgrind(vars_api):
+ return 'Valgrind' in vars_api.extra_tokens
class SkiaFlavorApi(recipe_api.RecipeApi):
- def get_flavor(self, builder_cfg):
+ def get_flavor(self, vars_api):
"""Return a flavor utils object specific to the given builder."""
- if is_flutter(builder_cfg):
+ if is_flutter(vars_api):
return flutter_flavor.FlutterFlavorUtils(self)
- if is_chromecast(builder_cfg):
+ if is_chromecast(vars_api):
return gn_chromecast_flavor.GNChromecastFlavorUtils(self)
- if is_chromebook(builder_cfg):
+ if is_chromebook(vars_api):
return gn_chromebook_flavor.GNChromebookFlavorUtils(self)
- if is_android(builder_cfg):
+ if is_android(vars_api):
return gn_android_flavor.GNAndroidFlavorUtils(self)
- elif is_ios(builder_cfg):
+ elif is_ios(vars_api):
return ios_flavor.iOSFlavorUtils(self)
- elif is_pdfium(builder_cfg):
+ elif is_pdfium(vars_api):
return pdfium_flavor.PDFiumFlavorUtils(self)
- elif is_valgrind(builder_cfg):
+ elif is_valgrind(vars_api):
return valgrind_flavor.ValgrindFlavorUtils(self)
else:
return gn_flavor.GNFlavorUtils(self)
def setup(self):
- self._f = self.get_flavor(self.m.vars.builder_cfg)
+ self._f = self.get_flavor(self.m.vars)
def step(self, name, cmd, **kwargs):
return self._f.step(name, cmd, **kwargs)
diff --git a/infra/bots/recipe_modules/flavor/flutter_flavor.py b/infra/bots/recipe_modules/flavor/flutter_flavor.py
index 6b95cb8..2bbf199 100644
--- a/infra/bots/recipe_modules/flavor/flutter_flavor.py
+++ b/infra/bots/recipe_modules/flavor/flutter_flavor.py
@@ -17,7 +17,7 @@
flutter_dir = self.m.vars.checkout_root.join('src')
configuration = self.m.vars.builder_cfg.get('configuration').lower()
- extra_config = self.m.vars.builder_cfg.get('extra_config', '')
+ extra_tokens = self.m.vars.extra_tokens
out_dir = configuration
with self.m.context(cwd=flutter_dir):
@@ -31,7 +31,7 @@
gn_args = [
'--runtime-mode=%s' % configuration,
]
- if 'Android' in extra_config:
+ if 'Android' in extra_tokens:
gn_args.append('--android')
out_dir = 'android_' + out_dir
diff --git a/infra/bots/recipe_modules/flavor/gn_android_flavor.py b/infra/bots/recipe_modules/flavor/gn_android_flavor.py
index f354611..6997541 100644
--- a/infra/bots/recipe_modules/flavor/gn_android_flavor.py
+++ b/infra/bots/recipe_modules/flavor/gn_android_flavor.py
@@ -261,7 +261,7 @@
def compile(self, unused_target):
compiler = self.m.vars.builder_cfg.get('compiler')
configuration = self.m.vars.builder_cfg.get('configuration')
- extra_config = self.m.vars.builder_cfg.get('extra_config', '')
+ extra_tokens = self.m.vars.extra_tokens
os = self.m.vars.builder_cfg.get('os')
target_arch = self.m.vars.builder_cfg.get('target_arch')
@@ -285,14 +285,16 @@
if configuration != 'Debug':
args['is_debug'] = 'false'
- if 'Vulkan' in extra_config:
+ if 'Vulkan' in extra_tokens:
args['ndk_api'] = 24
args['skia_enable_vulkan_debug_layers'] = 'false'
# If an Android API level is specified, use that.
- m = re.search(r'API(\d+)', extra_config)
- if m and len(m.groups()) == 1:
- args['ndk_api'] = m.groups()[0]
+ for t in extra_tokens:
+ m = re.search(r'API(\d+)', t)
+ if m and len(m.groups()) == 1:
+ args['ndk_api'] = m.groups()[0]
+ break
if extra_cflags:
args['extra_cflags'] = repr(extra_cflags).replace("'", '"')
diff --git a/infra/bots/recipe_modules/flavor/gn_flavor.py b/infra/bots/recipe_modules/flavor/gn_flavor.py
index a5aad9e..d75f9f4 100644
--- a/infra/bots/recipe_modules/flavor/gn_flavor.py
+++ b/infra/bots/recipe_modules/flavor/gn_flavor.py
@@ -56,7 +56,7 @@
"""Build Skia with GN."""
compiler = self.m.vars.builder_cfg.get('compiler', '')
configuration = self.m.vars.builder_cfg.get('configuration', '')
- extra_config = self.m.vars.builder_cfg.get('extra_config', '')
+ extra_tokens = self.m.vars.extra_tokens
os = self.m.vars.builder_cfg.get('os', '')
target_arch = self.m.vars.builder_cfg.get('target_arch', '')
@@ -90,7 +90,7 @@
cxx = emscripten_sdk + '/emscripten/incoming/em++'
extra_cflags.append('-Wno-unknown-warning-option')
- if 'Coverage' in extra_config:
+ if 'Coverage' in extra_tokens:
# See https://clang.llvm.org/docs/SourceBasedCodeCoverage.html for
# more info on using llvm to gather coverage information.
extra_cflags.append('-fprofile-instr-generate')
@@ -101,14 +101,18 @@
if compiler != 'MSVC' and configuration == 'Debug':
extra_cflags.append('-O1')
- if extra_config == 'Exceptions':
+ if 'Exceptions' in extra_tokens:
extra_cflags.append('/EHsc')
- if extra_config == 'Fast':
+ if 'Fast' in extra_tokens:
extra_cflags.extend(['-march=native', '-fomit-frame-pointer', '-O3',
'-ffp-contract=off'])
- if extra_config.startswith('SK'):
- extra_cflags.append('-D' + extra_config)
- if extra_config == 'MSAN':
+
+ # TODO(benjaminwagner): Same appears in compile.py to set CPPFLAGS. Are
+ # both needed?
+ if len(extra_tokens) == 1 and extra_tokens[0].startswith('SK'):
+ extra_cflags.append('-D' + extra_tokens[0])
+
+ if 'MSAN' in extra_tokens:
extra_ldflags.append('-L' + clang_linux + '/msan')
args = {}
@@ -117,16 +121,16 @@
if configuration != 'Debug':
args['is_debug'] = 'false'
- if extra_config == 'ANGLE':
+ if 'ANGLE' in extra_tokens:
args['skia_use_angle'] = 'true'
- if extra_config == 'CommandBuffer':
+ if 'CommandBuffer' in extra_tokens:
self.m.run.run_once(self.build_command_buffer)
- if extra_config == 'MSAN':
+ if 'MSAN' in extra_tokens:
args['skia_enable_gpu'] = 'false'
args['skia_use_fontconfig'] = 'false'
- if 'ASAN' in extra_config or 'UBSAN' in extra_config:
+ if 'ASAN' in extra_tokens or 'UBSAN' in extra_tokens:
args['skia_enable_spirv_validation'] = 'false'
- if extra_config == 'Mini':
+ if 'Mini' in extra_tokens:
args.update({
'is_component_build': 'true', # Proves we can link a coherent .so.
'is_official_build': 'true', # No debug symbols, no tools.
@@ -139,21 +143,21 @@
'skia_use_libwebp': 'false',
'skia_use_zlib': 'false',
})
- if extra_config == 'NoGPU':
+ if 'NoGPU' in extra_tokens:
args['skia_enable_gpu'] = 'false'
- if extra_config == 'EmbededResouces':
+ if 'EmbededResouces' in extra_tokens:
args['skia_embed_resoucres'] = 'true'
- if extra_config == 'Shared':
+ if 'Shared' in extra_tokens:
args['is_component_build'] = 'true'
- if 'Vulkan' in extra_config and not 'Android' in extra_config:
+ if 'Vulkan' in extra_tokens and not 'Android' in extra_tokens:
args['skia_enable_vulkan_debug_layers'] = 'false'
if self.m.vars.is_linux:
args['skia_vulkan_sdk'] = '"%s"' % linux_vulkan_sdk
if 'Win' in os:
args['skia_vulkan_sdk'] = '"%s"' % win_vulkan_sdk
- if 'Metal' in extra_config:
+ if 'Metal' in extra_tokens:
args['skia_use_metal'] = 'true'
- if 'CheckGeneratedFiles' in extra_config:
+ if 'CheckGeneratedFiles' in extra_tokens:
args['skia_compile_processors'] = 'true'
if compiler == 'Clang' and 'Win' in os:
args['clang_win'] = '"%s"' % self.m.vars.slave_dir.join('clang_win')
@@ -165,7 +169,7 @@
'skia_use_icu': 'false',
'skia_enable_gpu': 'false',
})
- if 'Goma' in extra_config:
+ if 'Goma' in extra_tokens:
json_file = self._get_goma_json()
self.m.cipd.set_service_account_credentials(json_file)
goma_package = ('infra_internal/goma/client/%s' %
@@ -182,9 +186,11 @@
ninja_args.extend(['-j', '100'])
sanitize = ''
- if 'SAN' in extra_config:
- sanitize = extra_config
- elif 'SafeStack' in extra_config:
+ for t in extra_tokens:
+ if t.endswith('SAN'):
+ sanitize = t
+ if 'SafeStack' in extra_tokens:
+ assert sanitize == ''
sanitize = 'safe-stack'
for (k,v) in {
@@ -192,7 +198,7 @@
'cxx': cxx,
'sanitize': sanitize,
'target_cpu': target_arch,
- 'target_os': 'ios' if 'iOS' in extra_config else '',
+ 'target_os': 'ios' if 'iOS' in extra_tokens else '',
'win_sdk': win_toolchain + '/win_sdk' if 'Win' in os else '',
'win_vc': win_toolchain + '/VC' if 'Win' in os else '',
}.iteritems():
@@ -212,7 +218,7 @@
try:
with self.m.context(cwd=self.m.vars.skia_dir):
self._py('fetch-gn', self.m.vars.skia_dir.join('bin', 'fetch-gn'))
- if 'CheckGeneratedFiles' in extra_config:
+ if 'CheckGeneratedFiles' in extra_tokens:
env['PATH'] = '%s:%%(PATH)s' % self.m.vars.skia_dir.join('bin')
self._py(
'fetch-clang-format',
@@ -236,11 +242,11 @@
def copy_extra_build_products(self, swarming_out_dir):
configuration = self.m.vars.builder_cfg.get('configuration', '')
- extra_config = self.m.vars.builder_cfg.get('extra_config', '')
+ extra_tokens = self.m.vars.extra_tokens
os = self.m.vars.builder_cfg.get('os', '')
win_vulkan_sdk = str(self.m.vars.slave_dir.join('win_vulkan_sdk'))
- if 'Win' in os and extra_config == 'Vulkan':
+ if 'Win' in os and 'Vulkan' in extra_tokens:
self.m.run.copy_build_products(
win_vulkan_sdk,
swarming_out_dir.join('out', configuration + '_x64'))
@@ -254,7 +260,7 @@
slave_dir = self.m.vars.slave_dir
clang_linux = str(slave_dir.join('clang_linux'))
- extra_config = self.m.vars.builder_cfg.get('extra_config', '')
+ extra_tokens = self.m.vars.extra_tokens
if self.m.vars.is_linux:
if (self.m.vars.builder_cfg.get('cpu_or_gpu', '') == 'GPU'
@@ -268,31 +274,31 @@
env['LIBGL_DRIVERS_PATH'] = str(dri_path)
env['VK_ICD_FILENAMES'] = str(dri_path.join('intel_icd.x86_64.json'))
- if 'Vulkan' in extra_config:
+ if 'Vulkan' in extra_tokens:
path.append(slave_dir.join('linux_vulkan_sdk', 'bin'))
ld_library_path.append(slave_dir.join('linux_vulkan_sdk', 'lib'))
- if 'SAN' in extra_config:
+ if any('SAN' in t for t in extra_tokens):
# Sanitized binaries may want to run clang_linux/bin/llvm-symbolizer.
path.append(clang_linux + '/bin')
elif self.m.vars.is_linux:
cmd = ['catchsegv'] + cmd
- if 'ASAN' == extra_config or 'UBSAN' in extra_config:
+ if 'ASAN' in extra_tokens or 'UBSAN' in extra_tokens:
env[ 'ASAN_OPTIONS'] = 'symbolize=1 detect_leaks=1'
env[ 'LSAN_OPTIONS'] = 'symbolize=1 print_suppressions=1'
env['UBSAN_OPTIONS'] = 'symbolize=1 print_stacktrace=1'
- if 'MSAN' == extra_config:
+ if 'MSAN' in extra_tokens:
# Find the MSAN-built libc++.
ld_library_path.append(clang_linux + '/msan')
- if 'TSAN' == extra_config:
+ if 'TSAN' in extra_tokens:
# We don't care about malloc(), fprintf, etc. used in signal handlers.
# If we're in a signal handler, we're already crashing...
env['TSAN_OPTIONS'] = 'report_signal_unsafe=0'
- if 'Coverage' in extra_config:
+ if 'Coverage' in extra_tokens:
# This is the output file for the coverage data. Just running the binary
# will produce the output. The output_file is in the swarming_out_dir and
# thus will be an isolated output of the Test step.
diff --git a/infra/bots/recipe_modules/vars/api.py b/infra/bots/recipe_modules/vars/api.py
index f43e6a9..92a61c2 100644
--- a/infra/bots/recipe_modules/vars/api.py
+++ b/infra/bots/recipe_modules/vars/api.py
@@ -120,6 +120,13 @@
arch = (self.builder_cfg.get('arch') or self.builder_cfg.get('target_arch'))
if ('Win' in self.builder_cfg.get('os', '') and arch == 'x86_64'):
self.configuration += '_x64'
+ self.extra_tokens = []
+ if len(self.builder_cfg.get('extra_config', '')) > 0:
+ if self.builder_cfg['extra_config'].startswith('SK'):
+ assert self.builder_cfg['extra_config'].isupper()
+ self.extra_tokens = [self.builder_cfg['extra_config']]
+ else:
+ self.extra_tokens = self.builder_cfg['extra_config'].split('_')
self.default_env.update({'SKIA_OUT': self.skia_out,
'BUILDTYPE': self.configuration})
@@ -221,4 +228,3 @@
''',
stdout=self.m.raw_io.output()).stdout.rstrip()
return self._swarming_task_id
-
diff --git a/infra/bots/recipe_modules/vars/examples/full.expected/Build-Debian9-Clang-x86_64-Release-SKNX_NO_SIMD.json b/infra/bots/recipe_modules/vars/examples/full.expected/Build-Debian9-Clang-x86_64-Release-SKNX_NO_SIMD.json
new file mode 100644
index 0000000..4594f9e
--- /dev/null
+++ b/infra/bots/recipe_modules/vars/examples/full.expected/Build-Debian9-Clang-x86_64-Release-SKNX_NO_SIMD.json
@@ -0,0 +1,35 @@
+[
+ {
+ "cmd": [
+ "python",
+ "-u",
+ "import os\nprint os.environ.get('SWARMING_BOT_ID', '')\n"
+ ],
+ "name": "get swarming bot id",
+ "stdout": "/path/to/tmp/",
+ "~followup_annotations": [
+ "@@@STEP_LOG_LINE@python.inline@import os@@@",
+ "@@@STEP_LOG_LINE@python.inline@print os.environ.get('SWARMING_BOT_ID', '')@@@",
+ "@@@STEP_LOG_END@python.inline@@@"
+ ]
+ },
+ {
+ "cmd": [
+ "python",
+ "-u",
+ "import os\nprint os.environ.get('SWARMING_TASK_ID', '')\n"
+ ],
+ "name": "get swarming task id",
+ "stdout": "/path/to/tmp/",
+ "~followup_annotations": [
+ "@@@STEP_LOG_LINE@python.inline@import os@@@",
+ "@@@STEP_LOG_LINE@python.inline@print os.environ.get('SWARMING_TASK_ID', '')@@@",
+ "@@@STEP_LOG_END@python.inline@@@"
+ ]
+ },
+ {
+ "name": "$result",
+ "recipe_result": null,
+ "status_code": 0
+ }
+]
\ No newline at end of file
diff --git a/infra/bots/recipe_modules/vars/examples/full.py b/infra/bots/recipe_modules/vars/examples/full.py
index 5eff062..b1a0af9 100644
--- a/infra/bots/recipe_modules/vars/examples/full.py
+++ b/infra/bots/recipe_modules/vars/examples/full.py
@@ -22,6 +22,7 @@
TEST_BUILDERS = [
+ 'Build-Debian9-Clang-x86_64-Release-SKNX_NO_SIMD',
'Build-Debian9-GCC-x86_64-Release-Flutter_Android',
'Build-Debian9-GCC-x86_64-Release-PDFium',
'Build-Mac-Clang-x86_64-Debug-CommandBuffer',
diff --git a/infra/bots/recipes/compile.py b/infra/bots/recipes/compile.py
index d628a00..25c8a95 100644
--- a/infra/bots/recipes/compile.py
+++ b/infra/bots/recipes/compile.py
@@ -26,16 +26,18 @@
return ['most']
-def get_extra_env_vars(builder_dict):
+def get_extra_env_vars(vars_api):
env = {}
- if builder_dict.get('compiler') == 'Clang':
+ if vars_api.builder_cfg.get('compiler') == 'Clang':
env['CC'] = '/usr/bin/clang'
env['CXX'] = '/usr/bin/clang++'
# SKNX_NO_SIMD, SK_USE_DISCARDABLE_SCALEDIMAGECACHE, etc.
- extra_config = builder_dict.get('extra_config', '')
- if extra_config.startswith('SK') and extra_config.isupper():
- env['CPPFLAGS'] = '-D' + extra_config
+ # TODO(benjaminwagner): Same appears in gn_flavor.py to set extra_cflags. Are
+ # both needed?
+ if (len(vars_api.extra_tokens) == 1 and
+ vars_api.extra_tokens[0].startswith('SK')):
+ env['CPPFLAGS'] = '-D' + vars_api.extra_tokens[0]
return env
@@ -43,7 +45,7 @@
def RunSteps(api):
api.core.setup()
- env = get_extra_env_vars(api.vars.builder_cfg)
+ env = get_extra_env_vars(api.vars)
build_targets = build_targets_from_builder_dict(api.vars.builder_cfg)
try:
diff --git a/infra/bots/recipes/perf.py b/infra/bots/recipes/perf.py
index 99e68af..b9be776 100644
--- a/infra/bots/recipes/perf.py
+++ b/infra/bots/recipes/perf.py
@@ -292,8 +292,7 @@
args.extend([k, api.vars.builder_cfg[k]])
# See skia:2789.
- extra_config_parts = api.vars.builder_cfg.get('extra_config', '').split('_')
- if 'AbandonGpuContext' in extra_config_parts:
+ if 'AbandonGpuContext' in api.vars.extra_tokens:
args.extend(['--abandonGpuContext'])
api.run(api.flavor.step, target, cmd=args,
diff --git a/infra/bots/recipes/test.py b/infra/bots/recipes/test.py
index 53aac01..53c2610 100644
--- a/infra/bots/recipes/test.py
+++ b/infra/bots/recipes/test.py
@@ -859,12 +859,11 @@
args.extend(dm_flags(api, api.vars.builder_name))
# See skia:2789.
- extra_config_parts = api.vars.builder_cfg.get('extra_config', '').split('_')
- if 'AbandonGpuContext' in extra_config_parts:
+ if 'AbandonGpuContext' in api.vars.extra_tokens:
args.append('--abandonGpuContext')
- if 'PreAbandonGpuContext' in extra_config_parts:
+ if 'PreAbandonGpuContext' in api.vars.extra_tokens:
args.append('--preAbandonGpuContext')
- if 'ReleaseAndAbandonGpuContext' in extra_config_parts:
+ if 'ReleaseAndAbandonGpuContext' in api.vars.extra_tokens:
args.append('--releaseAndAbandonGpuContext')
api.run(api.flavor.step, 'dm', cmd=args, abort_on_failure=False)