Skip to content

Conversation

@vector-of-bool
Copy link
Contributor

@vector-of-bool vector-of-bool commented Jan 21, 2026

Refer: MONGOCRYPT-874

This changeset simplifies the build scripts by removing custom environment variables, as well as some legacy cruft that is no longer relevant. It can be viewed on a per-commit basis.

Summary

  • 14682f5 Removes logic that sets -A and -T when invoking CMake. These are no longer relevant since we use Ninja on Windows and vs-env-run.ps1 sets the environment for us. Besides, the proper way to do that would be to use the CMAKE_GENERATOR_PLATFORM AND CMAKE_GENERATOR_TOOLSET environment variables to control the target for the VS (MSBuild) CMake generator.
  • fb16f91 Tosses out EXTRA_CFLAGS in favor of using CFLAGS and CXXFLAGS, which are recognized by CMake to initialize CMAKE_C_FLAGS and CMAKE_CXX_FLAGS. These are standard across build tools.
  • b0ae833 and 9238191 drop the MACOS_UNIVERSAL environment variable control in favor of CMake's standard CMAKE_OSX_ARCHITECTURES flag. The test case that asserts we generate a multi-arch binary remains.
  • 4c1bd8f Drops juggling the -G flag and the use_ninja expansion/environment variable in favor of using the CMAKE_GENERATOR environment variable.
    • Ideally, the build scripts should be written agnostic of the CMake generator, but our custom Ninja-loading code still does some work here. When we eventually adopt uv to manage Ninja, the ensure-ninja.sh and related scripts should be removed and our build scripts should not acknowledge the generator.
  • b3b3fef Significantly cleans up build_all.sh
    • We use environment variables to pass certain CMake controls: CMAKE_BUILD_TYPE, CMAKE_CONFIG_TYPE (controls the default for --build and ctest), CMAKE_INSTALL_PREFIX, and CMAKE_EXPORT_COMPILE_COMMANDS.
    • We rely much less on string splicing to form our command lines. The build_all.sh is now shellcheck-clean.
  • d009ba5 Use CC and CXX to control the compiler executables, not by setting CMAKE_C_COMPILER and CMAKE_CXX_COMPILER.
  • 8927fc0 Completely remove LIBMONGOCRYPT_EXTRA_CMAKE_FLAGS
  • 208c8f1 Drop ADDITIONAL_CMAKE_FLAGS
  • 83deb0b Drop the DEFAULT_BUILD_ONLY control. This didn't appear to be used anywhere.
  • 3bd5e7a Pass --fresh in build_all.sh when we invoke CMake each time.
  • 8df7cc7 Don't set -EHsc on Windows. This was incorrect because it should be checking for MSVC. It doesn't appear to have any effect on the build?
  • d0743af Don't set MAKEFLAGS to control parallelism. CMake respects CMAKE_BUILD_PARALLEL_LEVEL environment variable to control parallelism regardless of the underlying build tool.

@vector-of-bool vector-of-bool marked this pull request as ready for review January 21, 2026 21:07
@vector-of-bool vector-of-bool requested a review from a team as a code owner January 21, 2026 21:07
Comment on lines +68 to +71
# Prepend our custom C and CXX flags for any possible CMake builds
CFLAGS="$LIBMONGOCRYPT_COMPILE_FLAGS ${CFLAGS-}" \
CXXFLAGS="$LIBMONGOCRYPT_COMPILE_FLAGS ${CXXFLAGS-}" \
run_cmake "$@"
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious: why prepend instead of append?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason at all, just how I happened to write it.

@vector-of-bool vector-of-bool merged commit 790320d into mongodb:master Jan 23, 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants