Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Implement external build plan #1070

Merged
merged 4 commits into from
Oct 9, 2018
Merged

Implement external build plan #1070

merged 4 commits into from
Oct 9, 2018

Conversation

Xanewok
Copy link
Member

@Xanewok Xanewok commented Sep 27, 2018

From the commit description:

This adds a BuildGraph trait which aims to abstract away the API for
a build plan and it implements it for the in-process Cargo build plan
as well as the new, external one (for cargo build --build-plan
format).
In addition to that, since save-analysis (since rls-data 0.18.1)
includes the invocation used to compile a given crate, we try to
recreate the external build plan from the passed save-analysis files
via build_command config option.

This is squashed into a single commit not to miss anything during the rebase and because my branch diverged from head a bit and kept going back and forth; hopefully this will be legible.

TL;DR:

  • Moved plan.rs -> cargo_plan.rs
  • Added external build plan format (like in cargo build --build-plan)
  • Added and implemented BuildGraph trait for both plans
  • Recreate the build plan from invocations in save-analysis (for build_command case)

r? @nrc

@nrc
Copy link
Member

nrc commented Sep 28, 2018

Could you update Clippy so this runs tests on CI please?

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not done the deepest review, but I think this all looks good. It would be good if you could do a bunch of manual testing to ensure we keep working on different kinds of projects in different situations - we're close enough to the edition release that I'd hate to introduce new bugs.

@Xanewok
Copy link
Member Author

Xanewok commented Sep 28, 2018

Will do, thanks for taking a look!

I kept the old code path mostly intact with exception of CargoPlan::prepare_work - previously it was passed needs_rebuild: bool and manifest_path: &Path, both used to possibly recreate PackageMap and return a request to run Cargo.
Since the Cargo build plan is always initialized for a cargo() run, I changed the lazy PackageMap loading to always do it upfront but it seems like an okay thing to do, since whenever we request a build with BuildPriority::Cargo (explicit rebuild or build dir changed, we set needs_rebuild) we always end up running cargo(), thus always regenerating package map if needed.

@Xanewok
Copy link
Member Author

Xanewok commented Sep 28, 2018

error[E0061]: this function takes 6 parameters but 7 parameters were supplied
   --> src/build/rustc.rs:296:13
    |
296 | /             save::process_crate(
297 | |                 state.tcx.expect("missing tcx"),
298 | |                 state.expanded_crate.expect("missing crate"),
299 | |                 state.analysis.expect("missing analysis"),
...   |
309 | |                 },
310 | |             );
    | |_____________^ expected 6 parameters
error: aborting due to previous error

Blocked on rust-lang/rust#54356

@Xanewok Xanewok closed this Sep 29, 2018
@Xanewok Xanewok reopened this Sep 29, 2018
This adds a `BuildGraph` trait which aims to abstract away the API for
a build plan and it implements it for the in-process Cargo build plan
as well as the new, external one (for `cargo build --build-plan`
format).
In addition to that, since save-analysis (since rls-data 0.18.1)
includes the invocation used to compile a given crate, we try to
recreate the external build plan from the passed save-analysis files
via `build_command` config option.
@nrc nrc merged commit 440a985 into rust-lang:master Oct 9, 2018
@Xanewok Xanewok deleted the build-plan branch October 14, 2018 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants