Allow running cargo add foo when a foo.workspace = true already exists#10585
Allow running cargo add foo when a foo.workspace = true already exists#10585Muscraft wants to merge 1 commit intorust-lang:masterfrom
cargo add foo when a foo.workspace = true already exists#10585Conversation
cargo add foo when a foo.workspace = true already exists
a291941 to
2989335
Compare
|
I accidentally had my commits from an earlier branch that hadn't been merged under this one. It should be fixed now. |
There was a problem hiding this comment.
This is unrelated to the source
There was a problem hiding this comment.
Features and optional are unrelated to the source?
There was a problem hiding this comment.
See
pub struct Dependency {
/// The name of the dependency (as it is set in its `Cargo.toml` and known to crates.io)
pub name: String,
/// Whether the dependency is opted-in with a feature flag
pub optional: Option<bool>,
/// List of features to add (or None to keep features unchanged).
pub features: Option<IndexSet<String>>,
/// Whether default features are enabled
pub default_features: Option<bool>,
/// Where the dependency comes from
pub source: Option<Source>,
/// Non-default registry
pub registry: Option<String>,
/// If the dependency is renamed, this is the new name for the dependency
/// as a string. None if it is not renamed.
pub rename: Option<String>,
/// Features that are exposed by the dependency
pub available_features: BTreeMap<String, Vec<String>>,
}There was a problem hiding this comment.
Ah so we should make it so features from foo = { workspace = true, features = ["test"]} get put into the Dependency
There was a problem hiding this comment.
This will just print a bool. We should probably just print workspace
There was a problem hiding this comment.
Can this ever be false? Should we just have the presence of this assume true?
There was a problem hiding this comment.
That is up to you. I don't think it should be false ever since we check if it's false before we create it.
There was a problem hiding this comment.
Let's just have this be a unit struct for now. We can always add fields later
There was a problem hiding this comment.
We should delete all of the keys incompatible with workspace = true.
There was a problem hiding this comment.
The other branches should remove "workspace"
There was a problem hiding this comment.
Would it be more clear to say "{key}.workspace = false is unsupported"
src/cargo/ops/cargo_add/mod.rs
Outdated
There was a problem hiding this comment.
I might go as far as an unreachable!("registry depedencies required, found a workspace dependency")
src/cargo/ops/cargo_add/mod.rs
Outdated
There was a problem hiding this comment.
ditto
unreachable!("path or git dependency expected, found workspace dependency");
src/cargo/ops/cargo_add/mod.rs
Outdated
There was a problem hiding this comment.
Don't we need to be looking up the workspace's dependency and getting its query?
There was a problem hiding this comment.
Possibly, this was one area that I wasn't sure how we should be going about this. I know that this makes it so foo = { workspace = true, features = ["test"]} works.
There was a problem hiding this comment.
We have two feature lists
- What the user specified
- What the crate defines
This code is putting the user-specified features into the list of what the crate defines so anything the user enters will always be valid.
tests/testsuite/cargo_add.rs
Outdated
There was a problem hiding this comment.
#10581 will add a masquerade_as_nightly_cargo() function to Command.
There was a problem hiding this comment.
I saw that and put this in until that PR is merged or we plan to merge this one and then have a cleanup PR that fixes it
46b90e7 to
730f69a
Compare
730f69a to
395538d
Compare
395538d to
d0bf960
Compare
|
Closing as #10606 includes this commit |
Cargo add support for workspace inheritance Tracking issue: #8415 RFC: rust-lang/rfcs#2906 This PR adds all the required support for workspace inheritance within `cargo-add`. It was split up across a few different PRs as it required `snapbox` support from #10581 and a new `toml_edit` version from #10603. `@epage` and I decided to go ahead with this PR and add in some of the changes those PRs made. `@epage's` name on the commits is from helping to rewrite commits and some very minor additions. Changes: - #10585 - Muscraft#1 - Muscraft#3 - Muscraft#2 - Muscraft#4 r? `@epage`
Tracking issue: #8415
RFC: rust-lang/rfcs#2906
PRs in this RFC:
Cargo.toml#10517license-path, anddepednency.path#10538readme#10548rust-version#10563excludeandinclude#10565workspaceto… #10564InheritableFieldsin aLazyCellinside `… #10568key.workspace = truetokey = { workspace = true }#10584This is currently a draft
This PR focuses on adding initial support to allow running
cargo add foowhen afoo.workspace = truealready exists.Changes:
WorkspaceSourceto allow forfoo.workspace = trueMaybeWorkspaceto allow returning aWorkspaceSourcewhen trying toqueryfor a dependency or trying to get aSourceIdfor that query.Remaining implementation work for the RFC
cargo-addsupport, see epage's commentr? @epage