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

Workspace mode builds the whole workspace, even if not necessary #614

Closed
nrc opened this issue Dec 5, 2017 · 4 comments
Closed

Workspace mode builds the whole workspace, even if not necessary #614

nrc opened this issue Dec 5, 2017 · 4 comments
Milestone

Comments

@nrc
Copy link
Member

nrc commented Dec 5, 2017

If we have a crate inside a workspace and we open the crate in the RLS (not the whole workspace), the RLS builds the entire workspace and any deps, not just the crate which was opened.

This causes problems in the Rust distro where in order to run each test in workspace mode, we end up building the entire Rust distro.

cc @Xanewok

@DSpeckhals
Copy link
Contributor

I was wondering why I was getting beginBuild messages but nothing else when running tests locally in rust-lang/rust after preparing an RLS update there...

@Xanewok
Copy link
Member

Xanewok commented Dec 5, 2017

Oh crap. Sorry, I didn't foresee that problem before. @nrc thanks for looking into this, it seems that's the case.

So in workspace_mode, without analyze_package specified, we fall back to compiling as if we were running cargo check --all. It turns out that if you run cargo check --all inside a member package, it check-builds the whole workspace, including other member packages and their deps and so forth. If one just runs cargo check inside, it seems to implicitly call cargo check -p <cur_pkg_name> instead, limiting the build scope to only that package.

The quick Rust CI hack fix now would be to specify manually analyze_package for each test (I'll have to test that one, though), while ideally we would infer those config options, just like with build_bin etc.
So if user initialized RLS inside a workspace member package, we would infer analyze_package: <cur_pkg>, unless explicitly set by the user. We'd have to allow the user to explicitly say "null", so this might need introducing additional build_every_package: boolean | null or similar config option.

@sfackler
Copy link
Member

sfackler commented Dec 5, 2017

#613 happens with analyze_package set, so I think it'd be a separate issue.

@nrc
Copy link
Member Author

nrc commented Dec 5, 2017

So if user initialized RLS inside a workspace member package, we would infer analyze_package: <cur_pkg>, unless explicitly set by the user. We'd have to allow the user to explicitly say "null", so this might need introducing additional build_every_package: boolean | null or similar config option.

Yeah, I agree we should do this. I don't think we need to have a build_every_package option - if the user wants to build everything they should open the whole workspace, whereas if they want to build one crate, they should just open that crate (in fact, I hope this would mean we don't need analyze_package at all in the long term).

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

No branches or pull requests

4 participants