feat: add stage-slices key - #1669
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces first-class support for declaring Chisel slices via a new stage-slices key, while keeping (deprecated) slice support in stage-packages behind an opt-in/opt-out flag for downstream applications.
Changes:
- Add
stage-slicestoPartSpec, including mutual exclusivity withstage-packages, and propagatestage_packages_slice_supportthroughLifecycleManager/ProjectInfo. - Add a dedicated
ChiselSliceStrconstraint (regex + custom error message) and test coverage for slice naming rules. - Update package handling and executor plumbing so
stage-slicesbehaves likestage-packagesfor fetch/unpack and triggers adding chisel as a build snap when needed.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
craft_parts/parts.py |
Adds stage_slices, mutual exclusion with stage_packages, and context-based validation for deprecated slices in stage-packages. |
craft_parts/constraints.py |
Introduces ChiselSliceStr and shared regex-validator utility with a custom error message. |
craft_parts/utils/deb_utils.py |
Adds helpers to detect slices/packages and reuses them across deb/chisel handling. |
craft_parts/packages/deb.py |
Switches slice detection to shared deb_utils.has_slices and removes the local helper. |
craft_parts/executor/part_handler.py |
Treats stage-packages and stage-slices identically when fetching stage content. |
craft_parts/lifecycle_manager.py |
Threads stage_packages_slice_support to part building and ensures chisel snap is added when slices are present. |
craft_parts/infos.py |
Persists stage_packages_slice_support on ProjectInfo for consumers. |
craft_parts/state_manager/pull_state.py |
Tracks stage-slices in pull-state properties of interest. |
tests/unit/test_parts.py |
Expands spec unmarshal/marshal and validation tests for stage-slices and stage_packages_slice_support. |
tests/unit/test_lifecycle_manager.py |
Adds tests for ProjectInfo.stage_packages_slice_support and stage-slices triggering chisel. |
tests/unit/test_constraints.py |
New unit tests for ChiselSliceStr naming constraints and error messaging. |
tests/unit/packages/test_chisel.py |
Updates tests to use deb_utils.has_slices instead of the removed deb helper. |
tests/unit/executor/test_part_handler.py |
Tests identical handling of stage-packages vs stage-slices by the part handler. |
tests/unit/features/overlay/test_parts.py |
Updates overlay marshal/unmarshal expectations to include stage-slices. |
tests/conftest.py |
Adds an is_deb_based fixture to standardize deb-platform mocking. |
docs/reference/changelog.rst |
Documents the new stage-slices key and the stage_packages_slice_support parameter. |
docs/common/craft-parts/reference/part_properties.rst |
Adds stage_slices to the published part properties reference. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
2730dae to
f01780d
Compare
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
f01780d to
c8ebc9f
Compare
There was a problem hiding this comment.
This refactoring wasn't strictly necessary, but the duplication was bothering me and it made the validator more readable.
Plus craft-application will be able to leverage these for deprecation warnings.
| Tv = TypeVar("Tv") | ||
|
|
||
|
|
||
| def get_validator_by_regex( |
There was a problem hiding this comment.
This is a verbatim copy from craft-application.
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
| stage_packages = self._part.spec.stage_packages | ||
| # 'stage-packages' and 'stage-slices' are mutually exclusive, so at most one | ||
| # of these is populated. | ||
| stage_packages = self._part.spec.stage_packages or self._part.spec.stage_slices |
There was a problem hiding this comment.
We originally had debs or slices in stage packages, and the installation logic invoked _unpack_stage_packages/_unpack_stage_slices for each of the cases based on the contents of stage_packages. Now that we have specialized stage slices (which can be deb or other types of slices), it looks like we could move this decision one level up and invoke unpack_stage_packages/unpack_stage_slices from there instead of conflating everything into stage packages, and have two clean, separate paths for each format, wdyt?
There was a problem hiding this comment.
I'm not sure if that would gain us much. Since we still support slices in stage-packages, we still need to keep the logic that de-conflate slices from a stage-packages entry.
If we had entirely removed that, then yes I think your refactoring idea would be a great idea.
I'm not opinionated on whether that logic lives in craft_parts.packages.Repository or craft_parts.executor.PartHandler, but I don't see a strong reason to move it in this PR.
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
jahn-junior
left a comment
There was a problem hiding this comment.
The updated key descriptions LGTM. Thanks!
make lint && make test?https://docs.google.com/document/d/1tQUv6iMq06uq5-dMPAX0qnOr0Qx0NASA7aceuoZUc-I/edit?tab=t.0docs/reference/changelog.rst)?Adds a
stage-sliceskey per ST179.Split into 3 commits for your reviewing pleasure.
This is how I understood the division of responsibility between craft-parts and craft-application/downstream apps:
craft-parts
stage-packagesandstage-slices.craft-application
stage-packages.stage-packages.(CRAFT-5253)