Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support .rust-project.json (i.e. hidden) #17816

Closed
ojeda opened this issue Aug 7, 2024 · 14 comments · Fixed by #17818
Closed

Support .rust-project.json (i.e. hidden) #17816

ojeda opened this issue Aug 7, 2024 · 14 comments · Fixed by #17818
Labels
C-feature Category: feature request

Comments

@ojeda
Copy link

ojeda commented Aug 7, 2024

In the Linux kernel, we use rust-analyzer with a rust-project.json file.

However, it appears that there is no support for a hidden version of that configuration file, i.e. .rust-project.json, which means it is very visible when listing the root directory of the kernel sources (though only when generated, since the file is not committed into the repository and thus only visible when using rust-analyzer).

While this is only an issue for kernel developers that use Rust (i.e. so far a very small number compared to the total), since the file is normally not there, the file is not really something most kernel developers need to see, care or modify while working, so it is not really needed to show it in listings, and so a hidden .rust-project.json file would be better. I guess for other projects that may be the case as well.

This would follow other files like .clang-format, .gitignore, .gitattributes, .mailmap, .rustfmt.toml, etc. where configuration is not directly related to the project (say, a Kconfig file) but to tooling.

@alibektas
Copy link
Member

👋 Made a PR, but I will let others decide if they want to have it merged.

@bors bors closed this as completed in ddcd66b Aug 7, 2024
@ojeda
Copy link
Author

ojeda commented Aug 7, 2024

That was quick! Thanks a lot!

@davidbarsky
Copy link
Contributor

@ojeda out of curiosity, does Rust for Linux typically generate a rust-project.json on-the-fly for each user or do you commit one globally? I ask because if you generate it on the fly, #17246 might be nice UX improvement.

@ojeda
Copy link
Author

ojeda commented Aug 9, 2024

Yeah, we generate it on the fly (or, more precisely, currently the user triggers the generation when they want).

It sounds like your feature allows users to (re)generate the JSON file as needed by having rust-analyzer call a command that they configure into their discoverConfig (which can also watch extra files such as build system files). And ideally the kernel would provide the program (likely within the kernel source tree) to do what is right for the kernel build system. Users would still need to configure discoverConfig, but after that, it should "just work" and they don't need to care about. It also enables "dynamic" decisions on what to index or not, since the project can now customize that. Is that a right summary, or am I completely confused?

Thanks for the suggestion!

@davidbarsky
Copy link
Contributor

It sounds like your feature allows users to (re)generate the JSON file as needed by having rust-analyzer call a command that they configure into their discoverConfig (which can also watch extra files such as build system files). And ideally the kernel would provide the program (likely within the kernel source tree) to do what is right for the kernel build system. Users would still need to configure discoverConfig, but after that, it should "just work" and they don't need to care about. It also enables "dynamic" decisions on what to index or not, since the project can now customize that. Is that a right summary, or am I completely confused?

That is correct, yes! I provided a description of why this is handy in bazelbuild/rules_rust#2755 (comment), so that users wouldn't ever need to see the .rust-project.json on disk in the first place.

@ojeda
Copy link
Author

ojeda commented Aug 9, 2024

Then it sounds great -- I think the file being on-disk is not too bad (especially if hidden; although avoiding the file has other advantages, like being able to use rust-analyzer from a read-only location), but it is true that the regeneration part and the fine-grained possibilities are interesting.

