cc-rs icon indicating copy to clipboard operation
cc-rs copied to clipboard

Add API to Record Compile Invocation

Open schrieveslaach opened this issue 2 years ago • 11 comments

This commit adds API to record the compilation invocations in order to emit compile_commands.json a.k.a. JSON compilation database.

Fixes #497

See following screencast that uses Neovim with rust-analyzer and clangd as language servers:

output

schrieveslaach avatar May 26 '22 13:05 schrieveslaach

Not that I have an actual say, or that Alex wants to have one now, but consider https://github.com/rust-lang/cc-rs/pull/612#discussion_r677554949. ~Not even~ Something as simple as which was frown upon:-) But in either case, I for one would argue that [at the very least] making it optional would be more than appropriate.

dot-asm avatar Jun 01 '22 18:06 dot-asm

Not that I have an actual say, or that Alex wants to have one now, but consider #612 (comment). ~Not even~ Something as simple as which was frown upon:-) But in either case, I for one would argue that [at the very least] making it optional would be more than appropriate.

I switched to tinyjson to reduce compile times and I also added a feature flag so that user can opt in.

schrieveslaach avatar Jul 04 '22 08:07 schrieveslaach

@HKalbasi, as you have the need to use compile_commands.json (see #691), do you mind to test this PR and provide some feedback?

schrieveslaach avatar Jul 07 '22 07:07 schrieveslaach

Hi @schrieveslaach! unfortunately I'm using cc indirectly, and I'm totally newbie in c/c++ tooling, so it is not easy for me to experiment with this PR. But the result in the GIF looks great.

I just have a (probably newbie) question: would this design work if some project needs multiple calls to .compile? Or each call to .recorded_compile will override the previous dump?

HKalbasi avatar Jul 08 '22 14:07 HKalbasi

@HKalbasi, thanks for the response.

would this design work if some project needs multiple calls to .compile? Or each call to .recorded_compile will override the previous dump?

recorded_compile return a Vec<CompileCommand> which you have to pass to store_json_compilation_database to store the file as JSON. If you have multiple calls to recorded_compile you can collect the results in a single Vec<CompileCommand> before passing them to store_json_compilation_database.

Does that clarify your question?

schrieveslaach avatar Jul 23 '22 06:07 schrieveslaach

@m-ou-se, @Mark-Simulacrum, you seem to be the maintainers of the project. Do you mind to have a look at this PR?

schrieveslaach avatar Jul 24 '22 07:07 schrieveslaach

Does that clarify your question?

Yes. I thought .recorded_compile would generate the file on its own. But in this way everything is fine.

It might make sense to have some wrapper type over Vec<CompileCommands> with a .generate(file_name) method for convenience. Not really important, just something that might worth considering after this PR gets some attention from maintainers.

And thanks for this PR!

HKalbasi avatar Jul 24 '22 15:07 HKalbasi

Wouldn't it be more natural to have .record(<path-to-compilation-database>) method, as opposed to making users replace .compile() methods and perform extra steps actually saving the database?

BTW, is it correct understanding that the file name is actually fixed, compile_commands.json? So that the only variable is the target directory, right? But is it actually? I mean the file is surely searched for in specific locations. For example it's suggested that clangd starts in the directory where the source file is and walks up the tree. This sounds like it would be possible and even appropriate to infer even the destination directory for the database... In other words I wonder if it could be just .record(), or .store_compile_database(), or .save_compile_commands(), or... Or even more radical, why additional methods at all, just enable the feature and leave build.rs as is... Well, it all might be just wishful thinking, in which case my apologies for unnecessary distraction :-)

dot-asm avatar Jul 25 '22 12:07 dot-asm

I chose to add store_json_compilation_database as a separate function because if you have multiple calls to Build::recorded_compile you can collect them and store them in one file.

I checked the clangd docs and clangd searches for compile_commands.json in the following way:

We first check for a compilation database in the directory containing the source file, and then walk up into parent directories until we find one.

Running clangd --help reveals:

--compile-commands-dir= - Specify a path to look for compile_commands.json. If path is invalid, clangd will look in the current directory and parent paths of each source file

So, I assume what I could do is that recorded_compile could write compile_commands.json into the current directory (might be changed optionally, e.g. Build::compile_database_directory(path)). And if the file already exists than it can check if there is already an entry for the particular file and if so it updates it.

@dot-asm, @m-ou-se, @Mark-Simulacrum, @HKalbasi what do you think about that approach?

schrieveslaach avatar Sep 26 '22 08:09 schrieveslaach

looking forward to using this pr!

GopherJ avatar May 08 '23 08:05 GopherJ