Skip to content

Commit

Permalink
refactor(application): redesign pre-run validation
Browse files Browse the repository at this point in the history 
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
  • Loading branch information
@mr-cal
mr-cal committed Aug 28, 2024
1 parent 1fc4d23 commit 2c57004
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 82 deletions.
204 changes: 143 additions & 61 deletions snapcraft/application.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion snapcraft/cli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@
craft_cli.CommandGroup(
"Other",
[
*craft_application.commands.get_other_command_group().commands,
commands.LintCommand,
commands.InitCommand,
],
Expand Down
11 changes: 0 additions & 11 deletions tests/unit/commands/test_remote.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down
Loading

0 comments on commit 2c57004

Please sign in to comment.