From 32cb8008ab1e13e1b55176a1b224f2861a73c222 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 08:39:25 +0000 Subject: [PATCH 01/14] Initial plan From 4f46898bb1b25d34bf037f53049f02c0de4e0779 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 08:53:18 +0000 Subject: [PATCH 02/14] Add base snap injection functionality Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- craft_application/services/provider.py | 9 +++ craft_application/util/__init__.py | 2 + craft_application/util/snap_config.py | 40 +++++++++++++ tests/unit/services/test_provider.py | 83 ++++++++++++++++++++++++++ tests/unit/util/test_snap_config.py | 74 ++++++++++++++++++++++- 5 files changed, 207 insertions(+), 1 deletion(-) diff --git a/craft_application/services/provider.py b/craft_application/services/provider.py index 00d00222e..98bdae5b1 100644 --- a/craft_application/services/provider.py +++ b/craft_application/services/provider.py @@ -128,6 +128,15 @@ def setup(self) -> None: "host into the build environment because it is running " "as a snap." ) + + # Inject the base snap from the host if available + base_snap = util.get_snap_base(self._app.name) + if base_snap: + emit.debug( + f"Setting {base_snap} to be injected from the " + "host into the build environment." + ) + self.snaps.append(Snap(name=base_snap, channel=None, classic=False)) else: # use the snap name when installing from the store name = self._app.name diff --git a/craft_application/util/__init__.py b/craft_application/util/__init__.py index aade87a8c..82c56c553 100644 --- a/craft_application/util/__init__.py +++ b/craft_application/util/__init__.py @@ -38,6 +38,7 @@ SnapConfig, get_snap_config, is_running_from_snap, + get_snap_base, ) from craft_application.util.string import humanize_list, strtobool from craft_application.util.system import get_parallel_build_count @@ -57,6 +58,7 @@ "convert_architecture_deb_to_platform", "get_snap_config", "is_running_from_snap", + "get_snap_base", "is_valid_architecture", "SnapConfig", "humanize_list", diff --git a/craft_application/util/snap_config.py b/craft_application/util/snap_config.py index a3c0b9ec0..066ddade8 100644 --- a/craft_application/util/snap_config.py +++ b/craft_application/util/snap_config.py @@ -17,9 +17,11 @@ """Snap config file definitions and helpers.""" import os +import pathlib from typing import Any, Literal import pydantic +import yaml from craft_cli import emit from snaphelpers import SnapConfigOptions, SnapCtlError @@ -40,6 +42,44 @@ def is_running_from_snap(app_name: str) -> bool: return os.getenv("SNAP_NAME") == app_name and os.getenv("SNAP") is not None +def get_snap_base(app_name: str) -> str | None: + """Get the base snap name from snap.yaml. + + :param app_name: The name of the application. + + :returns: The base snap name (e.g., 'core24') or None if not running from a snap + or if the snap.yaml doesn't have a base defined. + """ + if not is_running_from_snap(app_name): + emit.debug( + f"Not reading snap base because {app_name} is not running as a snap." + ) + return None + + snap_dir = os.getenv("SNAP") + if not snap_dir: + emit.debug("SNAP environment variable not set.") + return None + + snap_yaml_path = pathlib.Path(snap_dir) / "meta" / "snap.yaml" + if not snap_yaml_path.exists(): + emit.debug(f"snap.yaml not found at {snap_yaml_path}") + return None + + try: + with open(snap_yaml_path, "r") as f: + snap_data = yaml.safe_load(f) + base = snap_data.get("base") + if base: + emit.debug(f"Found base snap: {base}") + else: + emit.debug("No base defined in snap.yaml") + return base + except (OSError, yaml.YAMLError) as error: + emit.debug(f"Failed to read or parse snap.yaml: {error!r}") + return None + + class SnapConfig(pydantic.BaseModel, extra="forbid"): """Data stored in a snap config. diff --git a/tests/unit/services/test_provider.py b/tests/unit/services/test_provider.py index 88f8573ca..6813860be 100644 --- a/tests/unit/services/test_provider.py +++ b/tests/unit/services/test_provider.py @@ -248,6 +248,89 @@ def test_install_snap( assert service.snaps == [] +def test_install_snap_with_base( + tmp_path, + monkeypatch, + app_metadata, + fake_project, + fake_process: pytest_subprocess.FakeProcess, + fake_services, +): + """Test that the base snap is injected when running from a snap.""" + monkeypatch.setattr("snaphelpers._ctl.Popen", subprocess.Popen) + fake_process.register( + ["/usr/bin/snapctl", "get", "-d", fake_process.any()], + stdout="{}", + occurrences=1000, + ) + # Set up snap environment + monkeypatch.setenv("SNAP_NAME", "testcraft") + monkeypatch.setenv("SNAP_INSTANCE_NAME", "testcraft") + monkeypatch.setenv("SNAP", str(tmp_path)) + monkeypatch.delenv("CRAFT_SNAP_CHANNEL", raising=False) + + # Create snap.yaml with base + meta_dir = tmp_path / "meta" + meta_dir.mkdir() + snap_yaml = meta_dir / "snap.yaml" + snap_yaml.write_text("name: testcraft\nbase: core24\nversion: 1.0\n") + + service = provider.ProviderService( + app_metadata, + fake_services, + work_dir=pathlib.Path(), + install_snap=True, + ) + service.setup() + + # Verify both the app snap and base snap are injected + assert service.snaps == [ + Snap(name="core24", channel=None, classic=False), + Snap(name="testcraft", channel=None, classic=True), + ] + + +def test_install_snap_without_base( + tmp_path, + monkeypatch, + app_metadata, + fake_project, + fake_process: pytest_subprocess.FakeProcess, + fake_services, +): + """Test that only app snap is injected when snap.yaml has no base.""" + monkeypatch.setattr("snaphelpers._ctl.Popen", subprocess.Popen) + fake_process.register( + ["/usr/bin/snapctl", "get", "-d", fake_process.any()], + stdout="{}", + occurrences=1000, + ) + # Set up snap environment + monkeypatch.setenv("SNAP_NAME", "testcraft") + monkeypatch.setenv("SNAP_INSTANCE_NAME", "testcraft") + monkeypatch.setenv("SNAP", str(tmp_path)) + monkeypatch.delenv("CRAFT_SNAP_CHANNEL", raising=False) + + # Create snap.yaml without base + meta_dir = tmp_path / "meta" + meta_dir.mkdir() + snap_yaml = meta_dir / "snap.yaml" + snap_yaml.write_text("name: testcraft\nversion: 1.0\n") + + service = provider.ProviderService( + app_metadata, + fake_services, + work_dir=pathlib.Path(), + install_snap=True, + ) + service.setup() + + # Verify only the app snap is injected (no base) + assert service.snaps == [ + Snap(name="testcraft", channel=None, classic=True), + ] + + @pytest.mark.parametrize( "additional_snaps", [ diff --git a/tests/unit/util/test_snap_config.py b/tests/unit/util/test_snap_config.py index 92e2593f3..013b3f8be 100644 --- a/tests/unit/util/test_snap_config.py +++ b/tests/unit/util/test_snap_config.py @@ -20,7 +20,12 @@ import pytest from craft_application.errors import CraftValidationError -from craft_application.util import SnapConfig, get_snap_config, is_running_from_snap +from craft_application.util import ( + SnapConfig, + get_snap_base, + get_snap_config, + is_running_from_snap, +) from snaphelpers import SnapCtlError @@ -151,3 +156,70 @@ def test_get_snap_config_handle_fetch_error(error, mock_config): mock_config.return_value.fetch.side_effect = error assert get_snap_config(app_name="testcraft") is None + + +def test_get_snap_base_success(tmp_path, monkeypatch): + """Test get_snap_base returns the base when snap.yaml exists.""" + # Set up environment + monkeypatch.setenv("SNAP_NAME", "testcraft") + monkeypatch.setenv("SNAP", str(tmp_path)) + + # Create snap.yaml with base + meta_dir = tmp_path / "meta" + meta_dir.mkdir() + snap_yaml = meta_dir / "snap.yaml" + snap_yaml.write_text("name: testcraft\nbase: core24\nversion: 1.0\n") + + assert get_snap_base("testcraft") == "core24" + + +def test_get_snap_base_not_from_snap(monkeypatch): + """Test get_snap_base returns None when not running from snap.""" + monkeypatch.delenv("SNAP_NAME", raising=False) + monkeypatch.delenv("SNAP", raising=False) + + assert get_snap_base("testcraft") is None + + +def test_get_snap_base_no_snap_env(monkeypatch): + """Test get_snap_base returns None when SNAP env var is not set.""" + monkeypatch.setenv("SNAP_NAME", "testcraft") + monkeypatch.delenv("SNAP", raising=False) + + assert get_snap_base("testcraft") is None + + +def test_get_snap_base_no_yaml(tmp_path, monkeypatch): + """Test get_snap_base returns None when snap.yaml doesn't exist.""" + monkeypatch.setenv("SNAP_NAME", "testcraft") + monkeypatch.setenv("SNAP", str(tmp_path)) + + assert get_snap_base("testcraft") is None + + +def test_get_snap_base_no_base_in_yaml(tmp_path, monkeypatch): + """Test get_snap_base returns None when snap.yaml has no base.""" + monkeypatch.setenv("SNAP_NAME", "testcraft") + monkeypatch.setenv("SNAP", str(tmp_path)) + + # Create snap.yaml without base + meta_dir = tmp_path / "meta" + meta_dir.mkdir() + snap_yaml = meta_dir / "snap.yaml" + snap_yaml.write_text("name: testcraft\nversion: 1.0\n") + + assert get_snap_base("testcraft") is None + + +def test_get_snap_base_invalid_yaml(tmp_path, monkeypatch): + """Test get_snap_base returns None when snap.yaml is invalid.""" + monkeypatch.setenv("SNAP_NAME", "testcraft") + monkeypatch.setenv("SNAP", str(tmp_path)) + + # Create invalid snap.yaml + meta_dir = tmp_path / "meta" + meta_dir.mkdir() + snap_yaml = meta_dir / "snap.yaml" + snap_yaml.write_text("invalid: yaml: content: [") + + assert get_snap_base("testcraft") is None From 7a774f747e94fc9c8438603000a48717f3b7f6d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 08:56:04 +0000 Subject: [PATCH 03/14] Fix linting issues in snap_config.py Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- craft_application/util/snap_config.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/craft_application/util/snap_config.py b/craft_application/util/snap_config.py index 066ddade8..ca69a3583 100644 --- a/craft_application/util/snap_config.py +++ b/craft_application/util/snap_config.py @@ -67,18 +67,20 @@ def get_snap_base(app_name: str) -> str | None: return None try: - with open(snap_yaml_path, "r") as f: + with snap_yaml_path.open() as f: snap_data = yaml.safe_load(f) - base = snap_data.get("base") - if base: - emit.debug(f"Found base snap: {base}") - else: - emit.debug("No base defined in snap.yaml") - return base except (OSError, yaml.YAMLError) as error: emit.debug(f"Failed to read or parse snap.yaml: {error!r}") return None + base = snap_data.get("base") + if base: + emit.debug(f"Found base snap: {base}") + return base + + emit.debug("No base defined in snap.yaml") + return None + class SnapConfig(pydantic.BaseModel, extra="forbid"): """Data stored in a snap config. From 9f07eff1f5dc1356f8a5f7cb23a5458b307c65ef Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 09:50:16 +0000 Subject: [PATCH 04/14] Use snaphelpers for reading snap metadata Refactored get_snap_base() to use snaphelpers.SnapMetadataFiles instead of manually reading the SNAP environment variable and yaml file. Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- craft_application/util/snap_config.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/craft_application/util/snap_config.py b/craft_application/util/snap_config.py index ca69a3583..7cb8c7f0e 100644 --- a/craft_application/util/snap_config.py +++ b/craft_application/util/snap_config.py @@ -17,13 +17,13 @@ """Snap config file definitions and helpers.""" import os -import pathlib from typing import Any, Literal import pydantic import yaml from craft_cli import emit from snaphelpers import SnapConfigOptions, SnapCtlError +from snaphelpers._meta import SnapMetadataFiles from craft_application.errors import CraftValidationError @@ -56,24 +56,18 @@ def get_snap_base(app_name: str) -> str | None: ) return None - snap_dir = os.getenv("SNAP") - if not snap_dir: - emit.debug("SNAP environment variable not set.") - return None - - snap_yaml_path = pathlib.Path(snap_dir) / "meta" / "snap.yaml" - if not snap_yaml_path.exists(): - emit.debug(f"snap.yaml not found at {snap_yaml_path}") - return None - try: - with snap_yaml_path.open() as f: - snap_data = yaml.safe_load(f) - except (OSError, yaml.YAMLError) as error: - emit.debug(f"Failed to read or parse snap.yaml: {error!r}") + metadata_files = SnapMetadataFiles() + snap_yaml = metadata_files.snap + if not snap_yaml.exists(): + emit.debug("snap.yaml not found") + return None + + base = snap_yaml.get("base") + except (OSError, KeyError, yaml.YAMLError) as error: + emit.debug(f"Failed to read snap metadata: {error!r}") return None - base = snap_data.get("base") if base: emit.debug(f"Found base snap: {base}") return base From 38375423ccc7cbbfeafe40a802da8a9b8cdb26d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 11:24:25 +0000 Subject: [PATCH 05/14] Fix linting and improve tests - Fix mypy type error by adding type annotation for base variable - Use parallel install instance name (testcraft_1) in test - Parametrize test_install_snap_with_base with multiple base options (core24, core22, core20, None) - Update occurrences to 50 (more realistic than 1000) - Add documentation explaining when snaps don't have a base (base snaps use type: base) Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- craft_application/util/snap_config.py | 2 +- tests/unit/services/test_provider.py | 69 ++++++++++++++++++++++----- 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/craft_application/util/snap_config.py b/craft_application/util/snap_config.py index 7cb8c7f0e..5c9b20174 100644 --- a/craft_application/util/snap_config.py +++ b/craft_application/util/snap_config.py @@ -63,7 +63,7 @@ def get_snap_base(app_name: str) -> str | None: emit.debug("snap.yaml not found") return None - base = snap_yaml.get("base") + base: str | None = snap_yaml.get("base") except (OSError, KeyError, yaml.YAMLError) as error: emit.debug(f"Failed to read snap metadata: {error!r}") return None diff --git a/tests/unit/services/test_provider.py b/tests/unit/services/test_provider.py index 6813860be..e18c70ac1 100644 --- a/tests/unit/services/test_provider.py +++ b/tests/unit/services/test_provider.py @@ -248,6 +248,42 @@ def test_install_snap( assert service.snaps == [] +@pytest.mark.parametrize( + ("base_snap", "expected_snaps"), + [ + pytest.param( + "core24", + [ + Snap(name="core24", channel=None, classic=False), + Snap(name="testcraft_1", channel=None, classic=True), + ], + id="with-core24-base", + ), + pytest.param( + "core22", + [ + Snap(name="core22", channel=None, classic=False), + Snap(name="testcraft_1", channel=None, classic=True), + ], + id="with-core22-base", + ), + pytest.param( + "core20", + [ + Snap(name="core20", channel=None, classic=False), + Snap(name="testcraft_1", channel=None, classic=True), + ], + id="with-core20-base", + ), + pytest.param( + None, + [ + Snap(name="testcraft_1", channel=None, classic=True), + ], + id="without-base", + ), + ], +) def test_install_snap_with_base( tmp_path, monkeypatch, @@ -255,25 +291,31 @@ def test_install_snap_with_base( fake_project, fake_process: pytest_subprocess.FakeProcess, fake_services, + base_snap, + expected_snaps, ): """Test that the base snap is injected when running from a snap.""" monkeypatch.setattr("snaphelpers._ctl.Popen", subprocess.Popen) fake_process.register( ["/usr/bin/snapctl", "get", "-d", fake_process.any()], stdout="{}", - occurrences=1000, + occurrences=50, ) # Set up snap environment monkeypatch.setenv("SNAP_NAME", "testcraft") - monkeypatch.setenv("SNAP_INSTANCE_NAME", "testcraft") + monkeypatch.setenv("SNAP_INSTANCE_NAME", "testcraft_1") monkeypatch.setenv("SNAP", str(tmp_path)) monkeypatch.delenv("CRAFT_SNAP_CHANNEL", raising=False) - # Create snap.yaml with base + # Create snap.yaml with or without base meta_dir = tmp_path / "meta" meta_dir.mkdir() snap_yaml = meta_dir / "snap.yaml" - snap_yaml.write_text("name: testcraft\nbase: core24\nversion: 1.0\n") + if base_snap: + snap_yaml.write_text(f"name: testcraft\nbase: {base_snap}\nversion: 1.0\n") + else: + # Base snaps themselves (like core24) don't have a base field + snap_yaml.write_text("name: testcraft\ntype: base\nversion: 1.0\n") service = provider.ProviderService( app_metadata, @@ -283,11 +325,8 @@ def test_install_snap_with_base( ) service.setup() - # Verify both the app snap and base snap are injected - assert service.snaps == [ - Snap(name="core24", channel=None, classic=False), - Snap(name="testcraft", channel=None, classic=True), - ] + # Verify the expected snaps are injected + assert service.snaps == expected_snaps def test_install_snap_without_base( @@ -298,12 +337,16 @@ def test_install_snap_without_base( fake_process: pytest_subprocess.FakeProcess, fake_services, ): - """Test that only app snap is injected when snap.yaml has no base.""" + """Test that only app snap is injected when snap.yaml has no base. + + This happens in practice for base snaps themselves (e.g., core24, core22) + which use 'type: base' instead of having a 'base' field. + """ monkeypatch.setattr("snaphelpers._ctl.Popen", subprocess.Popen) fake_process.register( ["/usr/bin/snapctl", "get", "-d", fake_process.any()], stdout="{}", - occurrences=1000, + occurrences=50, ) # Set up snap environment monkeypatch.setenv("SNAP_NAME", "testcraft") @@ -311,11 +354,11 @@ def test_install_snap_without_base( monkeypatch.setenv("SNAP", str(tmp_path)) monkeypatch.delenv("CRAFT_SNAP_CHANNEL", raising=False) - # Create snap.yaml without base + # Create snap.yaml without base (simulates a base snap) meta_dir = tmp_path / "meta" meta_dir.mkdir() snap_yaml = meta_dir / "snap.yaml" - snap_yaml.write_text("name: testcraft\nversion: 1.0\n") + snap_yaml.write_text("name: testcraft\ntype: base\nversion: 1.0\n") service = provider.ProviderService( app_metadata, From 204d75dc465a83ba5725dbf2aba053e666c1e0e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 12:47:57 +0000 Subject: [PATCH 06/14] Fix ruff linting error - remove trailing whitespace Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- tests/unit/services/test_provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/services/test_provider.py b/tests/unit/services/test_provider.py index e18c70ac1..01be31320 100644 --- a/tests/unit/services/test_provider.py +++ b/tests/unit/services/test_provider.py @@ -338,7 +338,7 @@ def test_install_snap_without_base( fake_services, ): """Test that only app snap is injected when snap.yaml has no base. - + This happens in practice for base snaps themselves (e.g., core24, core22) which use 'type: base' instead of having a 'base' field. """ From 5faca45827bce109ed85279d783ac1c40d853378 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 15:17:43 +0000 Subject: [PATCH 07/14] Remove redundant test_install_snap_without_base The without-base scenario is already covered by the parametrized test_install_snap_with_base test (without-base case). Testing base snaps themselves is a very unlikely scenario. Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- tests/unit/services/test_provider.py | 45 ---------------------------- 1 file changed, 45 deletions(-) diff --git a/tests/unit/services/test_provider.py b/tests/unit/services/test_provider.py index 01be31320..00178fb54 100644 --- a/tests/unit/services/test_provider.py +++ b/tests/unit/services/test_provider.py @@ -329,51 +329,6 @@ def test_install_snap_with_base( assert service.snaps == expected_snaps -def test_install_snap_without_base( - tmp_path, - monkeypatch, - app_metadata, - fake_project, - fake_process: pytest_subprocess.FakeProcess, - fake_services, -): - """Test that only app snap is injected when snap.yaml has no base. - - This happens in practice for base snaps themselves (e.g., core24, core22) - which use 'type: base' instead of having a 'base' field. - """ - monkeypatch.setattr("snaphelpers._ctl.Popen", subprocess.Popen) - fake_process.register( - ["/usr/bin/snapctl", "get", "-d", fake_process.any()], - stdout="{}", - occurrences=50, - ) - # Set up snap environment - monkeypatch.setenv("SNAP_NAME", "testcraft") - monkeypatch.setenv("SNAP_INSTANCE_NAME", "testcraft") - monkeypatch.setenv("SNAP", str(tmp_path)) - monkeypatch.delenv("CRAFT_SNAP_CHANNEL", raising=False) - - # Create snap.yaml without base (simulates a base snap) - meta_dir = tmp_path / "meta" - meta_dir.mkdir() - snap_yaml = meta_dir / "snap.yaml" - snap_yaml.write_text("name: testcraft\ntype: base\nversion: 1.0\n") - - service = provider.ProviderService( - app_metadata, - fake_services, - work_dir=pathlib.Path(), - install_snap=True, - ) - service.setup() - - # Verify only the app snap is injected (no base) - assert service.snaps == [ - Snap(name="testcraft", channel=None, classic=True), - ] - - @pytest.mark.parametrize( "additional_snaps", [ From a368c950a3a1602d819c24ccbbe24ae4a915c4a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 08:50:53 +0000 Subject: [PATCH 08/14] Add spread test for base snap injection Add a spread test that verifies the base snap is injected into managed instances with the correct revision matching the host system. Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- .../testcraft/inject-base-snap/task.yaml | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 tests/spread/testcraft/inject-base-snap/task.yaml diff --git a/tests/spread/testcraft/inject-base-snap/task.yaml b/tests/spread/testcraft/inject-base-snap/task.yaml new file mode 100644 index 000000000..826357740 --- /dev/null +++ b/tests/spread/testcraft/inject-base-snap/task.yaml @@ -0,0 +1,96 @@ +summary: Verify base snap is injected with correct revision in managed mode + +priority: 100 + +systems: + - ubuntu-22.04-64 # Test on 22.04 with a core24 snap + - ubuntu-24.04-64 + +prepare: | + # Install testcraft snap (we need to be running from snap for injection) + snap install testcraft --edge --classic || snap install testcraft --beta --classic || snap install testcraft --candidate --classic + + # Get the base snap name and revision from the testcraft snap + HOST_BASE=$(snap info testcraft --verbose | grep '^base:' | awk '{print $2}') + if [ -z "$HOST_BASE" ]; then + echo "Could not determine base snap from testcraft snap" + exit 1 + fi + + # Ensure the base snap is installed on the host + snap install "$HOST_BASE" || true + + # Get the revision of the base snap on the host + HOST_BASE_REVISION=$(snap list "$HOST_BASE" | tail -n 1 | awk '{print $3}') + echo "Host base snap: $HOST_BASE (revision $HOST_BASE_REVISION)" + + # Save for later use + echo "$HOST_BASE" > /tmp/base_snap_name + echo "$HOST_BASE_REVISION" > /tmp/base_snap_revision + +execute: | + mkdir inject-test + cd inject-test + + # Initialize a project + testcraft init + . /etc/os-release + sed -i "s/base: ubuntu@24.04/base: ubuntu@${VERSION_ID}/" testcraft.yaml + + # Set a short idle time for faster test + export CRAFT_IDLE_MINS=1 + + # Run testcraft pack in managed mode - this should inject the base snap + testcraft pack --verbose --debug + + # Get the base snap info + HOST_BASE=$(cat /tmp/base_snap_name) + HOST_BASE_REVISION=$(cat /tmp/base_snap_revision) + + # Connect to the managed instance and check if the base snap is present + # The instance name follows the pattern: testcraft-inject-test-- + ARCH=$(dpkg --print-architecture) + + # Find the LXD instance (testcraft uses LXD on Linux) + INSTANCE=$(lxc list --format csv -c n | grep "^testcraft-" | head -n 1) + + if [ -z "$INSTANCE" ]; then + echo "Could not find managed instance" + echo "Available instances:" + lxc list + exit 1 + fi + + echo "Found instance: $INSTANCE" + + # Check if the base snap is installed in the instance + INSTANCE_BASE_REVISION=$(lxc exec "$INSTANCE" -- snap list "$HOST_BASE" 2>/dev/null | tail -n 1 | awk '{print $3}' || echo "") + + if [ -z "$INSTANCE_BASE_REVISION" ]; then + echo "ERROR: Base snap '$HOST_BASE' is not installed in the managed instance" + echo "Installed snaps in instance:" + lxc exec "$INSTANCE" -- snap list + exit 1 + fi + + echo "Instance base snap: $HOST_BASE (revision $INSTANCE_BASE_REVISION)" + + # Verify the revisions match + if [ "$HOST_BASE_REVISION" != "$INSTANCE_BASE_REVISION" ]; then + echo "ERROR: Base snap revision mismatch!" + echo " Host revision: $HOST_BASE_REVISION" + echo " Instance revision: $INSTANCE_BASE_REVISION" + exit 1 + fi + + echo "SUCCESS: Base snap '$HOST_BASE' revision $HOST_BASE_REVISION matches on host and instance" + +restore: | + cd inject-test 2>/dev/null || true + testcraft clean || true + cd .. + rm -rf inject-test + rm -f /tmp/base_snap_name /tmp/base_snap_revision + + # Clean up testcraft snap + snap remove --purge testcraft || true From fc5f1965d8737d9e3453b91695fd97cf0f03619a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 10:16:52 +0000 Subject: [PATCH 09/14] Fix spread test to build testcraft snap locally The test now builds the testcraft snap from the repository using snapcraft instead of trying to install it from the snap store (where it doesn't exist). Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- .../spread/testcraft/inject-base-snap/task.yaml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/spread/testcraft/inject-base-snap/task.yaml b/tests/spread/testcraft/inject-base-snap/task.yaml index 826357740..5becfafc5 100644 --- a/tests/spread/testcraft/inject-base-snap/task.yaml +++ b/tests/spread/testcraft/inject-base-snap/task.yaml @@ -7,10 +7,15 @@ systems: - ubuntu-24.04-64 prepare: | - # Install testcraft snap (we need to be running from snap for injection) - snap install testcraft --edge --classic || snap install testcraft --beta --classic || snap install testcraft --candidate --classic + # Build the testcraft snap from the repository + cd "$PROJECT_PATH" + snap install snapcraft --classic + snapcraft pack - # Get the base snap name and revision from the testcraft snap + # Install the locally built testcraft snap + snap install --dangerous --classic testcraft_*.snap + + # Get the base snap name from the testcraft snap HOST_BASE=$(snap info testcraft --verbose | grep '^base:' | awk '{print $2}') if [ -z "$HOST_BASE" ]; then echo "Could not determine base snap from testcraft snap" @@ -92,5 +97,8 @@ restore: | rm -rf inject-test rm -f /tmp/base_snap_name /tmp/base_snap_revision - # Clean up testcraft snap + # Clean up testcraft snap and built snap file snap remove --purge testcraft || true + cd "$PROJECT_PATH" + rm -f testcraft_*.snap + snap remove --purge snapcraft || true From 0645e0c6b72b4008333f9c7dcd3ecde54836aedb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 23:48:19 +0000 Subject: [PATCH 10/14] Address code review feedback - Use walrus operator in provider.py for base_snap assignment - Remove import from private snaphelpers module, use direct file reading instead - Parametrize test_get_snap_base_success with real snap.yaml examples (core24, core22, core20, complex snap) - Parametrize test_get_snap_base_returns_none combining similar test cases Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- craft_application/services/provider.py | 3 +- craft_application/util/snap_config.py | 24 ++++-- tests/unit/util/test_snap_config.py | 109 ++++++++++++++++++------- 3 files changed, 97 insertions(+), 39 deletions(-) diff --git a/craft_application/services/provider.py b/craft_application/services/provider.py index 98bdae5b1..c34b977b2 100644 --- a/craft_application/services/provider.py +++ b/craft_application/services/provider.py @@ -130,8 +130,7 @@ def setup(self) -> None: ) # Inject the base snap from the host if available - base_snap = util.get_snap_base(self._app.name) - if base_snap: + if base_snap := util.get_snap_base(self._app.name): emit.debug( f"Setting {base_snap} to be injected from the " "host into the build environment." diff --git a/craft_application/util/snap_config.py b/craft_application/util/snap_config.py index 5c9b20174..f753d3554 100644 --- a/craft_application/util/snap_config.py +++ b/craft_application/util/snap_config.py @@ -17,13 +17,13 @@ """Snap config file definitions and helpers.""" import os +import pathlib from typing import Any, Literal import pydantic import yaml from craft_cli import emit from snaphelpers import SnapConfigOptions, SnapCtlError -from snaphelpers._meta import SnapMetadataFiles from craft_application.errors import CraftValidationError @@ -56,18 +56,24 @@ def get_snap_base(app_name: str) -> str | None: ) return None + snap_dir = os.getenv("SNAP") + if not snap_dir: + emit.debug("SNAP environment variable not set.") + return None + + snap_yaml_path = pathlib.Path(snap_dir) / "meta" / "snap.yaml" + if not snap_yaml_path.exists(): + emit.debug(f"snap.yaml not found at {snap_yaml_path}") + return None + try: - metadata_files = SnapMetadataFiles() - snap_yaml = metadata_files.snap - if not snap_yaml.exists(): - emit.debug("snap.yaml not found") - return None - - base: str | None = snap_yaml.get("base") - except (OSError, KeyError, yaml.YAMLError) as error: + with snap_yaml_path.open() as f: + snap_data = yaml.safe_load(f) + except (OSError, yaml.YAMLError) as error: emit.debug(f"Failed to read snap metadata: {error!r}") return None + base: str | None = snap_data.get("base") if base: emit.debug(f"Found base snap: {base}") return base diff --git a/tests/unit/util/test_snap_config.py b/tests/unit/util/test_snap_config.py index 013b3f8be..edd26e64b 100644 --- a/tests/unit/util/test_snap_config.py +++ b/tests/unit/util/test_snap_config.py @@ -158,8 +158,45 @@ def test_get_snap_config_handle_fetch_error(error, mock_config): assert get_snap_config(app_name="testcraft") is None -def test_get_snap_base_success(tmp_path, monkeypatch): - """Test get_snap_base returns the base when snap.yaml exists.""" +@pytest.mark.parametrize( + ("snap_yaml_content", "expected_base"), + [ + pytest.param( + # Simple snap with core24 + "name: testcraft\nbase: core24\nversion: 1.0\n", + "core24", + id="core24", + ), + pytest.param( + # Snap with core22 (common in 22.04 snaps) + "name: my-snap\nbase: core22\nversion: 2.0\nsummary: A test snap\n", + "core22", + id="core22", + ), + pytest.param( + # Snap with core20 + "name: hello-world\nversion: 6.4\nsummary: Hello world example\nbase: core20\n", + "core20", + id="core20", + ), + pytest.param( + # More complex snap.yaml similar to real snaps like snapcraft + """name: test-app +version: '8.0' +summary: Test application +description: | + A test application for craft-application +base: core24 +grade: stable +confinement: classic +""", + "core24", + id="complex-snap", + ), + ], +) +def test_get_snap_base_success(tmp_path, monkeypatch, snap_yaml_content, expected_base): + """Test get_snap_base returns the correct base from various snap.yaml formats.""" # Set up environment monkeypatch.setenv("SNAP_NAME", "testcraft") monkeypatch.setenv("SNAP", str(tmp_path)) @@ -168,9 +205,9 @@ def test_get_snap_base_success(tmp_path, monkeypatch): meta_dir = tmp_path / "meta" meta_dir.mkdir() snap_yaml = meta_dir / "snap.yaml" - snap_yaml.write_text("name: testcraft\nbase: core24\nversion: 1.0\n") + snap_yaml.write_text(snap_yaml_content) - assert get_snap_base("testcraft") == "core24" + assert get_snap_base("testcraft") == expected_base def test_get_snap_base_not_from_snap(monkeypatch): @@ -181,34 +218,50 @@ def test_get_snap_base_not_from_snap(monkeypatch): assert get_snap_base("testcraft") is None -def test_get_snap_base_no_snap_env(monkeypatch): - """Test get_snap_base returns None when SNAP env var is not set.""" - monkeypatch.setenv("SNAP_NAME", "testcraft") - monkeypatch.delenv("SNAP", raising=False) - - assert get_snap_base("testcraft") is None - - -def test_get_snap_base_no_yaml(tmp_path, monkeypatch): - """Test get_snap_base returns None when snap.yaml doesn't exist.""" +@pytest.mark.parametrize( + ("setup_snap_env", "create_yaml", "yaml_content", "reason"), + [ + pytest.param( + False, + False, + None, + "SNAP env var not set", + id="no-snap-env", + ), + pytest.param( + True, + False, + None, + "snap.yaml doesn't exist", + id="no-yaml", + ), + pytest.param( + True, + True, + "name: testcraft\nversion: 1.0\n", + "no base in snap.yaml", + id="no-base", + ), + ], +) +def test_get_snap_base_returns_none( + tmp_path, monkeypatch, setup_snap_env, create_yaml, yaml_content, reason +): + """Test get_snap_base returns None in various scenarios.""" monkeypatch.setenv("SNAP_NAME", "testcraft") - monkeypatch.setenv("SNAP", str(tmp_path)) - - assert get_snap_base("testcraft") is None + if setup_snap_env: + monkeypatch.setenv("SNAP", str(tmp_path)) + else: + monkeypatch.delenv("SNAP", raising=False) -def test_get_snap_base_no_base_in_yaml(tmp_path, monkeypatch): - """Test get_snap_base returns None when snap.yaml has no base.""" - monkeypatch.setenv("SNAP_NAME", "testcraft") - monkeypatch.setenv("SNAP", str(tmp_path)) - - # Create snap.yaml without base - meta_dir = tmp_path / "meta" - meta_dir.mkdir() - snap_yaml = meta_dir / "snap.yaml" - snap_yaml.write_text("name: testcraft\nversion: 1.0\n") + if create_yaml: + meta_dir = tmp_path / "meta" + meta_dir.mkdir() + snap_yaml = meta_dir / "snap.yaml" + snap_yaml.write_text(yaml_content) - assert get_snap_base("testcraft") is None + assert get_snap_base("testcraft") is None, f"Should return None when {reason}" def test_get_snap_base_invalid_yaml(tmp_path, monkeypatch): From c40ab1f24af7a04f39b7d1ba6bd95a73a2ff8882 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 03:31:25 +0000 Subject: [PATCH 11/14] Add changelog entry for base snap injection Added entry to docs/reference/changelog.rst documenting that base snaps are now injected into managed instances when running from a snap. Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- docs/reference/changelog.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst index 6f5fea08e..2ba5ad83d 100644 --- a/docs/reference/changelog.rst +++ b/docs/reference/changelog.rst @@ -15,6 +15,17 @@ Changelog For a complete list of commits, check out the `1.2.3`_ release on GitHub. +Unreleased +---------- + +Services +======== + +- When running from a snap, the application's base snap is now injected into + managed instances alongside the application snap. This prevents the managed + instance from downloading the base snap from the network when it's already + available on the host. + 6.0.1 (2025-11-19) ------------------ From f756850fc89ef7b62b5f882e4ad5a5286c4e57f9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 03:43:27 +0000 Subject: [PATCH 12/14] Fix base snap injection order and formatting - Move base snap injection before app snap to ensure correct dependency order - Fix extra space in sed command in spread test Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- craft_application/services/provider.py | 16 ++++++++-------- .../spread/testcraft/inject-base-snap/task.yaml | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/craft_application/services/provider.py b/craft_application/services/provider.py index c3d562817..3ab3e7bc3 100644 --- a/craft_application/services/provider.py +++ b/craft_application/services/provider.py @@ -120,6 +120,14 @@ def setup(self) -> None: self.snaps.extend(_REQUESTED_SNAPS.values()) if util.is_running_from_snap(self._app.name): + # Inject the base snap from the host if available + if base_snap := util.get_snap_base(self._app.name): + emit.debug( + f"Setting {base_snap} to be injected from the " + "host into the build environment." + ) + self.snaps.append(Snap(name=base_snap, channel=None, classic=False)) + # use the aliased name of the snap when injecting name = os.getenv("SNAP_INSTANCE_NAME", self._app.name) channel = None @@ -128,14 +136,6 @@ def setup(self) -> None: "host into the build environment because it is running " "as a snap." ) - - # Inject the base snap from the host if available - if base_snap := util.get_snap_base(self._app.name): - emit.debug( - f"Setting {base_snap} to be injected from the " - "host into the build environment." - ) - self.snaps.append(Snap(name=base_snap, channel=None, classic=False)) else: # use the snap name when installing from the store name = self._app.name diff --git a/tests/spread/testcraft/inject-base-snap/task.yaml b/tests/spread/testcraft/inject-base-snap/task.yaml index 5becfafc5..8a031fdb4 100644 --- a/tests/spread/testcraft/inject-base-snap/task.yaml +++ b/tests/spread/testcraft/inject-base-snap/task.yaml @@ -40,7 +40,7 @@ execute: | # Initialize a project testcraft init . /etc/os-release - sed -i "s/base: ubuntu@24.04/base: ubuntu@${VERSION_ID}/" testcraft.yaml + sed -i "s/base: ubuntu@24.04/base: ubuntu@${VERSION_ID}/" testcraft.yaml # Set a short idle time for faster test export CRAFT_IDLE_MINS=1 From e759892b23bef653316ba51f6cf3e67ac76b682b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 08:03:00 +0000 Subject: [PATCH 13/14] Simplify spread test and improve code clarity - Remove snapcraft build steps from spread test (testcraft is already available) - Improve comment clarity in provider.py to distinguish base snap and app snap injection - Make sed pattern more flexible to handle different Ubuntu version formats - Simplify test to just verify base snap presence without building from source Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- craft_application/services/provider.py | 2 +- .../testcraft/inject-base-snap/task.yaml | 24 +++---------------- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/craft_application/services/provider.py b/craft_application/services/provider.py index 3ab3e7bc3..2c5aa640f 100644 --- a/craft_application/services/provider.py +++ b/craft_application/services/provider.py @@ -128,7 +128,7 @@ def setup(self) -> None: ) self.snaps.append(Snap(name=base_snap, channel=None, classic=False)) - # use the aliased name of the snap when injecting + # Inject the app snap (use the aliased name when injecting from host) name = os.getenv("SNAP_INSTANCE_NAME", self._app.name) channel = None emit.debug( diff --git a/tests/spread/testcraft/inject-base-snap/task.yaml b/tests/spread/testcraft/inject-base-snap/task.yaml index 8a031fdb4..b751ff780 100644 --- a/tests/spread/testcraft/inject-base-snap/task.yaml +++ b/tests/spread/testcraft/inject-base-snap/task.yaml @@ -7,20 +7,8 @@ systems: - ubuntu-24.04-64 prepare: | - # Build the testcraft snap from the repository - cd "$PROJECT_PATH" - snap install snapcraft --classic - snapcraft pack - - # Install the locally built testcraft snap - snap install --dangerous --classic testcraft_*.snap - - # Get the base snap name from the testcraft snap - HOST_BASE=$(snap info testcraft --verbose | grep '^base:' | awk '{print $2}') - if [ -z "$HOST_BASE" ]; then - echo "Could not determine base snap from testcraft snap" - exit 1 - fi + # Determine the base snap for the test (use core24 for testing) + HOST_BASE="core24" # Ensure the base snap is installed on the host snap install "$HOST_BASE" || true @@ -40,7 +28,7 @@ execute: | # Initialize a project testcraft init . /etc/os-release - sed -i "s/base: ubuntu@24.04/base: ubuntu@${VERSION_ID}/" testcraft.yaml + sed -i "s/base: ubuntu@[0-9.]\\+/base: ubuntu@${VERSION_ID}/" testcraft.yaml # Set a short idle time for faster test export CRAFT_IDLE_MINS=1 @@ -96,9 +84,3 @@ restore: | cd .. rm -rf inject-test rm -f /tmp/base_snap_name /tmp/base_snap_revision - - # Clean up testcraft snap and built snap file - snap remove --purge testcraft || true - cd "$PROJECT_PATH" - rm -f testcraft_*.snap - snap remove --purge snapcraft || true From ebd660e75a9f9dc143e8e6d79528e368ba12c019 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 08:28:47 +0000 Subject: [PATCH 14/14] Improve spread test reliability - Use edge channel for core24 to ensure different revision from container preinstall - Add --project=testcraft to all lxc commands for proper project isolation - Restore core24 to stable channel during cleanup Co-authored-by: lengau <4305943+lengau@users.noreply.github.com> --- tests/spread/testcraft/inject-base-snap/task.yaml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/spread/testcraft/inject-base-snap/task.yaml b/tests/spread/testcraft/inject-base-snap/task.yaml index b751ff780..c5fe51abd 100644 --- a/tests/spread/testcraft/inject-base-snap/task.yaml +++ b/tests/spread/testcraft/inject-base-snap/task.yaml @@ -10,8 +10,8 @@ prepare: | # Determine the base snap for the test (use core24 for testing) HOST_BASE="core24" - # Ensure the base snap is installed on the host - snap install "$HOST_BASE" || true + # Install from edge channel to ensure different revision from container preinstall + snap install "$HOST_BASE" --channel=edge || true # Get the revision of the base snap on the host HOST_BASE_REVISION=$(snap list "$HOST_BASE" | tail -n 1 | awk '{print $3}') @@ -45,24 +45,24 @@ execute: | ARCH=$(dpkg --print-architecture) # Find the LXD instance (testcraft uses LXD on Linux) - INSTANCE=$(lxc list --format csv -c n | grep "^testcraft-" | head -n 1) + INSTANCE=$(lxc list --project=testcraft --format csv -c n | grep "^testcraft-" | head -n 1) if [ -z "$INSTANCE" ]; then echo "Could not find managed instance" echo "Available instances:" - lxc list + lxc list --project=testcraft exit 1 fi echo "Found instance: $INSTANCE" # Check if the base snap is installed in the instance - INSTANCE_BASE_REVISION=$(lxc exec "$INSTANCE" -- snap list "$HOST_BASE" 2>/dev/null | tail -n 1 | awk '{print $3}' || echo "") + INSTANCE_BASE_REVISION=$(lxc exec --project=testcraft "$INSTANCE" -- snap list "$HOST_BASE" 2>/dev/null | tail -n 1 | awk '{print $3}' || echo "") if [ -z "$INSTANCE_BASE_REVISION" ]; then echo "ERROR: Base snap '$HOST_BASE' is not installed in the managed instance" echo "Installed snaps in instance:" - lxc exec "$INSTANCE" -- snap list + lxc exec --project=testcraft "$INSTANCE" -- snap list exit 1 fi @@ -84,3 +84,6 @@ restore: | cd .. rm -rf inject-test rm -f /tmp/base_snap_name /tmp/base_snap_revision + + # Restore core24 to stable channel + snap refresh core24 --channel=stable || true