Skip to content

incr.comp.: Make sure cargo check is compatible with incremental compilation. #46058

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

Open
michaelwoerister opened this issue Nov 17, 2017 · 5 comments
Assignees
Labels
A-incr-comp Area: Incremental compilation C-enhancement Category: An issue proposing an enhancement or a PR with one. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-incr-comp Working group: Incremental compilation

Comments

@michaelwoerister
Copy link
Member

So far incremental compilation did not bring any benefit for cargo check because incremental compilation only cached post-trans artifacts and the whole point of cargo check is to exit from compilation before the costly trans and LLVM parts.

With #46004 this has changed and we'll keep adding more and more things to this pre-trans cache. As a consequence we should make sure that cargo check works in conjunction with CARGO_INCREMENTAL=1. It already might but we've never tested it and have no regression tests.

cc @nikomatsakis @rust-lang/cargo

@michaelwoerister michaelwoerister added A-incr-comp Area: Incremental compilation E-needs-mentor WG-incr-comp Working group: Incremental compilation labels Nov 17, 2017
@alexcrichton
Copy link
Member

I just verified that CARGO_INCREMENTAL=1 cargo check is indeed passing -Z incremental, so I think we're good there! (once -Z incremental-queries isn't necessary I think).

AFAIK we don't have a lot of tests in this regard though, it may be worth adding a few to Cargo maybe? (unsure though)

@michaelwoerister
Copy link
Member Author

it may be worth adding a few to Cargo maybe?

If it's not too much trouble, that would certainly help. And cargo would have to make sure to use a different cache directory for check builds than it uses for regular builds (or make sure that the crate disambiguator is different when building with check, which probably already is the case).

I was mostly thinking of making sure that incr. comp. works well together with the other commandline options that cargo check invokes rustc with (just --emit=metadata?).

@michaelwoerister
Copy link
Member Author

So, I've verified that everything works as expected. Would be great to have a regression test for this though.

@michaelwoerister michaelwoerister added E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. and removed E-needs-mentor labels Dec 27, 2017
@XAMPPRocky XAMPPRocky added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. labels Feb 19, 2018
@nnethercote
Copy link
Contributor

rustc-perf runs every benchmark under one or more incremental + check combinations. Is that sufficient?

@michaelwoerister
Copy link
Member Author

I'd prefer a proper test in src/test/incremental.

@pnkfelix pnkfelix self-assigned this Nov 17, 2020
@Noratrieb Noratrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation C-enhancement Category: An issue proposing an enhancement or a PR with one. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-incr-comp Working group: Incremental compilation
Projects
None yet
Development

No branches or pull requests

6 participants