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

feat: create a symlink of complie_commands.json on the source dir #190

Merged
merged 18 commits into from
Jan 30, 2023

Conversation

FeignClaims
Copy link
Contributor

@FeignClaims FeignClaims commented Jan 19, 2023

#190 (comment)

It helps clang based tools to find compile_commands.json when using out-of-source builds, and to find the correct compile_commands.json when handling multiple builds.
Already tested in my FeignClaims/cmake_starter_template (for the orign extension version, see cmake/SymlinkCompileCommands.cmake) when I switch between clang and gcc on MacOS and Windows.

Users should be notified to add compile_commands.json to .gitignore though.

src/Common.cmake Outdated Show resolved Hide resolved
@aminya
Copy link
Owner

aminya commented Jan 21, 2023

Users should be notified to add compile_commands.json to .gitignore though.

It would be great if we could automate the addition to .gitignore!

src/Common.cmake Outdated Show resolved Hide resolved
src/Common.cmake Outdated Show resolved Hide resolved
@FeignClaims
Copy link
Contributor Author

Users should be notified to add compile_commands.json to .gitignore though.

It would be great if we could automate the addition to .gitignore!

This should be optional then. I prefer to automately add it only if .gitignore exists.

@aminya
Copy link
Owner

aminya commented Jan 22, 2023

I tried this locally, but I got this error:

CMake Error at D:/GitHub/Cpp/project_options/src/Common.cmake:80 (cmake_host_system_information):
  cmake_host_system_information does not recognize <key> WINDOWS_REGISTRY
Call Stack (most recent call first):
  D:/GitHub/Cpp/project_options/src/Index.cmake:128 (common_project_options)
  CMakeLists.txt:53 (project_options)


CMake Error at D:/GitHub/Cpp/project_options/src/Common.cmake:96 (file):
  file failed to create symbolic link
  'D:/GitHub/Cpp/project_options/tests/myproj/compile_commands.json': A
  required privilege is not held by the client.

Call Stack (most recent call first):
  D:/GitHub/Cpp/project_options/src/Index.cmake:128 (common_project_options)
  CMakeLists.txt:53 (project_options)

@FeignClaims
Copy link
Contributor Author

FeignClaims commented Jan 23, 2023

I tried this locally, but I got this error:

CMake Error at D:/GitHub/Cpp/project_options/src/Common.cmake:80 (cmake_host_system_information):
  cmake_host_system_information does not recognize <key> WINDOWS_REGISTRY
Call Stack (most recent call first):
  D:/GitHub/Cpp/project_options/src/Index.cmake:128 (common_project_options)
  CMakeLists.txt:53 (project_options)


CMake Error at D:/GitHub/Cpp/project_options/src/Common.cmake:96 (file):
  file failed to create symbolic link
  'D:/GitHub/Cpp/project_options/tests/myproj/compile_commands.json': A
  required privilege is not held by the client.

Call Stack (most recent call first):
  D:/GitHub/Cpp/project_options/src/Index.cmake:128 (common_project_options)
  CMakeLists.txt:53 (project_options)

The command requires CMake 3.24+. Maybe that's the reason? The debug here requires a lot of tedious edit-and-test work, so I suggest just ignore this pull request after I fix this.

@FeignClaims
Copy link
Contributor Author

FeignClaims commented Jan 25, 2023

Should work now. Already tested on my windows virtual machine.

@FeignClaims
Copy link
Contributor Author

FeignClaims commented Jan 29, 2023

Here's a summay feature of my pr:

  • If .gitignore exists in ${CMAKE_SOURCE_DIR} and dosen't contain compile_commands.json, automatically add compile_commands.json into it. (this helps git based project.)
  • If cmake generates compile_commands.json:
    • If users don't run cmake as non-administrator on Windows, symlink ${CMAKE_BINARY_DIR}/compile_commands.json to ${CMAKE_SOURCE_DIR}/compile_commands.json. (this helps clang based tools.)
    • If users run cmake as non-administrator on Windows, add a custom command _copy_compile_commands which copies ${CMAKE_BINARY_DIR}/compile_commands.json to ${CMAKE_SOURCE_DIR}/compile_commands.json.

@FeignClaims
Copy link
Contributor Author

rebased on main and ready to merge now.

Copy link
Owner

@aminya aminya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use TRACE for the messages that are not necessary for the user.

src/Common.cmake Outdated Show resolved Hide resolved
src/Common.cmake Outdated Show resolved Hide resolved
@aminya aminya merged commit 08348bf into aminya:main Jan 30, 2023
@FeignClaims FeignClaims deleted the compile_commands branch January 31, 2023 02:05
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

Successfully merging this pull request may close these issues.

None yet

2 participants