Add $manifest_path substitution for cargo override commands

[Veykril]

We now allow using $manifest_path in the overrideCommands for build scripts and checkOnSave if their invocation strategy is per-workspace, which will be replaced by the path to the corresponding workspace's Cargo.toml or rust-project.json

[davidbarsky]

naive question: while i think it's reasonable to make this required to start with, do you think this can be expended to not be a file and instead load this rust-project.json over some process that dynamically creates this?

(idk if this question is better addressed in a dedicated issue.)

[Veykril]

I'm not sure what you mean, but for flychecking to occur, we already need to have a project structure which implies that there is at least one rust-project.json or Cargo.toml loaded

[davidbarsky]

...I think I mixed up what this PR was doing, I'm sorry about that! Bit tired.

Anyways: while I get that rust-project.json does need to be loaded for flycheck (and any analysis from rust-analyzer), I'm wondering if it needs to be loaded from file, as opposed to be read in via stdout or UDS, or anything along those lines.

[Veykril]

(I assume this is in regards to project loading, otherwise I am not sure I understand) not necessarily, I could imagine an lsp extension for fetching the project structure maybe?

[bors]

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

[RalfJung]

How does this close #10793? We still have the problem that errors are printed with paths relative to the package manifest, not the working directory.

[Veykril]

I don't quite recall the issue and this PR that well (I was planning on looking into it next week). But I think the rough idea was that this could work around the issue somehow? Or maybe it was only related to some of the comment in the linked issue.

Actually, I think the idea was that if the command uses the $manifest_path var, r-a could infer that the diagnostics will be relative to the manifest directory, not the working directory? Might be wrong but, again I'll have to check next week and read up on the past discussions.

[RalfJung]

I wrote a summary of the current situation as I understand it at #10793 (comment)

[bors]

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

[Veykril]

I'll close this. I don't think we should add this to the cargo runners. Anything fancy like this most likely has a more complicated/different build system than cargo and should instead rely on the project-json format which might support this in the end. (this includes the rustc workspace which should switch to that format)

[RalfJung]

Where can one find more information about this project-json format and how it works in RA?

[Veykril]

https://rust-analyzer.github.io/manual.html#non-cargo-based-projects I can write up some thought regarding that wrt what x.py should do for that later

Related, #16135

[RalfJung]

Hm, having to set the sysroot dir by hand makes it pretty much impossible to just ship such a project file and use it unaltered?

Also, rustc basically uses cargo. Having to duplicate all dependency information for RA would be a huge waste of effort IMO.

So I don't see a practical way for rustc (or Miri) to use that format, unfortunately.

[RalfJung]

But anyway, my real issue is #10793, not this PR. I am not sure if this substitution would even help that issue.

[Veykril]

Also, rustc basically uses cargo. Having to duplicate all dependency information for RA would be a huge waste of effort IMO.

It does not. In terms of check diagnostic output sure, but aside from that the workspace does not resemble a cargo workspace. It barely works in r-a right now with the hacks we have in place to make it somewhat work in the first place. Just open one of the library crates and you'll see a bunch of incorrect diagnostics. Proc-macros stop working if you add certain excluded workspaces like the cg_clif backend and a bunch of other things. That's the main reason why I want rust-lang/rust to move to rust-project.json.

Hm, having to set the sysroot dir by hand makes it pretty much impossible to just ship such a project file and use it unaltered?

In general a project would have a command or similar to produce the file for users, it is unfortunate to require a setup step to use r-a but in rust-lang/rust's case at least, that is already kind of required with x.py setup anyways.

But anyway, my real issue is #10793, not this PR. I am not sure if this substitution would even help that issue.

Probably not, that still requires fixing from cargo's side imo.

[RalfJung]

All dependency management in rustc is done via cargo. So in that sense rustc is using cargo. Having a copy of that dependency information somewhere else will inevitably result in that copy getting out-of-sync, and keeping it up-to-date will add yet more roadblocks to contributing to rustc.

[Veykril]

Yet plain cargo commands do not work. Bootstrap solely putting cargo commands together that work out is nice and all, but r-a has assumptions about normal cargo workspaces that break due to how the repo works. For one the sysroot is very special in the workspace and to properly support r-a to work in the library folders (which are at the same time the sysroot of the workspace) is already so complicated that half the things break in r-a.

There might be a way to fix this in a simple way, but if there is I don't know how. I've tried many times, every time fixing one thing in the rust-lang/rust workspace at the cost of breaking something else. So frankly speaking I gave up on trying. And ideally r-a wouldn't have to special case anything for the rust-lang workspace, but for that to be the case the workspace either needs to become more like a plain cargo workspace or switch to rust-project.json (as all non-plain cargo build systems need to).

[RalfJung]

Maybe there's a middle-ground where bootstrap can describe to RA what is special about this workspace. But giving up entirely and duplicating everything about how these crates need to be build is not a good solution either, IMO. I understand your frustration, but now you're swinging too far in the other direction I would say.

Anyway as we said above this PR is not on the critical path to improving the RA experience for rustc. rust-lang/cargo#9887 is the blocker here.

[davidbarsky]

Speaking as a person who spends a lot of time with Buck and Rust: while Cargo is a great build system/package manager, I've noticed that Cargo doesn't scale particularly well to more complex builds (which is fair! a general purpose build system is hard). I don't think Lukas is swinging too hard in the opposite direction, honestly. Some of the issues that rustc is running into here would be substantially easier to resolve with a rust-project.json-based workflow. It might be an unfamiliar path, but I'd be happy to talk about my experiences in other contexts.

(iirc, a colleague of mine replaced x.py with buck2 as an experiment and rust-analyzer worked.)

[lnicola]

The Rust for Linux project has a Makefile target that creates a rust-analyzer.json. I can imagine x.py being able to generate one.

[RalfJung]

Sounds like something that should be discussed with t-compiler or with the bootstrap subteam.

[Veykril]

There are multiple things relevant for this, I'll try writing down some notes regarding the problems the current rustc workspace has. But the gist is just because bootstrap makes use only of cargo for dependency resolution and assembling the actual workspace with a multitude of cargo commands does not make rust-lang/rust a cargo workspace and given bootstrap in rust-lang/rustc rust-project,json is should work out pretty well as a general solution in my eyes.

[Veykril]

https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/r-a.20support.20for.20rust-lang.2Frust.20via.20project-rust.2Ejson