Skip to content
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

feat(push): allow pushing composed components to registry #2996

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fibonacci1729
Copy link
Contributor

@fibonacci1729 fibonacci1729 commented Jan 27, 2025

WIP for now to flesh out the concept.

Implements #2988

Still working to add a test but the meat-and-potatoes is ready to serve.

@fibonacci1729 fibonacci1729 force-pushed the push branch 3 times, most recently from ee371d1 to 9176ca1 Compare January 30, 2025 18:10
@fibonacci1729 fibonacci1729 marked this pull request as ready for review January 30, 2025 18:10
for (dep_name, mut dep) in c.dependencies {
let source = dep
if !skip_compose {
let composed = spin_compose::compose(&ComponentSourceLoader, &c)
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels awkward for an assemble_layers function to contain composition logic.

What would be the tradeoffs of doing composition as e.g. a LockedApp -> LockedApp transformation invoked after locking, and not touching the (core) OCI code at all? I guess it would force us to create temp files rather than holding compositions only in memory... which is a bit yuck...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah i considered that but ended up abandoning it for "less code"... Happy to revisit if this is too awkward.

crates/oci/src/client.rs Outdated Show resolved Hide resolved
src/commands/registry.rs Outdated Show resolved Hide resolved
@fibonacci1729
Copy link
Contributor Author

cc/ @itowlson a simple test is now included.

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.

Feature Request: Add flag to registry push that pushes composed components
2 participants