Allow installation of multiple packages in one line #2601
Conversation
|
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
|
If I call |
|
You are correct, if |
| @@ -123,14 123,20 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> { | |||
| try!(SourceId::for_central(config)) | |||
| }; | |||
|
|
|||
| let krate = options.arg_crate.as_ref().map(|s| &s[..]); | |||
| let krate = options.arg_crate.as_ref().map(|s| s.iter().map(|t| &t[..]).collect::<Vec<_>>()); | |||
There was a problem hiding this comment.
The other versions of this statement are actually just converting Option<&String> to Option<&str> but I don't think that's necessary if we have Vec<String>, so this part can probably just get removed.
|
Thanks for the PR @pwoolcoc! I'm fine with the fail-fast semantics here, although we may be able to get away with "more transactional" semantics in terms of we don't install anything if anything fails. That may not be the most important thing in the world, however. |
|
☔ The latest upstream changes (presumably #2636) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Thanks @alexcrichton , I am taking a look at this today and hope to fix all the issues you brought up |
|
@alexcrichton what do you think the best way to handle the various "crate-selecting" options would be, in the presence of multiple crates? I.e., what does this do: Should we attempt to use the same source for all the listed crates? Or disable the various "crate-selecting" options when there are multiple crates passed (that would be |
|
Hm, that's a good question! I wonder if there's a way that we could specify this to docopt. Something like where you can pass all these options multiple times, but they're all associated with only the next (or previous) command line argument. That way you could install multiple things from multiple sources at multiple versions. That being said it may also indicate that this may not work out so hot... |
|
Ok, got a chance to chat about this with @wycats today, and the conclusion was that this seems fine to add, even with the limitation that you can't install packages from multiple sources. This is ultimately just a convenience so if you happen to just not be able to install multiple packages from multiple sources, you can always just run the command twice! |
|
@alexcrichton great! Thanks for the clarification. So, just to be clear, should we still try to pull all the packages from a specified source, or should we ignore the specified soure entirely (or throw an error when specifying a source for multiple packages)? |
|
Yeah this'd basically be a command where you can install multiple crates from one source, and you can just configure what that source is. Which is to say we should pull all the crates from the specified source, not throw it away |
|
☔ The latest upstream changes (presumably #2743) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@alexcrichton I think I need a bit of help here. In order to avoid re-updating the registry every time it starts to install another crate, I moved the loop into I have tried passing the I also tried changing it from Here is the exact place where we pass ownership: https://github.com/pwoolcoc/cargo/blob/issue-2585/src/cargo/ops/cargo_install.rs#L103 Any help you (or anyone else, really) can give would be appreciated. |
|
@pwoolcoc it may make the most sense to just use the same |
|
Closing due to inactivity, but feel free to resubmit with comments addressed! |
|
Hey ! @pwoolcoc: I am interested in helping on this one, let me know if you need any help. |
cargo install multiple crates rust-lang/rustup#986 for `cargo install` Revives #2601 @pwoolcoc, replaces #3075 @esclear, closes #2585 @kindlychung @cyplo Avoids the sticking point of the previous two PRs (multiple registry updates) by threading through a first-run boolean flag to decide whether `select_pkg` needs to call `source.update()`. There is still the issue that flags such as `--git` and `--vers` are "global" to the multiple packages you may be installing. The workaround is just to run `cargo install` separately. In the future we could add syntax like `cargo install foo=1.0 bar=2.5 quux=git://github.com/durka/quux#dev-branch` or something.
Closes #2585