Tracking issue: multiple targets/workspaces/packages, etc.

[nrc]

In many situations there is not just one primary crate - workspaces and packages are I think examples of this, basically. Another common one is where there is both a lib and bin (where cargo users would use --lib or --bin to filter builds). A related issue is where the primary crate is actually just a thin wrapper around a more interesting dep crate. There is also the case where a user wants to edit files in both the primary crate and a dep. The RLS doesn't really handle any of these scenarios very well.

Currently the only thing we can do about this is effectively set cargo check --lib by setting the build_lib option in rls.toml. We should expand the config file so we can set other filters and set a dep to be the primary crate for a project.

More long term we should:

• handle multiple active crates (this will require keeping files from more than one crate in the VFS and being smarter about how and what we rebuild)
• be able to infer sensible defaults for rls.toml options from the Cargo.toml
• offer UI for setting options for a project

cc #102

[sophiajt]

Diesel is an example of this style

[matklad]

being smarter about how and what we rebuild

Sorry for pushing this once again, but I really think that true rebuilding is not an option here. You should be able to have crates A and B, with B depending on A and with both crates modified (and not really able to compile fully). If the RLS wants to call into the compiler rustc should be able to handle more than one crate at once. That is, it should use A source code and not .rlib when compiling B.

I hope that this is included in the long-term plan for RLS/compiler integration. I totally understand why this restriction makes sense at the current stage :)

[bwasty]

Two cases I've run into that were not explicitly mentioned:

• examples in /src/example (cargo run --example <name>)
• multiple binaries in in /src/bin (cargo run --bin <name>)

For these it would be nice to not have to configure anything. Maybe the RLS could infer the settings from the directory path?

[Xanewok]

I probably should've posted more information on the work done for the workspaces here, sorry. More info can be also found over at #263.

Currently the new workspace mode can be enabled via "rust.workspace_mode" (needs "rust.unstable_features").
The default is to run something along the lines of cargo check --all. Additionally, in it, "rust.analyze_package" can be further specified to limit analysis only to a single package (as if -p <name> was used instead of --all).

Reporting diagnostics and lazy (i.e. not full) package rebuild should work now.
However, analysis data still requires more work (rust-dev-tools/rls-analysis#93).
As of now, the data may not be properly linked (the reloaded data seems to be valid only when used inside only last built package).

[nrc]

Status update. We're currently waiting on rust-dev-tools/rls-analysis#106 which is blocked on a Rust PR. Once that lands, we're basically done with our plan for workspaces (thanks @Xanewok !). Having both a lib and a bin (or multiple bins) should also be solved as part of that work, and build scripts (AIUI).

We can support any one of multiple targets using an option and there is work in progress (I think) for arbitrary Cargo features.

So, remaining work that I'm aware of:

• testing and debugging the various configurations (rustc and Servo would be good stress tests, but I think there might be other issues working with their build systems and general size/complexity. I don't think we need to handle such projects just yet, but things like Diesel and WebRender should be fine. Build scripts are probably an area we should test in detail).
• make sure that the options we offer are coherent, well-documented, cover most possibilities, and have sensible defaults.
• stabilise workspace support
• optimise (and hopefully remove) the distinction between single crate and workspace modes
• optimise the end to end compilation, i.e., can we make changes to Cargo or rustc that can make the RLS cleaner or faster?
• (related) support the RLS in tests and examples directories.

[rofrol]

rust-dev-tools/rls-analysis#106 merged

[nrc]

make sure that the options we offer are coherent, well-documented, cover most possibilities, and have sensible defaults.

The workspace-related options are:

workspace_mode: bool,
build_lib: Inferrable<bool>,
build_bin: Inferrable<Option<String>>,
all_targets: bool,
cfg_test: bool,

Thoughts:

• we should remove workspace_mode, it should always be on
• should all_targets be true by default? It means building takes longer, but it is often what people want.
• we should make cfg_test unstable - it doesn't work very well and all_targets is a better solution to getting indexing of test code
• I'm not sure where we are with build_lib and build_bin - we basically don't use them in workspace mode.

@Xanewok @alexheretic What do you think?

[alexheretic]

Yep I agree with these.

The bigger one is enabling all_targets by default which is something I already do personally and think makes sense from an rls user's perspective. I imagine many users are unaware of this feature right now.

[Xanewok]

Yeah, I guess that's fair.

The build_lib and build_bin could work together with analyze_package, but if we prefer to set that simply by initializing the RLS in the appropriate package directory, then I think we could remove them. Also building a single package should be relatively fast, so I don't think there are much gains from being able to specify only a single crate target.

Why doesn't cfg_test work too well? I think it does the equivalent of cargo check --tests now, which should work as expected (especially if all_targets work as expected now).

On that note, do you think we can prune non-workspace mode now in the codebase? Before doing that, I think it already has what it'd take to support the, currently lacking, single .rs file case (not Cargo), so this might be worth taking a look first.

[nrc]

The build_lib and build_bin could work together with analyze_package

Yeah, I guess the alternative is to make these work with workspace mode, I'm not sure how hard that would be. I'm not sure urgent that would be - aiui, we build everything in workspace mode with all_targets, so we get all bins and the lib, is there any advantage to specifying a particular binary? Given that binaries are always leaves in the dep graph, I don't think it would even make builds faster. I guess specifying build_lib would be faster because we would skip all bins.

Why doesn't cfg_test work too well?

Maybe I'm behind the times, I thought it had problems with the dead code lint.

On that note, do you think we can prune non-workspace mode now in the codebase?

Yes, although we should probably decide on build_lib/build_bin before that

I think it already has what it'd take to support the

it is workspace mode, or non-workspace mode? How much work do you think that would be?

[Xanewok]

is there any advantage to specifying a particular binary?

Probably not; I can imagine crate having multiple binaries and that it might be handy to only rebuild one, but if the lib is indexed once and shared across bins then I think we can remove it.

it is workspace mode, or non-workspace mode? How much work do you think that would be?

The non-workspace mode. It is effectively caching and running a single invocation of rustc, which is exactly the use case for a single file. However, it might be possible to do some unification and make it so that single file mode just creates a job queue with a single rustc job. (just an idea, didn't look into that)

[nrc]

Thinking about the single file case, I think that this is much lower priority than Cargo projects for 1.0 and that if we support it (and I do think we should) then we should find a way to do so in workspace mode so that we're not supporting these two separate modes. Worst case, when we want to implement, we can resurrect non-workspace mode.

[nrc]

Between f0a3c7a and bec14e8 I think everything here is addressed. The only remaining thing here is single file mode, which I think is addressed elsewhere