We are planning some major changes to the build system soon, so it is the right time to think about this. I see in the manual: "Warning: This format is provisional and subject to change." -- is there an expected timeline for stabilization or a tracking issue to follow or similar (maybe #17537)?

Thanks again!

@davidbarsky
Copy link
Contributor

Sorry for the delay in responding, Miguel. I was supposedly on vacation last week.

I also I had a big comment written here, but Chrome crashed and I lost it. Anyways.

I think the file being on-disk is not too bad (especially if hidden; although avoiding the file has other advantages, like being able to use rust-analyzer from a read-only location), but it is true that the regeneration part and the fine-grained possibilities are interesting.

I'd appreciate a bit more details about this (there might be an miscommunication here?), but rust-analyzer.workspace.discoverConfig does not expect rust-project.json to be written to disk at any point—it expects that it'd be communicated with rust-analyzer over stdout via a quasi-IPC mechanism.

We are planning some major changes to the build system soon, so it is the right time to think about this. I see in the manual: "Warning: This format is provisional and subject to change." -- is there an expected timeline for stabilization or a tracking issue to follow or similar (maybe #17537)?

I'm inclined to make a new tracking issue for announcing changes to non-Cargo build system integrations; a more specialized form of #4604, if you will. #17537 is more of a general architectural improvement that needs to happen to rust-analyzer anyways and it's not specific to non-Cargo build system integrations.

I don't have a timeline for stabilization, but I am mostly happy with the design of configuring rust-analyzer.workspace.discoverConfig, its IPC mechanism, and rust-project.json. In more detail, here's what I'm thinking:

  • The existence of this functionality is effectively stable; the format might change with notice on the as-of-yet-created tracking issue.
  • rust-analyzer might make some breaking changes to those in the future, but we'd support the older and version for some reasonable time period (3 months, maybe? I haven't spoken to anyone else on the team yet...).
  • the consumers of this tracking issue would be people who have the ability to update tooling/settings for a larger Rust project on people's behalf via tooling, a rust-analyzer.toml, or both. The hope is that the blast radius to end-users of these changes would be minimal and would be noticeable mostly to tooling-focused people. The breaking changes we'd make to the aforementioned details would be in service of:
    • better performance of rust-analyzer. For example, this might be the removal of numerical crate IDs in favor of strings in order to support some form of long-lived crate identity to make reindexing less costly.
    • Support of more advanced features (e.g., a hypothetical WASM plugin system or progressively indexing the crate graph without waiting on the build system...).
    • fix midlayer/design mistakes.
      • My perspective is skewed by big build systems like Buck and Bazel (and more focused ones like Cargo...) so I don't want to commit rust-analyzer to a design where it can't reasonably support other build systems. It serves nobody.

Let me know if that is acceptable for Rust for Linux for the time being!

@ojeda
Copy link
Author

ojeda commented Aug 12, 2024

Sorry for the delay in responding, Miguel. I was supposedly on vacation last week.

No need to apologize -- a couple days is a quick reply, even more so in that case! :)

I'd appreciate a bit more details about this (there might be an miscommunication here?), but rust-analyzer.workspace.discoverConfig does not expect rust-project.json to be written to disk at any point—it expects that it'd be communicated with rust-analyzer over stdout via a quasi-IPC mechanism.

Sorry, I was replying to the "so that users wouldn't ever need to see the .rust-project.json on disk in the first place." part, and what I meant is that, for us, having the file on-disk (like it currently is the case with rust-project.json), is not too bad, so the regeneration and the fine-grained possibilities would be the main reasons for us to switch anyway, rather than avoiding the file.

I'm inclined to make a new tracking issue for announcing changes to non-Cargo build system integrations;

Sounds good. I subscribed to those other two, just in case :)

I don't have a timeline for stabilization,

I see, thanks! (I was mainly asking in order to know if we needed to test and/or give feedback quickly before it got frozen.)

Let me know if that is acceptable for Rust for Linux for the time being!

So updating the details in the mainline kernel would not be a problem -- I would expect we can keep up with those changes.

However, for stable kernels (or even just older, unsupported kernels), I imagine it would eventually happen that you update the format, those few months pass, and then an LTS kernel would not work anymore.

For those stable/older kernels, it is not super critical, since development is mostly done in mainline, but I would imagine developers would still want to have support there.

So I guess that may force us (and other projects, I would imagine) to maintain a/the tool outside the kernel tree, and ask people to use that instead, at least for older kernels. Or, if at least rust-project.json is stable, I guess we could keep the tool in-tree, but have rust-project.json as a fallback. But I think rust-project.json is not completely stable either -- we needed to change something about its generation recently.

Does that make sense?

And thanks for all the details!

@davidbarsky
Copy link
Contributor

No need to apologize -- a couple days is a quick reply, even more so in that case! :)

I appreciate it!

Sorry, I was replying to the "so that users wouldn't ever need to see the .rust-project.json on disk in the first place." part, and what I meant is that, for us, having the file on-disk (like it currently is the case with rust-project.json), is not too bad, so the regeneration and the fine-grained possibilities would be the main reasons for us to switch anyway, rather than avoiding the file.

Gotcha, makes sense! I'm also in a position where I'm able to see a lot of different kinds of user behavior with rust-analyzer, and you'd be surprised where people want to open their editor, especially if they're not coming from Rust. rust-analyzer, as of today, is a bit rigid with where it wants to load/index things, but that should change with #17537.

I see, thanks! (I was mainly asking in order to know if we needed to test and/or give feedback quickly before it got frozen.)

Makes sense! Yeah, rust-analyzer's stability policy is to prefer not breaking users, but accept that it might sometimes happen.

So updating the details in the mainline kernel would not be a problem -- I would expect we can keep up with those changes.

makes sense, thanks for explaining the workflow!

For those stable/older kernels, it is not super critical, since development is mostly done in mainline, but I would imagine developers would still want to have support there.
So I guess that may force us (and other projects, I would imagine) to maintain a/the tool outside the kernel tree, and ask people to use that instead, at least for older kernels. Or, if at least rust-project.json is stable, I guess we could keep the tool in-tree, but have rust-project.json as a fallback. But I think rust-project.json is not completely stable either -- we needed to change something about its generation recently.
Does that make sense?

It does, yeah! If you're able to provide a consistent snapshot of the Rust environment at that point in time (rustc; rust-analyzer, etc.), nix-style, then I think that'll be ideal, but that might be an unreasonable lift. Note that if you don't provide that hermetic infrastructure, the breakage that end-users might experience wouldn't be along the lines of rust-analyzer refusing to work at all; it'd be more of a subtle degradation in proc macro expansion. This is, of course, modulo the changes you needed to make to generating a rust-project.json. My hope is that cases like those are an exception and that we wouldn't really have any major discontinuities in functionality that would break end-users.

@ojeda
Copy link
Author

ojeda commented Aug 13, 2024

If you're able to provide a consistent snapshot of the Rust environment at that point in time (rustc; rust-analyzer, etc.), nix-style, then I think that'll be ideal, but that might be an unreasonable lift.

If you mean something like a monorepo where everything is pinned, including the toolchain and other third-party code and so on, then no, we don't control developers' toolchains. The kernel supports a minimum version of every tool and developers are expected to bring their own toolchain (typically from their Linux distribution, but also other places).

This is, of course, modulo the changes you needed to make to generating a rust-project.json.

Do you mean to add the fallback now, or do you mean in general? i.e. if there are backwards incompatible changes to rust-project.json, in a way that make rust-analyzer not work at all (rather than degraded functionality), and there is no "stable version" expected (i.e. the manual says the format is provisional, but if there is no plan to declare a stable, perhaps versioned, rust-project.json eventually), then I think we should probably move the generation out of tree even for rust-project.json. That makes things fairly inconvenient for kernel developers though (and more complex for us, since now we need to have a tool that works across all kernel versions and potentially several rust-project.json formats too).

By the way, in my previous message I was assuming rust-analyzer only supports its latest release/version/tag here (or some small window), is that true? i.e. we can't easily tell developers to user an older version, because those are not supported anymore, right?

Thanks!

@Veykril
Copy link
Member

Veykril commented Aug 13, 2024

By the way, in my previous message I was assuming rust-analyzer only supports its latest release/version/tag here (or some small window), is that true? i.e. we can't easily tell developers to user an older version, because those are not supported anymore, right?

Yes rust-analyzer is generally only compatible with the last couple stable releases of the rust toolchain. There is no "hard limit" on the version range as the incompatability tends to come from the standard library assuming a specific compiler making it next to impossible for r-a to support all toolchains. That problem generally goes away if the users use the toolchain provided rust-analyzer as that should be compatible with the toolchain version itself (at the expense of not having newer features of the rolling rust-analyzer releases).

@ojeda
Copy link
Author

ojeda commented Aug 13, 2024

Ah, what I meant to ask was what releases of rust-analyzer (the tool) are supported by rust-analyzer (the project).

