Skip to content

CDRIVER-3433 support linking against windows static runtime #591

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

CDRIVER-3433 support linking against windows static runtime #591

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/libbson/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ if (ENABLE_STATIC MATCHES "ON|AUTO")
target_include_directories (bson_static INTERFACE $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src> $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/src>)
set_target_properties (bson_static PROPERTIES VERSION 0.0.0)
set_target_properties (bson_static PROPERTIES OUTPUT_NAME "${BSON_OUTPUT_BASENAME}-static-${BSON_API_VERSION}")
if (ENABLE_WINDOWS_STATIC_RUNTIME)
Copy link
Collaborator

@kevinAlbs kevinAlbs Nov 19, 2019

Choose a reason for hiding this comment

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

Can this be changed to a cached option? Similar to options here: https://github.com/mongodb/mongo-c-driver/blob/1.15.0/CMakeLists.txt/#L57

A cached option will show up after configuring with cmake, ending up in sort of a help menu. Users can get a list of cached options along with the help text by running cmake -LH or using cmake GUI or ccmake.

Arguably, the same should be done for mongodb/libmongocrypt#78, but I think it's much less likely anyone outside of our drivers team is using libmongocrypt directly.
Copy link
Member

@jmikola jmikola Nov 21, 2019

Choose a reason for hiding this comment

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

I'm not familiar with CMake, but should this option only exist on Windows platforms?

Coming from an Autotools perspective, I am imagining that this behaves similar to a configure option, where we'd simply prefer to have it omitted entirely from the list of supported options if we know it's not relevant to the platform. If that's not the case for CMake, please correct me.
Copy link
Collaborator

@kevinAlbs kevinAlbs Nov 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, some discussion/investigation has happened outside of this PR and I did not follow-up. This PR can hopefully be closed without merging. I was able to pass /MT through the relwithdebinfo Windows compile using -DCMAKE_C_FLAGS_RELWITHDEBINFO. @mbroadst if you can verify the changes on this branch work (https://github.com/kevinAlbs/libmongocrypt/tree/proposed-fix), shall we close this PR?

Though I do agree, if we did add this option, it'd probably make sense to have it Windows only.
target_compile_options (bson_static PUBLIC /MT)
endif ()

# We use CMAKE_THREAD_LIBS_INIT rather than Threads::Threads here because the
# latter fails when building on Mac OS X
target_link_libraries (bson_static ${CMAKE_THREAD_LIBS_INIT})
Expand Down