-
Notifications
You must be signed in to change notification settings - Fork 454
Only add the uninstall target if the C driver is the top level project #1352
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
Changes from 2 commits
73e9f24
de2558e
976ab56
f06de4f
c42d9be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,14 @@ | |
|
||
set (UNINSTALL_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE) | ||
|
||
# When added as a subdirectory in the mongocxx driver, there is a conflict in | ||
# the `uninstall` target causing the `make uninstall` command to fail. By | ||
# defining the uninstall target as a variable, the C++ driver is able to rename | ||
# this target in order to avoid the conflict. | ||
if (NOT DEFINED MONGOC_UNINSTALL_TARGET) | ||
set(MONGOC_UNINSTALL_TARGET uninstall) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the C++ driver renames the target, will running the uninstall target in the C++ driver only uninstall the C++ driver (and leave the C driver installed files)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this PR,
|
||
endif() | ||
|
||
if (WIN32) | ||
string (REPLACE "/" "\\\\" CMAKE_INSTALL_PREFIX_WIN32 | ||
"${CMAKE_INSTALL_PREFIX}" | ||
|
@@ -28,7 +36,7 @@ if (WIN32) | |
") | ||
install (FILES "${CMAKE_CURRENT_BINARY_DIR}/${UNINSTALL_PROG}" DESTINATION "${UNINSTALL_PROG_DIR}" PERMISSIONS ${UNINSTALL_PERMISSIONS}) | ||
|
||
add_custom_target (uninstall | ||
add_custom_target (${MONGOC_UNINSTALL_TARGET} | ||
COMMAND call "${CMAKE_CURRENT_BINARY_DIR}/${UNINSTALL_PROG}" | ||
) | ||
else () | ||
|
@@ -50,7 +58,7 @@ else () | |
") | ||
install (FILES "${CMAKE_CURRENT_BINARY_DIR}/${UNINSTALL_PROG}" DESTINATION "${UNINSTALL_PROG_DIR}" PERMISSIONS ${UNINSTALL_PERMISSIONS}) | ||
|
||
add_custom_target (uninstall | ||
add_custom_target (${MONGOC_UNINSTALL_TARGET} | ||
COMMAND sh "${CMAKE_CURRENT_BINARY_DIR}/${UNINSTALL_PROG}" | ||
) | ||
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.
Per https://gitlab.kitware.com/cmake/cmake/-/issues/23024#note_1098749, it may actually be more reasonable to simply not define the uninstall target at all if this project is not the top-level project (note:
PROJECT_IS_TOP_LEVEL
is a CMake 3.21 feature). The top-level project (i.e. the CXX Driver) would be responsible for defining theuninstall
target behavior to include (or ignore) this project's files.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.
This seems to work too. I don't have an opinion on which one we go with.