Skip to content

Add early dependency checks and fix packaging#252

Open
gursewak1997 wants to merge 1 commit intomainfrom
bcvk-250
Open

Add early dependency checks and fix packaging#252
gursewak1997 wants to merge 1 commit intomainfrom
bcvk-250

Conversation

@gursewak1997
Copy link
Copy Markdown
Collaborator

  • Check for bwrap/objcopy/systemctl early with clear errors
  • Add missing bubblewrap to bcvk.spec
  • Document all runtime dependencies

Closes: #250

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces bubblewrap as a mandatory dependency by updating the RPM spec file, the installation documentation, and the runtime binary validation logic. It also refines the documentation for existing prerequisites and adds explanatory comments regarding binary requirements. Review feedback recommends making the objcopy check conditional to prevent unnecessary dependencies for non-UKI images and suggests including qemu-img in the documentation to align with the package specifications.

Comment thread crates/kit/src/run_ephemeral.rs Outdated
Comment thread docs/src/installation.md Outdated
@gursewak1997 gursewak1997 force-pushed the bcvk-250 branch 2 times, most recently from 0f38d7d to 2aa37f0 Compare April 27, 2026 22:58
Comment thread contrib/packaging/bcvk.spec Outdated
ExclusiveArch: x86_64 aarch64

Requires: binutils
Requires: bubblewrap
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 think this is a dependency in the target system, not the host.

Not going to really hurt to require it on the host, but the original issue in #250 seems to mostly be about requirements in the target container.

So here and elsewhere, we should be clear about which component is required where.

And yes, the architecture is quite tricky with bubblewrap here, so I may be wrong.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You're right. I had the dependency location backwards. The architecture is tricky: we run podman run , and the target image's entrypoint.sh uses bwrap, then run_impl() inside that namespace uses systemctl/objcopy.

Updated now: So the spec now only has true host requirements (podman, qemu-kvm, virtiofsd, openssh-clients for libvirt SSH), and docs clearly separate the two contexts to address the #250 confusion.

@gursewak1997 gursewak1997 force-pushed the bcvk-250 branch 2 times, most recently from c61ba71 to 56e0ce5 Compare April 28, 2026 01:01
Signed-off-by: gursewak1997 <gursmangat@gmail.com>
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.

Minimal container requirements

2 participants