diff --git a/snapcraft/commands/remote.py b/snapcraft/commands/remote.py index 8a338ccd4f..68fd54db85 100644 --- a/snapcraft/commands/remote.py +++ b/snapcraft/commands/remote.py @@ -1,6 +1,6 @@ # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright 2022-2023 Canonical Ltd. +# Copyright 2022-2024 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License version 3 as @@ -91,18 +91,17 @@ def fill_parser(self, parser: argparse.ArgumentParser) -> None: parser.add_argument( "--status", action="store_true", help="display remote build status" ) - parser_target = parser.add_mutually_exclusive_group() - parser_target.add_argument( + parser.add_argument( "--build-on", + type=lambda arg: [arch.strip() for arch in arg.split(",")], metavar="arch", - nargs="+", help=HIDDEN, ) - parser_target.add_argument( + parser.add_argument( "--build-for", + type=lambda arg: [arch.strip() for arch in arg.split(",")], metavar="arch", - nargs="+", - help="architecture to build for", + help="comma-separated list of architectures to build for", ) parser.add_argument( "--build-id", metavar="build-id", help="specific build id to retrieve" @@ -141,7 +140,6 @@ def run(self, parsed_args: argparse.Namespace) -> None: if parsed_args.build_on: emit.message("Use --build-for instead of --build-on") - parsed_args.build_for = parsed_args.build_on if not parsed_args.launchpad_accept_public_upload and not confirm_with_user( _CONFIRMATION_PROMPT @@ -381,11 +379,24 @@ def _determine_architectures(self) -> List[str]: The build architectures can be set via the `--build-on` parameter or determined from the build-on architectures listed in the project's snapcraft.yaml. + To retain backwards compatibility, `--build-for` can also be used to + set the architectures. + :returns: A list of architectures. :raises SnapcraftError: If `--build-on` was provided and architectures are defined in the project's snapcraft.yaml. + :raises SnapcraftError: If `--build-on` and `--build-for` are both provided. """ + # argparse's `add_mutually_exclusive_group()` cannot be used because + # ArgumentParsingErrors executes the legacy remote-builder before this module + # can decide if the project is allowed to use the legacy remote-builder + if self._parsed_args.build_on and self._parsed_args.build_for: + raise SnapcraftError( + # use the same error as argparse produces for consistency + "Error: argument --build-for: not allowed with argument --build-on" + ) + project_architectures = self._get_project_build_on_architectures() if project_architectures and self._parsed_args.build_for: raise SnapcraftError( @@ -395,6 +406,8 @@ def _determine_architectures(self) -> List[str]: if project_architectures: archs = project_architectures + elif self._parsed_args.build_on: + archs = self._parsed_args.build_on elif self._parsed_args.build_for: archs = self._parsed_args.build_for else: diff --git a/tests/unit/commands/test_remote.py b/tests/unit/commands/test_remote.py index 10167887d0..e1222e4f43 100644 --- a/tests/unit/commands/test_remote.py +++ b/tests/unit/commands/test_remote.py @@ -1,6 +1,6 @@ # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright 2022-2023 Canonical Ltd. +# Copyright 2022-2024 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License version 3 as @@ -152,6 +152,43 @@ def test_command_accept_upload( mock_run_new_or_fallback_remote_build.assert_called_once() +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", "mock_confirm", "use_new_remote_build" +) +def test_command_new_build_arguments_mutually_exclusive(capsys, mocker): + """`--build-for` and `--build-on` are mutually exclusive in the new remote-build.""" + mocker.patch.object( + sys, + "argv", + ["snapcraft", "remote-build", "--build-on", "amd64", "--build-for", "arm64"], + ) + + cli.run() + + _, err = capsys.readouterr() + assert "Error: argument --build-for: not allowed with argument --build-on" in err + + +@pytest.mark.parametrize( + "create_snapcraft_yaml", LEGACY_BASES | {"core22"}, indirect=True +) +@pytest.mark.usefixtures("create_snapcraft_yaml", "mock_confirm") +def test_command_legacy_build_arguments_not_mutually_exclusive(mocker, mock_run_legacy): + """`--build-for` and `--build-on` are not mutually exclusive for legacy.""" + mocker.patch.object( + sys, + "argv", + ["snapcraft", "remote-build", "--build-on", "amd64", "--build-for", "arm64"], + ) + + cli.run() + + mock_run_legacy.assert_called_once() + + @pytest.mark.parametrize( "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True ) @@ -710,13 +747,56 @@ def test_determine_architectures_host_arch(mocker, mock_remote_builder): ) +@pytest.mark.parametrize("build_flag", ["--build-for", "--build-on"]) +@pytest.mark.parametrize( + ("archs", "expected_archs"), + [ + ("amd64", ["amd64"]), + ("amd64,arm64", ["amd64", "arm64"]), + ("amd64,amd64,arm64 ", ["amd64", "amd64", "arm64"]), + ], +) +@pytest.mark.parametrize( + "create_snapcraft_yaml", CURRENT_BASES | LEGACY_BASES, indirect=True +) +@pytest.mark.usefixtures( + "create_snapcraft_yaml", "mock_confirm", "use_new_remote_build" +) +def test_determine_architectures_provided_by_user_duplicate_arguments( + build_flag, archs, expected_archs, mocker, mock_remote_builder +): + """Argparse should only consider the last argument provided for build flags.""" + mocker.patch.object( + sys, + "argv", + # `--build-{for|on} armhf` should get silently ignored by argparse + ["snapcraft", "remote-build", build_flag, "armhf", build_flag, archs], + ) + + cli.run() + + mock_remote_builder.assert_called_with( + app_name="snapcraft", + build_id=None, + project_name="mytest", + architectures=expected_archs, + project_dir=Path(), + timeout=0, + ) + + +@pytest.mark.parametrize("build_flag", ["--build-for", "--build-on"]) @pytest.mark.parametrize( - ("args", "expected_archs"), + ("archs", "expected_archs"), [ - (["--build-for", "amd64"], ["amd64"]), - (["--build-for", "amd64", "arm64"], ["amd64", "arm64"]), + ("amd64", ["amd64"]), + ("amd64,arm64", ["amd64", "arm64"]), + ("amd64, arm64", ["amd64", "arm64"]), + ("amd64,arm64 ", ["amd64", "arm64"]), + ("amd64,arm64,armhf", ["amd64", "arm64", "armhf"]), + (" amd64 , arm64 , armhf ", ["amd64", "arm64", "armhf"]), # launchpad will accept and ignore duplicates - (["--build-for", "amd64", "amd64"], ["amd64", "amd64"]), + (" amd64 , arm64 , arm64 ", ["amd64", "arm64", "arm64"]), ], ) @pytest.mark.parametrize( @@ -726,10 +806,10 @@ def test_determine_architectures_host_arch(mocker, mock_remote_builder): "create_snapcraft_yaml", "mock_confirm", "use_new_remote_build" ) def test_determine_architectures_provided_by_user( - args, expected_archs, mocker, mock_remote_builder + build_flag, archs, expected_archs, mocker, mock_remote_builder ): """Use architectures provided by the user.""" - mocker.patch.object(sys, "argv", ["snapcraft", "remote-build"] + args) + mocker.patch.object(sys, "argv", ["snapcraft", "remote-build", build_flag, archs]) cli.run()