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

compdb: mergedCompileCommands for CMake subprojects #2029

Merged
merged 10 commits into from
Oct 11, 2021
Merged

compdb: mergedCompileCommands for CMake subprojects #2029

merged 10 commits into from
Oct 11, 2021

Conversation

Felix-El
Copy link
Contributor

@Felix-El Felix-El commented Aug 4, 2021

This changes visible behavior

The following changes are proposed:

Add an option to collect all compile_commands.json from the build directory and merge them into one for consumption by vscode-cpptools.

The purpose of this change

The option "Merged Compile Commands" will merge all compile_commands.json from the build directory into a new JSON file. This functionality is an enabler for IntelliSense in ExternalProject based "superbuilds".

Other Notes/Information

In order to get IntelliSense support with superbuilds, few settings are required:

{
    "cmake.mergedCompileCommands": "${workspaceFolder}/build/merged_compile_commands.json",
    "C_Cpp.default.compileCommands": "${workspaceFolder}/build/merged_compile_commands.json",
}

Signed-off-by: Felix Lelchuk <felix-el@users.noreply.github.com>
@ghost
Copy link

ghost commented Aug 4, 2021

CLA assistant check
All CLA requirements met.

@bobbrow bobbrow added this to the 1.9.0 milestone Aug 5, 2021
@andreeis
Copy link
Contributor

andreeis commented Oct 4, 2021

@Felix-El, can you describe how you can have more than one compile_commands.json in the same project? I am not familiar with this scenario and I need a repro to test your PR, before we accept it.

@Felix-El
Copy link
Contributor Author

Felix-El commented Oct 5, 2021

@Felix-El, can you describe how you can have more than one compile_commands.json in the same project? I am not familiar with this scenario and I need a repro to test your PR, before we accept it.

Sure, this feature can help a lot if you are, like I am, working with CMake subprojects, i.e. using ExternalProject.
Each subproject typically has its own private build, say build/subproj1, build/subproject2, etc.
Now if I let CMake generate the compile_commands.json I receive one of those files in the build folder of each subproject
(with nested subprojects there can be again multiple).
As such projects evolve it is impractical to carry along a list of project names or compile_commands.json paths, hence I decided to find them by crawling the build directory.

Here is a multi-subproject test I've added with a new CMake featrue (scheduled for 3.22): https://gitlab.kitware.com/cmake/cmake/-/tree/master/Tests/InstallMode
It should serve well to verify this PR.

@andreeis
Copy link
Contributor

andreeis commented Oct 8, 2021

@Felix-El, we have a suggestion if you can update your PR. Let's have instead one path setting for the merged compile commands ("cmake.mergedCompileCommands") and no boolean because we can deduce everything based on the path being set or not.
What do you think of this approach?

@Felix-El
Copy link
Contributor Author

Felix-El commented Oct 8, 2021

@Felix-El, we have a suggestion if you can update your PR. Let's have instead one path setting for the merged compile commands ("cmake.mergedCompileCommands") and no boolean because we can deduce everything based on the path being set or not. What do you think of this approach?

I assume that means the *.json crawl & collect logic would remain as proposed, just the configuration would add a cmake.mergedCompileCommands (which allows to create a separate merged JSON) instead of a cmake.collectCompileCommands (which would affect cmake.copyCompileCommands JSON).

If that is correct I totally agree!

@bobbrow
Copy link
Member

bobbrow commented Oct 8, 2021

Yes. We wanted to avoid requiring setting both the merge and copy settings to get predictable behavior.

This replaces the collectCompileCommands addition,
which altered the behavior of copyCompileCommands
into a standalone mergeCompileCommands config option
so both can co-exist without cross-interference.
@Felix-El Felix-El changed the title compdb: opt to use any compile_commands.json in build dir compdb: mergeCompileCommands for CMake subprojects Oct 9, 2021
@Felix-El Felix-El changed the title compdb: mergeCompileCommands for CMake subprojects compdb: mergedCompileCommands for CMake subprojects Oct 9, 2021
@Felix-El
Copy link
Contributor Author

Felix-El commented Oct 9, 2021

Ok, please give it another look...

@andreeis andreeis merged commit 73358f9 into microsoft:develop Oct 11, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants