-
Notifications
You must be signed in to change notification settings - Fork 454
Refactor uninstall-script generation #1380
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
Refactor uninstall-script generation #1380
Conversation
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.
I'm a little sad that you didn't like my generator script written in batch 😞
Seriously, though, this is much nicer. Aside from a few minor tweaks/questions, it looks good to me.
build/cmake/GeneratePkgConfig.cmake
Outdated
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.
I am assuming that the DEBUG messages were meant to be left in. I don't have a problem with them (I rather like the extra output), but I wanted to mention it in the event you intended to remove them and forgot.
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, they were useful while debugging and don't get in the way, so I just left them there.
) | ||
|
||
# If applicable, generate an "uninstall" target to run the uninstaller: | ||
if(CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR OR PROJECT_IS_TOP_LEVEL) |
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.
Is this how a collision is prevented between the uninstall generated in this project and, for instance, the uninstall generated in the C++ driver when it includes the C driver directly?
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. This is mostly the same as what was done previously, but with the addition of the newer CMake feature PROJECT_IS_TOP_LEVEL
, which was added to address this exact use case (on older CMake, that variable check will be a no-op).
Co-authored-by: Roberto C. Sánchez <[email protected]>
This aside change refactors and changes a few minor issues with the uninstall script:
DESTDIR
strangely (?)This change also removes the top-level
generate_uninstall/
directory, as well as the two generator scripts, instead replacing it with a singleGenerateUninstalle.cmaker
module (which should also be portable between projects).