Use build scripts from rustc source workspace

[eggyal]

The status quo ante (since #14348) is that, if a workspace has one or more packages with package.metadata.rust-analyzer.rustc_private = true, build scripts for those rustc_private crates are located either from building the workspace itself (if the workspace is the rustc source root) or else from the toolchain's target-libdir (as determined from invoking rustc --print target-libdir).

The problem with this approach is that, when working on rustc, there are such workspaces that are not themselves the rustc source root—and the toolchain target-libdir in such situations is not the appropriate one for use by the stage0 proc-macro server. For example, rustc_codegen_cranelift is opened as a workspace, it is not the rustc source root, and (owing to its rust-toolchain override file) its target-libdir is that of a specific nightly compiler; attempting to use the proc-macro dylibs therein gives rise to an ABI mismatch error.

The correct build scripts are instead located from building the opened rustc source root workspace, but finding these when building another workspace's crate graph requires the rustc source root workspace already to have been identified (and built). This PR therefore ensures that any such workspace is processed first, and that a reference thereto is passed into the crate graph construction functions for each workspace.

Incidentally, while working on debugging this issue, I noticed that such arising CrateOrigin::Rustc crates end up as duplicates of the CrateOrigin::Local crates from the opened rustc source root workspace—I already have a little deduplication tweak to the CrateGraph::extend function which I will open as a separate PR (and this pretty much halves the number of crates indexed by RA when working on rustc).

[Zalathar]

Crate duplication also causes other problems (e.g. #15656), so I’m curious to see whether your upcoming PR has the side-effect of fixing that (for rustc) too.

[eggyal]

On the crate deduplication front, I've eliminated a bunch of cases when merging sysroot (CrateOrigin::Lang) crates from different workspaces by ignoring the SysrootPublicDeps that rust-analyzer synthetically adds to them if they're in a loaded Cargo workspace (which also is the origin of various cyclic dependency errors that one sees). This change alone has a huge impact because pretty much everything depends upon them and the failure to deduplicate them causes dependency comparisons in downstream deduplications to fail, thus causing cascading failures/duplicates throughout the entire crate graph.

I haven't looked into the issue you've linked to see whether that alone would be sufficient, but the reduction in duplicates is very marked. However, I've been holding off opening the PR because there remain some further interesting duplicates that I think should also be eliminated... but perhaps it makes more sense to do this in small steps. I'll probably open a PR tomorrow either way.

[Veykril]

I wonder, wouldn't it be enough to just check if the projects workspace root is a child of the rustc one in this case? That is change the equivalence check from #14348 to path child check. The rustc dir itself should be correct for all already since the recommended config has it set to a path (opposed to leaving it to "discover")

[Veykril]

And yes, it does not surprise me that there are duplicated crates here. Iirc if you look into the library crates there are a bunch of type mismatches because all the library crates are duplicated and their dependencies are all over the place due to incorrect/missing deduplication there. I fixed part of that but not all and that also just broke other stuff yet again.

Found it #14962 (comment)

[eggyal]

wouldn't it be enough to just check if the projects workspace root is a child of the rustc one in this case?

In practice, probably yes. But conceptually there's nothing that requires all workspaces that are opened with a rustc project to be children of the rustc source route.

[eggyal]

Also, if by "just check" you mean simply to change if rustc_workspace.workspace_root() == cargo.workspace_root() to if cargo.workspace_root().starts_with(rustc_workspace.workspace_root()) or similar then no that alone wouldn't work, as rustc_workspace would still be the toolchain's target-libdir.

[Veykril]

Also, if by "just check" you mean simply to change if rustc_workspace.workspace_root() == cargo.workspace_root() to if cargo.workspace_root().starts_with(rustc_workspace.workspace_root()) or similar then no that alone wouldn't work, as rustc_workspace would still be the toolchain's target-libdir.

Huh? Doesn't the recommended vscode settings json in the rustc repo set the rustc-dir explicitly (via rustc.source config)? Then that should always point to the repo itself, not the toolchain. Even with a toolchain file, the setting should disable the discover mode for the rustc sources.

In practice, probably yes. But conceptually there's nothing that requires all workspaces that are opened with a rustc project to be children of the rustc source route.

And this I am happy to just not support. It's bad enough that we are having to hack things into r-a just to support the rustc repo's workspace as is. That OTOH is an even more exotic setup that I don't see value in trying to make work. I'd rather just pursue x.py emitting a rust-project.json to fix all of this in the end.

[eggyal]

Huh? Doesn't the recommended vscode settings json in the rustc repo set the rustc-dir explicitly (via rustc.source config)? Then that should always point to the repo itself, not the toolchain. Even with a toolchain file, the setting should disable the discover mode for the rustc sources.

Aaaah, yes, you're right. The rustc_workspace is correctly overridden by rustc.source config; it's the rustc_build_scripts that are still pointing at the toolchain's target-libdir... but with your proposed change we'd be ignoring those, and using the build scripts of the current cargo workspace instead. I think that still won't work though, because the build scripts of the cargo workspace are (I suppose, correctly) only those that x.py identified as being built for it, whereas the presence of rustc_private requires all transitive dependencies of rustc_driver to be present.

To give a concrete example, whilst rustc_codegen_cranelift uses rustc_private crates, it does not depend upon rustc_macros and therefore rustc_macros is not amongst the artifacts output by x.py for the cg-clif workspace when generating build scripts. However, because rustc_driver transitively depends upon it, it is still expected to be present in the workspace's build scripts.

I guess this could be resolved by including all build scripts in all workspaces? But that's perhaps a bit OTT. Or else perhaps more gracefully tolerating absence of a rustc_private crate/build script.

In practice, probably yes. But conceptually there's nothing that requires all workspaces that are opened with a rustc project to be children of the rustc source route.

And this I am happy to just not support. It's bad enough that we are having to hack things into r-a just to support the rustc repo's workspace as is. That OTOH is an even more exotic setup that I don't see value in trying to make work. I'd rather just pursue x.py emitting a rust-project.json to fix all of this in the end.

I'm on board with that.

[bors]

☔ The latest upstream changes (presumably #16153) made this pull request unmergeable. Please resolve the merge conflicts.

[Veykril]

I'll close this, as mentioned I think we (and with that I mean someone other than me) should instead investigate adding a command to x.py / having x.py generate a rust-project.json that contains a setup for the relevant profile of the developer. Then we can also finally remove some of the rustc workspace hacks in r-a.

I'm happy to help out with how to do that for anyone interested but I won't be working on that myself.

[eggyal]

@Veykril I'm once again running up against this problem, but have some time to try and implement the fix that you were proposing. However, having looked over the manual section on Non-Cargo Based Projects (which describes the rust-project.json format), I can't see how any of the existing settings can address this problem.

Were you proposing that the format be modified if need be? If so, then I'm not entirely sure what we would need to represent in the file that isn't already available to/known by RA elsewhere?

[Veykril]

There is a PR open that implements rust-project.json as a bootstrap step here rust-lang/rust#120611. I don't know the current status of the PR though

Also related zulip thread https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/rust-analyzer.20rust-project.2Ejson

The reason for why I'm pushing towards having bootstrap generate this file is that bootstrap knows its own hacks needed to make the workspaces work in the rust-lang/rust repo. So it only makes then for it to generate this file, instead of having rust-analyzer attempt to replicate bootstrap behavior here. As soon as you have a non cargo command / script / tool that is needed to invoke your builds you are out of default rust-analyzer project support (unless its a trivial wrapper that exposes the same cargo api surface).