Bug 274909 – [cmake] clangd auto-setup

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Alicia Boya García]

# Goals

This patch aims to bring out-of-the box clangd completion for WebKit
developers using CMake builds.

This patch attempts to clear some of the existing pitfalls,
particularly for CMake builds, so that more people take advantage of
Clangd without having to put conscious effort into troubleshooting.

This patch also makes WebKit stock clangd setup compatible with both
Mac and non-Mac platforms and allows developers to customize clangd
configuration without dirtying their repository.

---

Getting clangd right takes a lot of context and moving pieces, so this
is one of those very long commit messages. Sections with (NEW) in the
title explain changes introduced by this patch, whereas the rest hope to
explain the background and why those changes are necessary or useful.

 # Architecture (NEW)

Several scripts have been added in Tools/clangd. A diagram describing
the build system targets, dependencies and products is also included.

The clangd auto-setup needs to generate the following artifacts:
 * .clangd configuration file
 * compile_commands.json file/symlink in the WebKit repo root.

Starting with this patch `.clangd` is no longer committed in the
repository it is now .gitignore'd and auto-generated during the build.

 # Compatibility with XCode builds (NEW)

This patch doesn't improve or worsen the status of clangd support in
XCode builds.

XCode build users are still expected to generate compile_commands.json
manually as per the wiki states:
https://trac.webkit.org/wiki/Clangd#macOS1

    make release EXPORT_COMPILE_COMMANDS=YES
    generate-compile-commands WebKitBuild/Release

The generation of the `.clangd` file is also included in the XCode
build, as part of the WTF XCode project.

 # clangd config generation (NEW)

The script `update-clangd-config` autogenerates a `.clangd` file with an
«autogenerated» marker at the top. The file will only be updated as long
as that marker exists, therefore allowing end-user customization if
needed.

The `.clangd` file is generated from a Jinja template, which will produce
slightly different options for Mac and non-Mac platforms, fixing the
regression introduced in https://github.com/WebKit/WebKit/pull/23273.

`update-clangd-config` is made part of the build by default. For XCode
builds, it is generated as part of WTF. For CMake builds, it is enabled
as a default target as long as CLANGD_AUTO_SETUP is set, which is the
default when building inside the WebKitBuild directory.

 # clangd configuration changes (NEW)

This patch also makes several changes to the clangd configuration file
to fix issues detected during testing:

 * UnifiedSource files are no longer indexed.
 * The `-fcoroutines` flag is stripped, as it is not recognized by some
   versions of clang, and by extension, clangd.
 * `-xobjective-c++` is now only passed in macOS to avoid depending on
   macOS-only headers in other systems.
 * `config.h` is no longer prepended to files within the ThirdParty
   directory.

 # compile_commands.json generation: context

CMake already generates a compile_commands.json file when the Ninja or
Makefile file is generated as long as CMAKE_EXPORT_COMPILE_COMMANDS is
ON, which is already the default in DEVELOPER_MODE.

When using unified builds (which is the default), that
`compile_commands.json` is somewhat subpar, as the language server is
unaware of most of the source .cpp files and hence has to require to
heuristics to match them to other build commands.

In XCode builds, the `generate-compile-commands` scripts already includes
logic to resolve unified builds.

On the CMake side, a RewriteCompileCommands target exists that parses
that file along with unified sources to generate an improved file at
`<BUILD_DIR>/DeveloperTools/compile_commands.json`. Unfortunately, this
may not be obvious to many developers, which may be unaware of its
existence.

 # Same compile_commands.json path for non-unified builds (NEW)

The first change this patch makes is solving the unified builds vs
non-unified builds dichotomy by introducing a new build target for
non-unified builds named SymlinkCompileCommandsInDeveloperTools.

This way, **no matter the type of build**,
`<BUILD_DIR>/DeveloperTools/compile_commands.json` is the path to be
relied upon.

 # compile_commands.json vs builds

For a `compile_commands.json` file to be used, it needs to exist in a
parent directory of the sources being worked on.

Since any generated `compile_commands.json` is specific for a given build
(e.g. Debug vs Release), with build-specific flags, developers need to
make a choice of what build to use for the `compile_commands.json` in
their WebKit checkout.

In XCode builds, this is passed explicitly to the
generate-compile-commands command.

In CMake builds, this is usually accomplished by symlinking the file
into the root of the WebKit repository.

Developers who frequently switch and remove builds need to be careful to
not let the compile_commands.json symlink become broken, as that would
cause the language server to rely on much worsened heuristics.

 # compile_commands.json symlink autogeneration and management (NEW)

