zig icon indicating copy to clipboard operation
zig copied to clipboard

Implemented the JSON compilation database support

Open jonathanmarvens opened this issue 1 year ago • 11 comments
trafficstars

After commenting on #17365 yesterday, I decided I could do better than just +1 and complain, so I went ahead and implemented the JSON compilation database support. It likely could be implemented better, but I have been testing it and it works just fine. We can always improve it!

Now, all a user needs to do to get that sweet compile_commands.json is run zig build -fcompdb.

Resolves #17365.

jonathanmarvens avatar Dec 28 '23 10:12 jonathanmarvens

Note that you're currently only generating fragments which aren't a full compile_commands.json file. They have to be merged after the build happened.

~~Basically just do a "[" + join(",", fragments) + "]" and write that into an output file, best case in zig-cache/compile_commands.json or something~~

Ignore the above, but we have to merge all fragments, not only the ones per std.Build.Step.Compile, because otherwise projects with more than one compile step can only have a single subcompilation for completion

ikskuh avatar Dec 28 '23 11:12 ikskuh

Note that you're currently only generating fragments which aren't a full compile_commands.json file. They have to be merged after the build happened.

~Basically just do a "[" + join(",", fragments) + "]" and write that into an output file, best case in zig-cache/compile_commands.json or something~

Ignore the above, but we have to merge all fragments, not only the ones per std.Build.Step.Compile, because otherwise projects with more than one compile step can only have a single subcompilation for completion

@MasterQ32 Unless I'm misunderstanding you, the fragments do get merged in the generateCompdbFromEntryPathsSet() function. Have you tried compiling this branch and trying it out? If you haven't, please do and report back! Thank you.

jonathanmarvens avatar Dec 28 '23 11:12 jonathanmarvens

Unless I'm misunderstanding you, the fragments do get merged in the generateCompdbFromEntryPathsSet() function.

Yes, but only for a single compilation invocation, not for the whole build.

I didn't try yet, but:

You're creating one compile_commands.json for each invoked std.Build.Step.Compile. This means, for a bigger project of mine that would create roughly 30-40 compile_commands.json files instead of a single one.

You gotta collect all generated fragments in the root std.Build builder and then write the output files after

https://github.com/ziglang/zig/blob/a87142984ce82e9bfc68dd96ddc8f6ef0ee27bc5/lib/build_runner.zig#L341-L352

is executed, and collect all fragments from all std.Build.Step.Compile invocations from all std.Build instances.

ikskuh avatar Dec 28 '23 12:12 ikskuh

Unless I'm misunderstanding you, the fragments do get merged in the generateCompdbFromEntryPathsSet() function.

Yes, but only for a single compilation invocation, not for the whole build.

I didn't try yet, but:

You're creating one compile_commands.json for each invoked std.Build.Step.Compile. This means, for a bigger project of mine that would create roughly 30-40 compile_commands.json files instead of a single one.

You gotta collect all generated fragments in the root std.Build builder and then write the output files after

https://github.com/ziglang/zig/blob/a87142984ce82e9bfc68dd96ddc8f6ef0ee27bc5/lib/build_runner.zig#L341-L352

is executed, and collect all fragments from all std.Build.Step.Compile invocations from all std.Build instances.

@MasterQ32 Ah, I see your point. So far, I have tested this on multiple C/C++ build.zig setups, but I just realized the one thing they all have in common is that the root C source files are all being built in a single step, which is why I didn't consider this. I'll have to make some more time to do more testing and address what you've pointed out. I think it will require some significant changes, but we'll see. Any chance you have some example projects with build.zig files I can use?

jonathanmarvens avatar Dec 28 '23 12:12 jonathanmarvens

Any chance you have some example projects with build.zig files I can use?

https://github.com/MasterQ32/apigen builds itself one executable and one from a dependency. everything else is too complex to setup for a simple test.

ikskuh avatar Dec 28 '23 13:12 ikskuh

Any chance you have some example projects with build.zig files I can use?

https://github.com/MasterQ32/apigen builds itself one executable and one from a dependency. everything else is too complex to setup for a simple test.

@MasterQ32 Cool, thanks!

jonathanmarvens avatar Dec 28 '23 13:12 jonathanmarvens

@MasterQ32 My latest updates address the issue you pointed out. I also tested it on your repo; I had to update your repo a bit to make it work with the latest Zig changes, but other than that, zig build -fcompdb successfully generated the compile_commands.json that covers the entire build! Hopefully, you get a chance to build and test it out.

jonathanmarvens avatar Dec 29 '23 01:12 jonathanmarvens

Resolves #9323.

This PR is definitely an improvement on the status quo, but note that it does not actually resolve that issue. The ask there is specifically for zig cc -MJ to work in the same way as clang -MJ, because not all users of zig cc use the Zig build system.

alexrp avatar Dec 29 '23 02:12 alexrp

Resolves #9323.

This PR is definitely an improvement on the status quo, but note that it does not actually resolve that issue. The ask there is specifically for zig cc -MJ to work in the same way as clang -MJ, because not all users of zig cc use the Zig build system.

@alexrp Fair enough. I'll update the description to reflect that.

jonathanmarvens avatar Dec 29 '23 06:12 jonathanmarvens

Thanks @jonathanmarvens - I'm looking forward to reviewing this soon. Don't worry about merging master branch in. It's actually better if you just let it sit on green. When you do want to update the PR, a rebase makes things easier for the maintainer.

andrewrk avatar Jan 09 '24 07:01 andrewrk

Hey @jonathanmarvens - I know it's been a few monthss - I wanted to ask about the progress on this? (Mostly as an interested/invested watcher from the side).

Ttibsi avatar Feb 24 '24 16:02 Ttibsi

I fixed up these merge conflicts in a fork and rebased onto master. If anyone wants to use it: https://github.com/GalaxyShard/zig/tree/feature/add-compilation-database-support

Tested it on two of my projects, only issue was that using -fcompdb always recompiled every C and C++ file even when run twice in a row with no changes.

I'll make a PR if I fix the cache problem.

GalaxyShard avatar Aug 24 '24 04:08 GalaxyShard

only issue was that using -fcompdb always recompiled every C and C++ file even when run twice in a row with no changes.

ideally it should be changed to not compile files at all, thats how other tools such as cmake/meson works (in other words it should be a dry run that just outputs the commands to a json file), but other build steps such as generating files should run. Otherwise its painful to work on projects that take hours to compile (chromium for example).

dec05eba avatar Aug 24 '24 07:08 dec05eba