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

CMake config currently has a hard-coded include instead of CMAKE_INSTALL_INCLUDEDIR #134

Closed
thbeu opened this issue Jul 15, 2024 · 9 comments · Fixed by #139
Closed

CMake config currently has a hard-coded include instead of CMAKE_INSTALL_INCLUDEDIR #134

thbeu opened this issue Jul 15, 2024 · 9 comments · Fixed by #139

Comments

@thbeu
Copy link
Contributor

thbeu commented Jul 15, 2024

          Using absolute paths is a problem already now: The CMake config currently has a hard-coded `include` instead of `CMAKE_INSTALL_INCLUDEDIR`.

Originally posted by @dg0yt in #75 (comment)

@thbeu thbeu changed the title Using absolute paths is a problem already now: The CMake config currently has a hard-coded include instead of CMAKE_INSTALL_INCLUDEDIR. CMake config currently has a hard-coded include instead of CMAKE_INSTALL_INCLUDEDIR Jul 15, 2024
@rouault
Copy link
Member

rouault commented Aug 8, 2024

@thbeu is this issue still opened?

@thbeu
Copy link
Contributor Author

thbeu commented Aug 8, 2024

It's still here, but seems configurable.

shapelib/CMakeLists.txt

Lines 54 to 68 in 2cc2975

# Set up install locations.
set(
CMAKE_INSTALL_BINDIR bin
CACHE PATH "install location for user executables"
)
set(
CMAKE_INSTALL_LIBDIR lib
CACHE PATH "install location for object code libraries"
)
set(
CMAKE_INSTALL_INCLUDEDIR include
CACHE PATH "install location for C header files"
)

Note, that @dg0yt originally raised the issue.

@thbeu
Copy link
Contributor Author

thbeu commented Aug 8, 2024

Or rather here:

shapelib/CMakeLists.txt

Lines 133 to 137 in 2cc2975

target_include_directories(${PACKAGE}
INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
$<INSTALL_INTERFACE:include>
)

@dg0yt
Copy link
Contributor

dg0yt commented Aug 8, 2024

Or rather here:

shapelib/CMakeLists.txt

Lines 133 to 137 in 2cc2975

target_include_directories(${PACKAGE}
INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
$<INSTALL_INTERFACE:include>
)

And that's the INSTALL_INTERFACE which needs to be fixed.

@thbeu
Copy link
Contributor Author

thbeu commented Aug 8, 2024

Or rather here:

shapelib/CMakeLists.txt

Lines 133 to 137 in 2cc2975

target_include_directories(${PACKAGE}
INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
$<INSTALL_INTERFACE:include>
)

And that's the INSTALL_INTERFACE which needs to be fixed.

Right. Do you know how to do?

@dg0yt
Copy link
Contributor

dg0yt commented Aug 8, 2024

It was part of the comment: replace include with ${CMAKE_INSTALL_INCLUDEDIR}.

@dg0yt
Copy link
Contributor

dg0yt commented Aug 8, 2024

(This was initially suggested for a drive-by fix when using the same variable for the pc file.)

@thbeu
Copy link
Contributor Author

thbeu commented Aug 8, 2024

Right. As simple as that and available via #139.

@thbeu
Copy link
Contributor Author

thbeu commented Aug 8, 2024

(This was initially suggested for a drive-by fix when using the same variable for the pc file.)

Thanks for reminding. I changed to ${includedir} which we introduced in #135.

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 a pull request may close this issue.

3 participants