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

ClangdCProjectDescriptionListener doesn't respond to build configuration changes of CMake projects #435

Open
jonahgraham opened this issue Feb 20, 2025 · 6 comments
Assignees

Comments

@jonahgraham
Copy link
Member

In eclipse-cdt/cdt#1084 a fix was introduced to stop using "default" as the build folder, which was basically used when CDT couldn't figure out what the user actually asked for.

This change works fine, but it exposed that ClangdConfigurationFileManager is not updating .clangds CompilationDatabase path when the Core Build (CMake) active config changes. AFAICT This is because the ClangdCProjectDescriptionListener is not listening to sufficient events to see the change.

To reproduce (without eclipse-cdt/cdt#1084 applied)

  • Create CMake project
  • Immediately change (before first build) launch mode in launchbar from Run to Debug Image

Notice how .clangd continues to have CompilationDatabase: build/default and that some indexing isn't working

@betamaxbandit identified this, so he may be able to provide a more complete description

@jonahgraham
Copy link
Member Author

@betamaxbandit: I don't know if CDT LSP should listen to IResourceChangeEvents to see that IProject description change or if CDT's Core build should be sending out CProjectDescriptionEvent. I suspect the former, but I hope you have insight.

@betamaxbandit
Copy link

betamaxbandit commented Feb 20, 2025

Thanks to @jonahgraham for raising this issue. We were initially conversing via email, so I'll paste in here the findings I wrote about.

==

I used the msys2 info on “Before you begin” help page (eclipse-cdt/cdt#1086) to update my mingw installation with clangd.

Then I added the path in prefs:

Image

And set the c/c++ editor LSP as default.

Then I create a cmake project and this method is called:

org.eclipse.cdt.lsp.clangd.internal.config.ClangdConfigurationFileManager.buildConfiguration(IResource)

It returns null for the ICBuildConfiguration, because the active build config at this time is “default”; the CoreBuildLaunchBarTracker has not had the opportunity to get in and do it’s magic at this point.

It results in “"Cannot determine path to compile_commands.json"” log in setCompilationDatabasePath(…).

I notice a .clangd file created in the project folder with the following content:

CompileFlags:
  # When using clangd update this entry to point at the desired
  # configuration directory to pick up the compile_commands.json
  CompilationDatabase: build/default

Note the path “build/default” is used.

In the CMake project, when I change the build configuration (switch to another launch target or launch mode), I expected to see ClangdConfigurationFileManager.buildConfiguration called again and the .clangd file updated, but I did not see this.

Code navigation in the LSP editor seems to mostly work, but I can’t tell without a more rigorous test whether it’s using the correct info.

Image

However, it fails to find the config.h.in include for some reason and reports errors in the Problems view. I don’t know if this is related to the build config issue.

When I restart the workbench, I notice another version of buildConfiguration is called:
org.eclipse.cdt.lsp.clangd.internal.config.ClangdFallbackManager.buildConfiguration(IResource)

When this happens, the project’s active IBuildConfiguration is set correctly and so the correct ICBuildConfiguration
Is returned from build.getBuildConfiguration(active).

After the workbench restart, the errors markers have disappeared, and I can open config.h successfully.

Image

BTW, you can see what the expected build directory name is by expanding the build directory in the project. For each new build configuration a new directory is created (after clicking build in the launch bar). Since eclipse-cdt/cdt#1084, this will never be "default". The name is calculated by https://github.com/eclipse-cdt/cdt/blob/654e27076709ce2b8a9a42fd4acbc6b92bc3442c/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/ICBuildConfigurationProvider.java#L93

I'd like to know what the purpose of the .clangd file is. From what I can see, it doesn't seem to be used, but I expect I've missed something.

Cheers John

@betamaxbandit
Copy link

CProjectDescriptionEvent

@jonahgraham ,
Interesting question. I don't know much about CProjectDescriptionEvent.

I do know that when the CoreBuildLaunchBarTracker detects that a new build config should be set active, it calls*:

org.eclipse.core.resources.IProjectDescription.setActiveBuildConfig(String)
org.eclipse.core.resources.IProject.setDescription(IProjectDescription, IProgressMonitor)

I would have thought calling these two methods would cause the CProjectDescriptionEvent to be fired, but I don't know for sure.

*: https://github.com/eclipse-cdt/cdt/blob/654e27076709ce2b8a9a42fd4acbc6b92bc3442c/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/launch/CoreBuildLaunchBarTracker.java#L244-L245

@jonahgraham
Copy link
Member Author

I would have thought calling these two methods would cause the CProjectDescriptionEvent to be fired, but I don't know for sure.

Sadly not expected: CProjectDescriptionEvent is for modification of CDT project, while IProject.setDescription modifies Eclipse platform's project.

Should https://github.com/eclipse-cdt/cdt/blob/654e27076709ce2b8a9a42fd4acbc6b92bc3442c/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/launch/CoreBuildLaunchBarTracker.java#L247-L248 have an event as a side effect?

@jonahgraham
Copy link
Member Author

@betamaxbandit and I had a chat about this and listening to resource change events seems best to track the change to the project's active build config.

@jonahgraham jonahgraham self-assigned this Feb 20, 2025
@jonahgraham
Copy link
Member Author

I'm working on this - hopefully have something by RC1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants