Skip to content

fix: cmake configure_file path #41

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.

Sign up for GitHub

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

Open
marcfir wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: cmake configure_file path #41

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ if(${MBX_SUPPORT_EOE})
list(APPEND SRC_ETHERCAT src/eoe.c)
endif()

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cmake/cmake_config.h.in ${CMAKE_CURRENT_BINARY_DIR}/include/libethercat/config.h)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cmake/cmake_config.h.in ${CMAKE_CURRENT_SOURCE_DIR}/include/libethercat/config.h)
Copy link

@gemini-code-assist gemini-code-assist bot Sep 22, 2025

Choose a reason for hiding this comment

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

high

This change directs the generated config.h file into the source directory (CMAKE_CURRENT_SOURCE_DIR). This is generally considered bad practice in CMake as it pollutes the source tree with build artifacts. The original use of CMAKE_CURRENT_BINARY_DIR was correct because it places generated files in the build directory.

This is important for a few reasons:

  • Out-of-source builds: It allows for clean separation of source code and build artifacts. You can have multiple build directories (e.g., for debug, release, different toolchains) from a single source tree without them interfering with each other.
  • Version Control: It prevents generated files from being accidentally committed to your repository.

The original code followed these best practices. If the goal was to ensure config.h is installed, a better approach would be to add a specific install() rule for the file after it's generated in the binary directory, rather than changing its generation path.

I recommend reverting this change to adhere to standard CMake practices. The same principle also applies to settings.h on the next line, which should also ideally be generated into CMAKE_CURRENT_BINARY_DIR.

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cmake/cmake_config.h.in ${CMAKE_CURRENT_BINARY_DIR}/include/libethercat/config.h)

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cmake/settings.h.in ${CMAKE_CURRENT_SOURCE_DIR}/include/libethercat/settings.h)

# LIBS
Expand Down