Fix `cargo rustc` for test with implicit binary. by ehuss · Pull Request #5503 · rust-lang/cargo

ehuss · 2018-05-09T00:24:12Z

rust-highfive · 2018-05-09T00:24:15Z

(rust_highfive has picked a reviewer for you, use r? to override)

matklad · 2018-05-09T14:14:56Z

Fascinating! If I understand correctly, previously, integration tests and binaries could be built in parallel, because binaries were a prerequisite for running the test, now it is a per-requisite for building the test. I wonder if it is significant? One way to check this would be to do time cargo test --test testsuite --norun after modifying the lib.rs file. With this setup, we first should build cargo-the-library, and then the binary and the test either in parallel or sequentially. If the difference is significant, we might think about how to preserve the parallelism while fixing the bug. Or am I completely off base here about build parallelism changes?

That said, given that this code previously (before dependencies profiles) lived in compute_deps anyway, I am fine with merging it as is.

ehuss · 2018-05-09T16:28:44Z

I wonder if it is significant?

I thought the same thing, but fortunately JobQueue has a special case for this situation:

cargo/src/cargo/core/compiler/job_queue.rs

Lines 455 to 464 in caa7ac1

    
           Ok(targets 
        
               .iter() 
        
               .filter_map(|unit| { 
        
                   // Binaries aren't actually needed to *compile* tests, just to run 
        
                   // them, so we don't include this dependency edge in the job graph. 
        
                   if self.target.is_test() && unit.target.is_bin() { 
        
                       None 
        
                   } else { 
        
                       Some(Key::new(unit)) 
        
                   }

There are other ways to fix this. We could add special-case code to detect this scenario in compile_ws where it does a simple length check, but I think that would be awkward. This solution will affect #5460 (will need to explicitly add bins to the list to uplift), so it's not without impact. I couldn't really think of a better way to do it, though.

matklad · 2018-05-10T12:40:05Z

Ok!

@bors r+

bors · 2018-05-10T12:40:07Z

📌 Commit a187ba8 has been approved by matklad

bors · 2018-05-10T12:44:22Z

⌛ Testing commit a187ba8 with merge 7067bc8...

Fix `cargo rustc` for test with implicit binary. Fixes #5502

bors · 2018-05-10T13:19:11Z

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing 7067bc8 to master...

Regression fixes: - rust-lang/cargo#5503 - cargo rustc broken for tests in project with bins - rust-lang/cargo#5520 - (rust-lang#50640) shared proc-macro dependency built incorrectly Changes: - rust-lang/cargo#5522 - Add option to set user-agent - rust-lang/cargo#5519 - NFC: fix a couple of typos, found by codespell. - rust-lang/cargo#5513 - Fix `panic` for binaries built during tests. - rust-lang/cargo#5512 - simplify build_requirements - rust-lang/cargo#5301 - Add --build-plan for 'cargo build' - rust-lang/cargo#5460 - Be more conservative about which files are linked to the output dir. - rust-lang/cargo#5509 - Use the new stable - rust-lang/cargo#5507 - Does not print seconds fraction with minutes - rust-lang/cargo#5498 - Bump to 0.29.0 - rust-lang/cargo#5497 - Mention +nightly in ARCHITECTURE.md

Update Cargo Regression fixes: - rust-lang/cargo#5503 - cargo rustc broken for tests in project with bins - rust-lang/cargo#5520 - (rust-lang#50640) shared proc-macro dependency built incorrectly Changes: - rust-lang/cargo#5522 - Add option to set user-agent - rust-lang/cargo#5519 - NFC: fix a couple of typos, found by codespell. - rust-lang/cargo#5513 - Fix `panic` for binaries built during tests. - rust-lang/cargo#5512 - simplify build_requirements - rust-lang/cargo#5301 - Add --build-plan for 'cargo build' - rust-lang/cargo#5460 - Be more conservative about which files are linked to the output dir. - rust-lang/cargo#5509 - Use the new stable - rust-lang/cargo#5507 - Does not print seconds fraction with minutes - rust-lang/cargo#5498 - Bump to 0.29.0 - rust-lang/cargo#5497 - Mention +nightly in ARCHITECTURE.md

Unblocking PRs: - rust-lang/cargo#5535 - Ignore <tab> in libtest output. (unblocks rust-lang#50387) - rust-lang/cargo#5537 - Remove -Zno-trans test. (unblocks rust-lang#50615) - rust-lang/cargo#5540 - Fix tests when CARGO_TARGET_DIR is set. (unblocks self) Regression fixes: - rust-lang/cargo#5503 - cargo rustc broken for tests in project with bins - rust-lang/cargo#5520 - (rust-lang#50640) shared proc-macro dependency built incorrectly Changes: - rust-lang/cargo#5527 - Point Source Replacement to the Overriding Dependencies section - rust-lang/cargo#5533 - Detail how to run locally-built nightly cargo - rust-lang/cargo#5522 - Add option to set user-agent - rust-lang/cargo#5519 - NFC: fix a couple of typos, found by codespell. - rust-lang/cargo#5513 - Fix `panic` for binaries built during tests. - rust-lang/cargo#5512 - simplify build_requirements - rust-lang/cargo#5301 - Add --build-plan for 'cargo build' - rust-lang/cargo#5460 - Be more conservative about which files are linked to the output dir. - rust-lang/cargo#5509 - Use the new stable - rust-lang/cargo#5507 - Does not print seconds fraction with minutes - rust-lang/cargo#5498 - Bump to 0.29.0 - rust-lang/cargo#5497 - Mention +nightly in ARCHITECTURE.md

Update Cargo Unblocking PRs: - rust-lang/cargo#5535 - Ignore <tab> in libtest output. (unblocks rust-lang#50387) - rust-lang/cargo#5537 - Remove -Zno-trans test. (unblocks rust-lang#50615) - rust-lang/cargo#5540 - Fix tests when CARGO_TARGET_DIR is set. (unblocks self) Regression fixes: - rust-lang/cargo#5503 - cargo rustc broken for tests in project with bins - rust-lang/cargo#5520 - (rust-lang#50640) shared proc-macro dependency built incorrectly Changes: - rust-lang/cargo#5527 - Point Source Replacement to the Overriding Dependencies section - rust-lang/cargo#5533 - Detail how to run locally-built nightly cargo - rust-lang/cargo#5522 - Add option to set user-agent - rust-lang/cargo#5519 - NFC: fix a couple of typos, found by codespell. - rust-lang/cargo#5513 - Fix `panic` for binaries built during tests. - rust-lang/cargo#5512 - simplify build_requirements - rust-lang/cargo#5301 - Add --build-plan for 'cargo build' - rust-lang/cargo#5460 - Be more conservative about which files are linked to the output dir. - rust-lang/cargo#5509 - Use the new stable - rust-lang/cargo#5507 - Does not print seconds fraction with minutes - rust-lang/cargo#5498 - Bump to 0.29.0 - rust-lang/cargo#5497 - Mention +nightly in ARCHITECTURE.md

Unblocking PRs: - rust-lang/cargo#5535 - Ignore <tab> in libtest output. (unblocks rust-lang#50387) - rust-lang/cargo#5537 - Remove -Zno-trans test. (unblocks rust-lang#50615) - rust-lang/cargo#5540 - Fix tests when CARGO_TARGET_DIR is set. (unblocks self) Regression fixes: - rust-lang/cargo#5503 - cargo rustc broken for tests in project with bins - rust-lang/cargo#5520 - (rust-lang#50640) shared proc-macro dependency built incorrectly Changes: - rust-lang/cargo#5527 - Point Source Replacement to the Overriding Dependencies section - rust-lang/cargo#5533 - Detail how to run locally-built nightly cargo - rust-lang/cargo#5522 - Add option to set user-agent - rust-lang/cargo#5519 - NFC: fix a couple of typos, found by codespell. - rust-lang/cargo#5513 - Fix `panic` for binaries built during tests. - rust-lang/cargo#5512 - simplify build_requirements - rust-lang/cargo#5301 - Add --build-plan for 'cargo build' - rust-lang/cargo#5460 - Be more conservative about which files are linked to the output dir. - rust-lang/cargo#5509 - Use the new stable - rust-lang/cargo#5507 - Does not print seconds fraction with minutes - rust-lang/cargo#5498 - Bump to 0.29.0 - rust-lang/cargo#5497 - Mention +nightly in ARCHITECTURE.md

Update Cargo Unblocking PRs: - rust-lang/cargo#5535 - Ignore tab in libtest output. (unblocks #50387) - rust-lang/cargo#5537 - Remove -Zno-trans test. (unblocks #50615) - rust-lang/cargo#5540 - Fix tests when CARGO_TARGET_DIR is set. (unblocks self) Regression fixes: - rust-lang/cargo#5503 - cargo rustc broken for tests in project with bins - rust-lang/cargo#5520 - shared proc-macro dependency built incorrectly Changes: - rust-lang/cargo#5527 - Point Source Replacement to the Overriding Dependencies section - rust-lang/cargo#5533 - Detail how to run locally-built nightly cargo - rust-lang/cargo#5522 - Add option to set user-agent - rust-lang/cargo#5519 - NFC: fix a couple of typos, found by codespell. - rust-lang/cargo#5513 - Fix `panic` for binaries built during tests. - rust-lang/cargo#5512 - simplify build_requirements - rust-lang/cargo#5301 - Add --build-plan for 'cargo build' - rust-lang/cargo#5460 - Be more conservative about which files are linked to the output dir. - rust-lang/cargo#5509 - Use the new stable - rust-lang/cargo#5507 - Does not print seconds fraction with minutes - rust-lang/cargo#5498 - Bump to 0.29.0 - rust-lang/cargo#5497 - Mention +nightly in ARCHITECTURE.md The PR fixes #50640.

Fix cargo rustc for test with implicit binary.

a187ba8

Fixes rust-lang#5502

rust-highfive assigned matklad May 9, 2018

bors added a commit that referenced this pull request May 10, 2018

Auto merge of #5503 - ehuss:fix-rustc-test, r=matklad

7067bc8

Fix `cargo rustc` for test with implicit binary. Fixes #5502

bors merged commit a187ba8 into rust-lang:master May 10, 2018

ehuss added a commit to ehuss/cargo that referenced this pull request May 10, 2018

Update due to rust-lang#5503.

251502b

ehuss mentioned this pull request May 12, 2018

Update Cargo rust-lang/rust#50696

Merged

ehuss added this to the 1.28.0 milestone Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `cargo rustc` for test with implicit binary.#5503

Fix `cargo rustc` for test with implicit binary.#5503
bors merged 1 commit intorust-lang:masterfrom
ehuss:fix-rustc-test

ehuss commented May 9, 2018

Uh oh!

rust-highfive commented May 9, 2018

Uh oh!

matklad commented May 9, 2018

Uh oh!

ehuss commented May 9, 2018

Uh oh!

matklad commented May 10, 2018

Uh oh!

bors commented May 10, 2018

Uh oh!

bors commented May 10, 2018

Uh oh!

bors commented May 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ehuss commented May 9, 2018

Uh oh!

rust-highfive commented May 9, 2018

Uh oh!

matklad commented May 9, 2018

Uh oh!

ehuss commented May 9, 2018

Uh oh!

matklad commented May 10, 2018

Uh oh!

bors commented May 10, 2018

Uh oh!

bors commented May 10, 2018

Uh oh!

bors commented May 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants