From 7e166acdaeaccb860b46518568288b84a0bbe452 Mon Sep 17 00:00:00 2001 From: Ken Odegard Date: Wed, 7 Feb 2024 19:17:28 +0100 Subject: [PATCH] Fix nonzero exit for conda-build entrypoint (#5169) Add type hints & fix return type for execute functions. Updates tests that previously depended on `conda_build.cli.main_build.execute`'s return value. --- conda_build/cli/main_build.py | 28 ++++--- conda_build/cli/main_convert.py | 4 +- conda_build/cli/main_debug.py | 4 +- conda_build/cli/main_develop.py | 4 +- conda_build/cli/main_inspect.py | 4 +- conda_build/cli/main_metapackage.py | 4 +- conda_build/cli/main_render.py | 43 +++++------ conda_build/cli/main_skeleton.py | 4 +- conda_build/plugin.py | 36 +++++---- conda_build/utils.py | 2 +- news/5169-fix-nonzero-exitcode | 19 +++++ tests/cli/test_main_build.py | 111 +++++++++++++++++++--------- 12 files changed, 172 insertions(+), 91 deletions(-) create mode 100644 news/5169-fix-nonzero-exitcode diff --git a/conda_build/cli/main_build.py b/conda_build/cli/main_build.py index f84024df48..51c906ebc8 100644 --- a/conda_build/cli/main_build.py +++ b/conda_build/cli/main_build.py @@ -522,7 +522,7 @@ def check_action(recipe, config): return api.check(recipe, config=config) -def execute(args: Sequence[str] | None = None): +def execute(args: Sequence[str] | None = None) -> int: _, parsed = parse_args(args) config = get_or_merge_config(None, **parsed.__dict__) build.check_external() @@ -535,21 +535,22 @@ def execute(args: Sequence[str] | None = None): if "purge" in parsed.recipe: build.clean_build(config) - return + return 0 if "purge-all" in parsed.recipe: build.clean_build(config) config.clean_pkgs() - return + return 0 - outputs = None if parsed.output: config.verbose = False config.quiet = True config.debug = False - outputs = [output_action(recipe, config) for recipe in parsed.recipe] - elif parsed.test: - outputs = [] + for recipe in parsed.recipe: + output_action(recipe, config) + return 0 + + if parsed.test: failed_recipes = [] recipes = chain.from_iterable( glob(abspath(recipe), recursive=True) if "*" in recipe else [recipe] @@ -571,11 +572,13 @@ def execute(args: Sequence[str] | None = None): else: print("All tests passed") elif parsed.source: - outputs = [source_action(recipe, config) for recipe in parsed.recipe] + for recipe in parsed.recipe: + source_action(recipe, config) elif parsed.check: - outputs = [check_action(recipe, config) for recipe in parsed.recipe] + for recipe in parsed.recipe: + check_action(recipe, config) else: - outputs = api.build( + api.build( parsed.recipe, post=parsed.post, test_run_post=parsed.test_run_post, @@ -588,6 +591,7 @@ def execute(args: Sequence[str] | None = None): cache_dir=parsed.cache_dir, ) - if not parsed.output and len(utils.get_build_folders(config.croot)) > 0: + if utils.get_build_folders(config.croot): build.print_build_intermediate_warning(config) - return outputs + + return 0 diff --git a/conda_build/cli/main_convert.py b/conda_build/cli/main_convert.py index 14b5b4f9b0..7f50883172 100644 --- a/conda_build/cli/main_convert.py +++ b/conda_build/cli/main_convert.py @@ -120,7 +120,7 @@ def parse_args(args: Sequence[str] | None) -> tuple[ArgumentParser, Namespace]: return parser, parser.parse_args(args) -def execute(args: Sequence[str] | None = None): +def execute(args: Sequence[str] | None = None) -> int: _, parsed = parse_args(args) files = parsed.files del parsed.__dict__["files"] @@ -128,3 +128,5 @@ def execute(args: Sequence[str] | None = None): for f in files: f = abspath(expanduser(f)) api.convert(f, **parsed.__dict__) + + return 0 diff --git a/conda_build/cli/main_debug.py b/conda_build/cli/main_debug.py index b01b8b5e03..0fcdd8ab5c 100644 --- a/conda_build/cli/main_debug.py +++ b/conda_build/cli/main_debug.py @@ -88,7 +88,7 @@ def get_parser() -> ArgumentParser: return p -def execute(args: Sequence[str] | None = None): +def execute(args: Sequence[str] | None = None) -> int: parser = get_parser() parsed = parser.parse_args(args) @@ -118,3 +118,5 @@ def execute(args: Sequence[str] | None = None): f"Error: conda-debug encountered the following error:\n{e}", file=sys.stderr ) sys.exit(1) + + return 0 diff --git a/conda_build/cli/main_develop.py b/conda_build/cli/main_develop.py index 2c81a4edc1..694dde32b8 100644 --- a/conda_build/cli/main_develop.py +++ b/conda_build/cli/main_develop.py @@ -76,7 +76,7 @@ def parse_args(args: Sequence[str] | None) -> tuple[ArgumentParser, Namespace]: return parser, parser.parse_args(args) -def execute(args: Sequence[str] | None = None): +def execute(args: Sequence[str] | None = None) -> int: _, parsed = parse_args(args) prefix = determine_target_prefix(context, parsed) api.develop( @@ -87,3 +87,5 @@ def execute(args: Sequence[str] | None = None): clean=parsed.clean, uninstall=parsed.uninstall, ) + + return 0 diff --git a/conda_build/cli/main_inspect.py b/conda_build/cli/main_inspect.py index 58cba771dc..e71ee03137 100644 --- a/conda_build/cli/main_inspect.py +++ b/conda_build/cli/main_inspect.py @@ -184,7 +184,7 @@ def parse_args(args: Sequence[str] | None) -> tuple[ArgumentParser, Namespace]: return parser, parser.parse_args(args) -def execute(args: Sequence[str] | None = None): +def execute(args: Sequence[str] | None = None) -> int: parser, parsed = parse_args(args) if not parsed.subcommand: @@ -221,3 +221,5 @@ def execute(args: Sequence[str] | None = None): pprint(api.inspect_hash_inputs(parsed.packages)) else: parser.error(f"Unrecognized subcommand: {parsed.subcommand}.") + + return 0 diff --git a/conda_build/cli/main_metapackage.py b/conda_build/cli/main_metapackage.py index b4c610aea8..1af55abfbb 100644 --- a/conda_build/cli/main_metapackage.py +++ b/conda_build/cli/main_metapackage.py @@ -108,7 +108,9 @@ def parse_args(args: Sequence[str] | None) -> tuple[ArgumentParser, Namespace]: return parser, parser.parse_args(args) -def execute(args: Sequence[str] | None = None): +def execute(args: Sequence[str] | None = None) -> int: _, args = parse_args(args) channel_urls = args.__dict__.get("channel") or args.__dict__.get("channels") or () api.create_metapackage(channel_urls=channel_urls, **args.__dict__) + + return 0 diff --git a/conda_build/cli/main_render.py b/conda_build/cli/main_render.py index 155c0e7739..5710bace1c 100644 --- a/conda_build/cli/main_render.py +++ b/conda_build/cli/main_render.py @@ -14,6 +14,7 @@ from .. import __version__, api from ..conda_interface import ArgumentParser, add_parser_channels, cc_conda_build from ..config import get_channel_urls, get_or_merge_config +from ..deprecations import deprecated from ..utils import LoggingContext from ..variants import get_package_variants, set_language_env_vars @@ -189,7 +190,8 @@ def parse_args(args: Sequence[str] | None) -> tuple[ArgumentParser, Namespace]: return parser, parser.parse_args(args) -def execute(args: Sequence[str] | None = None, print_results: bool = True): +@deprecated.argument("24.1.1", "24.3.0", "print_results") +def execute(args: Sequence[str] | None = None) -> int: _, parsed = parse_args(args) config = get_or_merge_config(None, **parsed.__dict__) @@ -221,24 +223,23 @@ def execute(args: Sequence[str] | None = None, print_results: bool = True): f"Only one will be written to the file you specified ({parsed.file})." ) - if print_results: - if parsed.output: - with LoggingContext(logging.CRITICAL + 1): - paths = api.get_output_file_paths(metadata_tuples, config=config) - print("\n".join(sorted(paths))) - if parsed.file: - m = metadata_tuples[-1][0] - api.output_yaml(m, parsed.file, suppress_outputs=True) - else: - logging.basicConfig(level=logging.INFO) - for m, _, _ in metadata_tuples: - print("--------------") - print("Hash contents:") - print("--------------") - pprint(m.get_hash_contents()) - print("----------") - print("meta.yaml:") - print("----------") - print(api.output_yaml(m, parsed.file, suppress_outputs=True)) + if parsed.output: + with LoggingContext(logging.CRITICAL + 1): + paths = api.get_output_file_paths(metadata_tuples, config=config) + print("\n".join(sorted(paths))) + if parsed.file: + m = metadata_tuples[-1][0] + api.output_yaml(m, parsed.file, suppress_outputs=True) else: - return metadata_tuples + logging.basicConfig(level=logging.INFO) + for m, _, _ in metadata_tuples: + print("--------------") + print("Hash contents:") + print("--------------") + pprint(m.get_hash_contents()) + print("----------") + print("meta.yaml:") + print("----------") + print(api.output_yaml(m, parsed.file, suppress_outputs=True)) + + return 0 diff --git a/conda_build/cli/main_skeleton.py b/conda_build/cli/main_skeleton.py index ade4b14d0e..d314304e62 100644 --- a/conda_build/cli/main_skeleton.py +++ b/conda_build/cli/main_skeleton.py @@ -46,7 +46,7 @@ def parse_args(args: Sequence[str] | None) -> tuple[ArgumentParser, Namespace]: return parser, parser.parse_args(args) -def execute(args: Sequence[str] | None = None): +def execute(args: Sequence[str] | None = None) -> int: parser, parsed = parse_args(args) config = Config(**parsed.__dict__) @@ -62,3 +62,5 @@ def execute(args: Sequence[str] | None = None): version=parsed.version, config=config, ) + + return 0 diff --git a/conda_build/plugin.py b/conda_build/plugin.py index 16ac40bbb1..e86aa2a7af 100644 --- a/conda_build/plugin.py +++ b/conda_build/plugin.py @@ -1,55 +1,59 @@ # Copyright (C) 2014 Anaconda, Inc # SPDX-License-Identifier: BSD-3-Clause +from __future__ import annotations + +from typing import Sequence + import conda.plugins # lazy-import to avoid nasty import-time side effects when not using conda-build -def build(*args, **kwargs): +def build(args: Sequence[str]) -> int: from .cli.main_build import execute - execute(*args, **kwargs) + return execute(args) -def convert(*args, **kwargs): +def convert(args: Sequence[str]) -> int: from .cli.main_convert import execute - execute(*args, **kwargs) + return execute(args) -def debug(*args, **kwargs): +def debug(args: Sequence[str]) -> int: from .cli.main_debug import execute - execute(*args, **kwargs) + return execute(args) -def develop(*args, **kwargs): +def develop(args: Sequence[str]) -> int: from .cli.main_develop import execute - execute(*args, **kwargs) + return execute(args) -def inspect(*args, **kwargs): +def inspect(args: Sequence[str]) -> int: from .cli.main_inspect import execute - execute(*args, **kwargs) + return execute(args) -def metapackage(*args, **kwargs): +def metapackage(args: Sequence[str]) -> int: from .cli.main_metapackage import execute - execute(*args, **kwargs) + return execute(args) -def render(*args, **kwargs): +def render(args: Sequence[str]) -> int: from .cli.main_render import execute - execute(*args, **kwargs) + return execute(args) -def skeleton(*args, **kwargs): +def skeleton(args: Sequence[str]) -> int: from .cli.main_skeleton import execute - execute(*args, **kwargs) + return execute(args) @conda.plugins.hookimpl diff --git a/conda_build/utils.py b/conda_build/utils.py index bc1c20634c..c1a529082f 100644 --- a/conda_build/utils.py +++ b/conda_build/utils.py @@ -990,7 +990,7 @@ def get_site_packages(prefix, py_ver): return os.path.join(get_stdlib_dir(prefix, py_ver), "site-packages") -def get_build_folders(croot): +def get_build_folders(croot: str | os.PathLike | Path) -> list[str]: # remember, glob is not a regex. return glob(os.path.join(croot, "*" + "[0-9]" * 10 + "*"), recursive=True) diff --git a/news/5169-fix-nonzero-exitcode b/news/5169-fix-nonzero-exitcode new file mode 100644 index 0000000000..e66efe71a1 --- /dev/null +++ b/news/5169-fix-nonzero-exitcode @@ -0,0 +1,19 @@ +### Enhancements + +* + +### Bug fixes + +* Fix nonzero exitcode on success. (#5167 via #5169) + +### Deprecations + +* + +### Docs + +* + +### Other + +* diff --git a/tests/cli/test_main_build.py b/tests/cli/test_main_build.py index 90d1c4a629..04c2cda7ba 100644 --- a/tests/cli/test_main_build.py +++ b/tests/cli/test_main_build.py @@ -130,15 +130,23 @@ def test_build_output_build_path_multiple_recipes( assert output.rstrip().splitlines() == test_paths, error -def test_slash_in_recipe_arg_keeps_build_id(testing_workdir, testing_config): +def test_slash_in_recipe_arg_keeps_build_id( + testing_workdir: str, testing_config: Config +): args = [ os.path.join(metadata_dir, "has_prefix_files"), "--croot", testing_config.croot, "--no-anaconda-upload", ] - outputs = main_build.execute(args) - data = package_has_file(outputs[0], "binary-has-prefix", refresh_mode="forced") + main_build.execute(args) + + output = os.path.join( + testing_config.croot, + testing_config.host_subdir, + "conda-build-test-has-prefix-files-1.0-0.tar.bz2", + ) + data = package_has_file(output, "binary-has-prefix", refresh_mode="forced") assert data if hasattr(data, "decode"): data = data.decode("UTF-8") @@ -157,7 +165,7 @@ def test_build_long_test_prefix_default_enabled(mocker, testing_workdir): main_build.execute(args) -def test_build_no_build_id(testing_workdir, testing_config): +def test_build_no_build_id(testing_workdir: str, testing_config: Config): args = [ os.path.join(metadata_dir, "has_prefix_files"), "--no-build-id", @@ -166,8 +174,14 @@ def test_build_no_build_id(testing_workdir, testing_config): "--no-activate", "--no-anaconda-upload", ] - outputs = main_build.execute(args) - data = package_has_file(outputs[0], "binary-has-prefix", refresh_mode="forced") + main_build.execute(args) + + output = os.path.join( + testing_config.croot, + testing_config.host_subdir, + "conda-build-test-has-prefix-files-1.0-0.tar.bz2", + ) + data = package_has_file(output, "binary-has-prefix", refresh_mode="forced") assert data if hasattr(data, "decode"): data = data.decode("UTF-8") @@ -191,7 +205,7 @@ def test_build_multiple_recipes(testing_metadata, testing_workdir, testing_confi main_build.execute(args) -def test_build_output_folder(testing_workdir: str, testing_metadata): +def test_build_output_folder(testing_workdir: str, testing_metadata: MetaData): api.output_yaml(testing_metadata, "meta.yaml") out = Path(testing_workdir, "out") @@ -207,9 +221,10 @@ def test_build_output_folder(testing_workdir: str, testing_metadata): "--output-folder", str(out), ] - output = main_build.execute(args)[0] + main_build.execute(args) + assert ( - out / testing_metadata.config.host_subdir / os.path.basename(output) + out / testing_metadata.config.host_subdir / testing_metadata.pkg_fn() ).is_file() @@ -375,38 +390,53 @@ def test_activate_scripts_not_included(testing_workdir): assert not package_has_file(out, f) -def test_relative_path_croot(conda_build_test_recipe_envvar: str): +def test_relative_path_croot( + conda_build_test_recipe_envvar: str, testing_config: Config +): # this tries to build a package while specifying the croot with a relative path: # conda-build --no-test --croot ./relative/path + empty_sections = Path(metadata_dir, "empty_with_build_script") + croot = Path(".", "relative", "path") - empty_sections = os.path.join(metadata_dir, "empty_with_build_script") - croot_rel = os.path.join(".", "relative", "path") - args = ["--no-anaconda-upload", "--croot", croot_rel, empty_sections] - outputfile = main_build.execute(args) + args = ["--no-anaconda-upload", f"--croot={croot}", str(empty_sections)] + main_build.execute(args) - assert len(outputfile) == 1 - assert os.path.isfile(outputfile[0]) + assert len(list(croot.glob("**/*.tar.bz2"))) == 1 + assert ( + croot / testing_config.subdir / "empty_with_build_script-0.0-0.tar.bz2" + ).is_file() -def test_relative_path_test_artifact(conda_build_test_recipe_envvar: str): +def test_relative_path_test_artifact( + conda_build_test_recipe_envvar: str, testing_config: Config +): # this test builds a package into (cwd)/relative/path and then calls: # conda-build --test ./relative/path/{platform}/{artifact}.tar.bz2 - - empty_sections = os.path.join(metadata_dir, "empty_with_build_script") - croot_rel = os.path.join(".", "relative", "path") - croot_abs = os.path.abspath(os.path.normpath(croot_rel)) + empty_sections = Path(metadata_dir, "empty_with_build_script") + croot_rel = Path(".", "relative", "path") + croot_abs = croot_rel.resolve() # build the package - args = ["--no-anaconda-upload", "--no-test", "--croot", croot_abs, empty_sections] - output_file_abs = main_build.execute(args) - assert len(output_file_abs) == 1 + args = [ + "--no-anaconda-upload", + "--no-test", + f"--croot={croot_abs}", + str(empty_sections), + ] + main_build.execute(args) - output_file_rel = os.path.join( - croot_rel, os.path.relpath(output_file_abs[0], croot_abs) - ) + assert len(list(croot_abs.glob("**/*.tar.bz2"))) == 1 # run the test stage with relative path - args = ["--no-anaconda-upload", "--test", output_file_rel] + args = [ + "--no-anaconda-upload", + "--test", + os.path.join( + croot_rel, + testing_config.subdir, + "empty_with_build_script-0.0-0.tar.bz2", + ), + ] main_build.execute(args) @@ -414,17 +444,28 @@ def test_relative_path_test_recipe(conda_build_test_recipe_envvar: str): # this test builds a package into (cwd)/relative/path and then calls: # conda-build --test --croot ./relative/path/ /abs/path/to/recipe - empty_sections = os.path.join(metadata_dir, "empty_with_build_script") - croot_rel = os.path.join(".", "relative", "path") - croot_abs = os.path.abspath(os.path.normpath(croot_rel)) + empty_sections = Path(metadata_dir, "empty_with_build_script") + croot_rel = Path(".", "relative", "path") + croot_abs = croot_rel.resolve() # build the package - args = ["--no-anaconda-upload", "--no-test", "--croot", croot_abs, empty_sections] - output_file_abs = main_build.execute(args) - assert len(output_file_abs) == 1 + args = [ + "--no-anaconda-upload", + "--no-test", + f"--croot={croot_abs}", + str(empty_sections), + ] + main_build.execute(args) + + assert len(list(croot_abs.glob("**/*.tar.bz2"))) == 1 # run the test stage with relative croot - args = ["--no-anaconda-upload", "--test", "--croot", croot_rel, empty_sections] + args = [ + "--no-anaconda-upload", + "--test", + f"--croot={croot_rel}", + str(empty_sections), + ] main_build.execute(args)