Skip to content

Enable parallel compilation with MinGW and fix gmtime_s detection #1185

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 3 commits into from
Jan 20, 2023

Conversation

eramongodb
Copy link
Contributor

@eramongodb eramongodb commented Jan 20, 2023

Verified by this patch.

Increases the speed of MinGW compilation on Windows by passing the -j flag to CMake MinGW Make. No limit on concurrent processes is set in order to maximize use of the resources available on Evergreen.

As a drive-by fix, updated feature detection for MSVC-style gmtime_s in kms_request.c to be consistent with the logic used in bson-iso8601.c, as it may otherwise fail when building with MinGW depending on the build configurations used.

@eramongodb eramongodb requested a review from kevinAlbs January 20, 2023 16:15
@eramongodb eramongodb self-assigned this Jan 20, 2023
@eramongodb
Copy link
Contributor Author

Fixed PR description to indicate the -j flag is being passed to MinGW Make, not CMake. System-available CMake versions on Windows distros are currently 3.11, which do not support -j yet (introduced in 3.12).

As an additional drive-by fix, added default CMAKE_CXX_STANDARD to CMake configs when C++ is enabled to address MinGW failure to compile cpp_check.cpp presumably due to pre-C++11 defaults. Because test-libmongoc happens to be generated prior to build failure, and the BAT file does not have error handling to exit on failure, the task happened to continue successfully. This will be addressed in a separate PR.

Verified by this patch.

@eramongodb eramongodb requested a review from kevinAlbs January 20, 2023 17:05
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2023

Codecov Report

Merging #1185 (80333a1) into master (80333a1) will not change coverage.
The diff coverage is n/a.

❗ Current head 80333a1 differs from pull request most recent head dcc48f8. Consider uploading reports for the commit dcc48f8 to get more accurate results

@@           Coverage Diff           @@
##           master    #1185   +/-   ##
=======================================
  Coverage   63.72%   63.72%           
=======================================
  Files         328      328           
  Lines       84343    84343           
=======================================
  Hits        53750    53750           
  Misses      30593    30593           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@eramongodb eramongodb merged commit 3f41e4d into mongodb:master Jan 20, 2023
@eramongodb eramongodb deleted the cdriver-mingw branch January 20, 2023 19:19
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.

3 participants