From f367871ea881165440d55e7c0cceaea176d81ea9 Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Fri, 15 Mar 2024 22:24:30 +0100 Subject: [PATCH 1/3] fix b2 --help showing full binary path instead of just basename --- b2/_internal/_cli/shell.py | 13 ++++ b2/_internal/console_tool.py | 80 ++++++++++++++---------- changelog.d/+b2_path_in_help.fixed.md | 1 + doc/source/conf.py | 2 +- test/integration/test_b2_command_line.py | 4 +- test/integration/test_help.py | 28 +++++++++ 6 files changed, 92 insertions(+), 36 deletions(-) create mode 100644 changelog.d/+b2_path_in_help.fixed.md create mode 100644 test/integration/test_help.py diff --git a/b2/_internal/_cli/shell.py b/b2/_internal/_cli/shell.py index b175c7650..27e420004 100644 --- a/b2/_internal/_cli/shell.py +++ b/b2/_internal/_cli/shell.py @@ -10,6 +10,7 @@ import os import os.path +import shutil from typing import Optional @@ -19,3 +20,15 @@ def detect_shell() -> Optional[str]: if shell_var: return os.path.basename(shell_var) return None + + +def resolve_short_call_name(binary_path: str) -> str: + """ + Resolve the short name of the binary. + + If binary is in PATH, return only basename, otherwise return a full path. + This method is to be used with sys.argv[0] to resolve handy name for the user instead of full path. + """ + if shutil.which(binary_path) == binary_path: + return os.path.basename(binary_path) + return binary_path diff --git a/b2/_internal/console_tool.py b/b2/_internal/console_tool.py index eda3eb733..3488d8629 100644 --- a/b2/_internal/console_tool.py +++ b/b2/_internal/console_tool.py @@ -147,7 +147,7 @@ DEFAULT_THREADS, ) from b2._internal._cli.obj_loads import validated_loads -from b2._internal._cli.shell import detect_shell +from b2._internal._cli.shell import detect_shell, resolve_short_call_name from b2._internal._utils.uri import B2URI, B2FileIdURI, B2URIAdapter, B2URIBase from b2._internal.arg_parser import B2ArgumentParser from b2._internal.json_encoder import B2CliJsonEncoder @@ -183,13 +183,13 @@ def write(self, s): self.stdout.write(s) -# The name of an executable entry point -NAME = os.path.basename(sys.argv[0]) -if NAME.endswith('.py'): - version_name = re.search( - r'[\\/]b2[\\/]_internal[\\/](_{0,1}b2v\d+)[\\/]__main__.py', sys.argv[0] - ) - NAME = version_name.group(1) if version_name else 'b2' +def resolve_b2_bin_call_name(argv: list[str] | None = None) -> str: + call_name = resolve_short_call_name((argv or sys.argv)[0]) + if call_name.endswith('.py'): + version_name = re.search(r'[\\/]b2[\\/]_internal[\\/](_?b2v\d+)[\\/]__main__.py', call_name) + call_name = version_name.group(1) if version_name else 'b2' + return call_name + FILE_RETENTION_COMPATIBILITY_WARNING = """ .. warning:: @@ -200,7 +200,6 @@ def write(self, s): # Strings available to use when formatting doc strings. DOC_STRING_DATA = dict( - NAME=NAME, B2_ACCOUNT_INFO_ENV_VAR=B2_ACCOUNT_INFO_ENV_VAR, B2_ACCOUNT_INFO_DEFAULT_FILE=B2_ACCOUNT_INFO_DEFAULT_FILE, B2_ACCOUNT_INFO_PROFILE_FILE=B2_ACCOUNT_INFO_PROFILE_FILE, @@ -282,11 +281,12 @@ def format_account_info(account_info: AbstractAccountInfo) -> dict: class DescriptionGetter: - def __init__(self, described_cls): + def __init__(self, described_cls, **kwargs): self.described_cls = described_cls + self.kwargs = kwargs def __str__(self): - return self.described_cls._get_description() + return self.described_cls._get_description(**self.kwargs) class Described: @@ -296,17 +296,17 @@ class Described: """ @classmethod - def _get_description(cls): + def _get_description(cls, **kwargs): mro_docs = { - klass.__name__.upper(): klass.lazy_get_description() + klass.__name__.upper(): klass.lazy_get_description(**kwargs) for klass in cls.mro() if klass is not cls and klass.__doc__ and issubclass(klass, Described) } - return cls.__doc__.format(**DOC_STRING_DATA, **mro_docs) + return cls.__doc__.format(**kwargs, **DOC_STRING_DATA, **mro_docs) @classmethod - def lazy_get_description(cls): - return DescriptionGetter(cls) + def lazy_get_description(cls, **kwargs): + return DescriptionGetter(cls, **kwargs) class DefaultSseMixin(Described): @@ -830,7 +830,8 @@ def create_parser( subparsers: argparse._SubParsersAction | None = None, parents=None, for_docs=False, - name: str | None = None + name: str | None = None, + b2_binary_name: str | None = None, ) -> argparse.ArgumentParser: """ Creates a parser for the command. @@ -838,12 +839,15 @@ def create_parser( :param subparsers: subparsers object to which add new parser :param parents: created ArgumentParser `parents`, see `argparse.ArgumentParser` :param for_docs: if parser is to be used for documentation generation + :param name: action name + :param b2_binary_name: B2 binary call name :return: created parser """ if parents is None: parents = [] - description = cls._get_description() + b2_binary_name = b2_binary_name or resolve_b2_bin_call_name() + description = cls._get_description(NAME=b2_binary_name) if name: alias = None @@ -905,7 +909,12 @@ def create_parser( ) subparsers.required = True for subcommand in cls.subcommands_registry.values(): - subcommand.create_parser(subparsers=subparsers, parents=parents, for_docs=for_docs) + subcommand.create_parser( + subparsers=subparsers, + parents=parents, + for_docs=for_docs, + b2_binary_name=b2_binary_name + ) return parser @@ -1016,9 +1025,9 @@ def run(self, args): return super().run(args) @classmethod - def _get_description(cls): + def _get_description(cls, **kwargs): return ( - f'{super()._get_description()}\n\n' + f'{super()._get_description(**kwargs)}\n\n' f'.. warning::\n' f' This command is deprecated. Use ``{cls.replaced_by_cmd.name_and_alias()[0]}`` instead.\n' ) @@ -1083,7 +1092,7 @@ class B2(Command): @classmethod def name_and_alias(cls): - return NAME, None + return resolve_b2_bin_call_name(), None def _run(self, args): # Commands could be named via name or alias, so we fetch @@ -1113,6 +1122,7 @@ class AuthorizeAccount(Command): Stores an account auth token in a local cache, see + .. code-block:: {NAME} --help @@ -3912,10 +3922,11 @@ def _put_license_text(self, stream: io.StringIO, with_packages: bool = False): if with_packages: self._put_license_text_for_packages(stream) + b2_call_name = self.console_tool.b2_binary_name included_sources = get_included_sources() if included_sources: stream.write( - f'\n\nThird party libraries modified and included in {NAME} or {b2sdk.__name__}:\n' + f'\n\nThird party libraries modified and included in {b2_call_name} or {b2sdk.__name__}:\n' ) for src in included_sources: stream.write('\n') @@ -3928,7 +3939,7 @@ def _put_license_text(self, stream: io.StringIO, with_packages: bool = False): for file_name, file_content in src.files.items(): files_table.add_row([file_name, file_content]) stream.write(str(files_table)) - stream.write(f'\n\n{NAME} license:\n') + stream.write(f'\n\n{b2_call_name} license:\n') b2_license_file_text = (pathlib.Path(__file__).parent.parent / 'LICENSE').read_text(encoding='utf8') stream.write(b2_license_file_text) @@ -3959,10 +3970,13 @@ def _put_license_text_for_packages(self, stream: io.StringIO): modules_added.add(module_info['Name']) assert not (self.MODULES_TO_OVERRIDE_LICENSE_TEXT - modules_added) - stream.write(f'Licenses of all modules used by {NAME}, shipped with it in binary form:\n') + b2_call_name = self.console_tool.b2_binary_name + stream.write( + f'Licenses of all modules used by {b2_call_name}, shipped with it in binary form:\n' + ) stream.write(str(license_table)) stream.write( - f'\n\nSummary of all modules used by {NAME}, shipped with it in binary form:\n' + f'\n\nSummary of all modules used by {b2_call_name}, shipped with it in binary form:\n' ) stream.write(str(summary_table)) @@ -4054,7 +4068,7 @@ def _run(self, args): return 1 try: - autocomplete_install(NAME, shell=shell) + autocomplete_install(self.console_tool.b2_binary_name, shell=shell) except AutocompleteInstallError as e: raise CommandError(str(e)) from e self._print(f'Autocomplete successfully installed for {shell}.') @@ -4076,6 +4090,7 @@ def __init__(self, b2_api: B2Api | None, stdout, stderr): self.api = b2_api self.stdout = stdout self.stderr = stderr + self.b2_binary_name = 'b2' def _get_default_escape_cc_setting(self): escape_cc_env_var = os.environ.get(B2_ESCAPE_CONTROL_CHARACTERS, None) @@ -4090,7 +4105,8 @@ def _get_default_escape_cc_setting(self): def run_command(self, argv): signal.signal(signal.SIGINT, keyboard_interrupt_handler) - parser = B2.create_parser(name=argv[0]) + self.b2_binary_name = resolve_b2_bin_call_name(argv) + parser = B2.create_parser(name=self.b2_binary_name, b2_binary_name=self.b2_binary_name) AUTOCOMPLETE.cache_and_autocomplete(parser) args = parser.parse_args(argv[1:]) self._setup_logging(args, argv) @@ -4146,15 +4162,13 @@ def run_command(self, argv): except MissingAccountData as e: logger.exception('ConsoleTool missing account data error') self._print_stderr( - 'ERROR: {} Use: {} authorize-account or provide auth data with "{}" and "{}"' - ' environment variables'.format( - str(e), NAME, B2_APPLICATION_KEY_ID_ENV_VAR, B2_APPLICATION_KEY_ENV_VAR - ) + f'ERROR: {e} Use: {self.b2_binary_name} authorize-account or provide auth data with ' + f'{B2_APPLICATION_KEY_ID_ENV_VAR!r} and {B2_APPLICATION_KEY_ENV_VAR!r} environment variables' ) return 1 except B2Error as e: logger.exception('ConsoleTool command error') - self._print_stderr(f'ERROR: {str(e)}') + self._print_stderr(f'ERROR: {e}') return 1 except KeyboardInterrupt: logger.exception('ConsoleTool command interrupt') diff --git a/changelog.d/+b2_path_in_help.fixed.md b/changelog.d/+b2_path_in_help.fixed.md new file mode 100644 index 000000000..6bdffac37 --- /dev/null +++ b/changelog.d/+b2_path_in_help.fixed.md @@ -0,0 +1 @@ +Fix `b2 --help` showing full binary path instead of just basename. diff --git a/doc/source/conf.py b/doc/source/conf.py index 51f4e0f8b..294596d9a 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -213,7 +213,7 @@ def setup(_): reasonable way to piggy back that behaviour. Checking if the new file contents would the same as the old one (if any) is important, so that the automatic file-watcher/doc-builder doesn't fall into an endless loop. """ - main_help_text = str(B2.lazy_get_description()) + main_help_text = str(B2.lazy_get_description(NAME='b2')) main_help_text = textwrap.dedent(main_help_text) main_help_path = path.join(path.dirname(__file__), 'main_help.rst') diff --git a/test/integration/test_b2_command_line.py b/test/integration/test_b2_command_line.py index 5c0cb70e4..2e21e01e6 100755 --- a/test/integration/test_b2_command_line.py +++ b/test/integration/test_b2_command_line.py @@ -421,8 +421,8 @@ def test_account(b2_tool, cli_version): b2_tool.should_fail( ['create-bucket', bucket_name, 'allPrivate'], r'ERROR: Missing account data: \'NoneType\' object is not subscriptable (\(key 0\) )? ' - fr'Use: {cli_version}(\.(exe|EXE))? authorize-account or provide auth data with "B2_APPLICATION_KEY_ID" and ' - r'"B2_APPLICATION_KEY" environment variables' + fr'Use: {cli_version}(\.(exe|EXE))? authorize-account or provide auth data with \'B2_APPLICATION_KEY_ID\' and ' + r'\'B2_APPLICATION_KEY\' environment variables' ) os.remove(new_creds) diff --git a/test/integration/test_help.py b/test/integration/test_help.py new file mode 100644 index 000000000..bfa6410f0 --- /dev/null +++ b/test/integration/test_help.py @@ -0,0 +1,28 @@ +###################################################################### +# +# File: test/integration/test_help.py +# +# Copyright 2024 Backblaze Inc. All Rights Reserved. +# +# License https://www.backblaze.com/using_b2_code.html +# +###################################################################### +import platform +import re +import subprocess + + +def test_help(cli_version): + p = subprocess.run( + [cli_version, "--help"], + check=True, + capture_output=True, + text=True, + ) + + # verify help contains apiver binary name + expected_name = cli_version + if platform.system() == 'Windows': + expected_name += '.exe' + assert re.match(r"^_?b2(v\d+)?(\.exe)?$", expected_name) # test sanity check + assert f" {expected_name} --help" in p.stdout From 9adc9ed4c5f4fb26007448743f90c0bfe8874aa3 Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Fri, 15 Mar 2024 22:24:46 +0100 Subject: [PATCH 2/3] fix typo in noxfile docstring --- noxfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noxfile.py b/noxfile.py index 5522a110b..a7cdfc0c5 100644 --- a/noxfile.py +++ b/noxfile.py @@ -112,7 +112,7 @@ def get_version_key(path: pathlib.Path) -> int: def get_versions() -> list[str]: """ - "Almost" a copy of b2/_internalg/version_listing.py:get_versions(), because importing + "Almost" a copy of b2/_internal/version_listing.py:get_versions(), because importing the file directly seems impossible from the noxfile. """ # This sorting ensures that: From 223d9c95a522ded2754a392bf01ed8706a0a488b Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Fri, 15 Mar 2024 22:25:06 +0100 Subject: [PATCH 3/3] properly mark skipped test using pytest.skip --- test/integration/test_autocomplete.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/test_autocomplete.py b/test/integration/test_autocomplete.py index c877943e3..cd8cf73df 100644 --- a/test/integration/test_autocomplete.py +++ b/test/integration/test_autocomplete.py @@ -41,7 +41,7 @@ def cli_command(request) -> str: @pytest.fixture(scope="module") def autocomplete_installed(env, homedir, bashrc, cli_version, cli_command, is_running_on_docker): if is_running_on_docker: - return + pytest.skip('Not supported on Docker') shell = pexpect.spawn( f'bash -i -c "{cli_command} install-autocomplete"', env=env, logfile=sys.stderr.buffer