Skip to content

CXX-3288 do not initialize ENABLE_TESTS with the auto-downloaded C Driver #1404

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 2 commits into from
May 16, 2025

Conversation

eramongodb
Copy link
Contributor

Resolves CXX-3288. Split from #1403.

This PR delays the call to include(FetchMongoC) until after all top-level cache variables have been initialized (notably, including the ENABLE_TESTS option) to avoid the C Driver's own cache variable initialization taking priority over the C++ Driver when conditionally importing the auto-downloaded C Driver via add_subdirectory().


The auto-downloaded C Driver is observed to overwrite the default value (OFF) of the C++ Driver's ENABLE_TESTS cache variable with its own default (ON):

$ cmake -B local-build -LA
...
-- Required MongoDB C Driver libraries not found via find_package()
-- MongoDB C Driver library sources will be downloaded from GitHub
-- Downloading and configuring MongoDB C Driver 2.0.0...
...
-- Downloading Catch2... (!?)
...
-- Downloading Catch2... done.
...
ENABLE_TESTS:BOOL=ON (!?)

According to CMake documentation, set() shouldn't overwrite an existing cache variable:

Since cache entries are meant to provide user-settable values this does not overwrite existing cache entries by default. Use the FORCE option to overwrite existing entries.

The C++ Driver defines ENABLE_TESTS via option(), which creates a cache variable:

In CMake project mode, a boolean cache variable is created with the option value.

However, the C++ Driver does so long after the C Driver has already been imported via include(FetchMongoC). This means the C Driver's ENABLE_TESTS cache variable initialization via set(CACHE) has priority over the C++ Driver's call to option() which is therefore ignored:

25  | option(BUILD_TESTING "When ENABLE_TESTS=ON, include test targets in the \"all\" target")
    .
52  | include(FetchMongoC) # Initializes the ENABLE_TESTS cache variable with "ON".
    .
313 | option(ENABLE_TESTS "Enable building test targets." OFF) # Too late!
    .
320 | if(ENABLE_TESTS)
321 |     enable_testing()
322 | endif()
323 |
324 | add_subdirectory(src) # Sees ENABLE_TESTS=ON -> calls include(FetchCatch2).

To fix this bug, this PR moves the include(FetchMongoC) to come after option(ENABLE_TESTS) (and other top-level cache variables) and just before add_subdirectory(src) (where C Driver targets are required).

@eramongodb
Copy link
Contributor Author

eramongodb commented May 15, 2025

Macro guard task failures will be addressed by #1403. The fix to macro guard tests are also cherry-picked onto this PR branch.

@eramongodb eramongodb merged commit 39c9926 into mongodb:master May 16, 2025
21 checks passed
@eramongodb eramongodb deleted the cxx-3288 branch May 16, 2025 15:49
eramongodb added a commit to eramongodb/mongo-cxx-driver that referenced this pull request May 16, 2025
…iver (mongodb#1404)

* Include FetchMongoC after top-level cache variables are initialized
* Set missing ENABLE_TESTS=ON for macro guard tests
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