Skip to content
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

Implement install --all #5060

Closed

Conversation

matthiasbeyer
Copy link
Contributor

This patch tries to implement cargo install --all for installing all
"bin" crates from a workspace project.

Signed-off-by: Matthias Beyer [email protected]


I doubt that it is this simple, but I guess this is the first step.

Closes #4101


My cargo test fails, but I guess that's not because of my changes but rather because I cannot crosscompile:

test linker_and_ar ... FAILED

failures:

---- linker_and_ar stdout ----
	thread 'linker_and_ar' panicked at 'Cannot cross compile to i686-unknown-linux-gnu.

This failure can be safely ignored. If you would prefer to not see this
failure, you can set the environment variable CFG_DISABLE_CROSS_TESTS to "1".
', tests/cargotest/support/cross_compile.rs:88:4
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    linker_and_ar

test result: FAILED. 18 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--test cross-compile'

This patch tries to implement `cargo install --all` for installing all
"bin" crates from a workspace project.

Signed-off-by: Matthias Beyer <[email protected]>
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@matklad
Copy link
Member

matklad commented Feb 20, 2018

cc #4943 (comment), @withoutboats

@alexcrichton
Copy link
Member

Thanks! Could you be sure to add a test for this as well?

@matklad this may end up being somewhat orthogonal to that as "install the package/workspace at cwd" I think may just become a flag rather than being implicit

@matklad
Copy link
Member

matklad commented Feb 26, 2018

I am also unsure how this should interact with --bins and --examples...

For example, I believe that if you have a crate which contains an example, but does not contain a binary, than cargo install --all --examples will not install this example, which does seem surprising.

My gut feeling (which might be totally wrong! :) ) is that we should try do use the same semantics for --all which we use for all other commands. That is, to add an --exclude flag as well, and do something like this:

cargo/src/bin/build.rs

Lines 103 to 105 in ed8dfce

let spec = Packages::from_flags(options.flag_all,
&options.flag_exclude,
&options.flag_package)?;
.

@alexcrichton
Copy link
Member

Hm true yeah! I agree we should add --exclude for consistency if we add --all, but it's a good point about --bins and --examples... I dunno how to rationalize that without adding this though :(

@bors
Copy link
Collaborator

bors commented Mar 13, 2018

☔ The latest upstream changes (presumably #5152) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

I'm gonna close this for now as it's pretty out of date at this point, but it can of course be resubmitted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants