feature(cargo-miden): parse local path Miden dependencies from Cargo.toml
#485
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
greenhat
force-pushed
the
greenhat/i368-path-dependency
branch
4 times, most recently
from
37519e7 to
701f6df
Compare
greenhat
force-pushed
the
greenhat/i368-path-dependency
branch
from
0abf514 to
e2bf1d7
Compare
Introduce dependencies in `CompilerTest`.
greenhat
force-pushed
the
greenhat/i368-path-dependency
branch
from
e2bf1d7 to
388162f
Compare
|
@bitwalker I rebased it, and it's ready for a review. |
bitwalker
reviewed
May 14, 2025
| @@ -182,7 +182,7 @@ pub struct CompilerTestBuilder { | |||
| /// The extra MASM modules to link to the compiled MASM program | |||
| link_masm_modules: LinkMasmModules, | |||
| /// Extra flags to pass to the midenc driver | |||
| midenc_flags: Vec<Cow<'static, str>>, | |||
| midenc_flags: Vec<String>, | |||
There was a problem hiding this comment.
Any particular reason for this change? Most of the flags we pass are constant strings.
There was a problem hiding this comment.
The names of the dependencies (Miden package filenames) that passed via --link-library option. They are parsed from our section in Cargo.toml.
bitwalker
reviewed
May 14, 2025
| exec.with_dependencies(&package.manifest.dependencies).unwrap(); | ||
| let trace = exec.execute(&package.unwrap_program(), &test.session); | ||
| // TODO: uncomment after Paul's fix for mem intrinsics (failing u32 assert) is merged |
There was a problem hiding this comment.
The fix for this was merged, so this should be able to be uncommented now
bitwalker
reviewed
May 14, 2025
tools/cargo-miden/Cargo.toml
Outdated
| @@ -35,6 +35,9 @@ cargo-generate = "0.23" | |||
| path-absolutize = "3.1.1" | |||
| tokio.workspace = true | |||
| cargo-config2 = "0.1.24" | |||
| serde = { version = "1.0", features = ["derive"] } | |||
There was a problem hiding this comment.
Both serde and toml are workspace dependencies, and due to feature unification there isn't any benefit to specifying a smaller feature set here.
There was a problem hiding this comment.
Good spot! Thanks! Fixed.
bitwalker
approved these changes
May 14, 2025
`rust_sdk_cross_ctx_note` test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #368
The
rust_sdk_cross_ctx_notetest is using the localpathdependency for the account in itsCargo.toml.