-
Notifications
You must be signed in to change notification settings - Fork 5
Restructure package format, add Composite Packages #21
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
Conversation
This PR re-structures the format of the "package" structure, which also has implications for package-manifest.toml files. What did we change, and why? - Unified internal/external packages: There is information about packages which is shared between internal and external packages, so they've been pulled into the same structure. The "input" for a package is now described by "PackageSource", which can be one of "local" (you're building the package), "prebuilt" (you're downloading the package), "composite" (you're combining packages") or "manual" (you're supplying the package yourself). - The boolean "zone" flag has been updated to an enum, called "PackageOutput". This enum describes the different outputs that one should expect from a package. - The "Target" evaluation information has been moved to this package from the primary Omicron repository. This makes it easier to act on the 'only_for_target' and 'intermediate_only' fields in a useful way.
| pub enum ExternalPackageSource { | ||
| /// Downloads the package from the following URL: | ||
| /// | ||
| /// <https://buildomat.eng.oxide.computer/public/file/oxidecomputer/REPO/image/COMMIT/PACKAGE> | ||
| Prebuilt { | ||
| repo: String, | ||
| commit: String, | ||
| sha256: String, | ||
| }, | ||
| /// Expects that a package will be manually built and placed into the output | ||
| /// directory. | ||
| Manual, | ||
| } |
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.
Everything here has been merged into PackageSource below
jgallagher
left a comment
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.
LGTM! Left a handful of comments, only one of which is more than a style/nit suggestion (the question about whether composite packages can contain tarball packages).
andrewjstone
left a comment
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.
LGTM. Will be great to have this functionality.
|
|
||
| #[tokio::test] | ||
| #[tokio::test(flavor = "multi_thread")] | ||
| async fn test_composite_package() { |
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.
This test made usage very clear for me. Thank you!
This PR re-structures the format of the "package" structure, which also has implications for package-manifest.toml files.
What did we change, and why?
Unified internal/external packages: There is information about packages which is shared between internal and external packages, so they've been pulled into the same structure. The "input" for a package is now described by "PackageSource", which can be one of "local" (you're building the package), "prebuilt" (you're downloading the package), "composite" (you're combining packages") or "manual" (you're supplying the package yourself).
The boolean "zone" flag has been updated to an enum, called "PackageOutput". This enum describes the different outputs that one should expect from a package.
The "Target" evaluation information has been moved to this package from the primary Omicron repository. This makes it easier to act on the 'only_for_target' and 'intermediate_only' fields in a useful way.