In other words, if rust-analyzer only supports, say, the latest release or e.g. the latest month of releases (currently 2024-07-15, 2024-07-22, 2024-07-29, 2024-08-05 and 2024-08-12) and on top of that rust-project.json itself is not stable/versioned, then the only way out is to move the generation out of the kernel tree so that we can support the actually supported version of rust-analyzer and be able to adapt to changes to the file as time goes on.

@davidbarsky
Copy link
Contributor

If you mean something like a monorepo where everything is pinned, including the toolchain and other third-party code and so on, then no, we don't control developers' toolchains. The kernel supports a minimum version of every tool and developers are expected to bring their own toolchain (typically from their Linux distribution, but also other places).

Ah! That makes a lot of sense. I am completely unaware of how Linux development works.

Do you mean to add the fallback now, or do you mean in general? i.e. if there are backwards incompatible changes to rust-project.json, in a way that make rust-analyzer not work at all (rather than degraded functionality), and there is no "stable version" expected (i.e. the manual says the format is provisional, but if there is no plan to declare a stable, perhaps versioned, rust-project.json eventually), then I think we should probably move the generation out of tree even for rust-project.json. That makes things fairly inconvenient for kernel developers though (and more complex for us, since now we need to have a tool that works across all kernel versions and potentially several rust-project.json formats too).

I meant in general, but I'd like to step back a bit. While the documentation of rust-project.json says its provisional and subject to change, it's been pretty stable in practice over the past several years. The one breaking change I could see happening in the coming future is changing how crates are identified (instead of integer IDs, I was thinking of some sort of URI-esque approach), but even that change could potentially be done in a non-breaking manner.

Ah, what I meant to ask was what releases of rust-analyzer (the tool) are supported by rust-analyzer (the project).

Generally, only the latest stable release, but we can probably start versioning the rust-project.json format and support a version or two of the rust-project.json: I personally don't think it's an unreasonable burden on our (my?) end. I think we'd want to do a pass of the existing functionality on rust-project.json before attaching a version to it, however.

Do you mind sharing what change you needed to make in terms of rust-project.json generation/where that code is located? If it's of any help, I'd be be happy to review/see what y'all are doing there and provide some tips.

@ojeda
Copy link
Author

ojeda commented Aug 13, 2024

I meant in general, but I'd like to step back a bit. While the documentation of rust-project.json says its provisional and subject to change, it's been pretty stable in practice over the past several years. The one breaking change I could see happening in the coming future is changing how crates are identified (instead of integer IDs, I was thinking of some sort of URI-esque approach), but even that change could potentially be done in a non-breaking manner.

Yeah, I mentioned it mainly due to the recent change we needed (please see below).

Generally, only the latest stable release, but we can probably start versioning the rust-project.json format and support a version or two of the rust-project.json: I personally don't think it's an unreasonable burden on our (my?) end. I think we'd want to do a pass of the existing functionality on rust-project.json before attaching a version to it, however.

That sounds very reasonable.

Do you mind sharing what change you needed to make in terms of rust-project.json generation/where that code is located? If it's of any help, I'd be be happy to review/see what y'all are doing there and provide some tips.

It is this commit: Rust-for-Linux/linux@fe99216

And, of course, if you find anything that could be improved, that would be nice to know! Thanks!

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Aug 17, 2024
Very soon after we requested it [1], `rust-analyzer` added support for
`.rust-project.json` [2], i.e. the hidden variant of `.rust-project.json`.

While this is only a (tiny) issue for kernel developers that use Rust
(i.e. so far a very small number compared to the total), the file is not
really something most kernel developers need to see, care or modify while
working, so it is not really needed to show it by default in listings.

It also follows the pattern of other files that are not directly related
to the project (unlike e.g. a `Kconfig` file).

Thus migrate to it.

Cc: Ali Bektas <[email protected]>
Cc: Lukas Wirth <[email protected]>
Link: rust-lang/rust-analyzer#17816 [1]
Link: rust-lang/rust-analyzer#17818 [2]
Signed-off-by: Miguel Ojeda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature Category: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants