Skip to content

skip repacking if snap content not changed - #6342

Draft
cmatsuoka wants to merge 26 commits into
mainfrom
work/SNAPCRAFT-1364-Skip-repacking-if-artifact-contents-are-unchanged
Draft

skip repacking if snap content not changed#6342
cmatsuoka wants to merge 26 commits into
mainfrom
work/SNAPCRAFT-1364-Skip-repacking-if-artifact-contents-are-unchanged

Conversation

@cmatsuoka

@cmatsuoka cmatsuoka commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Mediate creation of metadata and asset files to only repack artifacts
if prime or post-prime contents have been changed. This includes
project or part definitions, snap metadata, manifests, hooks, and icon
and desktop files.


  • I've followed the contribution guidelines.
  • I've signed the CLA.
  • I've successfully run make lint && make test.
  • I've added or updated any relevant documentation.
  • In documents I changed, I added a meta description if one was missing.
  • I've updated the relevant release notes.

Copilot AI review requested due to automatic review settings July 14, 2026 12:30
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 <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
@cmatsuoka
cmatsuoka force-pushed the work/SNAPCRAFT-1364-Skip-repacking-if-artifact-contents-are-unchanged branch from 049b0b5 to 9993fb3 Compare July 14, 2026 12:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Snapcraft’s packaging flow to avoid repacking artifacts when primed/post-prime contents haven’t meaningfully changed, by mediating generation of metadata and asset files (snap.yaml, component.yaml, manifest, hooks, GUI assets, desktop/icon handling) and relying on craft-application’s conditional repack support.

Changes:

  • Upgrades craft-application/craft-providers and sets always_repack=False to enable conditional repack behavior.
  • Refactors snapcraft.services.package.Package to generate package files/assets via mediated writers and change-detection (including manifest comparison that ignores snapcraft-started-at).
  • Adds extensive unit tests plus a new spread test to validate “skip repack” behavior and asset/hook handling (including executability).

Reviewed changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
uv.lock Locks upgraded craft-application/craft-providers versions used by the new packaging behavior.
pyproject.toml Bumps craft-application dependency to enable conditional repack support.
snapcraft/application.py Disables unconditional repacking via always_repack=False.
snapcraft/services/package.py Core implementation of mediated metadata/assets and conditional repack change detection.
snapcraft/parts/setup_assets.py Exposes asset helper functions for reuse by the package service.
snapcraft/parts/desktop_file.py Adds render() to allow desktop file rewriting without immediate filesystem writes.
snapcraft/meta/component_yaml.py Adds get_metadata() helper to support mediated component.yaml generation.
tests/unit/services/test_package.py Adds unit coverage for mediated snap metadata/assets, manifest drift handling, and change detection.
tests/unit/services/test_package_components.py Extends component packaging tests for mediated assets and hook executability.
tests/unit/parts/test_setup_assets.py Updates tests to match newly exported setup-assets helper functions.
tests/spread/general/skip-repack/task.yaml New spread test validating that repacking is skipped when nothing changes.
tests/spread/general/skip-repack/snap/snapcraft.yaml Spread test snapcraft.yaml fixture for skip-repack scenario.
tests/spread/general/skip-repack/snap/icon.svg Spread test icon fixture to trigger repacks when modified.

Comment thread snapcraft/services/package.py
Comment thread snapcraft/services/package.py
Comment thread snapcraft/services/package.py
Comment thread snapcraft/services/package.py
Comment thread snapcraft/services/package.py Outdated
cmatsuoka and others added 9 commits July 14, 2026 12:55
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
@cmatsuoka
cmatsuoka marked this pull request as ready for review July 14, 2026 13:41
@cmatsuoka
cmatsuoka requested a review from mr-cal as a code owner July 14, 2026 13:41
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
@cmatsuoka
cmatsuoka requested a review from lengau July 14, 2026 15:03
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fairly hard to follow the changes in this file. Perhaps this is worth a live review.

Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>

@mr-cal mr-cal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, but I agree with @lengau that a live review would be useful as I'm also having a hard time understanding the package service.

)

@override
def pack(self, prime_dir: pathlib.Path, dest: pathlib.Path) -> list[pathlib.Path]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this old pack command now that it's been superseded by _pack()?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also _pack_components() is dead code too now

for hook in hooks_project_dir.iterdir():
meta_dir_hook = hooks_meta_dir / hook.name

_hardlink_or_copy(hook, meta_dir_hook)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is no longer referenced, can you drop it too?


return assets

def _resolve_icon_asset( # noqa: PLR0911

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is a near-verbatim copy of finalize_icon() in setup_assets.py, which will be tough to keep the 2 copies in-sync. Can we extract all of resolution logic from finalize_icon into another function to cut down on the duplication?


return assets

def _get_hook_assets(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a similar concern about maintaining duplicated code for this, _get_declared_hooks, and _validate_declared_hook_command_chains. My main concern with duplicated code is that core22 would diverge from core24+.

Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
@cmatsuoka

Copy link
Copy Markdown
Contributor Author

I think this change is too complex. I'm considering treating it as a spike and propose new PRs for each type of mediated metadata or asset file.

@cmatsuoka

Copy link
Copy Markdown
Contributor Author

Split snap.yaml creation as #6349

@cmatsuoka
cmatsuoka marked this pull request as draft July 15, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants