-
Notifications
You must be signed in to change notification settings - Fork 133
feat: into_package() for ProtocolLib and StandardLib + CI Artifact
#2859
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
base: next
Are you sure you want to change the base?
Changes from all commits
86f8f70
1163ceb
3f0e214
c06648a
0498c29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| use alloc::boxed::Box; | ||
| use alloc::sync::Arc; | ||
|
|
||
| use miden_mast_package::{Package, TargetType, Version}; | ||
|
|
||
| use crate::assembly::Library; | ||
| use crate::assembly::mast::MastForest; | ||
| use crate::utils::serde::Deserializable; | ||
|
|
@@ -10,6 +13,8 @@ use crate::utils::sync::LazyLock; | |
|
|
||
| const PROTOCOL_LIB_BYTES: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/assets/protocol.masl")); | ||
|
|
||
| const PROTOCOL_PACKAGE_NAME: &str = "protocol"; | ||
|
|
||
| // PROTOCOL LIBRARY | ||
| // ================================================================================================ | ||
|
|
||
|
|
@@ -21,6 +26,23 @@ impl ProtocolLib { | |
| pub fn mast_forest(&self) -> &Arc<MastForest> { | ||
| self.0.mast_forest() | ||
| } | ||
|
|
||
| /// Wraps this library into a [`Package`] named `PROTOCOL_PACKAGE_NAME`, | ||
| /// versioned with the `miden-protocol` crate's version. | ||
| pub fn into_package(self) -> Box<Package> { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if there are any meaningful tests we could add for
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think before this gets merged, we should switch to building the protocol and standards packages as Miden projects, i.e. via That way this function goes away, and packaging uses the standard workflow and tooling provided by the assembler, and we're guaranteed that all of the package metadata is correct.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Would this need to happen here in the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I believe so. I believe that would imply switching to a
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mmagician Yes, the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bitwalker regarding your suggestion to switch to using Option A - each dependency package with its own manifest:
Option B - only the userspace libraries (
|
||
| // The ProtocolLib's version is the same as the crate's as per the miden-protocol's | ||
| // Cargo.toml. | ||
| let version = Version::parse(env!("CARGO_PKG_VERSION")) | ||
| .expect("CARGO_PKG_VERSION must be valid semver"); | ||
|
|
||
| Package::from_library( | ||
| PROTOCOL_PACKAGE_NAME.into(), | ||
| version, | ||
| TargetType::Library, | ||
| Arc::new(self.0), | ||
| [], | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| impl AsRef<Library> for ProtocolLib { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| #!/usr/bin/env cargo | ||
|
|
||
| --- | ||
| [dependencies] | ||
| miden-protocol = { path = "../crates/miden-protocol" } | ||
| miden-standards = { path = "../crates/miden-standards" } | ||
| semver = "1" | ||
| --- | ||
|
|
||
| use std::env; | ||
|
|
||
| use miden_protocol::ProtocolLib; | ||
| use miden_standards::StandardsLib; | ||
|
|
||
| fn main() -> std::io::Result<()> { | ||
| // Must be run from the workspace root (CARGO_TARGET_DIR is not set for cargo scripts). | ||
| let workspace_root = env::current_dir().expect("could not read PWD"); | ||
| let packages_dir = workspace_root.join("target").join("packages"); | ||
| std::fs::create_dir_all(&packages_dir)?; | ||
|
|
||
| let protocol_pkg = ProtocolLib::default().into_package(); | ||
| protocol_pkg.write_masp_file(&packages_dir)?; | ||
| println!("wrote {}.masp to {}", protocol_pkg.name, packages_dir.display()); | ||
|
|
||
| let standards_pkg = StandardsLib::default().into_package(); | ||
| standards_pkg.write_masp_file(&packages_dir)?; | ||
| println!("wrote {}.masp to {}", standards_pkg.name, packages_dir.display()); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
mmagician marked this conversation as resolved.
|
||
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.
It looks like the last commit regressed the naming and we have the
miden-prefix again