Skip to content

ci(NODE-7409): Use CMake's builtin control for the MSVCRT link mode on libmongocrypt#122

Merged
dariakp merged 6 commits intomongodb-js:mainfrom
vector-of-bool:use-cmake-static-msvcrt-control
Feb 13, 2026
Merged

ci(NODE-7409): Use CMake's builtin control for the MSVCRT link mode on libmongocrypt#122
dariakp merged 6 commits intomongodb-js:mainfrom
vector-of-bool:use-cmake-static-msvcrt-control

Conversation

@vector-of-bool
Copy link
Contributor

Description

Summary of Changes

libmongocrypt provided a configuration parameter to control the type of the MSVC runtime library. This parameter was redundant with the control that is offered by CMake already. Prefer instead to use that control.

The parameter, CMAKE_MSVC_RUNTIME_LIBRARY, was added in CMake 3.15, which is the minimum CMake version required by libmongocrypt.

What is the motivation for this change?

We are trying to simplify libmongocrypt's build, which includes removing redundancy with built-in CMake functionality. This specifically is related to MONGOCRYPT-876, which seeks to remove the ENABLE_WINDOWS_STATIC_RUNTIME configuration parameter.

Double check the following

  • Lint is passing (npm run check:lint)
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

libmongocrypt provided a configuration parameter to control the type of
the MSVC runtime library. This parameter was redundant with the control
that is offered by CMake already. Prefer instead to use that control.

The parameter, CMAKE_MSVC_RUNTIME_LIBRARY, was added in CMake 3.15, which
is the minimum CMake version required by libmongocrypt.
@vector-of-bool vector-of-bool requested a review from a team as a code owner January 22, 2026 19:29
@baileympearson
Copy link
Collaborator

@vector-of-bool Looks like windows builds are failing

@vector-of-bool
Copy link
Contributor Author

The issue appears to be that it's spawning the subprocesss with shell: true and therefore the new angle brackets are being interpreted by cmd.exe as redirection operators.

I can't see in the commit history why shell: process.platform === 'win32' was added here. Does something on Windows require the cmd.exe processor?

I've removed the shell argument for now. If shell: true is required on Windows, there's another option to try in case that fails for some other reason.

@tadjik1 tadjik1 self-assigned this Jan 29, 2026
@tadjik1 tadjik1 added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jan 29, 2026
@tadjik1 tadjik1 removed their assignment Jan 29, 2026
@tadjik1 tadjik1 removed the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jan 29, 2026
@baileympearson
Copy link
Collaborator

@vector-of-bool We don't actually know, this is just something we've used in the past to make our tooling work. But that doesn't mean our scripts can't be made to work without it.

Have you tried escaping the angle brackets?

@baileympearson baileympearson self-assigned this Jan 29, 2026
@baileympearson baileympearson added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jan 29, 2026
@vector-of-bool
Copy link
Contributor Author

Angle bracket escaping should be unnecessary if the command is not being interpreted by a shell, so I really don't know why it fails now.

Regardless, the angle bracket magic is not totally necessary and we can just pass MultiThreaded without the Debug suffix.

baileympearson
baileympearson previously approved these changes Jan 29, 2026
tadjik1
tadjik1 previously approved these changes Jan 30, 2026
Copy link
Contributor

@tadjik1 tadjik1 left a comment

Choose a reason for hiding this comment

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

thank you! 👍

@vector-of-bool
Copy link
Contributor Author

For reference: The shell: true is required on Windows because the scripting executes certain programs that aren't .exe files (e.g. .bat scripts). Using cmd.exe as a command interpreter is required to execute .bat etc. as if they are regular programs.

Corrected the comment formatting for clarity.
@tadjik1 tadjik1 dismissed stale reviews from baileympearson and themself via e353c15 February 12, 2026 14:57
@dariakp dariakp merged commit 0c7efad into mongodb-js:main Feb 13, 2026
56 of 58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Primary Review In Review with primary reviewer, not yet ready for team's eyes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants