Skip to content

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

Merged
merged 5 commits into from
Jul 24, 2023
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions generate_uninstall/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

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 the uninstall target behavior to include (or ignore) this project's files.

if ("${CMAKE_SOURCE_DIR}" STREQUALS "${PROJECT_SOURCE_DIR}")
  add_custom_target(uninstall ...)
endif ()

Copy link
Contributor Author

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.

set(MONGOC_UNINSTALL_TARGET uninstall)
Copy link
Collaborator

Choose a reason for hiding this comment

The 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)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this PR, make uninstall will uninstall both the C and C++ drivers:

$ make install
$ tree -L 2 /tmp/cxxdriver/
/tmp/cxxdriver/
├── bin
│   └── mongoc-stat
├── include
│   ├── bsoncxx
│   ├── libbson-1.0
│   ├── libmongoc-1.0
│   └── mongocxx
├── lib
│   ├── cmake
│   ├── libbson-1.0.so -> libbson-1.0.so.0
│   ├── libbson-1.0.so.0 -> libbson-1.0.so.0.0.0
│   ├── libbson-1.0.so.0.0.0
│   ├── libbson-static-1.0.a
│   ├── libbsoncxx.so -> libbsoncxx.so._noabi
│   ├── libbsoncxx.so.0.0.0
│   ├── libbsoncxx.so._noabi -> libbsoncxx.so.0.0.0
│   ├── libmongoc-1.0.so -> libmongoc-1.0.so.0
│   ├── libmongoc-1.0.so.0 -> libmongoc-1.0.so.0.0.0
│   ├── libmongoc-1.0.so.0.0.0
│   ├── libmongoc-static-1.0.a
│   ├── libmongocxx.so -> libmongocxx.so._noabi
│   ├── libmongocxx.so.0.0.0
│   ├── libmongocxx.so._noabi -> libmongocxx.so.0.0.0
│   └── pkgconfig
└── share
    ├── mongo-c-driver
    └── mongo-cxx-driver

12 directories, 15 files
$ make uninstall
$ tree -L 2 /tmp/cxxdriver/
/tmp/cxxdriver/ [error opening dir]

0 directories, 0 files

endif()

if (WIN32)
string (REPLACE "/" "\\\\" CMAKE_INSTALL_PREFIX_WIN32
"${CMAKE_INSTALL_PREFIX}"
Expand All @@ -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 ()
Expand All @@ -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 ()