-
Notifications
You must be signed in to change notification settings - Fork 676
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
build: introduce ENABLE_DATE_INSTALL
to allow user opt-out of install
#752
base: master
Are you sure you want to change the base?
Conversation
When using `date` as a PRIVATE dependency, it is not required to install it. This flag give user using `date` with add_subdirectory the opportunity to disable install target Default behavior is conserved since ENABLE_DATE_INSTALL is ON
if( ENABLE_DATE_INSTALL ) | ||
include( GNUInstallDirs ) | ||
endif( ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it make sense to move this include()
in the installation section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No because we want CMAKE_INSTALL_INCLUDIR to be defined for target_include_directories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but the variable is not currently used in target_include_directories()
; I just saw #753 though, and they seem to complement each other. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes sorry for not making it clearer, I split bug fix and feature in different pr to make review easier/faster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as intended, just copy&paste to fix.
I was planning to create a patch like this myself.
Co-authored-by: Gerhard Olsson <6248932+gerhardol@users.noreply.github.com>
When using
date
as a PRIVATE dependency, it is not required to install it. This flag give user usingdate
with add_subdirectory the opportunity to disable install target Default behavior is conserved since ENABLE_DATE_INSTALL is ONfix #751