From 3ff8255275c447fb0d5970ba9caa8b405779c5d0 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 7 Jul 2026 19:25:55 -0300 Subject: [PATCH 01/23] feat: skip repacking if snap content is not changed Mediate creation of metadata and asset files to allow recreation of artifacts only if prime or post-prime content has changed. Signed-off-by: Claudio Matsuoka --- pyproject.toml | 2 +- snapcraft/services/package.py | 113 +++++++++++++++++- tests/unit/services/test_package.py | 52 ++++++++ .../unit/services/test_package_components.py | 36 ++++++ uv.lock | 14 +-- 5 files changed, 208 insertions(+), 9 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b6f741a29d..1cc86331b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ dynamic = ["version"] dependencies = [ "catkin-pkg==1.1.0; sys_platform == 'linux'", "click>=8.2", - "craft-application[remote]>=6.4.0", + "craft-application[remote]>=7.1.0", "craft-archives>=2.2.0", "craft-cli>=3.4.0", "craft-grammar>=2.3.0", diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index b641bee660..f79ad88218 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -24,6 +24,7 @@ from typing import Literal, cast from craft_application import PackageService +from craft_application.services.package import package_file from craft_application.util import strtobool from craft_cli import emit from typing_extensions import override @@ -43,6 +44,12 @@ class Package(PackageService): """Package service subclass for Snapcraft.""" + @property + @override + def supports_conditional_repack(self) -> bool: + """Disable ST160 pack orchestration until all post-prime writes are mediated.""" + return False + @override def setup(self) -> None: """Application-specific service setup.""" @@ -269,6 +276,98 @@ def _pack_components(self, dest: pathlib.Path) -> dict[str, pathlib.Path]: return component_map + def _get_pack_output(self) -> pathlib.Path: + """Return the configured pack output path.""" + return self.output_dir + + def _get_artifact_output_dir(self) -> pathlib.Path: + """Return the directory where component artifacts will be written.""" + output = self._get_pack_output() + if output and not output.is_dir(): + return output.parent.resolve() + + return output.resolve() + + def _get_default_artifact_path(self) -> pathlib.Path: + """Return the expected output path for the snap artifact.""" + output = self._get_pack_output() + output_dir = self._get_artifact_output_dir() + filename = pack._get_filename( # noqa: SLF001 + str(output) if output else None, + self._project.name, + process_version(self._project.version), + self._platform, + ) + if filename is None: + raise errors.SnapcraftError("Could not determine the snap artifact name.") + + return output_dir / filename + + def _get_component_artifact_path(self, component_name: str) -> pathlib.Path: + """Return the expected output path for a component artifact.""" + if self._project.components is None: + raise errors.SnapcraftError("Project does not contain any components.") + + component = self._project.components[component_name] + version = process_version(component.version or self._project.version) + filename = f"{self._project.name}+{component_name}_{version}.comp" + return self._get_artifact_output_dir() / filename + + @override + def get_artifacts(self) -> dict[str | None, pathlib.Path]: + """Get the expected output artifacts for the current pack operation.""" + artifacts: dict[str | None, pathlib.Path] = {None: self._get_default_artifact_path()} + + for component_name in self._project.get_component_names(): + artifacts[component_name] = self._get_component_artifact_path(component_name) + + return artifacts + + @override + def _pack(self, *, name: str | None = None, path: pathlib.Path) -> None: + """Pack a specific snap or component artifact.""" + if name in (None, "default"): + issues = linters.run_linters( + self._services.lifecycle.prime_dir, lint=self._project.lint + ) + status = linters.report(issues, intermediate=True) + + # In case of linter errors, stop execution and return the error code. + if status in (LinterStatus.ERRORS, LinterStatus.FATAL): + raise errors.LinterError("Linter errors found", exit_code=status) + + pack.pack_snap( + self._services.lifecycle.prime_dir, + output=str(path), + compression=self._project.compression, + name=self._project.name, + version=process_version(self._project.version), + target=self._platform, + ) + return + + if self._project.components is None or name not in self._project.components: + raise errors.SnapcraftError(f"Unknown component artifact {name!r}.") + + component = self._project.components[name] + compression = component.compression or self._project.compression + if component.compression: + emit.debug(f"Using {component.compression!r} compression for {name!r}.") + else: + emit.debug( + f"Using the snap's {self._project.compression!r} compression for {name!r}." + ) + + filename = pack.pack_component( + cast(Lifecycle, self._services.lifecycle).get_prime_dir(name), + compression=compression, + output_dir=path.parent, + ) + if filename != path.name: + raise errors.SnapcraftError( + f"Packed component {name!r} to unexpected file {filename!r}." + ) + @override def pack(self, prime_dir: pathlib.Path, dest: pathlib.Path) -> list[pathlib.Path]: """Create one or more packages as appropriate. @@ -316,6 +415,16 @@ def _get_assets_dir(self) -> pathlib.Path: # This is for backwards compatibility with setup_assets(...) return project_dir / "snap" + @package_file("meta/snap.yaml", partition_re="default") + def _get_snap_yaml(self, partition: str | None = None) -> str: + """Generate the snap.yaml file contents for the default partition.""" + if partition not in (None, "default"): + raise errors.SnapcraftError( + f"Cannot generate snap metadata for partition {partition!r}." + ) + + return self.metadata.to_yaml_string() + @override def write_metadata(self, path: pathlib.Path) -> None: """Write the project metadata to metadata.yaml in the given directory. @@ -324,7 +433,9 @@ def write_metadata(self, path: pathlib.Path) -> None: """ meta_dir = path / "meta" meta_dir.mkdir(parents=True, exist_ok=True) - self.metadata.to_yaml_file(meta_dir / "snap.yaml") + (meta_dir / "snap.yaml").write_text( + self._get_snap_yaml(), encoding="utf-8" + ) enable_manifest = strtobool(os.getenv("SNAPCRAFT_BUILD_INFO", "n")) diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 2cd7a428c8..fbaa3fefea 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -114,6 +114,58 @@ def test_metadata(default_project, fake_services, setup_project): ) +def test_get_snap_yaml(default_project, fake_services, setup_project): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + + assert package_service._get_snap_yaml() == dedent( + """\ + name: default + version: '1.0' + summary: default project + description: default project + license: MIT + architectures: + - amd64 + base: core24 + confinement: devmode + grade: devel + environment: + LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} + PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH + """ + ) + + +def test_get_artifacts(default_project, fake_services, setup_project, tmp_path): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + package_service.set_output_dir(tmp_path / "test-output.snap") + + assert package_service.get_artifacts() == { + None: tmp_path / "test-output.snap" + } + + +def test_pack_artifact_snap(default_project, fake_services, setup_project, mocker, tmp_path): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + mock_pack_snap = mocker.patch.object(pack, "pack_snap") + mocker.patch.object(linters, "run_linters") + mocker.patch.object(linters, "report") + + package_service._pack(name=None, path=tmp_path / "test-output.snap") + + mock_pack_snap.assert_called_once_with( + tmp_path / "prime", + name="default", + version="1.0", + compression="xz", + output=str(tmp_path / "test-output.snap"), + target="amd64", + ) + + def test_write_metadata(default_project, fake_services, setup_project, new_dir): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") diff --git a/tests/unit/services/test_package_components.py b/tests/unit/services/test_package_components.py index eeee8fb0c0..ca1dfc9889 100644 --- a/tests/unit/services/test_package_components.py +++ b/tests/unit/services/test_package_components.py @@ -157,6 +157,42 @@ def test_pack_component_compression( ) +@pytest.mark.usefixtures("enable_partitions_feature") +def test_get_artifacts(default_project, fake_services, setup_project, tmp_path): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + package_service.set_output_dir(tmp_path / "artifacts") + + assert package_service.get_artifacts() == { + None: tmp_path / "artifacts" / "default_1.0_amd64.snap", + "firstcomponent": tmp_path / "artifacts" / "default+firstcomponent_1.0.comp", + "secondcomponent": tmp_path / "artifacts" / "default+secondcomponent_1.0.comp", + } + + +@pytest.mark.usefixtures("enable_partitions_feature") +def test_pack_artifact_component( + default_project, fake_services, setup_project, mocker, tmp_path +): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + lifecycle_service = fake_services.get("lifecycle") + mock_pack_component = mocker.patch.object( + pack, + "pack_component", + return_value="default+firstcomponent_1.0.comp", + ) + + artifact_path = tmp_path / "artifacts" / "default+firstcomponent_1.0.comp" + package_service._pack(name="firstcomponent", path=artifact_path) + + mock_pack_component.assert_called_once_with( + lifecycle_service._work_dir / "partitions/component/firstcomponent/prime", + compression="xz", + output_dir=artifact_path.parent, + ) + + @pytest.mark.usefixtures("enable_partitions_feature") def test_write_metadata( default_project, diff --git a/uv.lock b/uv.lock index 813501c361..47fcf7f51d 100644 --- a/uv.lock +++ b/uv.lock @@ -490,7 +490,7 @@ toml = [ [[package]] name = "craft-application" -version = "6.4.0" +version = "7.1.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "annotated-types" }, @@ -512,9 +512,9 @@ dependencies = [ { name = "snap-http" }, { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/31/88/1a31a8085bf01ca5f00aeda88a5f5e4cff314fd4e56ee2a629f2c1d95ca3/craft_application-6.4.0.tar.gz", hash = "sha256:02b6afaaad0462f752c1c4715b6da4d7b7ee57bcd71b4ca38234cb18220a4354", size = 627605, upload-time = "2026-04-23T18:57:57.606Z" } +sdist = { url = "https://files.pythonhosted.org/packages/6b/c3/be96c01c1b92a7a7eb6cafa881ccd09a6b9e1db1355835fc98f001680cc3/craft_application-7.1.0.tar.gz", hash = "sha256:e0a3fb2080bf1dd6daf8b570262716f79a5f307a87d3ed2b986eaca50e07efed", size = 663202, upload-time = "2026-07-07T22:02:40.491Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/88/21/38bf81b8327990d9b8a174abeddaf06f36434bfc924aaa275079627babf1/craft_application-6.4.0-py3-none-any.whl", hash = "sha256:ee587eaeb9570628a548189f81813f9517951ce367e87eb56ff01034f8dd59e9", size = 211843, upload-time = "2026-04-23T18:57:55.388Z" }, + { url = "https://files.pythonhosted.org/packages/eb/ce/169bdacdfbda0fe8a880d914be0794fc5379b7ec577546444f1572f4762d/craft_application-7.1.0-py3-none-any.whl", hash = "sha256:3319310644d1d935fd7e2fb285fa38bc87c490f3d4fb72fed934e6202164b7bc", size = 212571, upload-time = "2026-07-07T22:02:38.317Z" }, ] [package.optional-dependencies] @@ -605,7 +605,7 @@ wheels = [ [[package]] name = "craft-providers" -version = "3.6.0" +version = "3.7.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "packaging" }, @@ -615,9 +615,9 @@ dependencies = [ { name = "requests" }, { name = "requests-unixsocket2" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/47/bc/db06baf74ff9538282eb265f99b0147f3b8b775da583a9c6aa88a9d59227/craft_providers-3.6.0.tar.gz", hash = "sha256:dfffebb4a9f09f763b293fe396f9f4f17d917e18167d98b61f76cffd55556250", size = 325499, upload-time = "2026-04-30T17:43:56.728Z" } +sdist = { url = "https://files.pythonhosted.org/packages/86/f5/81525f63af39d73be1fae45fc169eb036781d745a0220f95d9b7f6165549/craft_providers-3.7.1.tar.gz", hash = "sha256:473e6228720dda9f042eae8b9584a1f1c41d6fd987c0fd9db0d69609d3c5f283", size = 380268, upload-time = "2026-07-02T14:03:49.44Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/ce/88/a8cebf86e8fda0c542e70c40374d3edc0bf4cf92ba55151e3a4f60695253/craft_providers-3.6.0-py3-none-any.whl", hash = "sha256:3b3ee87e535f1a29555012ba8d050ae6ca9afbc91fc2f273656d57052a307854", size = 121782, upload-time = "2026-04-30T17:43:54.567Z" }, + { url = "https://files.pythonhosted.org/packages/72/ba/c86bb1f2b356299f2971e613071bd30eef39719d372fb1e3f116b078d1ca/craft_providers-3.7.1-py3-none-any.whl", hash = "sha256:594efc39b84af865f4b3733b6658ec9e34ff6ff2bb98a1ed3785fe82d4afdc83", size = 120989, upload-time = "2026-07-02T14:03:47.659Z" }, ] [[package]] @@ -2518,7 +2518,7 @@ types = [ requires-dist = [ { name = "catkin-pkg", marker = "sys_platform == 'linux'", specifier = "==1.1.0" }, { name = "click", specifier = ">=8.2" }, - { name = "craft-application", extras = ["remote"], specifier = ">=6.4.0" }, + { name = "craft-application", extras = ["remote"], specifier = ">=7.1.0" }, { name = "craft-archives", specifier = ">=2.2.0" }, { name = "craft-cli", specifier = ">=3.4.0" }, { name = "craft-grammar", specifier = ">=2.3.0" }, From e885eb10464d2356eb001464ea4f018f709503bb Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 7 Jul 2026 19:58:36 -0300 Subject: [PATCH 02/23] feat: mediate manifest.yaml Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 70 ++++++++++++++++++++++++++--- tests/unit/services/test_package.py | 60 +++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 5 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index f79ad88218..03836c9364 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -23,8 +23,9 @@ import shutil from typing import Literal, cast +import yaml from craft_application import PackageService -from craft_application.services.package import package_file +from craft_application.services.package import PackageFileEntry, package_file from craft_application.util import strtobool from craft_cli import emit from typing_extensions import override @@ -425,6 +426,60 @@ def _get_snap_yaml(self, partition: str | None = None) -> str: return self.metadata.to_yaml_string() + @package_file("snap/manifest.yaml", partition_re="default") + def _get_manifest_yaml(self, partition: str | None = None) -> str | None: + """Generate the manifest.yaml file contents for the default partition.""" + if partition not in (None, "default"): + raise errors.SnapcraftError( + f"Cannot generate snap manifest for partition {partition!r}." + ) + + if not strtobool(os.getenv("SNAPCRAFT_BUILD_INFO", "n")): + return None + + lifecycle_service = cast(Lifecycle, self._services.lifecycle) + return lifecycle_service.generate_manifest().to_yaml_string() + + @override + def _package_file_changed( + self, package_file: PackageFileEntry, partition_name: str | None + ) -> bool: + """Return whether a generated package file differs from prime contents.""" + if package_file.relative_path == pathlib.PurePosixPath("snap/manifest.yaml"): + # craft-application normally compares package files byte-for-byte, but + # manifest.yaml embeds snapcraft-started-at, which changes on every run. + return self._manifest_changed(partition_name) + + return super()._package_file_changed(package_file, partition_name) + + def _manifest_changed(self, partition_name: str | None) -> bool: + """Return whether the mediated manifest differs from the primed one. + + Ignores ``snapcraft-started-at`` because it changes on every Snapcraft + invocation. Other fields that can drift with the build environment + (``snapcraft-version``, OS release IDs, ``image-info``, build/stage + package lists) are intentionally considered meaningful content changes + that should trigger a repack. + """ + content = self._get_manifest_yaml(partition_name) + destination = self._prime_dir_for(partition_name) / "snap" / "manifest.yaml" + + if content is None: + return destination.exists() + + if not destination.is_file(): + return True + + existing = yaml.safe_load(destination.read_text(encoding="utf-8")) + generated = yaml.safe_load(content) + + if isinstance(existing, dict): + existing.pop("snapcraft-started-at", None) + if isinstance(generated, dict): + generated.pop("snapcraft-started-at", None) + + return existing != generated + @override def write_metadata(self, path: pathlib.Path) -> None: """Write the project metadata to metadata.yaml in the given directory. @@ -441,12 +496,17 @@ def write_metadata(self, path: pathlib.Path) -> None: # Snapcraft's Lifecycle implementation is what we need to refer to for typing lifecycle_service = cast(Lifecycle, self._services.lifecycle) - if enable_manifest: - snap_dir = path / "snap" + snap_dir = path / "snap" + manifest_path = snap_dir / "manifest.yaml" + manifest_contents = self._get_manifest_yaml() + if manifest_contents is not None: snap_dir.mkdir(parents=True, exist_ok=True) - manifest = lifecycle_service.generate_manifest() - manifest.to_yaml_file(snap_dir / "manifest.yaml") + manifest_path.write_text(manifest_contents, encoding="utf-8") + else: + manifest_path.unlink(missing_ok=True) + if enable_manifest: + snap_dir.mkdir(parents=True, exist_ok=True) project_file = self._services.get("project").resolve_project_file_path() shutil.copy(project_file, snap_dir) diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index fbaa3fefea..42196a4716 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -166,6 +166,50 @@ def test_pack_artifact_snap(default_project, fake_services, setup_project, mocke ) +def test_get_manifest_yaml_disabled( + monkeypatch, default_project, fake_services, setup_project +): + monkeypatch.delenv("SNAPCRAFT_BUILD_INFO", raising=False) + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + + assert package_service._get_manifest_yaml() is None + + +def test_get_manifest_yaml_enabled( + monkeypatch, default_project, fake_services, setup_project +): + monkeypatch.setenv("SNAPCRAFT_BUILD_INFO", "1") + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + + manifest = yaml.safe_load(cast("str", package_service._get_manifest_yaml())) + + assert manifest["name"] == "default" + assert manifest["version"] == "1.0" + assert manifest["architectures"] == ["amd64"] + assert manifest["grade"] == "devel" + + +def test_manifest_changed_ignores_started_at( + monkeypatch, default_project, fake_services, setup_project, tmp_path +): + monkeypatch.setenv("SNAPCRAFT_BUILD_INFO", "1") + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + prime_dir = tmp_path / "prime" + snap_dir = prime_dir / "snap" + snap_dir.mkdir(parents=True) + + manifest = yaml.safe_load(cast("str", package_service._get_manifest_yaml())) + manifest["snapcraft-started-at"] = "2001-02-03T04:05:06Z" + (snap_dir / "manifest.yaml").write_text( + yaml.safe_dump(manifest), encoding="utf-8" + ) + + assert package_service._manifest_changed(None) is False + + def test_write_metadata(default_project, fake_services, setup_project, new_dir): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") @@ -196,6 +240,22 @@ def test_write_metadata(default_project, fake_services, setup_project, new_dir): assert not (prime_dir / "snap" / "manifest.yaml").exists() +def test_write_metadata_removes_manifest_when_disabled( + monkeypatch, default_project, fake_services, setup_project, tmp_path +): + monkeypatch.delenv("SNAPCRAFT_BUILD_INFO", raising=False) + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + + manifest_path = tmp_path / "prime" / "snap" / "manifest.yaml" + manifest_path.parent.mkdir(parents=True) + manifest_path.write_text("stale manifest", encoding="utf-8") + + package_service.write_metadata(tmp_path / "prime") + + assert not manifest_path.exists() + + def test_write_metadata_with_manifest( monkeypatch, default_project, fake_services, setup_project, tmp_path ): From 0256d4f67245c5228a4255c57c9036850d960ad3 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 7 Jul 2026 20:10:56 -0300 Subject: [PATCH 03/23] feat: mediate component.yaml and project copy Signed-off-by: Claudio Matsuoka --- snapcraft/meta/component_yaml.py | 31 +++++---- snapcraft/services/package.py | 57 +++++++++++++--- tests/unit/services/test_package.py | 67 +++++++++++++++++++ .../unit/services/test_package_components.py | 27 ++++++++ 4 files changed, 159 insertions(+), 23 deletions(-) diff --git a/snapcraft/meta/component_yaml.py b/snapcraft/meta/component_yaml.py index 1e278294bf..6eeee50bef 100644 --- a/snapcraft/meta/component_yaml.py +++ b/snapcraft/meta/component_yaml.py @@ -39,18 +39,8 @@ class ComponentMetadata(SnapcraftMetadata): provenance: str | None = None -def write( - project: models.Project, component_name: str, component_prime_dir: Path -) -> None: - """Create a component.yaml file. - - :param project: The snapcraft project. - :param component_name: Name of the component. - :param component_prime_dir: The directory containing the component's primed contents. - """ - meta_dir = component_prime_dir / "meta" - meta_dir.mkdir(parents=True, exist_ok=True) - +def get_metadata(project: models.Project, component_name: str) -> ComponentMetadata: + """Create the component metadata model for a project component.""" if not project.components: raise SnapcraftError("Project does not contain any components.") @@ -59,7 +49,7 @@ def write( if not component: raise SnapcraftError("Component does not exist.") - component_metadata = ComponentMetadata( + return ComponentMetadata( component=f"{project.name}+{component_name}", type=component.type, version=component.version, @@ -68,4 +58,17 @@ def write( provenance=project.provenance, ) - component_metadata.to_yaml_file(meta_dir / "component.yaml") + +def write( + project: models.Project, component_name: str, component_prime_dir: Path +) -> None: + """Create a component.yaml file. + + :param project: The snapcraft project. + :param component_name: Name of the component. + :param component_prime_dir: The directory containing the component's primed contents. + """ + meta_dir = component_prime_dir / "meta" + meta_dir.mkdir(parents=True, exist_ok=True) + + get_metadata(project, component_name).to_yaml_file(meta_dir / "component.yaml") diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index 03836c9364..301feea0f7 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -440,6 +440,42 @@ def _get_manifest_yaml(self, partition: str | None = None) -> str | None: lifecycle_service = cast(Lifecycle, self._services.lifecycle) return lifecycle_service.generate_manifest().to_yaml_string() + @package_file("meta/component.yaml", partition_re=r"component/.+") + def _get_component_yaml(self, partition: str | None = None) -> str: + """Generate component.yaml contents for a component partition.""" + if partition is None or not partition.startswith("component/"): + raise errors.SnapcraftError( + f"Cannot generate component metadata for partition {partition!r}." + ) + + component_name = partition.split("/", 1)[1] + return component_yaml.get_metadata(self._project, component_name).to_yaml_string() + + @override + def _gen_extra_assets( + self, partition_name: str | None = None + ) -> list[tuple[str | bytes | None | pathlib.Path, pathlib.Path]]: + """Generate mediated post-prime assets for a partition. + + A ``(None, destination)`` entry is returned when the asset should not + exist in prime; craft-application will delete any stale copy at that + destination. + """ + if partition_name not in (None, "default"): + return [] + + project_file = self._services.get("project").resolve_project_file_path() + destination = self._prime_dir_for(partition_name) / "snap" / project_file.name + source: pathlib.Path | None = ( + project_file if self._project_file_copy_enabled() else None + ) + return [(source, destination)] + + @staticmethod + def _project_file_copy_enabled() -> bool: + """Return whether the project file should be copied into snap/.""" + return bool(strtobool(os.getenv("SNAPCRAFT_BUILD_INFO", "n"))) + @override def _package_file_changed( self, package_file: PackageFileEntry, partition_name: str | None @@ -492,8 +528,6 @@ def write_metadata(self, path: pathlib.Path) -> None: self._get_snap_yaml(), encoding="utf-8" ) - enable_manifest = strtobool(os.getenv("SNAPCRAFT_BUILD_INFO", "n")) - # Snapcraft's Lifecycle implementation is what we need to refer to for typing lifecycle_service = cast(Lifecycle, self._services.lifecycle) snap_dir = path / "snap" @@ -505,10 +539,14 @@ def write_metadata(self, path: pathlib.Path) -> None: else: manifest_path.unlink(missing_ok=True) - if enable_manifest: + project_file_path = self._services.get("project").resolve_project_file_path() + source = project_file_path if self._project_file_copy_enabled() else None + destination = snap_dir / project_file_path.name + if source is not None: snap_dir.mkdir(parents=True, exist_ok=True) - project_file = self._services.get("project").resolve_project_file_path() - shutil.copy(project_file, snap_dir) + shutil.copy(source, destination) + else: + destination.unlink(missing_ok=True) assets_dir = self._get_assets_dir() setup_assets( @@ -520,10 +558,11 @@ def write_metadata(self, path: pathlib.Path) -> None: ) for component in self._project.get_component_names(): - component_yaml.write( - project=self._project, - component_name=component, - component_prime_dir=lifecycle_service.get_prime_dir(component), + component_prime_dir = lifecycle_service.get_prime_dir(component) + component_meta_dir = component_prime_dir / "meta" + component_meta_dir.mkdir(parents=True, exist_ok=True) + (component_meta_dir / "component.yaml").write_text( + self._get_component_yaml(f"component/{component}"), encoding="utf-8" ) @property diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 42196a4716..cf036a2bf7 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -191,6 +191,54 @@ def test_get_manifest_yaml_enabled( assert manifest["grade"] == "devel" +def test_project_file_copy_disabled( + monkeypatch, default_project, fake_services, setup_project +): + monkeypatch.delenv("SNAPCRAFT_BUILD_INFO", raising=False) + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + + assert package_service._project_file_copy_enabled() is False + + +def test_project_file_copy_enabled( + monkeypatch, default_project, fake_services, setup_project +): + monkeypatch.setenv("SNAPCRAFT_BUILD_INFO", "1") + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + + assert package_service._project_file_copy_enabled() is True + + +def test_gen_extra_assets_includes_project_file( + monkeypatch, default_project, fake_services, setup_project, tmp_path +): + monkeypatch.setenv("SNAPCRAFT_BUILD_INFO", "1") + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + + project_file = fake_services.get("project").resolve_project_file_path() + + assert package_service._gen_extra_assets() == [ + (project_file, tmp_path / "prime" / "snap" / project_file.name) + ] + + +def test_gen_extra_assets_removes_project_file_when_disabled( + monkeypatch, default_project, fake_services, setup_project, tmp_path +): + monkeypatch.delenv("SNAPCRAFT_BUILD_INFO", raising=False) + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + + project_file = fake_services.get("project").resolve_project_file_path() + + assert package_service._gen_extra_assets() == [ + (None, tmp_path / "prime" / "snap" / project_file.name) + ] + + def test_manifest_changed_ignores_started_at( monkeypatch, default_project, fake_services, setup_project, tmp_path ): @@ -283,6 +331,25 @@ def test_write_metadata_with_manifest( assert manifest.name == snap_yaml["name"] assert manifest.grade == snap_yaml["grade"] assert manifest.architectures == snap_yaml["architectures"] + project_file = fake_services.get("project").resolve_project_file_path() + assert (prime_dir / "snap" / project_file.name).read_text() == project_file.read_text() + + +def test_write_metadata_removes_project_file_when_disabled( + monkeypatch, default_project, fake_services, setup_project, tmp_path +): + monkeypatch.delenv("SNAPCRAFT_BUILD_INFO", raising=False) + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + + project_file = fake_services.get("project").resolve_project_file_path() + copied_project_file = tmp_path / "prime" / "snap" / project_file.name + copied_project_file.parent.mkdir(parents=True) + copied_project_file.write_text("stale project file", encoding="utf-8") + + package_service.write_metadata(tmp_path / "prime") + + assert not copied_project_file.exists() @pytest.fixture(params=["snap", "build-aux/snap"]) diff --git a/tests/unit/services/test_package_components.py b/tests/unit/services/test_package_components.py index ca1dfc9889..deff5757c4 100644 --- a/tests/unit/services/test_package_components.py +++ b/tests/unit/services/test_package_components.py @@ -161,6 +161,7 @@ def test_pack_component_compression( def test_get_artifacts(default_project, fake_services, setup_project, tmp_path): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") + (tmp_path / "artifacts").mkdir() package_service.set_output_dir(tmp_path / "artifacts") assert package_service.get_artifacts() == { @@ -193,6 +194,32 @@ def test_pack_artifact_component( ) +@pytest.mark.usefixtures("enable_partitions_feature") +def test_get_component_yaml(default_project, fake_services, setup_project): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + + assert package_service._get_component_yaml("component/firstcomponent") == dedent( + """\ + component: default+firstcomponent + type: test + version: '1.0' + summary: first component + description: lorem ipsum + """ + ) + + assert package_service._get_component_yaml("component/secondcomponent") == dedent( + """\ + component: default+secondcomponent + type: test + version: '1.0' + summary: second component + description: lorem ipsum + """ + ) + + @pytest.mark.usefixtures("enable_partitions_feature") def test_write_metadata( default_project, From fdfde7f3ef6b97cf61ee00936b1f09e5c7640cc3 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 7 Jul 2026 20:19:01 -0300 Subject: [PATCH 04/23] feat: mediate hook files Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 113 +++++++++++------- tests/unit/services/test_package.py | 61 ++++++++-- .../unit/services/test_package_components.py | 42 +++++-- 3 files changed, 160 insertions(+), 56 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index 301feea0f7..0244302d01 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -314,6 +314,14 @@ def _get_component_artifact_path(self, component_name: str) -> pathlib.Path: filename = f"{self._project.name}+{component_name}_{version}.comp" return self._get_artifact_output_dir() / filename + @override + def _prime_dir_for(self, partition_name: str | None) -> pathlib.Path: + """Return the prime directory for the default snap or a component artifact.""" + if partition_name in (None, "default"): + return self._services.lifecycle.prime_dir + + return cast(Lifecycle, self._services.lifecycle).get_prime_dir(partition_name) + @override def get_artifacts(self) -> dict[str | None, pathlib.Path]: """Get the expected output artifacts for the current pack operation.""" @@ -440,16 +448,15 @@ def _get_manifest_yaml(self, partition: str | None = None) -> str | None: lifecycle_service = cast(Lifecycle, self._services.lifecycle) return lifecycle_service.generate_manifest().to_yaml_string() - @package_file("meta/component.yaml", partition_re=r"component/.+") + @package_file("meta/component.yaml", partition_re=r"(?!default$).+") def _get_component_yaml(self, partition: str | None = None) -> str: """Generate component.yaml contents for a component partition.""" - if partition is None or not partition.startswith("component/"): + if partition is None or partition == "default": raise errors.SnapcraftError( f"Cannot generate component metadata for partition {partition!r}." ) - component_name = partition.split("/", 1)[1] - return component_yaml.get_metadata(self._project, component_name).to_yaml_string() + return component_yaml.get_metadata(self._project, partition).to_yaml_string() @override def _gen_extra_assets( @@ -461,21 +468,68 @@ def _gen_extra_assets( exist in prime; craft-application will delete any stale copy at that destination. """ - if partition_name not in (None, "default"): - return [] + assets = self._get_hook_assets(partition_name) - project_file = self._services.get("project").resolve_project_file_path() - destination = self._prime_dir_for(partition_name) / "snap" / project_file.name - source: pathlib.Path | None = ( - project_file if self._project_file_copy_enabled() else None - ) - return [(source, destination)] + if partition_name in (None, "default"): + project_file = self._services.get("project").resolve_project_file_path() + destination = self._prime_dir_for(partition_name) / "snap" / project_file.name + source: pathlib.Path | None = ( + project_file if self._project_file_copy_enabled() else None + ) + assets.insert(0, (source, destination)) + + return assets + + def _get_hook_assets( + self, partition_name: str | None = None + ) -> list[tuple[pathlib.Path, pathlib.Path]]: + """Generate hook assets for the default or component partition. + + Project-provided hooks are added after built hooks so they keep their + existing precedence when both target the same meta/hooks path. + """ + prime_dir = self._prime_dir_for(partition_name) + assets_dir = self._get_partition_assets_dir(partition_name) + destination_dir = prime_dir / "meta" / "hooks" + assets: list[tuple[pathlib.Path, pathlib.Path]] = [] + + built_snap_hooks = prime_dir / "snap" / "hooks" + if built_snap_hooks.is_dir(): + for hook in sorted(built_snap_hooks.iterdir()): + assets.append((hook, destination_dir / hook.name)) + + project_hooks_dir = assets_dir / "hooks" + if project_hooks_dir.is_dir(): + for hook in sorted(project_hooks_dir.iterdir()): + assets.append((hook, destination_dir / hook.name)) + + return assets + + def _get_partition_assets_dir(self, partition_name: str | None = None) -> pathlib.Path: + """Return the project assets directory for a default or component partition.""" + assets_dir = self._get_assets_dir() + if partition_name in (None, "default"): + return assets_dir + + return assets_dir / "component" / partition_name @staticmethod def _project_file_copy_enabled() -> bool: """Return whether the project file should be copied into snap/.""" return bool(strtobool(os.getenv("SNAPCRAFT_BUILD_INFO", "n"))) + @override + def _write_asset( + self, source: str | bytes | None | pathlib.Path, destination: pathlib.Path + ) -> None: + """Write a generated package file or extra asset into prime.""" + if isinstance(source, pathlib.Path): + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copy(source, destination) + return + + super()._write_asset(source, destination) + @override def _package_file_changed( self, package_file: PackageFileEntry, partition_name: str | None @@ -539,14 +593,9 @@ def write_metadata(self, path: pathlib.Path) -> None: else: manifest_path.unlink(missing_ok=True) - project_file_path = self._services.get("project").resolve_project_file_path() - source = project_file_path if self._project_file_copy_enabled() else None - destination = snap_dir / project_file_path.name - if source is not None: - snap_dir.mkdir(parents=True, exist_ok=True) - shutil.copy(source, destination) - else: - destination.unlink(missing_ok=True) + self._materialize_extra_assets(None) + for component in self._project.get_component_names(): + self._materialize_extra_assets(component) assets_dir = self._get_assets_dir() setup_assets( @@ -562,7 +611,7 @@ def write_metadata(self, path: pathlib.Path) -> None: component_meta_dir = component_prime_dir / "meta" component_meta_dir.mkdir(parents=True, exist_ok=True) (component_meta_dir / "component.yaml").write_text( - self._get_component_yaml(f"component/{component}"), encoding="utf-8" + self._get_component_yaml(component), encoding="utf-8" ) @property @@ -598,32 +647,12 @@ def _hardlink_or_copy(source: pathlib.Path, destination: pathlib.Path) -> bool: def meta_directory_handler(assets_dir: pathlib.Path, path: pathlib.Path): - """Handle hooks and gui assets from Snapcraft. + """Handle gui assets from Snapcraft. :param assets_dir: directory with project assets. :param path: directory to write assets to. """ meta_dir = path / "meta" - built_snap_hooks = path / "snap" / "hooks" - hooks_project_dir = assets_dir / "hooks" - - hooks_meta_dir = meta_dir / "hooks" - - if built_snap_hooks.is_dir(): - hooks_meta_dir.mkdir(parents=True, exist_ok=True) - for hook in built_snap_hooks.iterdir(): - meta_dir_hook = hooks_meta_dir / hook.name - # Remove to always refresh to the latest - meta_dir_hook.unlink(missing_ok=True) - meta_dir_hook.hardlink_to(hook) - - # Overwrite any built hooks with project level ones - if hooks_project_dir.is_dir(): - hooks_meta_dir.mkdir(parents=True, exist_ok=True) - for hook in hooks_project_dir.iterdir(): - meta_dir_hook = hooks_meta_dir / hook.name - - _hardlink_or_copy(hook, meta_dir_hook) # Write any gui assets gui_project_dir = assets_dir / "gui" diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index cf036a2bf7..2718b8b4ed 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -239,6 +239,52 @@ def test_gen_extra_assets_removes_project_file_when_disabled( ] +@pytest.fixture(params=["snap", "build-aux/snap"]) +def project_hooks_dir(in_project_path, request): + hooks_dir = in_project_path / request.param / "hooks" + hooks_dir.mkdir(parents=True) + yield hooks_dir + + +def test_gen_extra_assets_includes_default_hook_assets( + default_project, fake_services, setup_project, project_hooks_dir, tmp_path +): + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + built_hooks_dir = tmp_path / "prime" / "snap" / "hooks" + built_hooks_dir.mkdir(parents=True) + (built_hooks_dir / "configure").write_text("built_configure", encoding="utf-8") + (project_hooks_dir / "install").write_text("project_install", encoding="utf-8") + + extra_assets = package_service._gen_extra_assets() + + assert (built_hooks_dir / "configure", tmp_path / "prime" / "meta" / "hooks" / "configure") in extra_assets + assert ( + project_hooks_dir / "install", + tmp_path / "prime" / "meta" / "hooks" / "install", + ) in extra_assets + + +def test_gen_extra_assets_project_hooks_override_built_hooks( + default_project, fake_services, setup_project, project_hooks_dir, tmp_path +): + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + built_hooks_dir = tmp_path / "prime" / "snap" / "hooks" + built_hooks_dir.mkdir(parents=True) + built_hook = built_hooks_dir / "configure" + project_hook = project_hooks_dir / "configure" + built_hook.write_text("built_configure", encoding="utf-8") + project_hook.write_text("project_configure", encoding="utf-8") + + extra_assets = package_service._get_hook_assets() + + assert extra_assets == [ + (built_hook, tmp_path / "prime" / "meta" / "hooks" / "configure"), + (project_hook, tmp_path / "prime" / "meta" / "hooks" / "configure"), + ] + + def test_manifest_changed_ignores_started_at( monkeypatch, default_project, fake_services, setup_project, tmp_path ): @@ -352,13 +398,6 @@ def test_write_metadata_removes_project_file_when_disabled( assert not copied_project_file.exists() -@pytest.fixture(params=["snap", "build-aux/snap"]) -def project_hooks_dir(in_project_path, request): - hooks_dir = in_project_path / request.param / "hooks" - hooks_dir.mkdir(parents=True) - yield hooks_dir - - def test_write_metadata_with_project_hooks( default_project, fake_services, setup_project, project_hooks_dir, tmp_path ): @@ -367,6 +406,8 @@ def test_write_metadata_with_project_hooks( # Create some hooks (project_hooks_dir / "configure").write_text("configure_hook") (project_hooks_dir / "install").write_text("install_hook") + (project_hooks_dir / "configure").chmod(0o755) + (project_hooks_dir / "install").chmod(0o755) prime_dir = tmp_path / "prime" meta_dir = prime_dir / "meta" @@ -396,8 +437,10 @@ def test_write_metadata_with_project_hooks( # and not a wrapped hook. assert (meta_dir / "hooks" / "configure").exists() assert (meta_dir / "hooks" / "configure").read_text() == "configure_hook" + assert (meta_dir / "hooks" / "configure").stat().st_mode & 0o111 assert (meta_dir / "hooks" / "install").exists() assert (meta_dir / "hooks" / "install").read_text() == "install_hook" + assert (meta_dir / "hooks" / "install").stat().st_mode & 0o111 def test_write_metadata_with_built_hooks( @@ -411,6 +454,8 @@ def test_write_metadata_with_built_hooks( built_hooks_dir.mkdir(parents=True) (built_hooks_dir / "configure").write_text("configure_hook") (built_hooks_dir / "install").write_text("install_hook") + (built_hooks_dir / "configure").chmod(0o755) + (built_hooks_dir / "install").chmod(0o755) package_service.write_metadata(prime_dir) @@ -438,8 +483,10 @@ def test_write_metadata_with_built_hooks( # and not a wrapped hook. assert (meta_dir / "hooks" / "configure").exists() assert (meta_dir / "hooks" / "configure").read_text() == "configure_hook" + assert (meta_dir / "hooks" / "configure").stat().st_mode & 0o111 assert (meta_dir / "hooks" / "install").exists() assert (meta_dir / "hooks" / "install").read_text() == "install_hook" + assert (meta_dir / "hooks" / "install").stat().st_mode & 0o111 def test_write_metadata_with_project_gui( diff --git a/tests/unit/services/test_package_components.py b/tests/unit/services/test_package_components.py index deff5757c4..9db143f926 100644 --- a/tests/unit/services/test_package_components.py +++ b/tests/unit/services/test_package_components.py @@ -58,8 +58,8 @@ def extra_project_params(extra_project_params): @pytest.fixture(params=["snap", "build-aux/snap"]) -def project_assets_dir(new_dir, request): - assets_dir = new_dir / request.param +def project_assets_dir(in_project_path, request): + assets_dir = in_project_path / request.param assets_dir.mkdir(parents=True) yield assets_dir @@ -199,7 +199,7 @@ def test_get_component_yaml(default_project, fake_services, setup_project): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") - assert package_service._get_component_yaml("component/firstcomponent") == dedent( + assert package_service._get_component_yaml("firstcomponent") == dedent( """\ component: default+firstcomponent type: test @@ -209,7 +209,7 @@ def test_get_component_yaml(default_project, fake_services, setup_project): """ ) - assert package_service._get_component_yaml("component/secondcomponent") == dedent( + assert package_service._get_component_yaml("secondcomponent") == dedent( """\ component: default+secondcomponent type: test @@ -220,6 +220,29 @@ def test_get_component_yaml(default_project, fake_services, setup_project): ) +@pytest.mark.usefixtures("enable_partitions_feature") +def test_get_hook_assets_for_component( + default_project, fake_services, setup_project, project_assets_dir, tmp_path +): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + component_prime_dir = tmp_path / "partitions" / "component" / "firstcomponent" / "prime" + built_hooks_dir = component_prime_dir / "snap" / "hooks" + built_hooks_dir.mkdir(parents=True) + built_hook = built_hooks_dir / "install" + project_hook = project_assets_dir / "component" / "firstcomponent" / "hooks" / "configure" + project_hook.parent.mkdir(parents=True) + built_hook.write_text("built_install", encoding="utf-8") + project_hook.write_text("project_configure", encoding="utf-8") + built_hook.chmod(0o755) + project_hook.chmod(0o755) + + assert package_service._get_hook_assets("firstcomponent") == [ + (built_hook, component_prime_dir / "meta" / "hooks" / "install"), + (project_hook, component_prime_dir / "meta" / "hooks" / "configure"), + ] + + @pytest.mark.usefixtures("enable_partitions_feature") def test_write_metadata( default_project, @@ -246,9 +269,9 @@ def test_write_metadata( # Create some hooks (project_assets_dir / "component/firstcomponent/hooks").mkdir(parents=True) - (project_assets_dir / "component/firstcomponent/hooks/install").write_text( - "install_hook" - ) + component_hook = project_assets_dir / "component/firstcomponent/hooks/install" + component_hook.write_text("install_hook") + component_hook.chmod(0o755) (project_assets_dir / "post-refresh").write_text("post-refresh") prime_dir = tmp_path / "prime" @@ -321,3 +344,8 @@ def test_write_metadata( description: lorem ipsum """ ) + component_meta_hook = ( + lifecycle_service.get_prime_dir("firstcomponent") / "meta" / "hooks" / "install" + ) + assert component_meta_hook.read_text() == "install_hook" + assert component_meta_hook.stat().st_mode & 0o111 From c02b3a9a6b26b5086d1980966cf9831513297318 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Mon, 13 Jul 2026 18:21:08 -0300 Subject: [PATCH 05/23] feat: mediate creation of icon and desktop assets Signed-off-by: Claudio Matsuoka --- snapcraft/application.py | 1 + snapcraft/parts/desktop_file.py | 13 +- snapcraft/parts/setup_assets.py | 34 +- snapcraft/services/package.py | 370 ++++++++++++++---- .../spread/general/skip-repack/snap/icon.svg | 1 + .../general/skip-repack/snap/snapcraft.yaml | 13 + tests/spread/general/skip-repack/task.yaml | 28 ++ tests/unit/parts/test_setup_assets.py | 26 +- tests/unit/services/test_package.py | 324 ++++++++++++++- .../unit/services/test_package_components.py | 79 ++++ 10 files changed, 775 insertions(+), 114 deletions(-) create mode 100644 tests/spread/general/skip-repack/snap/icon.svg create mode 100644 tests/spread/general/skip-repack/snap/snapcraft.yaml create mode 100644 tests/spread/general/skip-repack/task.yaml diff --git a/snapcraft/application.py b/snapcraft/application.py index 0e0f628e50..7041585d1e 100644 --- a/snapcraft/application.py +++ b/snapcraft/application.py @@ -50,6 +50,7 @@ mandatory_adoptable_fields=list(models.MANDATORY_ADOPTABLE_FIELDS), docs_url="https://documentation.ubuntu.com/snapcraft/{version}", enable_pro_support=True, + always_repack=False, ) diff --git a/snapcraft/parts/desktop_file.py b/snapcraft/parts/desktop_file.py index c3f5ec737c..7395688030 100644 --- a/snapcraft/parts/desktop_file.py +++ b/snapcraft/parts/desktop_file.py @@ -19,6 +19,7 @@ from __future__ import annotations import configparser +import io import os import shlex from typing import TYPE_CHECKING @@ -113,14 +114,19 @@ def _parse_and_reformat(self, *, icon_path: str | None = None) -> None: for section in self._parser.sections(): self._parse_and_reformat_section(section=section, icon_path=icon_path) + def render(self, *, icon_path: str | None = None) -> str: + """Return the rewritten desktop file contents.""" + self._parse_and_reformat(icon_path=icon_path) + output = io.StringIO() + self._parser.write(output, space_around_delimiters=False) + return output.getvalue() + def write(self, *, gui_dir: Path, icon_path: str | None = None) -> None: """Write the desktop file. :param gui_dir: The desktop file destination directory. :param icon_path: The icon corresponding to this desktop file. """ - self._parse_and_reformat(icon_path=icon_path) - gui_dir.mkdir(parents=True, exist_ok=True) # Rename the desktop file to match the app name. This will help @@ -131,5 +137,4 @@ def write(self, *, gui_dir: Path, icon_path: str | None = None) -> None: # Unlikely. A desktop file in meta/gui/ already existed for # this app. Let's pretend it wasn't there and overwrite it. target.unlink() - with target.open("w", encoding="utf-8") as target_file: - self._parser.write(target_file, space_around_delimiters=False) + target.write_text(self.render(icon_path=icon_path), encoding="utf-8") diff --git a/snapcraft/parts/setup_assets.py b/snapcraft/parts/setup_assets.py index b5d6429ee1..c0040a31bd 100644 --- a/snapcraft/parts/setup_assets.py +++ b/snapcraft/parts/setup_assets.py @@ -92,7 +92,7 @@ def setup_assets( if project.apps: for app_name, app in project.apps.items(): - _validate_command_chain( + validate_command_chain( app.command_chain, name=f"app {app_name!r}", prime_dir=prime_dir ) @@ -121,11 +121,11 @@ def copy_assets( if meta_directory_handler: meta_directory_handler(assets_dir, prime_dir) else: - _write_snap_directory( + write_snap_directory( assets_dir=assets_dir, prime_dir=prime_dir, meta_dir=prime_dir / "meta" ) # create wrappers for hooks in the snap/hooks directory - _create_hook_wrappers(prime_dir) + create_hook_wrappers(prime_dir) def setup_hooks(hooks: dict[str, models.Hook] | None, prime_dir: Path) -> None: @@ -139,15 +139,15 @@ def setup_hooks(hooks: dict[str, models.Hook] | None, prime_dir: Path) -> None: if hooks: for hook_name, hook in hooks.items(): if hook.command_chain: - _validate_command_chain( + validate_command_chain( hook.command_chain, name=f"hook {hook_name!r}", prime_dir=prime_dir ) - _ensure_hook(hooks_dir / hook_name) + ensure_hook(hooks_dir / hook_name) # Ensure all hooks are executable if hooks_dir.is_dir(): for hook in hooks_dir.iterdir(): - _ensure_hook_executable(hook) + ensure_hook_executable(hook) def _finalize_icon( @@ -162,7 +162,7 @@ def _finalize_icon( # Nothing to do if no icon is configured, search for existing icon. if icon is None: - return _find_icon_file(assets_dir) + return find_icon_file(assets_dir) # Extracted appstream icon paths will either: # (1) point to a file relative to prime @@ -196,21 +196,21 @@ def _finalize_icon( _copy_file(parsed_path, target_icon_path) else: # No icon found, fall back to searching for existing icon. - return _find_icon_file(assets_dir) + return find_icon_file(assets_dir) else: raise RuntimeError(f"Unexpected icon path: {parsed_url!r}") return target_icon_path -def _find_icon_file(assets_dir: Path) -> Path | None: +def find_icon_file(assets_dir: Path) -> Path | None: for icon_path in (assets_dir / "gui/icon.png", assets_dir / "gui/icon.svg"): if icon_path.is_file(): return icon_path return None -def _validate_command_chain( +def validate_command_chain( command_chain: list[str], *, name: str, prime_dir: Path ) -> None: """Verify if each item in the command chain is executable.""" @@ -236,7 +236,7 @@ def _is_executable(path: Path) -> bool: return bool(mode & stat.S_IXUSR or mode & stat.S_IXGRP or mode & stat.S_IXOTH) -def _write_snap_directory(*, assets_dir: Path, prime_dir: Path, meta_dir: Path) -> None: +def write_snap_directory(*, assets_dir: Path, prime_dir: Path, meta_dir: Path) -> None: """Record manifest and copy assets found under the assets directory. These assets have priority over any code generated assets and include: @@ -263,7 +263,7 @@ def _write_snap_directory(*, assets_dir: Path, prime_dir: Path, meta_dir: Path) _copy_file(source, destination, follow_symlinks=True) -def _ensure_hook(hook_path: Path) -> None: +def ensure_hook(hook_path: Path) -> None: """Create a stub for hook_path if it does not exist. A stub for hook_name is generated if a command-chain entry is defined @@ -280,7 +280,7 @@ def _ensure_hook(hook_path: Path) -> None: hook_path.write_text("#!/bin/true\n") -def _ensure_hook_executable(hook_path: Path) -> None: +def ensure_hook_executable(hook_path: Path) -> None: """Ensure hook is executable. :param hook_path: file path of the hook @@ -289,7 +289,7 @@ def _ensure_hook_executable(hook_path: Path) -> None: hook_path.chmod(0o755) -def _create_hook_wrappers(prime_dir: Path) -> None: +def create_hook_wrappers(prime_dir: Path) -> None: """Create wrappers for hooks. Hooks in the snap/hooks/ directory are typically built by parts. @@ -313,11 +313,11 @@ def _create_hook_wrappers(prime_dir: Path) -> None: # create a wrapper for each hook for hook in hooks_in_snap_dir: - _ensure_hook_executable(hook) - _write_hook_wrapper(hook.name, hooks_meta_dir / hook.name) + ensure_hook_executable(hook) + write_hook_wrapper(hook.name, hooks_meta_dir / hook.name) -def _write_hook_wrapper(hook_name: str, wrapper_path: Path) -> None: +def write_hook_wrapper(hook_name: str, wrapper_path: Path) -> None: """Write hook wrapper file. The wrapper is a minimal shell script that calls a hook in $SNAP/snap/hooks/ diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index 0244302d01..7dfa610d99 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -21,8 +21,11 @@ import os import pathlib import shutil +import stat +import urllib.parse from typing import Literal, cast +import requests import yaml from craft_application import PackageService from craft_application.services.package import PackageFileEntry, package_file @@ -30,14 +33,15 @@ from craft_cli import emit from typing_extensions import override -from snapcraft import errors, linters, models, pack +from snapcraft import const, errors, linters, models, pack from snapcraft.errors import SnapcraftPrecreationEscapesPrimeError from snapcraft.linters import LinterStatus from snapcraft.meta import component_yaml, snap_yaml from snapcraft.models import ContentPlug from snapcraft.parts import extract_metadata as extract from snapcraft.parts import update_metadata as update -from snapcraft.parts.setup_assets import setup_assets +from snapcraft.parts.desktop_file import DesktopFile +from snapcraft.parts.setup_assets import find_icon_file, validate_command_chain from snapcraft.services import Lifecycle from snapcraft.utils import process_version @@ -45,11 +49,8 @@ class Package(PackageService): """Package service subclass for Snapcraft.""" - @property - @override - def supports_conditional_repack(self) -> bool: - """Disable ST160 pack orchestration until all post-prime writes are mediated.""" - return False + _REMOTE_FETCH_TIMEOUT = 120 + _HOOK_STUB = "#!/bin/true\n" @override def setup(self) -> None: @@ -78,9 +79,10 @@ def _project(self) -> models.Project: return cast(models.Project, self._project_service.get()) @override - def _extra_project_updates(self) -> None: + def _extra_project_updates(self) -> bool: # Update the project from parse-info data. project_info = self._services.lifecycle.project_info + project_before = self._project.marshal() extracted_metadata = extract.extract_lifecycle_metadata( self._project.adopt_info, self._project_service.get_parse_info(), @@ -93,19 +95,25 @@ def _extra_project_updates(self) -> None: assets_dir=self._get_assets_dir(), prime_dir=project_info.prime_dir, ) + project_changed = self._project.marshal() != project_before + prime_changed = False # precreate content targets and layout sources when using core26+ # and bare base snaps if self._project.get_effective_base() not in ("core22", "core24"): - self._precreate_layout_targets() - self._precreate_plug_targets() + layout_changed = self._precreate_layout_targets() + plug_changed = self._precreate_plug_targets() + prime_changed = layout_changed or plug_changed + + return project_changed or prime_changed - def _precreate_layout_targets(self) -> None: + def _precreate_layout_targets(self) -> bool: """Create layout targets ahead of time for snapd to avoid ENOENT errors.""" if self._project.layout is None: - return + return False emit.debug("Pre-creating layout targets inside of snap") + changed = False for src, layout in self._project.layout.items(): result = self._parse_layout_target(src, layout) @@ -125,6 +133,7 @@ def _precreate_layout_targets(self) -> None: f"Layout target directory {path!r} maps to {str(file)!r} inside of the snap" ) emit.debug(f"Creating {str(file)!r} in the prime directory") + changed = changed or not to_create.exists() to_create.mkdir(0o0755, parents=True, exist_ok=True) case "bind-file": emit.debug( @@ -132,14 +141,18 @@ def _precreate_layout_targets(self) -> None: ) emit.debug(f"Creating {str(file)!r} in the prime directory") to_create.parent.mkdir(0o0755, parents=True, exist_ok=True) + changed = changed or not to_create.exists() to_create.touch(0o0644) - def _precreate_plug_targets(self) -> None: + return changed + + def _precreate_plug_targets(self) -> bool: """Create plug targets ahead of time for snapd to avoid ENOENT errors.""" if self._project.plugs is None: - return + return False emit.debug("Pre-creating plug targets inside of snap") + changed = False plug_targets = [] for name, plug in self._project.plugs.items(): @@ -159,8 +172,11 @@ def _precreate_plug_targets(self) -> None: f"Plug target directory {target!r} maps to {str(file)!r} inside of the snap" ) emit.debug(f"Creating {str(file)!r} in the prime directory") + changed = changed or not to_create.exists() to_create.mkdir(0o0755, parents=True, exist_ok=True) + return changed + @staticmethod def _parse_layout_target( src: str, layout: dict[Literal["symlink", "bind", "bind-file", "type"], str] @@ -335,6 +351,8 @@ def get_artifacts(self) -> dict[str | None, pathlib.Path]: @override def _pack(self, *, name: str | None = None, path: pathlib.Path) -> None: """Pack a specific snap or component artifact.""" + self._ensure_hook_assets_executable(name) + if name in (None, "default"): issues = linters.run_linters( self._services.lifecycle.prime_dir, lint=self._project.lint @@ -468,7 +486,12 @@ def _gen_extra_assets( exist in prime; craft-application will delete any stale copy at that destination. """ - assets = self._get_hook_assets(partition_name) + assets = [ + *self._get_hook_assets(partition_name), + *self._get_gui_assets(partition_name), + *self._get_desktop_assets(partition_name), + *self._get_icon_assets(partition_name), + ] if partition_name in (None, "default"): project_file = self._services.get("project").resolve_project_file_path() @@ -477,34 +500,194 @@ def _gen_extra_assets( project_file if self._project_file_copy_enabled() else None ) assets.insert(0, (source, destination)) + assets.extend(self._get_system_metadata_assets()) + + return assets + + def _get_desktop_assets( + self, partition_name: str | None = None + ) -> list[tuple[str | None, pathlib.Path]]: + """Generate rewritten desktop files for a partition.""" + if partition_name not in (None, "default") or not self._project.apps: + return [] + + prime_dir = self._prime_dir_for(partition_name) + icon_path = self._get_effective_icon_path(partition_name) + assets: list[tuple[str | None, pathlib.Path]] = [] + + for app_name, app in self._project.apps.items(): + if not app.desktop: + continue + + desktop_file = DesktopFile( + snap_name=self._project.name, + app_name=app_name, + filename=app.desktop, + prime_dir=prime_dir, + ) + destination = prime_dir / "meta" / "gui" / f"{app_name}.desktop" + assets.append( + ( + desktop_file.render(icon_path=icon_path), + destination, + ) + ) + + return assets + + def _get_icon_assets( + self, partition_name: str | None = None + ) -> list[tuple[bytes | pathlib.Path | None, pathlib.Path]]: + """Generate icon assets for the default partition.""" + if partition_name not in (None, "default"): + return [] + + icon_asset = self._resolve_icon_asset(partition_name) + if icon_asset is None: + return [] + + source, destination = icon_asset + return [(source, destination)] + + def _get_system_metadata_assets( + self, + ) -> list[tuple[pathlib.Path | None, pathlib.Path]]: + """Generate gadget/kernel metadata assets for the default partition.""" + prime_dir = self._prime_dir_for(None) + project_dir = self._services.lifecycle.project_info.project_dir + assets: list[tuple[pathlib.Path | None, pathlib.Path]] = [] + + if self._project.type == const.ProjectType.GADGET: + gadget_yaml = project_dir / "gadget.yaml" + if not gadget_yaml.exists(): + raise errors.SnapcraftError("gadget.yaml is required for gadget snaps") + assets.append((gadget_yaml, prime_dir / "meta" / "gadget.yaml")) + + if self._project.type == const.ProjectType.KERNEL: + kernel_yaml = project_dir / "kernel.yaml" + assets.append( + ( + kernel_yaml if kernel_yaml.exists() else None, + prime_dir / "meta" / "kernel.yaml", + ) + ) return assets def _get_hook_assets( self, partition_name: str | None = None - ) -> list[tuple[pathlib.Path, pathlib.Path]]: + ) -> list[tuple[str | pathlib.Path, pathlib.Path]]: """Generate hook assets for the default or component partition. Project-provided hooks are added after built hooks so they keep their existing precedence when both target the same meta/hooks path. """ + assets = self._get_project_assets( + partition_name, + source_subdir="hooks", + destination_subdir="meta/hooks", + include_built_subdir="snap/hooks", + ) + + existing_hooks = {destination.name for _source, destination in assets} + prime_dir = self._prime_dir_for(partition_name) + for hook_name in self._get_declared_hooks(partition_name): + if hook_name not in existing_hooks: + assets.append((self._HOOK_STUB, prime_dir / "meta" / "hooks" / hook_name)) + + return assets + + def _get_gui_assets( + self, partition_name: str | None = None + ) -> list[tuple[pathlib.Path, pathlib.Path]]: + """Generate static gui assets for the default or component partition.""" + return self._get_project_assets( + partition_name, + source_subdir="gui", + destination_subdir="meta/gui", + ) + + def _get_project_assets( + self, + partition_name: str | None, + *, + source_subdir: str, + destination_subdir: str, + include_built_subdir: str | None = None, + ) -> list[tuple[pathlib.Path, pathlib.Path]]: + """Generate mediated assets copied from project or prime subdirectories.""" prime_dir = self._prime_dir_for(partition_name) assets_dir = self._get_partition_assets_dir(partition_name) - destination_dir = prime_dir / "meta" / "hooks" + destination_dir = prime_dir / destination_subdir assets: list[tuple[pathlib.Path, pathlib.Path]] = [] - built_snap_hooks = prime_dir / "snap" / "hooks" - if built_snap_hooks.is_dir(): - for hook in sorted(built_snap_hooks.iterdir()): - assets.append((hook, destination_dir / hook.name)) + if include_built_subdir is not None: + built_dir = prime_dir / include_built_subdir + if built_dir.is_dir(): + for asset in sorted(built_dir.iterdir()): + assets.append((asset, destination_dir / asset.name)) - project_hooks_dir = assets_dir / "hooks" - if project_hooks_dir.is_dir(): - for hook in sorted(project_hooks_dir.iterdir()): - assets.append((hook, destination_dir / hook.name)) + project_dir = assets_dir / source_subdir + if project_dir.is_dir(): + for asset in sorted(project_dir.iterdir()): + assets.append((asset, destination_dir / asset.name)) return assets + def _resolve_icon_asset( + self, partition_name: str | None = None + ) -> tuple[bytes | pathlib.Path, pathlib.Path] | None: + """Resolve the icon asset that should be added to prime.""" + prime_dir = self._prime_dir_for(partition_name) + assets_dir = self._get_partition_assets_dir(partition_name) + + if partition_name not in (None, "default"): + return None + + icon = self._project.icon + + if icon is None: + icon_path = find_icon_file(assets_dir) + if icon_path is None: + return None + return (icon_path, prime_dir / icon_path.relative_to(assets_dir.parent)) + + parsed_url = urllib.parse.urlparse(icon) + parsed_path = pathlib.Path(parsed_url.path) + icon_ext = parsed_path.suffix[1:] + target_icon_path = prime_dir / "meta" / "gui" / f"icon.{icon_ext}" + + if parsed_url.scheme in ["http", "https"]: + emit.progress(f"Fetching icon from {icon!r}") + icon_data = requests.get(icon, timeout=self._REMOTE_FETCH_TIMEOUT).content + return (icon_data, target_icon_path) + + if parsed_url.scheme: + raise RuntimeError(f"Unexpected icon path: {parsed_url!r}") + + source_path = prime_dir / ( + parsed_path.relative_to("/") if parsed_path.is_absolute() else parsed_path + ) + if source_path.exists(): + return (source_path, target_icon_path) + if parsed_path.exists(): + return (parsed_path, target_icon_path) + + icon_path = find_icon_file(assets_dir) + if icon_path is None: + return None + + return (icon_path, prime_dir / icon_path.relative_to(assets_dir.parent)) + + def _get_effective_icon_path(self, partition_name: str | None = None) -> str | None: + """Return the path desktop rewrites should reference for the icon.""" + icon_asset = self._resolve_icon_asset(partition_name) + if icon_asset is None: + return None + + _source, destination = icon_asset + return str(destination.relative_to(self._prime_dir_for(partition_name))) + def _get_partition_assets_dir(self, partition_name: str | None = None) -> pathlib.Path: """Return the project assets directory for a default or component partition.""" assets_dir = self._get_assets_dir() @@ -513,6 +696,36 @@ def _get_partition_assets_dir(self, partition_name: str | None = None) -> pathli return assets_dir / "component" / partition_name + def _get_declared_hooks( + self, partition_name: str | None = None + ) -> dict[str, models.Hook]: + """Return hook declarations for the given partition.""" + if partition_name in (None, "default"): + return self._project.hooks or {} + + if self._project.components is None: + return {} + + component = self._project.components.get(partition_name) + if component is None: + return {} + + return component.hooks or {} + + def _validate_declared_hook_command_chains( + self, partition_name: str | None = None + ) -> None: + """Validate command-chain entries for declared hooks.""" + prime_dir = self._prime_dir_for(partition_name) + + for hook_name, hook in self._get_declared_hooks(partition_name).items(): + if hook.command_chain: + validate_command_chain( + hook.command_chain, + name=f"hook {hook_name!r}", + prime_dir=prime_dir, + ) + @staticmethod def _project_file_copy_enabled() -> bool: """Return whether the project file should be copied into snap/.""" @@ -523,12 +736,23 @@ def _write_asset( self, source: str | bytes | None | pathlib.Path, destination: pathlib.Path ) -> None: """Write a generated package file or extra asset into prime.""" + if source is None: + super()._write_asset(source, destination) + return + if isinstance(source, pathlib.Path): destination.parent.mkdir(parents=True, exist_ok=True) shutil.copy(source, destination) - return + else: + super()._write_asset(source, destination) + + if self._is_hook_asset(destination) and not destination.stat().st_mode & stat.S_IEXEC: + destination.chmod(0o755) - super()._write_asset(source, destination) + @staticmethod + def _is_hook_asset(path: pathlib.Path) -> bool: + """Return whether a path points to a mediated hook file.""" + return path.parent.name == "hooks" and path.parent.parent.name == "meta" @override def _package_file_changed( @@ -576,43 +800,50 @@ def write_metadata(self, path: pathlib.Path) -> None: :param path: The path to the prime directory. """ - meta_dir = path / "meta" - meta_dir.mkdir(parents=True, exist_ok=True) - (meta_dir / "snap.yaml").write_text( - self._get_snap_yaml(), encoding="utf-8" - ) + path.mkdir(parents=True, exist_ok=True) - # Snapcraft's Lifecycle implementation is what we need to refer to for typing - lifecycle_service = cast(Lifecycle, self._services.lifecycle) - snap_dir = path / "snap" - manifest_path = snap_dir / "manifest.yaml" - manifest_contents = self._get_manifest_yaml() - if manifest_contents is not None: - snap_dir.mkdir(parents=True, exist_ok=True) - manifest_path.write_text(manifest_contents, encoding="utf-8") - else: - manifest_path.unlink(missing_ok=True) + self._write_metadata_assets_changed() - self._materialize_extra_assets(None) - for component in self._project.get_component_names(): - self._materialize_extra_assets(component) + def _write_metadata_assets_changed(self) -> bool: + """Create mediated assets during post-prime setup.""" + changed = False - assets_dir = self._get_assets_dir() - setup_assets( - self._project, - assets_dir=assets_dir, - project_dir=self._services.lifecycle.project_info.project_dir, - prime_dirs=lifecycle_service.prime_dirs, - meta_directory_handler=meta_directory_handler, - ) + for partition_name in self.get_artifacts(): + self._validate_declared_hook_command_chains(partition_name) - for component in self._project.get_component_names(): - component_prime_dir = lifecycle_service.get_prime_dir(component) - component_meta_dir = component_prime_dir / "meta" - component_meta_dir.mkdir(parents=True, exist_ok=True) - (component_meta_dir / "component.yaml").write_text( - self._get_component_yaml(component), encoding="utf-8" - ) + for pkg_file in self._package_files(partition_name): + if self._package_file_changed(pkg_file, partition_name): + changed = True + generator = getattr(self, pkg_file.method_name) + self._write_asset( + generator(partition_name), + self._prime_dir_for(partition_name) / pkg_file.relative_path, + ) + + for source, destination in self._gen_extra_assets(partition_name): + if self._asset_changed(source, destination, partition_name): + changed = True + self._write_asset(source, destination) + + if self._ensure_hook_assets_executable(partition_name): + changed = True + + self._project_was_updated = self._project_was_updated or changed + return changed + + def _ensure_hook_assets_executable(self, partition_name: str | None = None) -> bool: + """Ensure mediated hooks under meta/hooks are executable.""" + hooks_dir = self._prime_dir_for(partition_name) / "meta" / "hooks" + if not hooks_dir.is_dir(): + return False + + changed = False + for hook_path in hooks_dir.iterdir(): + if not hook_path.stat().st_mode & stat.S_IEXEC: + hook_path.chmod(0o755) + changed = True + + return changed @property def metadata(self) -> snap_yaml.SnapMetadata: @@ -644,22 +875,3 @@ def _hardlink_or_copy(source: pathlib.Path, destination: pathlib.Path) -> bool: return False return True - - -def meta_directory_handler(assets_dir: pathlib.Path, path: pathlib.Path): - """Handle gui assets from Snapcraft. - - :param assets_dir: directory with project assets. - :param path: directory to write assets to. - """ - meta_dir = path / "meta" - - # Write any gui assets - gui_project_dir = assets_dir / "gui" - gui_meta_dir = meta_dir / "gui" - if gui_project_dir.is_dir(): - gui_meta_dir.mkdir(parents=True, exist_ok=True) - for gui in gui_project_dir.iterdir(): - meta_dir_gui = gui_meta_dir / gui.name - - _hardlink_or_copy(gui, meta_dir_gui) diff --git a/tests/spread/general/skip-repack/snap/icon.svg b/tests/spread/general/skip-repack/snap/icon.svg new file mode 100644 index 0000000000..8933941f0a --- /dev/null +++ b/tests/spread/general/skip-repack/snap/icon.svg @@ -0,0 +1 @@ +This is an icon. Believe me. diff --git a/tests/spread/general/skip-repack/snap/snapcraft.yaml b/tests/spread/general/skip-repack/snap/snapcraft.yaml new file mode 100644 index 0000000000..b860afbd1c --- /dev/null +++ b/tests/spread/general/skip-repack/snap/snapcraft.yaml @@ -0,0 +1,13 @@ +name: skip-repack +base: core24 +version: '0.1' +summary: Skip-repack test +description: Skip-repack test. +grade: stable +confinement: strict +icon: snap/icon.svg + +parts: + my-part: + plugin: nil + source: src diff --git a/tests/spread/general/skip-repack/task.yaml b/tests/spread/general/skip-repack/task.yaml new file mode 100644 index 0000000000..5e11c2f0ab --- /dev/null +++ b/tests/spread/general/skip-repack/task.yaml @@ -0,0 +1,28 @@ +summary: Check if repacking is skipped + +environment: + PROJECT_NAME: "test-proj" + +systems: + - ubuntu-24.04* + +execute: | + mkdir "${PROJECT_NAME}" + cp -rap snap "${PROJECT_NAME}" + mkdir "${PROJECT_NAME}/src" + touch "${PROJECT_NAME}/foo.txt" + pushd "${PROJECT_NAME}" + + snapcraft pack 2>&1 | MATCH "Packing" + snapcraft pack 2>&1 | MATCH "Skipping pack" + echo "Change" >> snap/icon.svg + snapcraft pack 2>&1 | MATCH "Packing" + snapcraft pack 2>&1 | MATCH "Skipping pack" + echo "echo hello world" >> snap/hooks/configure + chmod +x snap/hooks/configure + snapcraft pack 2>&1 | MATCH "Packing" + + popd + +restore: | + rm -rf ./"${PROJECT_NAME}" diff --git a/tests/unit/parts/test_setup_assets.py b/tests/unit/parts/test_setup_assets.py index 5e7be689c0..350ddd5272 100644 --- a/tests/unit/parts/test_setup_assets.py +++ b/tests/unit/parts/test_setup_assets.py @@ -25,11 +25,11 @@ from snapcraft import errors, models from snapcraft.parts import setup_assets as parts_setup_assets from snapcraft.parts.setup_assets import ( - _create_hook_wrappers, - _ensure_hook, - _ensure_hook_executable, - _validate_command_chain, - _write_hook_wrapper, + create_hook_wrappers, + ensure_hook, + ensure_hook_executable, + validate_command_chain, + write_hook_wrapper, setup_assets, ) @@ -532,7 +532,7 @@ def test_setup_assets_hook_command_chain_error(self, yaml_data, new_dir): def test_command_chain_path_not_found(self, new_dir): with pytest.raises(errors.SnapcraftError) as raised: - _validate_command_chain(["file-not-found"], name="foo", prime_dir=new_dir) + validate_command_chain(["file-not-found"], name="foo", prime_dir=new_dir) assert str(raised.value) == ( "Failed to generate snap metadata: The command-chain item 'file-not-found' " @@ -546,7 +546,7 @@ def test_command_chain_path_not_executable(self, new_dir): Path("file-not-executable").touch() with pytest.raises(errors.SnapcraftError) as raised: - _validate_command_chain( + validate_command_chain( ["file-executable", "file-not-executable"], name="foo", prime_dir=new_dir, @@ -561,7 +561,7 @@ def test_command_chain_path_not_executable(self, new_dir): def test_ensure_hook(new_dir): """Verify creation of executable placeholder hooks.""" hook_path: Path = new_dir / "configure" - _ensure_hook(hook_path) + ensure_hook(hook_path) assert hook_path.exists() assert hook_path.read_text() == "#!/bin/true\n" @@ -573,7 +573,7 @@ def test_ensure_hook_does_not_overwrite(new_dir): hook_path.write_text("#!/bin/python3\n") hook_path.chmod(0o700) - _ensure_hook(hook_path) + ensure_hook(hook_path) assert hook_path.exists() assert hook_path.read_text() == "#!/bin/python3\n" @@ -581,13 +581,13 @@ def test_ensure_hook_does_not_overwrite(new_dir): def test_ensure_hook_executable(new_dir): - """Verify _ensure_hook_executable makes a file executable.""" + """Verify ensure_hook_executable makes a file executable.""" # create a non-executable file hook_path: Path = new_dir / "configure" hook_path.write_text("#!/bin/true\n") hook_path.chmod(0o644) - _ensure_hook_executable(hook_path) + ensure_hook_executable(hook_path) assert hook_path.exists() assert oct(hook_path.stat().st_mode)[-3:] == "755" @@ -605,7 +605,7 @@ def test_create_hook_wrappers(new_dir): hook.write_text("#!/bin/true\n") hook.chmod(0o644) - _create_hook_wrappers(new_dir) + create_hook_wrappers(new_dir) # verify prime/meta/hooks directory was created hooks_meta_dir = new_dir / "meta" / "hooks" @@ -634,7 +634,7 @@ def test_write_hook_wrapper(new_dir): hook_wrapper_dir.mkdir() hook_wrapper = hook_wrapper_dir / hook_name - _write_hook_wrapper(hook_name, hook_wrapper) + write_hook_wrapper(hook_name, hook_wrapper) # verify content of hook wrapper assert hook_wrapper.exists() diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 2718b8b4ed..62cb05c665 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -21,6 +21,7 @@ from contextlib import AbstractContextManager, nullcontext from pathlib import Path from textwrap import dedent +from types import SimpleNamespace from typing import Any, cast import pytest @@ -29,7 +30,7 @@ from craft_cli.pytest_plugin import RecordingEmitter from pytest_mock import MockerFixture -from snapcraft import __version__, linters, meta, models, pack +from snapcraft import __version__, const, linters, meta, models, pack from snapcraft.errors import SnapcraftPrecreationEscapesPrimeError from snapcraft.meta import ExtractedMetadata from snapcraft.parts import extract_metadata, update_metadata @@ -191,6 +192,13 @@ def test_get_manifest_yaml_enabled( assert manifest["grade"] == "devel" +def test_supports_conditional_repack(default_project, fake_services, setup_project): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + + assert package_service.supports_conditional_repack is True + + def test_project_file_copy_disabled( monkeypatch, default_project, fake_services, setup_project ): @@ -285,6 +293,111 @@ def test_gen_extra_assets_project_hooks_override_built_hooks( ] +def test_get_gui_assets(default_project, fake_services, setup_project, in_project_path, tmp_path): + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + project_gui_dir = in_project_path / "snap" / "gui" + project_gui_dir.mkdir(parents=True) + desktop = project_gui_dir / "default.default.desktop" + icon = project_gui_dir / "icon.png" + desktop.write_text("desktop_file", encoding="utf-8") + icon.write_text("package_png_icon", encoding="utf-8") + + assert package_service._get_gui_assets() == [ + (desktop, tmp_path / "prime" / "meta" / "gui" / "default.default.desktop"), + (icon, tmp_path / "prime" / "meta" / "gui" / "icon.png"), + ] + + +def test_get_desktop_assets(default_project, fake_services, setup_project, tmp_path): + project_data = default_project.marshal() + project_data["apps"] = { + "app1": {"command": "bin/test", "desktop": "test.desktop"} + } + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + prime_dir = tmp_path / "prime" + (prime_dir / "bin").mkdir(parents=True) + (prime_dir / "bin" / "test").write_text("#!/bin/true\n", encoding="utf-8") + (prime_dir / "bin" / "test").chmod(0o755) + (prime_dir / "test.desktop").write_text( + dedent( + """\ + [Desktop Entry] + Name=test + Exec=test + Type=Application + Icon=/usr/share/icons/test.png + """ + ), + encoding="utf-8", + ) + (prime_dir / "usr/share/icons").mkdir(parents=True) + (prime_dir / "usr/share/icons" / "test.png").write_text("icon", encoding="utf-8") + + assert package_service._get_desktop_assets() == [ + ( + dedent( + """\ + [Desktop Entry] + Name=test + Exec=default.app1 + Type=Application + Icon=${SNAP}/usr/share/icons/test.png + + """ + ), + tmp_path / "prime" / "meta" / "gui" / "app1.desktop", + ) + ] + + +def test_get_icon_assets_remote(monkeypatch, default_project, fake_services, setup_project, mocker, tmp_path): + project_data = default_project.marshal() + project_data["icon"] = "https://example.com/icon.png" + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + mock_response = mocker.Mock() + mock_response.content = b"png-data" + mocker.patch("snapcraft.services.package.requests.get", return_value=mock_response) + + assert package_service._get_icon_assets() == [ + (b"png-data", tmp_path / "prime" / "meta" / "gui" / "icon.png") + ] + + +def test_get_system_metadata_assets_gadget( + default_project, fake_services, setup_project, in_project_path, tmp_path +): + project_data = default_project.marshal() + project_data["type"] = "gadget" + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + gadget_yaml = in_project_path / "gadget.yaml" + gadget_yaml.write_text("volumes: {}\n", encoding="utf-8") + + assert package_service._get_system_metadata_assets() == [ + (gadget_yaml, tmp_path / "prime" / "meta" / "gadget.yaml") + ] + + +def test_get_system_metadata_assets_kernel_missing( + default_project, fake_services, setup_project, mocker, tmp_path +): + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + mocker.patch.object( + type(package_service), + "_project", + new_callable=mocker.PropertyMock, + return_value=SimpleNamespace(type=const.ProjectType.KERNEL), + ) + + assert package_service._get_system_metadata_assets() == [ + (None, tmp_path / "prime" / "meta" / "kernel.yaml") + ] + + def test_manifest_changed_ignores_started_at( monkeypatch, default_project, fake_services, setup_project, tmp_path ): @@ -304,6 +417,57 @@ def test_manifest_changed_ignores_started_at( assert package_service._manifest_changed(None) is False +def test_needs_packing_when_write_metadata_changes_prime( + default_project, fake_services, setup_project, tmp_path +): + project_data = default_project.marshal() + project_data["apps"] = { + "app1": {"command": "bin/test", "desktop": "test.desktop"} + } + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + artifact_path = tmp_path / "test-output.snap" + artifact_path.write_text("artifact", encoding="utf-8") + + prime_dir = tmp_path / "prime" + (prime_dir / "bin").mkdir(parents=True) + (prime_dir / "bin" / "test").write_text("#!/bin/true\n", encoding="utf-8") + (prime_dir / "bin" / "test").chmod(0o755) + (prime_dir / "test.desktop").write_text( + dedent( + """\ + [Desktop Entry] + Name=test + Exec=test + Type=Application + """ + ), + encoding="utf-8", + ) + + package_service.write_metadata(prime_dir) + + assert package_service.needs_packing(None) is True + + +def test_write_metadata_second_run_is_stable( + default_project, fake_services, setup_project, tmp_path +): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + prime_dir = tmp_path / "prime" + + package_service.write_metadata(prime_dir) + snap_yaml = prime_dir / "meta" / "snap.yaml" + first_mtime = snap_yaml.stat().st_mtime_ns + + package_service._project_was_updated = False + package_service.write_metadata(prime_dir) + + assert package_service._project_was_updated is False + assert snap_yaml.stat().st_mtime_ns == first_mtime + + def test_write_metadata(default_project, fake_services, setup_project, new_dir): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") @@ -489,6 +653,39 @@ def test_write_metadata_with_built_hooks( assert (meta_dir / "hooks" / "install").stat().st_mode & 0o111 +def test_write_metadata_generates_declared_hook_stub_as_executable( + default_project, fake_services, setup_project, tmp_path +): + project_data = default_project.marshal() + project_data["hooks"] = {"post-refresh": {}} + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + + prime_dir = tmp_path / "prime" + package_service.write_metadata(prime_dir) + + hook_path = prime_dir / "meta" / "hooks" / "post-refresh" + assert hook_path.read_text() == "#!/bin/true\n" + assert hook_path.stat().st_mode & 0o111 + + +def test_write_metadata_makes_non_executable_hook_asset_executable( + default_project, fake_services, setup_project, project_hooks_dir, tmp_path +): + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + project_hook = project_hooks_dir / "configure" + project_hook.write_text("configure_hook", encoding="utf-8") + project_hook.chmod(0o644) + + prime_dir = tmp_path / "prime" + package_service.write_metadata(prime_dir) + + hook_path = prime_dir / "meta" / "hooks" / "configure" + assert hook_path.read_text() == "configure_hook" + assert hook_path.stat().st_mode & 0o111 + + def test_write_metadata_with_project_gui( default_project, fake_services, setup_project, in_project_path, tmp_path ): @@ -532,6 +729,45 @@ def test_write_metadata_with_project_gui( assert (meta_dir / "gui" / "icon.png").read_text() == "package_png_icon" +def test_write_metadata_generates_desktop_and_marks_project_updated( + default_project, fake_services, setup_project, tmp_path +): + project_data = default_project.marshal() + project_data["apps"] = { + "app1": {"command": "bin/test", "desktop": "test.desktop"} + } + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + prime_dir = tmp_path / "prime" + (prime_dir / "bin").mkdir(parents=True) + (prime_dir / "bin" / "test").write_text("#!/bin/true\n", encoding="utf-8") + (prime_dir / "bin" / "test").chmod(0o755) + (prime_dir / "test.desktop").write_text( + dedent( + """\ + [Desktop Entry] + Name=test + Exec=test + Type=Application + """ + ), + encoding="utf-8", + ) + + package_service.write_metadata(prime_dir) + + assert (prime_dir / "meta" / "gui" / "app1.desktop").read_text() == dedent( + """\ + [Desktop Entry] + Name=test + Exec=default.app1 + Type=Application + + """ + ) + assert package_service._project_was_updated is True + + def test_update_project_parse_info( default_project, fake_services, setup_project, in_project_path, tmp_path, mocker ): @@ -570,6 +806,60 @@ def test_update_project_parse_info( ) +def test_update_project_parse_info_unchanged_does_not_mark_project_updated( + default_project, fake_services, setup_project, in_project_path, tmp_path, mocker +): + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + project_service = fake_services.get("project") + lifecycle = fake_services.lifecycle + project_info = lifecycle.project_info + project_info.execution_finished = True + + fake_metadata = ExtractedMetadata() + mocker.patch.object( + extract_metadata, "extract_lifecycle_metadata", return_value=[fake_metadata] + ) + mocker.patch.object(update_metadata, "update_from_extracted_metadata") + mocker.patch.object( + project_service, + "get_parse_info", + return_value={"my-part": ["file.metadata.xml"]}, + ) + + package_service.update_project() + + assert package_service._project_was_updated is False + + +def test_update_project_parse_info_changed_marks_project_updated( + default_project, fake_services, setup_project, in_project_path, tmp_path, mocker +): + project_data = default_project.marshal() + project_data["license"] = None + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + project_service = fake_services.get("project") + lifecycle = fake_services.lifecycle + project_info = lifecycle.project_info + project_info.execution_finished = True + + fake_metadata = ExtractedMetadata(license="GPL-3.0") + mocker.patch.object( + extract_metadata, "extract_lifecycle_metadata", return_value=[fake_metadata] + ) + mocker.patch.object( + project_service, + "get_parse_info", + return_value={"my-part": ["file.metadata.xml"]}, + ) + + package_service.update_project() + + assert project_service.get().license == "GPL-3.0" + assert package_service._project_was_updated is True + + def test_extra_project_updates_makes_targets_core26( snapcraft_yaml: Callable[..., Any], setup_project: Callable[..., Any], @@ -591,6 +881,38 @@ def test_extra_project_updates_makes_targets_core26( mock_precreate_plugs.assert_called_once() +def test_extra_project_updates_core26_no_precreate_changes_not_marked_updated( + snapcraft_yaml: Callable[..., Any], + setup_project: Callable[..., Any], + fake_services: ServiceFactory, + mocker: MockerFixture, +) -> None: + setup_project(fake_services, snapcraft_yaml(base="core26")) + package_service = fake_services.get("package") + mocker.patch.object(package_service, "_precreate_layout_targets", return_value=False) + mocker.patch.object(package_service, "_precreate_plug_targets", return_value=False) + + package_service.update_project() + + assert package_service._project_was_updated is False + + +def test_extra_project_updates_core26_precreate_change_marks_updated( + snapcraft_yaml: Callable[..., Any], + setup_project: Callable[..., Any], + fake_services: ServiceFactory, + mocker: MockerFixture, +) -> None: + setup_project(fake_services, snapcraft_yaml(base="core26")) + package_service = fake_services.get("package") + mocker.patch.object(package_service, "_precreate_layout_targets", return_value=True) + mocker.patch.object(package_service, "_precreate_plug_targets", return_value=False) + + package_service.update_project() + + assert package_service._project_was_updated is True + + @pytest.mark.parametrize( "base", [ diff --git a/tests/unit/services/test_package_components.py b/tests/unit/services/test_package_components.py index 9db143f926..75707f062c 100644 --- a/tests/unit/services/test_package_components.py +++ b/tests/unit/services/test_package_components.py @@ -21,6 +21,7 @@ from unittest.mock import call import pytest +from pytest_mock import MockerFixture from snapcraft import linters, pack @@ -240,6 +241,26 @@ def test_get_hook_assets_for_component( assert package_service._get_hook_assets("firstcomponent") == [ (built_hook, component_prime_dir / "meta" / "hooks" / "install"), (project_hook, component_prime_dir / "meta" / "hooks" / "configure"), + ("#!/bin/true\n", component_prime_dir / "meta" / "hooks" / "post-refresh"), + ] + + +@pytest.mark.usefixtures("enable_partitions_feature") +def test_get_gui_assets_for_component( + default_project, fake_services, setup_project, project_assets_dir, tmp_path +): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + component_prime_dir = tmp_path / "partitions" / "component" / "firstcomponent" / "prime" + desktop = project_assets_dir / "component" / "firstcomponent" / "gui" / "first.desktop" + icon = project_assets_dir / "component" / "firstcomponent" / "gui" / "icon.png" + desktop.parent.mkdir(parents=True) + desktop.write_text("desktop_file", encoding="utf-8") + icon.write_text("component_icon", encoding="utf-8") + + assert package_service._get_gui_assets("firstcomponent") == [ + (desktop, component_prime_dir / "meta" / "gui" / "first.desktop"), + (icon, component_prime_dir / "meta" / "gui" / "icon.png"), ] @@ -272,6 +293,9 @@ def test_write_metadata( component_hook = project_assets_dir / "component/firstcomponent/hooks/install" component_hook.write_text("install_hook") component_hook.chmod(0o755) + component_gui = project_assets_dir / "component/firstcomponent/gui/icon.png" + component_gui.parent.mkdir(parents=True) + component_gui.write_text("component_icon") (project_assets_dir / "post-refresh").write_text("post-refresh") prime_dir = tmp_path / "prime" @@ -349,3 +373,58 @@ def test_write_metadata( ) assert component_meta_hook.read_text() == "install_hook" assert component_meta_hook.stat().st_mode & 0o111 + assert ( + lifecycle_service.get_prime_dir("firstcomponent") / "meta" / "gui" / "icon.png" + ).read_text() == "component_icon" + + +@pytest.mark.usefixtures("enable_partitions_feature") +def test_write_metadata_generates_component_declared_hook_stub( + default_project, fake_services, setup_project, tmp_path +): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + lifecycle_service = fake_services.get("lifecycle") + command_chain_exe = ( + lifecycle_service.get_prime_dir("firstcomponent") / "test-command-chain" + ) + command_chain_exe.parent.mkdir(parents=True, exist_ok=True) + command_chain_exe.touch() + command_chain_exe.chmod(0o755) + + package_service.write_metadata(tmp_path / "prime") + + hook_path = ( + lifecycle_service.get_prime_dir("firstcomponent") + / "meta" + / "hooks" + / "post-refresh" + ) + assert hook_path.read_text() == "#!/bin/true\n" + assert hook_path.stat().st_mode & 0o111 + + +@pytest.mark.usefixtures("enable_partitions_feature") +def test_pack_component_makes_organized_meta_hook_executable( + default_project, fake_services, setup_project, tmp_path, mocker: MockerFixture +): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + lifecycle_service = fake_services.get("lifecycle") + component_prime_dir = lifecycle_service.get_prime_dir("firstcomponent") + remove_hook = component_prime_dir / "meta" / "hooks" / "remove" + remove_hook.parent.mkdir(parents=True, exist_ok=True) + remove_hook.write_text("#!/bin/true\n", encoding="utf-8") + remove_hook.chmod(0o664) + + mocker.patch.object(linters, "run_linters") + mocker.patch.object(linters, "report") + mock_pack_component = mocker.patch.object(pack, "pack_component", return_value="default+firstcomponent_1.0.comp") + + package_service._pack( + name="firstcomponent", + path=tmp_path / "default+firstcomponent_1.0.comp", + ) + + assert remove_hook.stat().st_mode & 0o111 + mock_pack_component.assert_called_once() From 9993fb3fd52bfe253238370cb773cc3e0ba63730 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 09:28:13 -0300 Subject: [PATCH 06/23] chore: reuse existing helpers instead of reimplementing Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index 7dfa610d99..f21d281f54 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -41,7 +41,11 @@ from snapcraft.parts import extract_metadata as extract from snapcraft.parts import update_metadata as update from snapcraft.parts.desktop_file import DesktopFile -from snapcraft.parts.setup_assets import find_icon_file, validate_command_chain +from snapcraft.parts.setup_assets import ( + ensure_hook_executable, + find_icon_file, + validate_command_chain, +) from snapcraft.services import Lifecycle from snapcraft.utils import process_version @@ -746,8 +750,8 @@ def _write_asset( else: super()._write_asset(source, destination) - if self._is_hook_asset(destination) and not destination.stat().st_mode & stat.S_IEXEC: - destination.chmod(0o755) + if self._is_hook_asset(destination): + ensure_hook_executable(destination) @staticmethod def _is_hook_asset(path: pathlib.Path) -> bool: @@ -840,7 +844,7 @@ def _ensure_hook_assets_executable(self, partition_name: str | None = None) -> b changed = False for hook_path in hooks_dir.iterdir(): if not hook_path.stat().st_mode & stat.S_IEXEC: - hook_path.chmod(0o755) + ensure_hook_executable(hook_path) changed = True return changed From 6281ff978f06fd839309bd193a02777967db5786 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 12:55:05 +0000 Subject: [PATCH 07/23] chore: better filtering for hook file creation Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index f21d281f54..b2e9fdbea3 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -843,6 +843,8 @@ def _ensure_hook_assets_executable(self, partition_name: str | None = None) -> b changed = False for hook_path in hooks_dir.iterdir(): + if not hook_path.is_file(): + continue if not hook_path.stat().st_mode & stat.S_IEXEC: ensure_hook_executable(hook_path) changed = True From e541acf18372e355a1065490c965ca24571d6a42 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 09:48:30 -0300 Subject: [PATCH 08/23] chore: handle asset clash issue raised in review Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 10 ++++-- tests/unit/services/test_package.py | 32 ++++++++++++++++++- .../unit/services/test_package_components.py | 7 ++-- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index b2e9fdbea3..cf5c7c5a50 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -583,8 +583,7 @@ def _get_hook_assets( ) -> list[tuple[str | pathlib.Path, pathlib.Path]]: """Generate hook assets for the default or component partition. - Project-provided hooks are added after built hooks so they keep their - existing precedence when both target the same meta/hooks path. + Project-provided hooks override built hooks with the same filename. """ assets = self._get_project_assets( partition_name, @@ -628,7 +627,14 @@ def _get_project_assets( if include_built_subdir is not None: built_dir = prime_dir / include_built_subdir if built_dir.is_dir(): + overridden_assets = set[str]() + project_dir = assets_dir / source_subdir + if project_dir.is_dir(): + overridden_assets = {asset.name for asset in project_dir.iterdir()} + for asset in sorted(built_dir.iterdir()): + if asset.name in overridden_assets: + continue assets.append((asset, destination_dir / asset.name)) project_dir = assets_dir / source_subdir diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 62cb05c665..89c95f4042 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -148,6 +148,18 @@ def test_get_artifacts(default_project, fake_services, setup_project, tmp_path): } +def test_get_artifacts_defaults_to_cwd( + default_project, fake_services, setup_project, monkeypatch, tmp_path +): + setup_project(fake_services, default_project.marshal()) + package_service = fake_services.get("package") + monkeypatch.chdir(tmp_path) + + assert package_service.get_artifacts() == { + None: tmp_path / "default_1.0_amd64.snap" + } + + def test_pack_artifact_snap(default_project, fake_services, setup_project, mocker, tmp_path): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") @@ -288,11 +300,29 @@ def test_gen_extra_assets_project_hooks_override_built_hooks( extra_assets = package_service._get_hook_assets() assert extra_assets == [ - (built_hook, tmp_path / "prime" / "meta" / "hooks" / "configure"), (project_hook, tmp_path / "prime" / "meta" / "hooks" / "configure"), ] +def test_write_metadata_project_hook_overrides_newer_built_hook( + default_project, fake_services, setup_project, project_hooks_dir, tmp_path +): + setup_project(fake_services, default_project.marshal(), write_project=True) + package_service = fake_services.get("package") + built_hooks_dir = tmp_path / "prime" / "snap" / "hooks" + built_hooks_dir.mkdir(parents=True) + built_hook = built_hooks_dir / "configure" + project_hook = project_hooks_dir / "configure" + project_hook.write_text("project_configure", encoding="utf-8") + built_hook.write_text("built_configure", encoding="utf-8") + + package_service.write_metadata(tmp_path / "prime") + + assert (tmp_path / "prime" / "meta" / "hooks" / "configure").read_text() == ( + "project_configure" + ) + + def test_get_gui_assets(default_project, fake_services, setup_project, in_project_path, tmp_path): setup_project(fake_services, default_project.marshal(), write_project=True) package_service = fake_services.get("package") diff --git a/tests/unit/services/test_package_components.py b/tests/unit/services/test_package_components.py index 75707f062c..630888cab4 100644 --- a/tests/unit/services/test_package_components.py +++ b/tests/unit/services/test_package_components.py @@ -231,16 +231,15 @@ def test_get_hook_assets_for_component( built_hooks_dir = component_prime_dir / "snap" / "hooks" built_hooks_dir.mkdir(parents=True) built_hook = built_hooks_dir / "install" - project_hook = project_assets_dir / "component" / "firstcomponent" / "hooks" / "configure" + project_hook = project_assets_dir / "component" / "firstcomponent" / "hooks" / "install" project_hook.parent.mkdir(parents=True) built_hook.write_text("built_install", encoding="utf-8") - project_hook.write_text("project_configure", encoding="utf-8") + project_hook.write_text("project_install", encoding="utf-8") built_hook.chmod(0o755) project_hook.chmod(0o755) assert package_service._get_hook_assets("firstcomponent") == [ - (built_hook, component_prime_dir / "meta" / "hooks" / "install"), - (project_hook, component_prime_dir / "meta" / "hooks" / "configure"), + (project_hook, component_prime_dir / "meta" / "hooks" / "install"), ("#!/bin/true\n", component_prime_dir / "meta" / "hooks" / "post-refresh"), ] From 47a34c795cf9442371126ce7d0499790c2a666a7 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 09:53:15 -0300 Subject: [PATCH 09/23] chore: handle icon retrieval errors Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 10 +++++++- tests/unit/services/test_package.py | 39 ++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index cf5c7c5a50..4ea9efd22f 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -669,7 +669,15 @@ def _resolve_icon_asset( if parsed_url.scheme in ["http", "https"]: emit.progress(f"Fetching icon from {icon!r}") - icon_data = requests.get(icon, timeout=self._REMOTE_FETCH_TIMEOUT).content + try: + response = requests.get(icon, timeout=self._REMOTE_FETCH_TIMEOUT) + response.raise_for_status() + except requests.RequestException as err: + raise errors.SnapcraftError( + f"Failed to fetch icon from {icon!r}.", details=str(err) + ) from err + + icon_data = response.content return (icon_data, target_icon_path) if parsed_url.scheme: diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 89c95f4042..98de106759 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -29,8 +29,9 @@ from craft_application import ServiceFactory from craft_cli.pytest_plugin import RecordingEmitter from pytest_mock import MockerFixture +import requests -from snapcraft import __version__, const, linters, meta, models, pack +from snapcraft import __version__, const, errors, linters, meta, models, pack from snapcraft.errors import SnapcraftPrecreationEscapesPrimeError from snapcraft.meta import ExtractedMetadata from snapcraft.parts import extract_metadata, update_metadata @@ -389,6 +390,7 @@ def test_get_icon_assets_remote(monkeypatch, default_project, fake_services, set package_service = fake_services.get("package") mock_response = mocker.Mock() mock_response.content = b"png-data" + mock_response.raise_for_status.return_value = None mocker.patch("snapcraft.services.package.requests.get", return_value=mock_response) assert package_service._get_icon_assets() == [ @@ -396,6 +398,41 @@ def test_get_icon_assets_remote(monkeypatch, default_project, fake_services, set ] +def test_get_icon_assets_remote_http_error( + default_project, fake_services, setup_project, mocker +): + project_data = default_project.marshal() + project_data["icon"] = "https://example.com/icon.png" + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + mock_response = mocker.Mock() + mock_response.raise_for_status.side_effect = requests.HTTPError("404 Client Error") + mocker.patch("snapcraft.services.package.requests.get", return_value=mock_response) + + with pytest.raises(errors.SnapcraftError, match="Failed to fetch icon") as raised: + package_service._get_icon_assets() + + assert raised.value.details == "404 Client Error" + + +def test_get_icon_assets_remote_request_error( + default_project, fake_services, setup_project, mocker +): + project_data = default_project.marshal() + project_data["icon"] = "https://example.com/icon.png" + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + mocker.patch( + "snapcraft.services.package.requests.get", + side_effect=requests.RequestException("temporary failure in name resolution"), + ) + + with pytest.raises(errors.SnapcraftError, match="Failed to fetch icon") as raised: + package_service._get_icon_assets() + + assert raised.value.details == "temporary failure in name resolution" + + def test_get_system_metadata_assets_gadget( default_project, fake_services, setup_project, in_project_path, tmp_path ): From da832714b26660564923746bd4d227f880d37984 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 09:56:52 -0300 Subject: [PATCH 10/23] chore: update docstring Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index 4ea9efd22f..84f3307afb 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -814,9 +814,12 @@ def _manifest_changed(self, partition_name: str | None) -> bool: @override def write_metadata(self, path: pathlib.Path) -> None: - """Write the project metadata to metadata.yaml in the given directory. + """Write mediated package metadata and assets into the given prime directory. - :param path: The path to the prime directory. + This includes generated package files and mediated assets such as hooks, + desktop files, icons, and other metadata content. + + :param path: The prime directory to update. """ path.mkdir(parents=True, exist_ok=True) From 7303ca3e1bc5d6efe4ba7f0776faedcc180249a9 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 10:08:44 -0300 Subject: [PATCH 11/23] chore: address linter warnings Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 2 +- tests/unit/parts/test_setup_assets.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index 84f3307afb..e447012ad6 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -644,7 +644,7 @@ def _get_project_assets( return assets - def _resolve_icon_asset( + def _resolve_icon_asset( # noqa: PLR0911 self, partition_name: str | None = None ) -> tuple[bytes | pathlib.Path, pathlib.Path] | None: """Resolve the icon asset that should be added to prime.""" diff --git a/tests/unit/parts/test_setup_assets.py b/tests/unit/parts/test_setup_assets.py index 350ddd5272..4a5d6457d5 100644 --- a/tests/unit/parts/test_setup_assets.py +++ b/tests/unit/parts/test_setup_assets.py @@ -28,9 +28,9 @@ create_hook_wrappers, ensure_hook, ensure_hook_executable, + setup_assets, validate_command_chain, write_hook_wrapper, - setup_assets, ) From d0ae763f26632b67376284598a0c104289923dc3 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 10:09:11 -0300 Subject: [PATCH 12/23] chore: address linter warnings Signed-off-by: Claudio Matsuoka --- tests/unit/services/test_package.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 98de106759..b64f0e8d2a 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -25,11 +25,11 @@ from typing import Any, cast import pytest +import requests import yaml from craft_application import ServiceFactory from craft_cli.pytest_plugin import RecordingEmitter from pytest_mock import MockerFixture -import requests from snapcraft import __version__, const, errors, linters, meta, models, pack from snapcraft.errors import SnapcraftPrecreationEscapesPrimeError From f91dfc0371b1094bbef5ad4abc98b5e201de35b4 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 10:13:10 -0300 Subject: [PATCH 13/23] chore: address linter warnings Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index e447012ad6..a5fe3e247f 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -236,7 +236,8 @@ def _maybe_get_target_in_snap(path: str) -> pathlib.Path | None: exist (no-op). :param path: String path to investigate - :returns: A Path object that needs to be created, or None if nothing needs to be done.""" + :returns: A Path object that needs to be created, or None if nothing needs to be done. + """ # Make explicit references to the snap root ($SNAP) relative if path.startswith("$SNAP/"): path = path.removeprefix("$SNAP/") @@ -345,10 +346,14 @@ def _prime_dir_for(self, partition_name: str | None) -> pathlib.Path: @override def get_artifacts(self) -> dict[str | None, pathlib.Path]: """Get the expected output artifacts for the current pack operation.""" - artifacts: dict[str | None, pathlib.Path] = {None: self._get_default_artifact_path()} + artifacts: dict[str | None, pathlib.Path] = { + None: self._get_default_artifact_path() + } for component_name in self._project.get_component_names(): - artifacts[component_name] = self._get_component_artifact_path(component_name) + artifacts[component_name] = self._get_component_artifact_path( + component_name + ) return artifacts @@ -499,7 +504,9 @@ def _gen_extra_assets( if partition_name in (None, "default"): project_file = self._services.get("project").resolve_project_file_path() - destination = self._prime_dir_for(partition_name) / "snap" / project_file.name + destination = ( + self._prime_dir_for(partition_name) / "snap" / project_file.name + ) source: pathlib.Path | None = ( project_file if self._project_file_copy_enabled() else None ) @@ -596,7 +603,9 @@ def _get_hook_assets( prime_dir = self._prime_dir_for(partition_name) for hook_name in self._get_declared_hooks(partition_name): if hook_name not in existing_hooks: - assets.append((self._HOOK_STUB, prime_dir / "meta" / "hooks" / hook_name)) + assets.append( + (self._HOOK_STUB, prime_dir / "meta" / "hooks" / hook_name) + ) return assets @@ -706,7 +715,9 @@ def _get_effective_icon_path(self, partition_name: str | None = None) -> str | N _source, destination = icon_asset return str(destination.relative_to(self._prime_dir_for(partition_name))) - def _get_partition_assets_dir(self, partition_name: str | None = None) -> pathlib.Path: + def _get_partition_assets_dir( + self, partition_name: str | None = None + ) -> pathlib.Path: """Return the project assets directory for a default or component partition.""" assets_dir = self._get_assets_dir() if partition_name in (None, "default"): From 6b466d11e14d4ae24bb58245bb708466b3f2b688 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 10:22:24 -0300 Subject: [PATCH 14/23] chore: address linter warnings Signed-off-by: Claudio Matsuoka --- tests/unit/services/test_package.py | 105 ++++++++++++---------------- 1 file changed, 44 insertions(+), 61 deletions(-) diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index b64f0e8d2a..6060d7a495 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -120,8 +120,7 @@ def test_get_snap_yaml(default_project, fake_services, setup_project): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") - assert package_service._get_snap_yaml() == dedent( - """\ + assert package_service._get_snap_yaml() == dedent("""\ name: default version: '1.0' summary: default project @@ -135,8 +134,7 @@ def test_get_snap_yaml(default_project, fake_services, setup_project): environment: LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH - """ - ) + """) def test_get_artifacts(default_project, fake_services, setup_project, tmp_path): @@ -144,9 +142,7 @@ def test_get_artifacts(default_project, fake_services, setup_project, tmp_path): package_service = fake_services.get("package") package_service.set_output_dir(tmp_path / "test-output.snap") - assert package_service.get_artifacts() == { - None: tmp_path / "test-output.snap" - } + assert package_service.get_artifacts() == {None: tmp_path / "test-output.snap"} def test_get_artifacts_defaults_to_cwd( @@ -161,7 +157,9 @@ def test_get_artifacts_defaults_to_cwd( } -def test_pack_artifact_snap(default_project, fake_services, setup_project, mocker, tmp_path): +def test_pack_artifact_snap( + default_project, fake_services, setup_project, mocker, tmp_path +): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") mock_pack_snap = mocker.patch.object(pack, "pack_snap") @@ -279,7 +277,10 @@ def test_gen_extra_assets_includes_default_hook_assets( extra_assets = package_service._gen_extra_assets() - assert (built_hooks_dir / "configure", tmp_path / "prime" / "meta" / "hooks" / "configure") in extra_assets + assert ( + built_hooks_dir / "configure", + tmp_path / "prime" / "meta" / "hooks" / "configure", + ) in extra_assets assert ( project_hooks_dir / "install", tmp_path / "prime" / "meta" / "hooks" / "install", @@ -324,7 +325,9 @@ def test_write_metadata_project_hook_overrides_newer_built_hook( ) -def test_get_gui_assets(default_project, fake_services, setup_project, in_project_path, tmp_path): +def test_get_gui_assets( + default_project, fake_services, setup_project, in_project_path, tmp_path +): setup_project(fake_services, default_project.marshal(), write_project=True) package_service = fake_services.get("package") project_gui_dir = in_project_path / "snap" / "gui" @@ -342,9 +345,7 @@ def test_get_gui_assets(default_project, fake_services, setup_project, in_projec def test_get_desktop_assets(default_project, fake_services, setup_project, tmp_path): project_data = default_project.marshal() - project_data["apps"] = { - "app1": {"command": "bin/test", "desktop": "test.desktop"} - } + project_data["apps"] = {"app1": {"command": "bin/test", "desktop": "test.desktop"}} setup_project(fake_services, project_data, write_project=True) package_service = fake_services.get("package") prime_dir = tmp_path / "prime" @@ -352,15 +353,13 @@ def test_get_desktop_assets(default_project, fake_services, setup_project, tmp_p (prime_dir / "bin" / "test").write_text("#!/bin/true\n", encoding="utf-8") (prime_dir / "bin" / "test").chmod(0o755) (prime_dir / "test.desktop").write_text( - dedent( - """\ + dedent("""\ [Desktop Entry] Name=test Exec=test Type=Application Icon=/usr/share/icons/test.png - """ - ), + """), encoding="utf-8", ) (prime_dir / "usr/share/icons").mkdir(parents=True) @@ -368,22 +367,22 @@ def test_get_desktop_assets(default_project, fake_services, setup_project, tmp_p assert package_service._get_desktop_assets() == [ ( - dedent( - """\ + dedent("""\ [Desktop Entry] Name=test Exec=default.app1 Type=Application Icon=${SNAP}/usr/share/icons/test.png - """ - ), + """), tmp_path / "prime" / "meta" / "gui" / "app1.desktop", ) ] -def test_get_icon_assets_remote(monkeypatch, default_project, fake_services, setup_project, mocker, tmp_path): +def test_get_icon_assets_remote( + monkeypatch, default_project, fake_services, setup_project, mocker, tmp_path +): project_data = default_project.marshal() project_data["icon"] = "https://example.com/icon.png" setup_project(fake_services, project_data, write_project=True) @@ -477,9 +476,7 @@ def test_manifest_changed_ignores_started_at( manifest = yaml.safe_load(cast("str", package_service._get_manifest_yaml())) manifest["snapcraft-started-at"] = "2001-02-03T04:05:06Z" - (snap_dir / "manifest.yaml").write_text( - yaml.safe_dump(manifest), encoding="utf-8" - ) + (snap_dir / "manifest.yaml").write_text(yaml.safe_dump(manifest), encoding="utf-8") assert package_service._manifest_changed(None) is False @@ -488,9 +485,7 @@ def test_needs_packing_when_write_metadata_changes_prime( default_project, fake_services, setup_project, tmp_path ): project_data = default_project.marshal() - project_data["apps"] = { - "app1": {"command": "bin/test", "desktop": "test.desktop"} - } + project_data["apps"] = {"app1": {"command": "bin/test", "desktop": "test.desktop"}} setup_project(fake_services, project_data, write_project=True) package_service = fake_services.get("package") artifact_path = tmp_path / "test-output.snap" @@ -501,14 +496,12 @@ def test_needs_packing_when_write_metadata_changes_prime( (prime_dir / "bin" / "test").write_text("#!/bin/true\n", encoding="utf-8") (prime_dir / "bin" / "test").chmod(0o755) (prime_dir / "test.desktop").write_text( - dedent( - """\ + dedent("""\ [Desktop Entry] Name=test Exec=test Type=Application - """ - ), + """), encoding="utf-8", ) @@ -544,8 +537,7 @@ def test_write_metadata(default_project, fake_services, setup_project, new_dir): package_service.write_metadata(prime_dir) - assert (meta_dir / "snap.yaml").read_text() == dedent( - """\ + assert (meta_dir / "snap.yaml").read_text() == dedent("""\ name: default version: '1.0' summary: default project @@ -559,8 +551,7 @@ def test_write_metadata(default_project, fake_services, setup_project, new_dir): environment: LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH - """ - ) + """) assert not (prime_dir / "snap" / "manifest.yaml").exists() @@ -609,7 +600,9 @@ def test_write_metadata_with_manifest( assert manifest.grade == snap_yaml["grade"] assert manifest.architectures == snap_yaml["architectures"] project_file = fake_services.get("project").resolve_project_file_path() - assert (prime_dir / "snap" / project_file.name).read_text() == project_file.read_text() + assert ( + prime_dir / "snap" / project_file.name + ).read_text() == project_file.read_text() def test_write_metadata_removes_project_file_when_disabled( @@ -645,8 +638,7 @@ def test_write_metadata_with_project_hooks( package_service.write_metadata(prime_dir) - assert (meta_dir / "snap.yaml").read_text() == dedent( - """\ + assert (meta_dir / "snap.yaml").read_text() == dedent("""\ name: default version: '1.0' summary: default project @@ -660,8 +652,7 @@ def test_write_metadata_with_project_hooks( environment: LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH - """ - ) + """) assert (meta_dir / "hooks").exists() # Ensure the hook is the one we provided in the project @@ -691,8 +682,7 @@ def test_write_metadata_with_built_hooks( package_service.write_metadata(prime_dir) meta_dir = prime_dir / "meta" - assert (meta_dir / "snap.yaml").read_text() == dedent( - """\ + assert (meta_dir / "snap.yaml").read_text() == dedent("""\ name: default version: '1.0' summary: default project @@ -706,8 +696,7 @@ def test_write_metadata_with_built_hooks( environment: LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH - """ - ) + """) assert (meta_dir / "hooks").exists() # Ensure the hook is the one we provided in the project @@ -769,8 +758,7 @@ def test_write_metadata_with_project_gui( package_service.write_metadata(prime_dir) - assert (meta_dir / "snap.yaml").read_text() == dedent( - """\ + assert (meta_dir / "snap.yaml").read_text() == dedent("""\ name: default version: '1.0' summary: default project @@ -784,8 +772,7 @@ def test_write_metadata_with_project_gui( environment: LD_LIBRARY_PATH: ${SNAP_LIBRARY_PATH}${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH - """ - ) + """) assert (meta_dir / "gui").exists() # Ensure the hook is the one we provided in the project @@ -800,9 +787,7 @@ def test_write_metadata_generates_desktop_and_marks_project_updated( default_project, fake_services, setup_project, tmp_path ): project_data = default_project.marshal() - project_data["apps"] = { - "app1": {"command": "bin/test", "desktop": "test.desktop"} - } + project_data["apps"] = {"app1": {"command": "bin/test", "desktop": "test.desktop"}} setup_project(fake_services, project_data, write_project=True) package_service = fake_services.get("package") prime_dir = tmp_path / "prime" @@ -810,28 +795,24 @@ def test_write_metadata_generates_desktop_and_marks_project_updated( (prime_dir / "bin" / "test").write_text("#!/bin/true\n", encoding="utf-8") (prime_dir / "bin" / "test").chmod(0o755) (prime_dir / "test.desktop").write_text( - dedent( - """\ + dedent("""\ [Desktop Entry] Name=test Exec=test Type=Application - """ - ), + """), encoding="utf-8", ) package_service.write_metadata(prime_dir) - assert (prime_dir / "meta" / "gui" / "app1.desktop").read_text() == dedent( - """\ + assert (prime_dir / "meta" / "gui" / "app1.desktop").read_text() == dedent("""\ [Desktop Entry] Name=test Exec=default.app1 Type=Application - """ - ) + """) assert package_service._project_was_updated is True @@ -956,7 +937,9 @@ def test_extra_project_updates_core26_no_precreate_changes_not_marked_updated( ) -> None: setup_project(fake_services, snapcraft_yaml(base="core26")) package_service = fake_services.get("package") - mocker.patch.object(package_service, "_precreate_layout_targets", return_value=False) + mocker.patch.object( + package_service, "_precreate_layout_targets", return_value=False + ) mocker.patch.object(package_service, "_precreate_plug_targets", return_value=False) package_service.update_project() From 0ac9ce17d3c3a996dced3d4028ad86922ffd868d Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 10:29:36 -0300 Subject: [PATCH 15/23] chore: address linter warnings Signed-off-by: Claudio Matsuoka --- tests/unit/services/test_assertions.py | 28 ++++---------- tests/unit/services/test_confdbs.py | 18 +++------ .../unit/services/test_package_components.py | 38 ++++++++++--------- tests/unit/services/test_validationsets.py | 12 ++---- 4 files changed, 39 insertions(+), 57 deletions(-) diff --git a/tests/unit/services/test_assertions.py b/tests/unit/services/test_assertions.py index cc32c4b895..579ebd11a6 100644 --- a/tests/unit/services/test_assertions.py +++ b/tests/unit/services/test_assertions.py @@ -169,23 +169,19 @@ def _normalize_assertions( @override def _generate_yaml_from_model(self, assertion: FakeAssertion) -> str: - return textwrap.dedent( - """\ + return textwrap.dedent("""\ test-field-1: test-value-1 test-field-2: 0 - """ - ) + """) @override def _generate_yaml_from_template( self, name: str, account_id: str, **kwargs: dict[str, Any] ) -> str: - return textwrap.dedent( - """\ + return textwrap.dedent("""\ test-field-1: default-value-1 test-field-2: 0 - """ - ) + """) @override def _get_success_message(self, assertion: FakeAssertion) -> str: @@ -210,14 +206,10 @@ def test_list_assertions_table(fake_assertion_service, emitter): output_format=const.OutputFormat.table, name="test-confb" ) - emitter.assert_message( - textwrap.dedent( - """\ + emitter.assert_message(textwrap.dedent("""\ test-field-1 test-field-2 test-value-1 0 - test-value-2 100""" - ) - ) + test-value-2 100""")) def test_list_assertions_json(fake_assertion_service, emitter): @@ -226,9 +218,7 @@ def test_list_assertions_json(fake_assertion_service, emitter): output_format=const.OutputFormat.json, name="test-confb" ) - emitter.assert_message( - textwrap.dedent( - """\ + emitter.assert_message(textwrap.dedent("""\ { "fake assertions": [ { @@ -240,9 +230,7 @@ def test_list_assertions_json(fake_assertion_service, emitter): "test-field-2": 100 } ] - }""" - ) - ) + }""")) def test_list_assertions_unknown_format(fake_assertion_service): diff --git a/tests/unit/services/test_confdbs.py b/tests/unit/services/test_confdbs.py index c4d623bd28..0617c3471a 100644 --- a/tests/unit/services/test_confdbs.py +++ b/tests/unit/services/test_confdbs.py @@ -130,8 +130,7 @@ def test_generate_yaml_from_model(fake_confdb_schema_assertion, fake_services): ) yaml_data = confdb_schemas_service._generate_yaml_from_model(assertion) - assert yaml_data == textwrap.dedent( - """\ + assert yaml_data == textwrap.dedent("""\ account-id: test-account-id name: test-confdb # The revision for this confdb-schema @@ -158,8 +157,7 @@ def test_generate_yaml_from_model(fake_confdb_schema_assertion, fake_services): } } - """ - ) + """) def test_generate_yaml_from_model_with_summary( @@ -195,8 +193,7 @@ def test_generate_yaml_from_model_with_summary( ) yaml_data = confdb_schemas_service._generate_yaml_from_model(assertion) - assert yaml_data == textwrap.dedent( - """\ + assert yaml_data == textwrap.dedent("""\ account-id: test-account-id name: test-confdb summary: This is a test confdb-schema summary. @@ -225,8 +222,7 @@ def test_generate_yaml_from_model_with_summary( } } - """ - ) + """) def test_generate_yaml_from_template(fake_services): @@ -236,8 +232,7 @@ def test_generate_yaml_from_template(fake_services): name="test-confdb", account_id="test-account-id" ) - expected_yaml = textwrap.dedent( - """\ + expected_yaml = textwrap.dedent("""\ account-id: test-account-id name: test-confdb summary: Summary of the confdb-schema @@ -261,8 +256,7 @@ def test_generate_yaml_from_template(fake_services): } } } - """ - ) + """) assert yaml_data.strip() == expected_yaml.strip() diff --git a/tests/unit/services/test_package_components.py b/tests/unit/services/test_package_components.py index 630888cab4..b1a3a21714 100644 --- a/tests/unit/services/test_package_components.py +++ b/tests/unit/services/test_package_components.py @@ -200,25 +200,21 @@ def test_get_component_yaml(default_project, fake_services, setup_project): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") - assert package_service._get_component_yaml("firstcomponent") == dedent( - """\ + assert package_service._get_component_yaml("firstcomponent") == dedent("""\ component: default+firstcomponent type: test version: '1.0' summary: first component description: lorem ipsum - """ - ) + """) - assert package_service._get_component_yaml("secondcomponent") == dedent( - """\ + assert package_service._get_component_yaml("secondcomponent") == dedent("""\ component: default+secondcomponent type: test version: '1.0' summary: second component description: lorem ipsum - """ - ) + """) @pytest.mark.usefixtures("enable_partitions_feature") @@ -227,11 +223,15 @@ def test_get_hook_assets_for_component( ): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") - component_prime_dir = tmp_path / "partitions" / "component" / "firstcomponent" / "prime" + component_prime_dir = ( + tmp_path / "partitions" / "component" / "firstcomponent" / "prime" + ) built_hooks_dir = component_prime_dir / "snap" / "hooks" built_hooks_dir.mkdir(parents=True) built_hook = built_hooks_dir / "install" - project_hook = project_assets_dir / "component" / "firstcomponent" / "hooks" / "install" + project_hook = ( + project_assets_dir / "component" / "firstcomponent" / "hooks" / "install" + ) project_hook.parent.mkdir(parents=True) built_hook.write_text("built_install", encoding="utf-8") project_hook.write_text("project_install", encoding="utf-8") @@ -250,8 +250,12 @@ def test_get_gui_assets_for_component( ): setup_project(fake_services, default_project.marshal()) package_service = fake_services.get("package") - component_prime_dir = tmp_path / "partitions" / "component" / "firstcomponent" / "prime" - desktop = project_assets_dir / "component" / "firstcomponent" / "gui" / "first.desktop" + component_prime_dir = ( + tmp_path / "partitions" / "component" / "firstcomponent" / "prime" + ) + desktop = ( + project_assets_dir / "component" / "firstcomponent" / "gui" / "first.desktop" + ) icon = project_assets_dir / "component" / "firstcomponent" / "gui" / "icon.png" desktop.parent.mkdir(parents=True) desktop.write_text("desktop_file", encoding="utf-8") @@ -302,8 +306,7 @@ def test_write_metadata( package_service.write_metadata(prime_dir) - assert (meta_dir / "snap.yaml").read_text() == dedent( - """\ + assert (meta_dir / "snap.yaml").read_text() == dedent("""\ name: default version: '1.0' summary: default project @@ -341,8 +344,7 @@ def test_write_metadata( summary: second component description: lorem ipsum type: test - """ - ) + """) assert ( lifecycle_service.get_prime_dir("firstcomponent") / "meta" / "component.yaml" @@ -418,7 +420,9 @@ def test_pack_component_makes_organized_meta_hook_executable( mocker.patch.object(linters, "run_linters") mocker.patch.object(linters, "report") - mock_pack_component = mocker.patch.object(pack, "pack_component", return_value="default+firstcomponent_1.0.comp") + mock_pack_component = mocker.patch.object( + pack, "pack_component", return_value="default+firstcomponent_1.0.comp" + ) package_service._pack( name="firstcomponent", diff --git a/tests/unit/services/test_validationsets.py b/tests/unit/services/test_validationsets.py index 5fbc79d448..947f5d98e5 100644 --- a/tests/unit/services/test_validationsets.py +++ b/tests/unit/services/test_validationsets.py @@ -113,8 +113,7 @@ def test_generate_yaml_from_model(fake_validation_set_assertion, fake_services): fake_validation_set_assertion() ) - assert yaml_data == textwrap.dedent( - """\ + assert yaml_data == textwrap.dedent("""\ account-id: test-account-id name: test-validation-set sequence: 5 @@ -130,8 +129,7 @@ def test_generate_yaml_from_model(fake_validation_set_assertion, fake_services): presence: required revision: 10 component-without-revision: invalid - """ - ) + """) def test_generate_yaml_from_template(fake_services): @@ -141,8 +139,7 @@ def test_generate_yaml_from_template(fake_services): name="test-validation-set", account_id="test-account-id", sequence=100 ) - expected_yaml = textwrap.dedent( - """\ + expected_yaml = textwrap.dedent("""\ account-id: test-account-id name: test-validation-set sequence: 100 @@ -160,8 +157,7 @@ def test_generate_yaml_from_template(fake_services): # presence: [required|optional|invalid] # Presence of the component. Required. # revision: # The revision of the component, required if the snap's revision is given. # Otherwise, not allowed. - """ - ) + """) assert yaml_data.strip() == expected_yaml.strip() From 4a7f0b5cbada0f9ecf9559906b576a48cc74a529 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 11:32:50 -0300 Subject: [PATCH 16/23] chore: address linter warnings Signed-off-by: Claudio Matsuoka --- tests/unit/services/test_assertions.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/unit/services/test_assertions.py b/tests/unit/services/test_assertions.py index 579ebd11a6..8b6e802255 100644 --- a/tests/unit/services/test_assertions.py +++ b/tests/unit/services/test_assertions.py @@ -206,10 +206,12 @@ def test_list_assertions_table(fake_assertion_service, emitter): output_format=const.OutputFormat.table, name="test-confb" ) - emitter.assert_message(textwrap.dedent("""\ + emitter.assert_message( + textwrap.dedent("""\ test-field-1 test-field-2 test-value-1 0 - test-value-2 100""")) + test-value-2 100""") + ) def test_list_assertions_json(fake_assertion_service, emitter): @@ -218,7 +220,8 @@ def test_list_assertions_json(fake_assertion_service, emitter): output_format=const.OutputFormat.json, name="test-confb" ) - emitter.assert_message(textwrap.dedent("""\ + emitter.assert_message( + textwrap.dedent("""\ { "fake assertions": [ { @@ -230,7 +233,8 @@ def test_list_assertions_json(fake_assertion_service, emitter): "test-field-2": 100 } ] - }""")) + }""") + ) def test_list_assertions_unknown_format(fake_assertion_service): From a536eb99327cf9cbe13df58d057ad2d789aa91f6 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 11:35:38 -0300 Subject: [PATCH 17/23] chore: address linter warning Signed-off-by: Claudio Matsuoka --- tests/unit/cli/test_version.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/unit/cli/test_version.py b/tests/unit/cli/test_version.py index c6e99ce0e4..0c71ab70fa 100644 --- a/tests/unit/cli/test_version.py +++ b/tests/unit/cli/test_version.py @@ -30,10 +30,7 @@ def test_version_command(mocker): "craft_application.commands.other.VersionCommand.run" ) app.run() - assert mock_version_cmd.mock_calls == [ - call(argparse.Namespace()), - call().__bool__(), - ] + assert mock_version_cmd.mock_calls == [call(argparse.Namespace())] def test_version_argument(mocker, emitter): From b7f8d083ad20160d412462183a4bb0740ff1a0b8 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 12:03:12 -0300 Subject: [PATCH 18/23] chore: remove unused methods Signed-off-by: Claudio Matsuoka --- snapcraft/commands/lifecycle.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/snapcraft/commands/lifecycle.py b/snapcraft/commands/lifecycle.py index 1d4b014725..7ba95755f3 100644 --- a/snapcraft/commands/lifecycle.py +++ b/snapcraft/commands/lifecycle.py @@ -84,19 +84,6 @@ def needs_project(self, parsed_args: argparse.Namespace) -> bool: emit.debug("Loading project because a directory was not provided.") return True - @override - def run_managed(self, parsed_args: argparse.Namespace) -> bool: - """Return whether the command should run in managed mode or not. - - Packing a directory always runs locally. - """ - if parsed_args.directory: - emit.debug("Not running managed mode because a directory was provided.") - return False - - return super().run_managed(parsed_args) - - class TryCommand(PackCommand): """Prepare the parts for ``snap try``.""" @@ -109,11 +96,6 @@ class TryCommand(PackCommand): """ ) - @override - def run_managed(self, parsed_args: argparse.Namespace) -> bool: - """Overridden to return false, such that the command fails early.""" - return False - @override def _run( self, From a8cf43e8841b79a5ed7b87651cf0385e98ccf712 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 18:12:23 -0300 Subject: [PATCH 19/23] chore: address linter warning Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index a5fe3e247f..f078969386 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -592,11 +592,14 @@ def _get_hook_assets( Project-provided hooks override built hooks with the same filename. """ - assets = self._get_project_assets( - partition_name, - source_subdir="hooks", - destination_subdir="meta/hooks", - include_built_subdir="snap/hooks", + assets: list[tuple[str | pathlib.Path, pathlib.Path]] = [] + assets.extend( + self._get_project_assets( + partition_name, + source_subdir="hooks", + destination_subdir="meta/hooks", + include_built_subdir="snap/hooks", + ) ) existing_hooks = {destination.name for _source, destination in assets} From 159cc2d7e203e837926216b3c48660f67f019429 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 18:16:48 -0300 Subject: [PATCH 20/23] chore: fix mediated icon asset error Signed-off-by: Claudio Matsuoka --- snapcraft/parts/desktop_file.py | 7 +++++ tests/unit/services/test_package.py | 44 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/snapcraft/parts/desktop_file.py b/snapcraft/parts/desktop_file.py index 7395688030..379d516f19 100644 --- a/snapcraft/parts/desktop_file.py +++ b/snapcraft/parts/desktop_file.py @@ -89,6 +89,13 @@ def _parse_and_reformat_section( if icon_path is not None: icon = icon_path + # An explicit icon path is a mediated asset destination that may + # not have been written into prime yet. + icon = icon[1:] if icon.startswith("/") else icon + icon = icon[8:] if icon.startswith("${SNAP}") else icon + self._parser[section]["Icon"] = os.path.join("${SNAP}", icon) + return + # Strip any leading slash. icon = icon[1:] if icon.startswith("/") else icon diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 6060d7a495..1dc692f86a 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -380,6 +380,50 @@ def test_get_desktop_assets(default_project, fake_services, setup_project, tmp_p ] +def test_get_desktop_assets_with_mediated_icon( + default_project, fake_services, setup_project, tmp_path +): + project_data = default_project.marshal() + project_data["icon"] = "/usr/share/icons/test.svg" + project_data["apps"] = {"app1": {"command": "bin/test", "desktop": "test.desktop"}} + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + prime_dir = tmp_path / "prime" + (prime_dir / "bin").mkdir(parents=True) + (prime_dir / "bin" / "test").write_text("#!/bin/true\n", encoding="utf-8") + (prime_dir / "bin" / "test").chmod(0o755) + (prime_dir / "test.desktop").write_text( + dedent( + """\ + [Desktop Entry] + Name=test + Exec=test + Type=Application + Icon=/usr/share/icons/test.svg + """ + ), + encoding="utf-8", + ) + (prime_dir / "usr/share/icons").mkdir(parents=True) + (prime_dir / "usr/share/icons" / "test.svg").write_text("icon", encoding="utf-8") + + assert package_service._get_desktop_assets() == [ + ( + dedent( + """\ + [Desktop Entry] + Name=test + Exec=default.app1 + Type=Application + Icon=${SNAP}/meta/gui/icon.svg + + """ + ), + tmp_path / "prime" / "meta" / "gui" / "app1.desktop", + ) + ] + + def test_get_icon_assets_remote( monkeypatch, default_project, fake_services, setup_project, mocker, tmp_path ): From f2e820225c55b7a08c6c19f795fbd38d2b1ebbed Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 18:20:50 -0300 Subject: [PATCH 21/23] chore: address linter warning Signed-off-by: Claudio Matsuoka --- snapcraft/commands/lifecycle.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/snapcraft/commands/lifecycle.py b/snapcraft/commands/lifecycle.py index 7ba95755f3..9e01309641 100644 --- a/snapcraft/commands/lifecycle.py +++ b/snapcraft/commands/lifecycle.py @@ -35,13 +35,11 @@ class PackCommand(craft_application.commands.lifecycle.PackCommand): name = "pack" help_msg = "Create the final artifact" - overview = textwrap.dedent( - """ + overview = textwrap.dedent(""" Process parts and create a snap file containing the project payload with the provided metadata. If a directory is specified, pack its contents instead. - """ - ) + """) @override def _fill_parser(self, parser: argparse.ArgumentParser) -> None: @@ -84,17 +82,16 @@ def needs_project(self, parsed_args: argparse.Namespace) -> bool: emit.debug("Loading project because a directory was not provided.") return True + class TryCommand(PackCommand): """Prepare the parts for ``snap try``.""" name = "try" help_msg = 'Prepare a snap for "snap try".' - overview = textwrap.dedent( - """ + overview = textwrap.dedent(""" Process parts and expose the ``prime`` directory containing the final payload, in preparation for ``snap try prime``. - """ - ) + """) @override def _run( From bfbbb1ed805ec7254b9f94fc59c25d9acf1cbbde Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 21:16:22 -0300 Subject: [PATCH 22/23] chore: handle empty environment variables Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 18 +++++- tests/spread/general/skip-repack/task.yaml | 6 +- tests/unit/conftest.py | 9 ++- tests/unit/services/test_package.py | 72 ++++++++++++++++++++++ 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index f078969386..c231f072aa 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -885,12 +885,28 @@ def _ensure_hook_assets_executable(self, partition_name: str | None = None) -> b @property def metadata(self) -> snap_yaml.SnapMetadata: """Get the metadata model for this project.""" - return snap_yaml.get_metadata_from_project( + metadata = snap_yaml.get_metadata_from_project( self._project, self._services.lifecycle.prime_dir, arch=self._build_for, ) + raw_environment = self._project_service.get_raw().get("environment") + if not isinstance(raw_environment, dict): + return metadata + + if metadata.environment is None: + metadata.environment = {} + + for key, value in raw_environment.items(): + if value is None and metadata.environment is not None: + metadata.environment.pop(key, None) + + if metadata.environment == {}: + metadata.environment = None + + return metadata + def _hardlink_or_copy(source: pathlib.Path, destination: pathlib.Path) -> bool: """Try to hardlink and fallback to copy if it fails. diff --git a/tests/spread/general/skip-repack/task.yaml b/tests/spread/general/skip-repack/task.yaml index 5e11c2f0ab..5774e631ef 100644 --- a/tests/spread/general/skip-repack/task.yaml +++ b/tests/spread/general/skip-repack/task.yaml @@ -18,7 +18,11 @@ execute: | echo "Change" >> snap/icon.svg snapcraft pack 2>&1 | MATCH "Packing" snapcraft pack 2>&1 | MATCH "Skipping pack" - echo "echo hello world" >> snap/hooks/configure + mkdir -p snap/hooks + cat <<'EOF' > snap/hooks/configure + #!/bin/sh + exit 0 + EOF chmod +x snap/hooks/configure snapcraft pack 2>&1 | MATCH "Packing" diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 3732c4f518..ab2313fbfd 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -589,7 +589,14 @@ class FakeProjectService(services.Project): # This is a final method, but we're overriding it here for convenience when # doing internal testing. def _load_raw_project(self): # ty: ignore[override-of-final-method] - return fake_project.marshal() + project = fake_project.marshal() + self._app_preprocess_project( + project, + build_on="amd64", + build_for="amd64", + platform="amd64", + ) + return project # Don't care if the project file exists during this testing. # Silencing B019 because we're replicating an inherited method. diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 1dc692f86a..0323a9ab40 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -600,6 +600,78 @@ def test_write_metadata(default_project, fake_services, setup_project, new_dir): assert not (prime_dir / "snap" / "manifest.yaml").exists() +def test_write_metadata_omits_default_ld_library_path_when_null( + default_project, fake_services, setup_project, tmp_path +): + project_data = default_project.marshal() + project_data["confinement"] = "strict" + project_data["environment"] = { + "LD_LIBRARY_PATH": None, + "PATH": "$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH", + "TEST_VARIABLE": "test-1", + } + project_data["apps"] = {"paths-one-null": {"command": "usr/bin/hello"}} + setup_project(fake_services, project_data) + package_service = fake_services.get("package") + + package_service.write_metadata(tmp_path / "prime") + + assert (tmp_path / "prime" / "meta" / "snap.yaml").read_text() == dedent( + """\ + name: default + version: '1.0' + summary: default project + description: default project + license: MIT + architectures: + - amd64 + base: core24 + apps: + paths-one-null: + command: usr/bin/hello + confinement: strict + grade: devel + environment: + PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH + TEST_VARIABLE: test-1 + """ + ) + + +def test_write_metadata_omits_environment_block_when_all_default_entries_null( + default_project, fake_services, setup_project, tmp_path +): + project_data = default_project.marshal() + project_data["confinement"] = "strict" + project_data["environment"] = { + "LD_LIBRARY_PATH": None, + "PATH": None, + } + project_data["apps"] = {"paths-all-null": {"command": "usr/bin/hello"}} + setup_project(fake_services, project_data) + package_service = fake_services.get("package") + + package_service.write_metadata(tmp_path / "prime") + + assert (tmp_path / "prime" / "meta" / "snap.yaml").read_text() == dedent( + """\ + name: default + version: '1.0' + summary: default project + description: default project + license: MIT + architectures: + - amd64 + base: core24 + apps: + paths-all-null: + command: usr/bin/hello + confinement: strict + grade: devel + """ + ) + + def test_write_metadata_removes_manifest_when_disabled( monkeypatch, default_project, fake_services, setup_project, tmp_path ): From a23571ceda4e6d1b385b1019fef3bd697bc27f35 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Tue, 14 Jul 2026 21:28:56 -0300 Subject: [PATCH 23/23] chore: fix icon download issue Signed-off-by: Claudio Matsuoka --- snapcraft/services/package.py | 13 +++++++++ tests/unit/services/test_package.py | 44 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/snapcraft/services/package.py b/snapcraft/services/package.py index c231f072aa..0f83d79c47 100644 --- a/snapcraft/services/package.py +++ b/snapcraft/services/package.py @@ -711,6 +711,19 @@ def _resolve_icon_asset( # noqa: PLR0911 def _get_effective_icon_path(self, partition_name: str | None = None) -> str | None: """Return the path desktop rewrites should reference for the icon.""" + icon = self._project.icon + if icon is not None: + parsed_url = urllib.parse.urlparse(icon) + if parsed_url.scheme in ["http", "https"]: + icon_ext = pathlib.Path(parsed_url.path).suffix[1:] + destination = ( + self._prime_dir_for(partition_name) + / "meta" + / "gui" + / f"icon.{icon_ext}" + ) + return str(destination.relative_to(self._prime_dir_for(partition_name))) + icon_asset = self._resolve_icon_asset(partition_name) if icon_asset is None: return None diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py index 0323a9ab40..746f8abaaa 100644 --- a/tests/unit/services/test_package.py +++ b/tests/unit/services/test_package.py @@ -441,6 +441,50 @@ def test_get_icon_assets_remote( ] +def test_get_desktop_assets_with_remote_mediated_icon_does_not_fetch( + default_project, fake_services, setup_project, mocker, tmp_path +): + project_data = default_project.marshal() + project_data["icon"] = "https://example.com/icon.png" + project_data["apps"] = {"app1": {"command": "bin/test", "desktop": "test.desktop"}} + setup_project(fake_services, project_data, write_project=True) + package_service = fake_services.get("package") + prime_dir = tmp_path / "prime" + (prime_dir / "bin").mkdir(parents=True) + (prime_dir / "bin" / "test").write_text("#!/bin/true\n", encoding="utf-8") + (prime_dir / "bin" / "test").chmod(0o755) + (prime_dir / "test.desktop").write_text( + dedent( + """\ + [Desktop Entry] + Name=test + Exec=test + Type=Application + Icon=test.png + """ + ), + encoding="utf-8", + ) + requests_get = mocker.patch("snapcraft.services.package.requests.get") + + assert package_service._get_desktop_assets() == [ + ( + dedent( + """\ + [Desktop Entry] + Name=test + Exec=default.app1 + Type=Application + Icon=${SNAP}/meta/gui/icon.png + + """ + ), + tmp_path / "prime" / "meta" / "gui" / "app1.desktop", + ) + ] + requests_get.assert_not_called() + + def test_get_icon_assets_remote_http_error( default_project, fake_services, setup_project, mocker ):