-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make chisel a build snap #464
base: main
Are you sure you want to change the base?
Make chisel a build snap #464
Conversation
b473145
to
6829757
Compare
Here are successful examples of running
|
@cjdcordeiro @tigarmo This is ready for review. See examples of running it above. Before merging this, PR #637 needs to be merged first, and function |
cfddc15
to
4248878
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Short and sweet. I'm adding a label to be explicit that canonical/craft-parts#637 should be merged first
# this will be removed once part_has_slices is added to craft-parts | ||
def part_has_slices(data: dict[str, Any]) -> bool: | ||
stage_packages = data.get("stage-packages", []) | ||
return any(name for name in stage_packages if "_" in name) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still within this PR right? Since we're waiting for canonical/craft-parts#637
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
|
||
services = cast(RockcraftServiceFactory, self._services) | ||
image_service = services.image | ||
image_info = image_service.obtain_image() | ||
needs_chisel = any(part for part in parts if part_has_slices(parts[part])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this will also slightly change after canonical/craft-parts#637
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it will become craft_parts.part_has_slices
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can move detection and chisel installing to craft-parts, it already does that with other tools such as git.
Hey everyone. I wanted to check in on this PR; I'm worried its getting a little stale. 🦇 |
We're still waiting for chisel to release its first stable release, so I guess let's keep it open until then. |
Chisel v1.0.0 has been released in candidate channel. It will be released in stable channel in about 2 weeks. |
Chisel is now owned by canonical (@rebornplusplus ) so this PR can be resumed |
We also need this PR (or a new release) for 1.0.0 to become available in rockcraft |
Chisel isn't wanted to be embedded in rockcraft snap, so it is set to be a build-snap instead.