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

Fix missing INTERFACE_INCLUDE_DIRECTORIES in exported config #182

Merged
merged 1 commit into from
Jul 16, 2022

Conversation

arghness
Copy link
Contributor

@arghness arghness commented Jul 8, 2022

CMAKE_INSTALL_INCLUDEDIR was used before including GNUInstallDirs, causing the first configuration to miss INTERFACE_INCLUDE_DIRECTORIES. Configuring a second time worked, though. Tested with CMake 3.17.5.

I haven't included it here, as I think it might be a breaking change for existing users, but I'd also suggest changing the name of the configuration file from Config.cmake to v8pp-config.cmake or v8ppConfig.cmake, as currently it looks like the config filename needs to be included in the find(). e.g. find_package(v8pp REQUIRED CONFIGS Config.cmake)

For the supported filenames, see the "Config mode" section in https://cmake.org/cmake/help/latest/command/find_package.html#search-modes

CMAKE_INSTALL_INCLUDEDIR was used before including GNUInstallDirs, causing the first configuration to miss INTERFACE_INCLUDE_DIRECTORIES. Configuring a second time worked, though.
Copy link
Owner

@pmed pmed left a comment

Choose a reason for hiding this comment

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

Hi,

thanks a lot!

@pmed pmed merged commit de4037d into pmed:master Jul 16, 2022
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.

2 participants