From c55ab028df6800fa3a9819164af2cd8b34d3acf0 Mon Sep 17 00:00:00 2001 From: Jianhui Harold Date: Tue, 12 Nov 2019 13:03:08 +0800 Subject: [PATCH] Add reasonable warning when command on wheel extentions without --include-whl-extensions (#133) * Optimize help message of linter * Add another warning for command runing on a wheel installed extension without --include-whl-extensions * Clean code * Partially fix Azure/azure-cli#11140 * Partially fix Azure/azure-cli-extensions#1078 --- azdev/params.py | 11 +++++++++-- azdev/utilities/path.py | 40 +++++++++++++++++++--------------------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/azdev/params.py b/azdev/params.py index 11e0ea37..40860879 100644 --- a/azdev/params.py +++ b/azdev/params.py @@ -19,7 +19,10 @@ class Flag(object): # pylint: disable=too-many-statements def load_arguments(self, _): - modules_type = CLIArgumentType(nargs='*', help="Space-separated list of modules or extensions to check. Omit to check all or use 'CLI' or 'EXT' to check only CLI modules or extensions respectively.") + modules_type = CLIArgumentType(nargs='*', + help="Space-separated list of modules or extensions (dev mode) to check. " + "Use 'CLI' to check built-in modules or 'EXT' to check extensions. " + "Omit to check all. ") with ArgumentsContext(self, '') as c: c.argument('private', action='store_true', help='Target the private repo.') @@ -65,12 +68,16 @@ def load_arguments(self, _): with ArgumentsContext(self, 'cli update-setup') as c: c.argument('pin', action='store_true', help='Pin the module versions in azure-cli\'s setup.py file.') + # region linter with ArgumentsContext(self, 'linter') as c: c.positional('modules', modules_type) c.argument('rules', options_list=['--rules', '-r'], nargs='+', help='Space-separated list of rules to run. Omit to run all rules.') c.argument('rule_types', options_list=['--rule-types', '-t'], nargs='+', choices=['params', 'commands', 'command_groups', 'help_entries'], help='Space-separated list of rule types to run. Omit to run all.') c.argument('ci_exclusions', action='store_true', help='Force application of CI exclusions list when run locally.') - c.argument('include_whl_extensions', action='store_true', help='Allow running the linter on non-dev extensions (those installed using `az extension add ...`).') + c.argument('include_whl_extensions', + action='store_true', + help="Allow running linter on extensions installed by `az extension add`.") + # endregion with ArgumentsContext(self, 'perf') as c: c.argument('runs', type=int, help='Number of runs to average performance over.') diff --git a/azdev/utilities/path.py b/azdev/utilities/path.py index 8985b39c..5064f19b 100644 --- a/azdev/utilities/path.py +++ b/azdev/utilities/path.py @@ -119,6 +119,7 @@ def make_dirs(path): def get_name_index(invert=False, include_whl_extensions=False): """ Returns a dictionary containing the long and short names of modules and extensions is {SHORT:LONG} format or {LONG:SHORT} format when invert=True. """ + from azure.cli.core.extension import EXTENSIONS_DIR # pylint: disable=import-error table = {} cli_repo_path = get_cli_repo_path() @@ -135,7 +136,6 @@ def get_name_index(invert=False, include_whl_extensions=False): ext_paths = [x for x in find_files(ext_repo_paths, '*.*-info') if 'site-packages' not in x] whl_ext_paths = [] if include_whl_extensions: - from azure.cli.core.extension import EXTENSIONS_DIR # pylint: disable=import-error whl_ext_paths = [x for x in find_files(EXTENSIONS_DIR, '*.*-info') if 'site-packages' not in x] def _update_table(paths, key): @@ -190,6 +190,7 @@ def get_path_table(include_only=None, include_whl_extensions=False): } } """ + from azure.cli.core.extension import EXTENSIONS_DIR # pylint: disable=import-error # determine whether the call will filter or return all if isinstance(include_only, str): @@ -208,40 +209,30 @@ def get_path_table(include_only=None, include_whl_extensions=False): modules_paths = glob(paths) core_paths = glob(os.path.normcase(os.path.join(cli_repo_path, 'src', '*', 'setup.py'))) ext_paths = [x for x in find_files(ext_repo_paths, '*.*-info') if 'site-packages' not in x] - whl_ext_paths = [] - if include_whl_extensions: - from azure.cli.core.extension import EXTENSIONS_DIR # pylint: disable=import-error - whl_ext_paths = [x for x in find_files(EXTENSIONS_DIR, '*.*-info') if 'site-packages' not in x] + whl_ext_paths = [x for x in find_files(EXTENSIONS_DIR, '*.*-info') if 'site-packages' not in x] - def _update_table(paths, key): + def _update_table(package_paths, key): if key not in table: table[key] = {} - folder = None - long_name = None - short_name = None - for path in paths: + + for path in package_paths: folder = os.path.dirname(path) base_name = os.path.basename(folder) - # determine long-names + if key == 'ext': short_name = base_name - for item in os.listdir(folder): - if item.startswith(EXTENSION_PREFIX): - long_name = item - break + long_name = next((item for item in os.listdir(folder) if item.startswith(EXTENSION_PREFIX)), None) elif base_name.startswith(COMMAND_MODULE_PREFIX): - long_name = base_name short_name = base_name.replace(COMMAND_MODULE_PREFIX, '') or '__main__' + long_name = base_name else: short_name = base_name long_name = '{}{}'.format(COMMAND_MODULE_PREFIX, base_name) if get_all: table[key][long_name if key == 'ext' else short_name] = folder - continue elif not include_only: - # nothing left to filter - return + return # nothing left to filter else: # check and update filter if short_name in include_only: @@ -256,9 +247,16 @@ def _update_table(paths, key): _update_table(modules_paths, 'mod') _update_table(core_paths, 'core') _update_table(ext_paths, 'ext') - _update_table(whl_ext_paths, 'ext') + if include_whl_extensions: + _update_table(whl_ext_paths, 'ext') if include_only: - raise CLIError('unrecognized names: {}'.format(' '.join(include_only))) + whl_extensions = [mod for whl_ext_path in whl_ext_paths for mod in include_only if mod in whl_ext_path] + if whl_extensions: + err = 'extension(s): [ {} ] installed from a wheel may need --include-whl-extensions option'.format( + ', '.join(whl_extensions)) + raise CLIError(err) + + raise CLIError('unrecognized modules: [ {} ]'.format(', '.join(include_only))) return table