This patch introduces automatic compile_commands.json symlink handling.
A new target is introduced that intentionally runs with every build:
`UpdateCompileCommandsSymlink`, which relies on the
`update-symlink-from-candidates` script, also introduced by this patch.
The script is meant to be fast enough to not introduce any perceivable
delay in builds.

`update-symlink-from-candidates` receives as arguments a symlink path and
a YAML file listing a series of candidate paths. The symlink is updated
to point to the first candidate path that exists.

This list of candidate paths is declared in a new .gitignore'd
`compile_commands.conf` file that is generated early during any CMake
build that uses `CLANGD_AUTO_SETUP`.

`compile_commands.conf` will be automatically updated from
`Tools/clangd/compile_commands.conf.example`; but similarly to `.clangd`,
a user can disable this and manage the file on their own, in this case
by setting the `user_managed` property inside the file to True.

 # Testing

clangd auto-setup has been tested with a Linux x86_64 machine using
webkit-container-sdk and Visual Studio Code, both with the default GCC
and with clang 16, with a fresh installation of VS Code (no settings).

It is possible to confirm if a file passes under clangd by using the
clangd --check command along with the compile lines used during
compilation, which can be queried from compile_commands.json.

A couple barebones scripts showing this usage have been provided in
Tools/clangd as documentation: clangd-check-file and
clangd-check-codebase.

I was able to run clangd-check-codebase with very few .cpp files
failing, and the failing files had good reasons for failing, like
missing includes that were obscured by unified builds.

[Alicia Boya García]

Pull request: https://github.com/WebKit/WebKit/pull/29313

[EWS]

Committed 280922@main (0a169e24c5d4): <https://commits.webkit.org/280922@main>

Reviewed commits have been landed. Closing PR #29313 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/131636190>

[Robert Jenner]

Reopened Bugzilla.
[cmake] clangd auto-setup, tracking revert in https://bugs.webkit.org/show_bug.cgi?id=274909.

[EWS]

Committed 280928@main (7122adc82225): <https://commits.webkit.org/280928@main>

Reviewed commits have been landed. Closing PR #30771 and removing active labels.

[Robert Jenner]

(In reply to EWS from comment #2)
> Committed 280922@main (0a169e24c5d4):
> <https://commits.webkit.org/280922@main>
> 
> Reviewed commits have been landed. Closing PR #29313 and removing active
> labels.

This change broke all Apple builds. We had to revert it. It specifically broke things with the error of:

update-clangd-config: No such file or directory
Command PhaseScriptExecution failed with a nonzero exit code

[Elliott Williams]

(In reply to Robert Jenner from comment #6)
> (In reply to EWS from comment #2)
> > Committed 280922@main (0a169e24c5d4):
> > <https://commits.webkit.org/280922@main>
> > 
> > Reviewed commits have been landed. Closing PR #29313 and removing active
> > labels.
> 
> This change broke all Apple builds. We had to revert it. It specifically
> broke things with the error of:
> 
> update-clangd-config: No such file or directory
> Command PhaseScriptExecution failed with a nonzero exit code

Right, this broke Apple's internal build, because we do not include the `Tools/` directory when we submit WebKit to build for our OSes.

I'm a little confused why you chose to add a generation step to WTF.xcodeproj, when the goal of this patch is to enhance the CMake build?

That said, if you do need to keep the script phase in WTF.xcodeproj, you could change it to only run the generation step on local builds:

    if [ "${DEPLOYMENT_LOCATION}" != YES ]; then
        "${SCRIPT_INPUT_FILE_0}" "${SCRIPT_INPUT_FILE_1}" "${SCRIPT_OUTPUT_FILE_0}"
    fi


This will run the script when you're building Xcode locally, and skip it when Xcode is installing to a deployment location, like it does in the internal OS build.

Let me know if you'd like me to help test the next revision -- sorry for the trouble!

[Alicia Boya García]

(In reply to Elliott Williams from comment #7)
> I'm a little confused why you chose to add a generation step to
> WTF.xcodeproj, when the goal of this patch is to enhance the CMake build?

So that the previously committed, now autogenerated .clangd file is also autogenerated on XCode builds.

[Alicia Boya García]

Pull request: https://github.com/WebKit/WebKit/pull/30784

[EWS]

Committed 282175@main (4b85cbe26eff): <https://commits.webkit.org/282175@main>

Reviewed commits have been landed. Closing PR #30784 and removing active labels.