Add PG source dir to system includes #6590
Conversation
erimatnor
added
the
build
CMakeLists.txt
Outdated
| # includes BEFORE the installed PG include files. This will allow the LSP | ||
| # (e.g., clangd) to navigate to the PostgreSQL source instead of the install | ||
| # path directory that only has the headers. | ||
| include_directories(BEFORE SYSTEM ${PG_SOURCE_DIR}/src/include) |
There was a problem hiding this comment.
We have include_directories(SYSTEM ${PG_INCLUDEDIR_SERVER}) in build-defs.cmake already, I wonder why it doesn't work for clangd.
There was a problem hiding this comment.
Using includes from source might be slightly confusing, because Timescale is compiled against the ${PG_INCLUDEDIR_SERVER}, which doesn't necessarily match the source includes. E.g. if you're using several out-of-source builds for Postgres with different configurations, the pg_config.h will be different.
There was a problem hiding this comment.
We have
include_directories(SYSTEM ${PG_INCLUDEDIR_SERVER})inbuild-defs.cmakealready, I wonder why it doesn't work for clangd.
include_directories(SYSTEM ${PG_INCLUDEDIR_SERVER}) points to a different directory.
Good point about the pg_config.h. We could make this an optional override on the command line to protect against it.
I also tried using CMAKE_C_STANDARD_INCLUDE_DIRECTORIES on the command line but that always puts the PG source include after the installed header include directory, which won't help.
There was a problem hiding this comment.
The problem is that your IDE can't go to definition from the installed headers, right? That's why you want the ones from source. Personally I just added the entire PG source into the IDE project and it works, but I'm using QT Creator, not sure how this translates to clangd.
There was a problem hiding this comment.
Yeah, that's the problem. I am using Emacs + clangd, although I think it is a problem for all IDEs that use language server protocol (LSP) based on compile_commands.json unless they build their own index using some other method.
There was a problem hiding this comment.
@akuzm Added an option (default OFF) to enable this behavior. Now it won't interfere with out-of-source builds.
Add a CMake option to add the PostgreSQL source directory as a system include for the build system. This will ensure that the PG source include directory will end up in a generated `compile_commands.json`, which can be used by language servers (e.g., clangd) to navigate from the TimescaleDB source directly to the PostgreSQL source.
erimatnor
force-pushed
the
include-pg-source-dir
branch
from
03584d8
to
469a2b6
Compare
erimatnor
changed the title
|
@antekresic, @nikkhils: please review this pull request.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6590 +/- ##
==========================================
- Coverage 79.82% 79.52% -0.30%
==========================================
Files 190 190
Lines 37285 36972 -313
Branches 9458 9373 -85
==========================================
- Hits 29761 29403 -358
- Misses 3122 3146 +24
- Partials 4402 4423 +21 ☔ View full report in Codecov by Sentry. |
Add a CMake option to add the PostgreSQL source directory as a system include for the build system. This will ensure that the PG source include directory will end up in a generated
compile_commands.json, which can be used by language servers (e.g., clangd) to navigate from the TimescaleDB source directly to the PostgreSQL source.Disable-check: force-changelog-file