Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: fail if we know the project is core24 #4557

Merged
merged 5 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/spread.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ jobs:
env:
SPREAD_GOOGLE_KEY: ${{ secrets.SPREAD_GOOGLE_KEY }}
UA_TOKEN: ${{ secrets.UA_TOKEN }}
run: spread ${{ matrix.spread-jobs }}
# Only run core24 tests for now; fix this once all those tests are OK
run: spread ${{ matrix.spread-jobs }}:tests/spread/core24/

- name: Discard spread workers
if: always()
Expand Down
5 changes: 5 additions & 0 deletions .shellcheckrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# NOTE: these disables are only necessary while the core24 spread tests are
# being fixed; they refer to unreachable code, unused variable, etc.
# This file must be removed once all tests in tests/spread/core24/ are fixed.
disable=SC2317,SC2034,SC2154

48 changes: 44 additions & 4 deletions snapcraft/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
from __future__ import annotations

import os
import pathlib
import signal
import sys
from typing import Any

import craft_cli
from craft_application import Application, AppMetadata, util
Expand All @@ -41,6 +43,11 @@
class Snapcraft(Application):
"""Snapcraft application definition."""

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Whether we know that we should use the core24-based codepath.
self._known_core24 = False
syu-w marked this conversation as resolved.
Show resolved Hide resolved

@override
def _configure_services(self, platform: str | None, build_for: str | None) -> None:
if build_for is None:
Expand All @@ -55,10 +62,22 @@ def command_groups(self):
# TODO: Remove this once we've got lifecycle commands and version migrated.
return self._command_groups

def run(self) -> int:
"""Fall back to the old snapcraft entrypoint."""
self._get_dispatcher()
raise errors.ClassicFallback()
def _resolve_project_path(self, project_dir: pathlib.Path | None) -> pathlib.Path:
"""Overridden to handle the two possible locations for snapcraft.yaml."""
if project_dir is None:
project_dir = pathlib.Path.cwd()

try:
return super()._resolve_project_path(project_dir / "snap")
except FileNotFoundError:
return super()._resolve_project_path(project_dir)

@property
def app_config(self) -> dict[str, Any]:
"""Overridden to add "core" knowledge to the config."""
config = super().app_config
config["core24"] = self._known_core24
return config

@override
def _get_dispatcher(self) -> craft_cli.Dispatcher:
Expand All @@ -68,6 +87,7 @@ def _get_dispatcher(self) -> craft_cli.Dispatcher:

:returns: A ready-to-run Dispatcher object
"""
# pylint: disable=too-many-statements
# Set the logging level to DEBUG for all craft-libraries. This is OK even if
# the specific application doesn't use a specific library, the call does not
# import the package.
Expand All @@ -81,6 +101,26 @@ def _get_dispatcher(self) -> craft_cli.Dispatcher:
streaming_brief=True,
)

# Handle "multiplexing" of Snapcraft "codebases" depending on the
# project's base (if any). Here, we handle the case where there *is*
# a project and it's core24, which means it should definitely fall into
# the craft-application-based flow.
try:
existing_project = self._resolve_project_path(None)
except FileNotFoundError:
# No project file - don't know if we should use core24 code or not.
pass
else:
with existing_project.open() as file:
yaml_data = util.safe_yaml_load(file)
base = yaml_data.get("base")
build_base = yaml_data.get("build-base")
if "core24" in (base, build_base) or build_base == "devel":
# We know for sure that we're handling a core24 project
self._known_core24 = True
else:
raise errors.ClassicFallback()

dispatcher = craft_cli.Dispatcher(
self.app.name,
self.command_groups,
Expand Down
20 changes: 19 additions & 1 deletion snapcraft/commands/unimplemented.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

from snapcraft import commands, errors

# pylint: disable=missing-class-docstring
# pylint: disable=missing-class-docstring,unused-argument


class UnimplementedMixin:
Expand All @@ -33,8 +33,26 @@ class UnimplementedMixin:
@final
def run(self, parsed_args: argparse.Namespace) -> None:
"""Execute a command's functionality."""
if self.config["core24"]: # type: ignore[attr-defined]
# We know that this is a core24 project, but the actual command has
# not yet been ported.
command_name = self.name # type: ignore[attr-defined]
raise RuntimeError(
f'"{command_name}" command is not implemented for core24!'
)

# Fallback to the codepaths for non-core24-code.
raise errors.ClassicFallback()

def run_managed(
self,
parsed_args: argparse.Namespace, # noqa: ARG002 (the unused argument is for subclasses)
) -> bool:
"""Overridden to always return False, for now."""
return False

always_load_project: bool = False


class ExportLogin(
UnimplementedMixin, commands.StoreExportLoginCommand
Expand Down
9 changes: 7 additions & 2 deletions snapcraft/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@
from craft_cli import CraftError


class ClassicFallback(Exception):
"""Temporary class to fall back to non craft-application launcher."""
class ClassicFallback(BaseException):
"""Temporary class to fall back to non craft-application launcher.

Note that it inherits from BaseException so that it passes through
craft-application, is caught by Snapcraft itself and then redirected to
non-core24 codepaths.
"""


class SnapcraftError(CraftError):
Expand Down
35 changes: 34 additions & 1 deletion spread.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,40 @@ suites:
- ubuntu-22.04*

tests/spread/core24/:
summary: core24 tests
summary: core24 specific tests
systems:
- ubuntu-22.04*

tests/spread/core24/environment/:
summary: core24 environment tests
systems:
- ubuntu-22.04*

tests/spread/core24/scriptlets/:
summary: core24 scriptlet tests
systems:
- ubuntu-22.04*

tests/spread/core24/manifest/:
summary: core24 manifest tests
systems:
- ubuntu-22.04*

tests/spread/core24/architectures/:
summary: core24 architecture tests
systems:
- ubuntu-22.04*

tests/spread/core24/linters/:
summary: core24 linter tests
environment:
SNAPCRAFT_ENABLE_DEVELOPER_DEBUG: "n"

systems:
- ubuntu-22.04*

tests/spread/core24/patchelf/:
summary: core24 patchelf tests
systems:
- ubuntu-22.04*

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--build-for arm64
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
arg-match_1.0_arm64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: arg-match
version: "1.0"
summary: test
description: |
Only snaps should be built where `build-for` matches the
command line argument `--build-for <architecture>`.
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: amd64
build-for: amd64
- build-on: [amd64, arm64]
build-for: arm64

parts:
nil:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--build-for armhf
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: env-var-no-match
version: "1.0"
summary: test
description: |
No snaps should be built if there no `build-for` that matches
command line argument `--build-for <architecture>`.
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: amd64
build-for: amd64
- build-on: [amd64, arm64]
build-for: arm64

parts:
nil:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
build-for-all_1.0_all.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: build-for-all
version: "1.0"
summary: test
description: |
Test building of `build-for: all`
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: amd64
build-for: all

parts:
nil:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: build-on-no-match
version: "1.0"
summary: test
description: |
When build-on doesn't match the host architecture,
no snaps should be built.
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: arm64
build-for: arm64
- build-on: armhf
build-for: armhf

parts:
nil:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
SNAPCRAFT_BUILD_FOR="arm64"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
env-var-match_1.0_arm64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: env-var-match
version: "1.0"
summary: test
description: |
Only snaps should be built where `build-for` matches the
environmental variable `SNAPCRAFT_BUILD_FOR`.
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: amd64
build-for: amd64
- build-on: [amd64, arm64]
build-for: arm64

parts:
nil:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
SNAPCRAFT_BUILD_FOR=armhf
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: env-var-no-match
version: "1.0"
summary: test
description: |
No snaps should be built if there no `build-for`
matches the environmental variable `SNAPCRAFT_BUILD_FOR`.
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: amd64
build-for: amd64
- build-on: [amd64, arm64]
build-for: arm64

parts:
nil:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
multiple-build-for_1.0_amd64.snap
multiple-build-for_1.0_arm64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: multiple-build-for
version: "1.0"
summary: test
description: |
Building on amd64 should produce 2 snaps,
one snap for amd64 and one snap for arm64.
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: amd64
build-for: amd64
- build-on: [amd64, arm64]
build-for: arm64

parts:
nil:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
multiple-build-for_1.0_amd64.snap
multiple-build-for_1.0_arm64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: multiple-build-for
version: "1.0"
summary: test
description: |
Building on amd64 should produce 2 snaps,
one snap for amd64 and one snap for arm64.
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: amd64
build-for: amd64
- build-on: [amd64, arm64]
build-for: arm64

parts:
nil:
plugin: nil
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
single-arch_1.0_amd64.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: single-arch
version: "1.0"
summary: test
description: |
A single architecture should create a single snap file.
grade: devel
confinement: strict
base: core24
build-base: devel
architectures:
- build-on: amd64
build-for: amd64

parts:
nil:
plugin: nil
Loading
Loading