Devtools: refactor shell configuration logic. This patch splits the shell configuration logic into part that computes the final list of options (shell_config) and the part that configures the shell abstraction accordingly. This becomes needed as information that affects a shell run (script command-line arguments and inferred checkout paths), and we would like to have another (configuration file). This patch also includesthe logic that infers default paths in Chromium-like checkouts in the common configuration logic, so that it now applies to `mojo_test` as well as to `mojo_run`. R=qsr@chromium.org BUG= Review URL: https://codereview.chromium.org/1256323011 . Cr-Mirrored-From: https://github.com/domokit/mojo Cr-Mirrored-Commit: 035f2cc50b8e2aacb3b3a7a8dc504bedfb46bb90
diff --git a/devtoolslib/paths.py b/devtoolslib/paths.py index 3347c75..1b0bff4 100644 --- a/devtoolslib/paths.py +++ b/devtoolslib/paths.py
@@ -8,6 +8,7 @@ checkouts. """ +import collections import os.path import sys @@ -26,13 +27,13 @@ return None -def infer_mojo_paths(is_android, is_debug, target_cpu): - """Infers the locations of select build output artifacts in a regular Mojo - checkout. +def infer_paths(is_android, is_debug, target_cpu): + """Infers the locations of select build output artifacts in a regular + Chromium-like checkout. This should grow thinner or disappear as we introduce + per-repo config files, see https://github.com/domokit/devtools/issues/28. Returns: - Tuple of path dictionary, error message. Only one of the two will be - not-None. + Defaultdict with the inferred paths. """ build_dir = (('android_' if is_android else '') + (target_cpu + '_' if target_cpu else '') + @@ -40,20 +41,19 @@ out_build_dir = os.path.join('out', build_dir) root_path = find_ancestor_with(out_build_dir) + paths = collections.defaultdict(lambda: None) if not root_path: - return None, ('Failed to find build directory: ' + out_build_dir) + return paths - paths = {} - paths['root'] = root_path build_dir_path = os.path.join(root_path, out_build_dir) - paths['build'] = build_dir_path + paths['build_dir_path'] = build_dir_path if is_android: - paths['shell'] = os.path.join(build_dir_path, 'apks', 'MojoShell.apk') - paths['adb'] = os.path.join(root_path, 'third_party', 'android_tools', + paths['shell_path'] = os.path.join(build_dir_path, 'apks', 'MojoShell.apk') + paths['adb_path'] = os.path.join(root_path, 'third_party', 'android_tools', 'sdk', 'platform-tools', 'adb') else: - paths['shell'] = os.path.join(build_dir_path, 'mojo_shell') - return paths, None + paths['shell_path'] = os.path.join(build_dir_path, 'mojo_shell') + return paths # Based on Chromium //tools/find_depot_tools.py.
diff --git a/devtoolslib/shell_arguments.py b/devtoolslib/shell_arguments.py index bd818e8..f8db053 100644 --- a/devtoolslib/shell_arguments.py +++ b/devtoolslib/shell_arguments.py
@@ -2,8 +2,10 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -"""Functions that configure the shell before it is run manipulating its argument -list. +"""Produces configured shell abstractions. + +This module knows how to produce a configured shell abstraction based on +shell_config.ShellConfig. """ import os.path @@ -17,7 +19,11 @@ # so that caching works between subsequent runs with the same command line. _LOCAL_ORIGIN_PORT = 31840 _MAPPINGS_BASE_PORT = 31841 -_SKY_SERVER_PORT = 9998 + + +class ShellConfigurationException(Exception): + """Represents an error preventing creating a functional shell abstraction.""" + pass def _is_web_url(dest): @@ -164,95 +170,55 @@ return arguments -def add_shell_arguments(parser): - """Adds argparse arguments allowing to configure shell abstraction using - configure_shell() below. +def get_shell(shell_config, shell_args): """ - # Arguments configuring the shell run. - parser.add_argument('--shell-path', help='Path of the Mojo shell binary.') - parser.add_argument('--android', help='Run on Android', - action='store_true') - parser.add_argument('--origin', help='Origin for mojo: URLs. This can be a ' - 'web url or a local directory path.') - parser.add_argument('--map-url', action='append', - help='Define a mapping for a url in the format ' - '<url>=<url-or-local-file-path>') - parser.add_argument('--map-origin', action='append', - help='Define a mapping for a url origin in the format ' - '<origin>=<url-or-local-file-path>') - parser.add_argument('--sky', action='store_true', - help='Maps mojo:sky_viewer as the content handler for ' - 'dart apps.') - parser.add_argument('-v', '--verbose', action="store_true", - help="Increase output verbosity") - - android_group = parser.add_argument_group('Android-only', - 'These arguments apply only when --android is passed.') - android_group.add_argument('--adb-path', help='Path of the adb binary.') - android_group.add_argument('--target-device', help='Device to run on.') - android_group.add_argument('--logcat-tags', help='Comma-separated list of ' - 'additional logcat tags to display.') - - desktop_group = parser.add_argument_group('Desktop-only', - 'These arguments apply only when running on desktop.') - desktop_group.add_argument('--use-osmesa', action='store_true', - help='Configure the native viewport service ' - 'for off-screen rendering.') - - -class ShellConfigurationException(Exception): - """Represents an error preventing creating a functional shell abstraction.""" - pass - - -def configure_shell(config_args, shell_args): - """ - Produces a shell abstraction configured using the parsed arguments defined in - add_shell_arguments(). + Produces a shell abstraction configured according to |shell_config|. Args: - config_args: Parsed arguments added using add_shell_arguments(). + shell_config: Instance of shell_config.ShellConfig. shell_args: Additional raw shell arguments to be passed to the shell. We need to take these into account as some parameters need to appear only once on the argument list (e.g. url-mappings) so we need to coalesce any overrides and the existing value into just one argument. Returns: - A tuple of (shell, shell_args). + A tuple of (shell, shell_args). |shell| is the configured shell abstraction, + |shell_args| is updated list of shell arguments. Throws: ShellConfigurationException if shell abstraction could not be configured. """ - if config_args.android: - verbose_pipe = sys.stdout if config_args.verbose else None + if shell_config.android: + verbose_pipe = sys.stdout if shell_config.verbose else None - shell = AndroidShell(config_args.adb_path, config_args.target_device, - logcat_tags=config_args.logcat_tags, + shell = AndroidShell(shell_config.adb_path, shell_config.target_device, + logcat_tags=shell_config.logcat_tags, verbose_pipe=verbose_pipe) + device_status, error = shell.CheckDevice() if not device_status: raise ShellConfigurationException('Device check failed: ' + error) - if config_args.shell_path: - shell.InstallApk(config_args.shell_path) + if shell_config.shell_path: + shell.InstallApk(shell_config.shell_path) else: - if not config_args.shell_path: + if not shell_config.shell_path: raise ShellConfigurationException('Can not run without a shell binary. ' 'Please pass --shell-path.') - shell = LinuxShell(config_args.shell_path) - if config_args.use_osmesa: + shell = LinuxShell(shell_config.shell_path) + if shell_config.use_osmesa: shell_args.append('--args-for=mojo:native_viewport_service --use-osmesa') - shell_args = _apply_mappings(shell, shell_args, config_args.map_url, - config_args.map_origin) + shell_args = _apply_mappings(shell, shell_args, shell_config.map_url_list, + shell_config.map_origin_list) - if config_args.origin: - if _is_web_url(config_args.origin): - shell_args.append('--origin=' + config_args.origin) + if shell_config.origin: + if _is_web_url(shell_config.origin): + shell_args.append('--origin=' + shell_config.origin) else: - shell_args.extend(configure_local_origin(shell, config_args.origin, - fixed_port=True)) + shell_args.extend(configure_local_origin(shell, shell_config.origin, + fixed_port=True)) - if config_args.sky: + if shell_config.sky: shell_args = _configure_sky(shell_args) return shell, shell_args
diff --git a/devtoolslib/shell_config.py b/devtoolslib/shell_config.py new file mode 100644 index 0000000..acc9c61 --- /dev/null +++ b/devtoolslib/shell_config.py
@@ -0,0 +1,121 @@ +# Copyright 2015 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +"""Configuration for the shell abstraction. + +This module declares ShellConfig and knows how to compute it from command-line +arguments, applying any default paths inferred from the checkout, configuration +file, etc. +""" + +from devtoolslib import paths + + +class ShellConfig(object): + """Configuration for the shell abstraction.""" + + def __init__(self): + self.android = None + self.shell_path = None + self.origin = None + self.map_url_list = None + self.map_origin_list = None + self.sky = None + self.verbose = None + + # Android-only. + self.adb_path = None + self.target_device = None + self.logcat_tags = None + + # Desktop-only. + self.use_osmesa = None + + +def add_shell_arguments(parser): + """Adds argparse arguments allowing to configure shell abstraction using + configure_shell() below. + """ + # Arguments configuring the shell run. + parser.add_argument('--android', help='Run on Android', + action='store_true') + parser.add_argument('--shell-path', help='Path of the Mojo shell binary.') + parser.add_argument('--origin', help='Origin for mojo: URLs. This can be a ' + 'web url or a local directory path.') + parser.add_argument('--map-url', action='append', + help='Define a mapping for a url in the format ' + '<url>=<url-or-local-file-path>') + parser.add_argument('--map-origin', action='append', + help='Define a mapping for a url origin in the format ' + '<origin>=<url-or-local-file-path>') + parser.add_argument('--sky', action='store_true', + help='Maps mojo:sky_viewer as the content handler for ' + 'dart apps.') + parser.add_argument('-v', '--verbose', action="store_true", + help="Increase output verbosity") + + android_group = parser.add_argument_group('Android-only', + 'These arguments apply only when --android is passed.') + android_group.add_argument('--adb-path', help='Path of the adb binary.') + android_group.add_argument('--target-device', help='Device to run on.') + android_group.add_argument('--logcat-tags', help='Comma-separated list of ' + 'additional logcat tags to display.') + + desktop_group = parser.add_argument_group('Desktop-only', + 'These arguments apply only when running on desktop.') + desktop_group.add_argument('--use-osmesa', action='store_true', + help='Configure the native viewport service ' + 'for off-screen rendering.') + + # Arguments allowing to indicate the configuration we are targeting when + # running within a Chromium-like checkout. These will go away once we have + # devtools config files, see https://github.com/domokit/devtools/issues/28. + chromium_config_group = parser.add_argument_group('Chromium configuration', + 'These arguments allow to infer paths to tools and build results ' + 'when running withing a Chromium-like checkout') + debug_group = chromium_config_group.add_mutually_exclusive_group() + debug_group.add_argument('--debug', help='Debug build (default)', + default=True, action='store_true') + debug_group.add_argument('--release', help='Release build', default=False, + dest='debug', action='store_false') + chromium_config_group.add_argument('--target-cpu', + help='CPU architecture to run for.', + choices=['x64', 'x86', 'arm']) + + +def get_shell_config(script_args): + """Processes command-line options defined in add_shell_arguments(), applying + any inferred default paths and produces an instance of ShellConfig. + + Returns: + An instance of ShellConfig. + """ + # Infer paths based on the Chromium configuration options + # (--debug/--release, etc.), if running within a Chromium-like checkout. + inferred_paths = paths.infer_paths(script_args.android, script_args.debug, + script_args.target_cpu) + + shell_config = ShellConfig() + + shell_config.android = script_args.android + shell_config.shell_path = (script_args.shell_path or + inferred_paths['shell_path']) + shell_config.origin = script_args.origin + shell_config.map_url_list = script_args.map_url + shell_config.map_origin_list = script_args.map_origin + shell_config.sky = script_args.sky + shell_config.verbose = script_args.verbose + + # Android-only. + shell_config.adb_path = (script_args.adb_path or inferred_paths['adb_path']) + shell_config.target_device = script_args.target_device + shell_config.logcat_tags = script_args.logcat_tags + + # Desktop-only. + shell_config.use_osmesa = script_args.use_osmesa + + if (shell_config.android and not shell_config.origin and + inferred_paths['build_dir_path']): + shell_config.origin = inferred_paths['build_dir_path'] + return shell_config
diff --git a/mojo_run b/mojo_run index ee43274..f7d30df 100755 --- a/mojo_run +++ b/mojo_run
@@ -7,8 +7,8 @@ import logging import sys -from devtoolslib import paths from devtoolslib import shell_arguments +from devtoolslib import shell_config _USAGE = ("mojo_run " "[--args-for=<mojo-app>] " @@ -55,23 +55,8 @@ logging.basicConfig() parser = argparse.ArgumentParser(usage=_USAGE, description=_DESCRIPTION) + shell_config.add_shell_arguments(parser) - # Arguments allowing to indicate the configuration we are targeting when - # running within a Chromium-like checkout. These will go away once we have - # devtools config files, see https://github.com/domokit/devtools/issues/28. - chromium_config_group = parser.add_argument_group('Chromium configuration', - 'These arguments allow to infer paths to tools and build results ' - 'when running withing a Chromium-like checkout') - debug_group = chromium_config_group.add_mutually_exclusive_group() - debug_group.add_argument('--debug', help='Debug build (default)', - default=True, action='store_true') - debug_group.add_argument('--release', help='Release build', default=False, - dest='debug', action='store_false') - chromium_config_group.add_argument('--target-cpu', - help='CPU architecture to run for.', - choices=['x64', 'x86', 'arm']) - - shell_arguments.add_shell_arguments(parser) parser.add_argument('--no-debugger', action="store_true", help='Do not spawn mojo:debugger.') parser.add_argument('--window-manager', default=_DEFAULT_WINDOW_MANAGER, @@ -80,31 +65,10 @@ _DEFAULT_WINDOW_MANAGER) script_args, shell_args = parser.parse_known_args() - - # Infer paths based on the config if running within a Chromium-like checkout. - mojo_paths, _ = paths.infer_mojo_paths(script_args.android, - script_args.debug, - script_args.target_cpu) - if mojo_paths: - if script_args.android and not script_args.adb_path: - script_args.adb_path = mojo_paths['adb'] - if script_args.android and not script_args.origin: - script_args.origin = mojo_paths['build'] - if not script_args.shell_path: - script_args.shell_path = mojo_paths['shell'] - - if script_args.verbose: - print 'Running within a Chromium-style checkout.' - print ' - using the locally built shell at: ' + script_args.shell_path - if script_args.origin: - print ' - using the origin: ' + script_args.origin - if script_args.android: - print ' - using the adb path: ' + script_args.adb_path - elif script_args.verbose: - print 'Running outside a Chromium-style checkout.' + config = shell_config.get_shell_config(script_args) try: - shell, shell_args = shell_arguments.configure_shell(script_args, shell_args) + shell, shell_args = shell_arguments.get_shell(config, shell_args) except shell_arguments.ShellConfigurationException as e: print e return 1
diff --git a/mojo_test b/mojo_test index d59bacc..e2100dd 100755 --- a/mojo_test +++ b/mojo_test
@@ -15,6 +15,7 @@ from devtoolslib import apptest_dart from devtoolslib import apptest_gtest from devtoolslib import shell_arguments +from devtoolslib import shell_config _DESCRIPTION = """Runner for Mojo application tests. @@ -55,14 +56,13 @@ description=_DESCRIPTION) parser.add_argument("test_list_file", type=file, help="a file listing apptests to run") + shell_config.add_shell_arguments(parser) - # Common shell configuration arguments. - shell_arguments.add_shell_arguments(parser) - script_args, common_shell_args = parser.parse_known_args() + script_args, shell_args = parser.parse_known_args() + config = shell_config.get_shell_config(script_args) try: - shell, common_shell_args = shell_arguments.configure_shell( - script_args, common_shell_args) + shell, common_shell_args = shell_arguments.get_shell(config, shell_args) except shell_arguments.ShellConfigurationException as e: print e return 1