-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Updated the soldeer version to 0.2.18 and added extra CLI tests #8441
Updated the soldeer version to 0.2.18 and added extra CLI tests #8441
Conversation
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.
Hey, this adds 3 C libraries as dependencies as you can see by the Cargo.lock diff. I would really prefer if this was not the case.
We dropped the git2 library a while ago in favor of just using the git CLI, see
foundry/crates/cli/src/utils/mod.rs
Line 276 in 0e07ca5
pub struct Git<'a> { |
In your case you can probably just get away with running Command::new("git").args(["clone", a, b])
Thanks for the suggestion. I am gonna try to get around just with the cli then. |
crates/config/src/soldeer.rs
Outdated
|
||
/// The commit in case git is used as dependency retrieval | ||
#[serde(default, skip_serializing_if = "Option::is_none")] | ||
pub commit: Option<String>, |
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 can be rev
like in cargo, which can be a commit or a tag
crates/forge/tests/cli/soldeer.rs
Outdated
"#; | ||
|
||
let actual_foundry_contents = read_file_to_string(&foundry_file); | ||
assert_eq!(foundry_contents, actual_foundry_contents); |
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.
We've recently updated our test suite, you should use assert_data_eq!(read_file_to_string(x), str![[""]])
and then run the test to update the assertion automatically. See #8389
crates/forge/bin/cmd/soldeer.rs
Outdated
@@ -6,6 +6,8 @@ use soldeer::commands::Subcommands; | |||
// CLI arguments for `forge soldeer`. | |||
#[derive(Clone, Debug, Parser)] | |||
#[clap(override_usage = "forge soldeer install [DEPENDENCY]~[VERSION] <REMOTE_URL> | |||
forge soldeer install [DEPENDENCY]~[VERSION] <GIT_URL> | |||
forge soldeer install [DEPENDENCY]~[VERSION] <GIT_URL> <COMMIT> |
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.
same here, the CLI options should probably mimic the config keys, so --rev
@DaniPopes |
Motivation
We added new functionality to Soldeer, using a git repo as a dependency.
mario-eth/soldeer#48
This feature has been requested even tho I do not encourage abusing it but it can be used for various unpublished repos.
Solution
Now we can use git to pull dependencies. It supports github and gitlab
Example:
Or using custom commit hashes or tags