diff --git a/snapcraft/application.py b/snapcraft/application.py index d19a6be4a41..d01a6d479bd 100644 --- a/snapcraft/application.py +++ b/snapcraft/application.py @@ -22,9 +22,8 @@ import os import pathlib import sys -from typing import Any, Optional +from typing import Any -import craft_application.commands as craft_app_commands import craft_cli import craft_parts import craft_store @@ -35,7 +34,7 @@ import snapcraft import snapcraft_legacy -from snapcraft import cli, errors, models, services, store +from snapcraft import cli, commands, errors, models, services, store from snapcraft.extensions import apply_extensions from snapcraft.models.project import SnapcraftBuildPlanner, apply_root_packages from snapcraft.parts import set_global_environment @@ -66,7 +65,6 @@ def _get_esm_error_for_base(base: str) -> None: """Raise an error appropriate for the base under ESM.""" - channel: str | None = None match base: case "core": channel = "4.x" @@ -150,11 +148,15 @@ def _configure_services(self, provider_name: str | None) -> None: super()._configure_services(provider_name) - # TODO: remove this override (#4911) @property def command_groups(self): - """Short-circuit the standard command groups for now.""" - return self._command_groups + """Replace craft-application's LifecycleCommand group.""" + _command_groups = super().command_groups + for index, command_group in enumerate(_command_groups): + if command_group.name == "Lifecycle": + _command_groups[index] = cli.CORE24_LIFECYCLE_COMMAND_GROUP + + return _command_groups @override def _resolve_project_path(self, project_dir: pathlib.Path | None) -> pathlib.Path: @@ -177,6 +179,22 @@ def app_config(self) -> dict[str, Any]: config["core24"] = self._known_core24 return config + @override + def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None: + """Do any final setup before running the command. + + :raises SnapcraftError: If the wrong codebase is chosen for the project. + :raises SnapcraftError: If the project uses a base that is not supported by the + current version of Snapcraft. + """ + if self._snapcraft_yaml_data: + base = self._snapcraft_yaml_data.get("base") + build_base = self._snapcraft_yaml_data.get("build-base") + _get_esm_error_for_base(base) + self._ensure_remote_build_supported(base, build_base) + + super()._pre_run(dispatcher) + @override def run(self) -> int: try: @@ -240,74 +258,139 @@ def _get_argv_command() -> str | None: return None - @override - def _get_dispatcher(self) -> craft_cli.Dispatcher: - """Handle multiplexing of Snapcraft "codebases" depending on the project's base. + def _check_for_classic_fallback(self) -> None: + """Check for and raise a ClassicFallback if an older codebase should be used. + + The project should use the classic fallback path for any of the following: + 1. Running a lifecycle command for a core20 or core22 snap + 2. Using the default command (pack) for a core20 or core22 snap + 3. Remote builds for a core20 snap + 4. Remote builds for a core22 snap with the `force-fallback` strategy + + Exception: If `--version` or `-V` is passed, do not use the classic fallback. - The ClassicFallback-based flow is used in any of the following scenarios: - - there is no project to load - - for core20 remote builds if SNAPCRAFT_REMOTE_BUILD_STRATEGY is not "disable-fallback" - - for core22 remote builds if SNAPCRAFT_REMOTE_BUILD_STRATEGY is "force-fallback" + If none of the above conditions are met, then the default craft-application + code path should be used. - The craft-application-based flow is used in any of the following scenarios: - - the project base is core24 or newer - - for the "version" command + :raises ClassicFallback: If the project should use the classic fallback code path. """ argv_command = self._get_argv_command() - if argv_command == "lint": - # We don't need to check for core24 if we're just linting - return super()._get_dispatcher() + + # Exception: If `--version` or `-V` is passed, do not use the classic fallback. + if {"--version", "-V"}.intersection(sys.argv): + return if self._snapcraft_yaml_data: base = self._snapcraft_yaml_data.get("base") build_base = self._snapcraft_yaml_data.get("build-base") - _get_esm_error_for_base(base) + is_classic_fallback_base = bool( + {"core20", "core22"}.intersection({base, build_base}) + ) + classic_lifecycle_commands = [ + command.name for command in cli.CORE22_LIFECYCLE_COMMAND_GROUP.commands + ] + + if is_classic_fallback_base: + # 1. Fallback for lifecycle commands for core20 and core22 snaps + if argv_command in classic_lifecycle_commands: + raise errors.ClassicFallback() + + # 2. Fallback for the default command for core20 and core22 snaps + if argv_command is None: + raise errors.ClassicFallback() + + build_strategy = os.environ.get("SNAPCRAFT_REMOTE_BUILD_STRATEGY", None) - if argv_command == "remote-build" and any( - b in ("core20", "core22") for b in (base, build_base) - ): - build_strategy = os.environ.get("SNAPCRAFT_REMOTE_BUILD_STRATEGY", None) - if build_strategy not in ( - "force-fallback", - "disable-fallback", - "", - None, + if argv_command == "remote-build": + # 3. Fallback for core20 snaps to use the legacy remote builder (#4885) + # Note that a `core20` snap with 'disable-fallback' set will follow the + # craft-application codepath and an error will be raised after the + # dispatcher is created. + if "core20" in (base, build_base) and ( + not build_strategy or build_strategy == "force-fallback" ): - raise errors.SnapcraftError( - message=( - f"Unknown value {build_strategy!r} in environment variable " - "'SNAPCRAFT_REMOTE_BUILD_STRATEGY'. " - ), - resolution=( - "Valid values are 'disable-fallback' and 'force-fallback'." - ), - ) - - # core20 must use the legacy remote builder because the Project model - # cannot parse core20 snapcraft.yaml schemas (#4885) - if "core20" in (base, build_base): - if build_strategy == "disable-fallback": - raise errors.SnapcraftError( - message=( - "'SNAPCRAFT_REMOTE_BUILD_STRATEGY=disable-fallback' " - "cannot be used for core20 snaps." - ), - resolution=( - "Unset the environment variable or use 'force-fallback'." - ), - ) raise errors.ClassicFallback() - # Use craft-application unless explicitly forced to use legacy snapcraft + # 4. Fallback for core22 remote builds with the `force-fallback` strategy if ( "core22" in (base, build_base) and build_strategy == "force-fallback" ): raise errors.ClassicFallback() - elif not self._known_core24 and not ( - argv_command == "version" or "--version" in sys.argv or "-V" in sys.argv - ): - raise errors.ClassicFallback() + + @staticmethod + def _ensure_remote_build_supported(base, build_base) -> None: + """Ensure the version of remote build is supported for the project. + + 1. SNAPCRAFT_REMOTE_BUILD_STRATEGY must be unset, 'disable-fallback', or + 'force-fallback' + 2. core20 projects must use the legacy remote builder + 3. core24 and newer projects must use the craft-application remote builder + + :raises SnapcraftError: If the environment variable `SNAPCRAFT_REMOTE_BUILD_STRATEGY` + is invalid. + :raises SnapcraftError: If the remote build version cannot be used for the project. + """ + build_strategy = os.environ.get("SNAPCRAFT_REMOTE_BUILD_STRATEGY", None) + + # 1. SNAPCRAFT_REMOTE_BUILD_STRATEGY must be unset, 'disable-fallback', or 'force-fallback' + if build_strategy and build_strategy not in ( + "force-fallback", + "disable-fallback", + ): + raise errors.SnapcraftError( + message=( + f"Unknown value {build_strategy!r} in environment variable " + "'SNAPCRAFT_REMOTE_BUILD_STRATEGY'. " + ), + resolution=( + "Valid values are 'disable-fallback' and 'force-fallback'." + ), + ) + + # 2. core20 projects must use the legacy remote builder (#4885) + if "core20" in (base, build_base) and build_strategy == "disable-fallback": + raise errors.SnapcraftError( + message=( + "'SNAPCRAFT_REMOTE_BUILD_STRATEGY=disable-fallback' " + "cannot be used for core20 snaps." + ), + resolution=( + "Unset the environment variable or set it to 'force-fallback'." + ), + ) + + # 3. core24 and newer projects must use the craft-application remote builder + elif ( + not {"core20", "core22"}.intersection({base, build_base}) + and build_strategy == "force-fallback" + ): + raise errors.SnapcraftError( + message=( + "'SNAPCRAFT_REMOTE_BUILD_STRATEGY=force-fallback' cannot " + "be used for core24 and newer snaps." + ), + resolution=( + "Unset the environment variable or set it to 'disable-fallback'." + ), + ) + + @override + def _get_dispatcher(self) -> craft_cli.Dispatcher: + """Handle multiplexing of Snapcraft "codebases" depending on the project's base. + + ClassicFallback errors must be raised before creating the Dispatcher. + + - The codebase for core24 and newer commands uses craft-application to + create and manage the Dispatcher. + - The codebase for core22 commands creates its own Dispatcher and handles + errors and exit codes. + - The codebase for core20 commands uses the legacy snapcraft codebase which + handles logging, errors, and exit codes internally. + + :raises ClassicFallback: If the core20 or core22 codebases should be used. + """ + self._check_for_classic_fallback() return super()._get_dispatcher() @override @@ -318,7 +401,7 @@ def _create_dispatcher(self) -> craft_cli.Dispatcher: self.command_groups, summary=str(self.app.summary), extra_global_args=self._global_arguments, - default_command=craft_app_commands.lifecycle.PackCommand, + default_command=commands.PackCommand, ) @override @@ -335,10 +418,9 @@ def create_app() -> Snapcraft: app = Snapcraft( app=APP_METADATA, services=snapcraft_services, - extra_loggers={"snapcraft.remote"}, ) - for group in [cli.CORE24_LIFECYCLE_COMMAND_GROUP, *cli.COMMAND_GROUPS]: + for group in cli.COMMAND_GROUPS: app.add_command_group(group.name, group.commands) return app diff --git a/snapcraft/cli.py b/snapcraft/cli.py index 6b2ca2de789..c916b9ff5e0 100644 --- a/snapcraft/cli.py +++ b/snapcraft/cli.py @@ -143,7 +143,6 @@ craft_cli.CommandGroup( "Other", [ - *craft_application.commands.get_other_command_group().commands, commands.LintCommand, commands.InitCommand, ], diff --git a/tests/unit/commands/test_remote.py b/tests/unit/commands/test_remote.py index 714eaa80c72..cba23b38b6f 100644 --- a/tests/unit/commands/test_remote.py +++ b/tests/unit/commands/test_remote.py @@ -32,7 +32,6 @@ from craft_application.remote.utils import get_build_id from snapcraft import application, const -from snapcraft.errors import ClassicFallback from snapcraft.utils import get_host_architecture # remote-build control logic may check if the working dir is a git repo, @@ -239,16 +238,6 @@ def test_remote_build_sudo_warns(emitter, snapcraft_yaml, base, mock_run_remote_ ) mock_run_remote_build.assert_called_once() - -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services") -def test_cannot_load_snapcraft_yaml(capsys, mocker): - """Raise an error if the snapcraft.yaml does not exist.""" - app = application.create_app() - - with pytest.raises(ClassicFallback): - app.run() - - @pytest.mark.parametrize("base", const.CURRENT_BASES) @pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services") def test_launchpad_timeout_default(mocker, snapcraft_yaml, base, fake_services): diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index b48df3600c6..5f50309a7c7 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -31,7 +31,7 @@ from snapcraft import application, const, services from snapcraft.commands import PackCommand -from snapcraft.errors import ClassicFallback, SnapcraftError +from snapcraft.errors import ClassicFallback from snapcraft.models.project import Architecture @@ -409,12 +409,10 @@ def test_esm_pass(mocker, snapcraft_yaml, base): mock_dispatch.assert_called_once() -@pytest.mark.parametrize( - "envvar", ["force-fallback", "disable-fallback", "badvalue", None] -) +@pytest.mark.parametrize("envvar", ["disable-fallback", None]) @pytest.mark.parametrize("base", const.CURRENT_BASES - {"core22"}) @pytest.mark.usefixtures("mock_confirm", "mock_remote_build_argv") -def test_run_envvar( +def test_run_remote_build_core24( monkeypatch, snapcraft_yaml, base, @@ -422,7 +420,7 @@ def test_run_envvar( mock_remote_build_run, mock_run_legacy, ): - """Bases core24 and later run new remote-build regardless of envvar.""" + """Bases core24 and later use the new remote-build.""" snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} snapcraft_yaml(**snapcraft_yaml_dict) @@ -437,6 +435,24 @@ def test_run_envvar( mock_run_legacy.assert_not_called() +@pytest.mark.parametrize("base", const.CURRENT_BASES - {"core22"}) +@pytest.mark.usefixtures("mock_confirm", "mock_remote_build_argv") +def test_run_remote_build_core24_error(monkeypatch, snapcraft_yaml, base, capsys): + """Error if using force-fallback for core24 or newer.""" + snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} + snapcraft_yaml(**snapcraft_yaml_dict) + monkeypatch.setenv("SNAPCRAFT_REMOTE_BUILD_STRATEGY", "force-fallback") + monkeypatch.setattr("sys.argv", ["snapcraft", "remote-build"]) + + application.main() + + _, err = capsys.readouterr() + assert ( + "'SNAPCRAFT_REMOTE_BUILD_STRATEGY=force-fallback' cannot be used " + "for core24 and newer snaps" + ) in err + + @pytest.mark.parametrize("base", const.LEGACY_BASES) @pytest.mark.usefixtures("mock_confirm", "mock_remote_build_argv") def test_run_envvar_disable_fallback_core20(snapcraft_yaml, base, monkeypatch, capsys): @@ -449,7 +465,10 @@ def test_run_envvar_disable_fallback_core20(snapcraft_yaml, base, monkeypatch, c application.main() _, err = capsys.readouterr() - assert f"'SNAPCRAFT_REMOTE_BUILD_STRATEGY=disable-fallback' cannot be used for {base} snaps" in err + assert ( + f"'SNAPCRAFT_REMOTE_BUILD_STRATEGY=disable-fallback' cannot be used for {base} snaps" + in err + ) @pytest.mark.parametrize("base", const.LEGACY_BASES | {"core22"}) @@ -500,7 +519,7 @@ def test_run_envvar_force_fallback_empty_core22( mock_run_legacy.assert_not_called() -@pytest.mark.parametrize("base", const.LEGACY_BASES | {"core22"}) +@pytest.mark.parametrize("base", const.LEGACY_BASES | const.CURRENT_BASES) @pytest.mark.usefixtures("mock_confirm", "mock_remote_build_argv") def test_run_envvar_invalid(snapcraft_yaml, base, monkeypatch, capsys): """core20 and core22 bases raise an error if the envvar is invalid.""" @@ -513,7 +532,10 @@ def test_run_envvar_invalid(snapcraft_yaml, base, monkeypatch, capsys): application.main() _, err = capsys.readouterr() - assert "Unknown value 'badvalue' in environment variable 'SNAPCRAFT_REMOTE_BUILD_STRATEGY'" in err + assert ( + "Unknown value 'badvalue' in environment variable 'SNAPCRAFT_REMOTE_BUILD_STRATEGY'" + in err + ) @pytest.mark.parametrize(