Fix building multiple binaries that do not have path spacified in Cargo.toml #3609
Conversation
|
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. |
|
Ups, seems that I have broken the build. Unfortunately, I am not able to make cross-compilation test to work on my machine, so I am not able to run the tests locally. Are there any guide for running cargo test (especially those that include cross-compilation)? |
|
@jmatraszek looks great to me, thanks for the PR! The test failures look like it's just overly-pedantic path matching in the test, so you should be able to just relax the assertions with some well-placed Otherwise to run cross-compilation tests locally if you're using rustup you can just install the targets through rustup: |
|
Hi @alexcrichton, BTW. Running tests by |
jmatraszek
force-pushed
the
build_proper_binary
branch
from
ccd8654 to
d95983f
Compare
|
@bors: r+ Ok well let's see what Travis says |
|
📌 Commit d95983f has been approved by |
|
⌛ Testing commit d95983f with merge 2b63c4f... |
Fix building multiple binaries that do not have path spacified in Cargo.toml When multiple binaries are specified in Cargo.toml, the binaries that do not have `path` specified are build from `src/main.rs`. Discovered here: rust-lang-nursery/thanks#40 (comment). This was caused by setting for a binary a main layout here https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml.rs#L478, which caused `normalize` to not fallback to default binary path here https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml.rs#L1149 (as `bin.path` was always `Some("/path/to/main.rs")`. Added a test and fixed this by not using `layout.main()`, so right now for bins without `path` specified we fallback to default path inferred from bin's name (e.g. `src/bin/foo.rs`), test if the file exists and only if it doesn't -- fallback to `src/main.rs`. I do not have any knowledge about Cargo's design, so I am not sure if this is the proper place to test for file existence.
|
☀️ Test successful - status-appveyor, status-travis |
When multiple binaries are specified in Cargo.toml, the binaries that do not have
pathspecified are build fromsrc/main.rs. Discovered here: rust-lang-nursery/thanks#40 (comment).This was caused by setting for a binary a main layout here https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml.rs#L478, which caused
normalizeto not fallback to default binary path here https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml.rs#L1149 (asbin.pathwas alwaysSome("/path/to/main.rs").Added a test and fixed this by not using
layout.main(), so right now for bins withoutpathspecified we fallback to default path inferred from bin's name (e.g.src/bin/foo.rs), test if the file exists and only if it doesn't -- fallback tosrc/main.rs.I do not have any knowledge about Cargo's design, so I am not sure if this is the proper place to test for file existence.