-
Notifications
You must be signed in to change notification settings - Fork 547
Use uvx for CMake and Server Toolchain for Ninja #1428
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
Conversation
kevinAlbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The efforts to move towards uv and Ninja are much appreciated. LGTM with a fix to macOS builds.
Aside: I would like to avoid manually patching uv-installer.sh, but I do not see a clear alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the PR to include a new patch-uv-installer.sh script which automatically patches the uv-installer.sh script to enable checksum validation. The list of checksums to be validated with is embedded in the patch-uv-installer.sh script along with a download_checksums helper function which may be used to update the list of checksums.
Updated the PR to use uv tool install as designed and intended rather than the uv run --isolated ... pattern initially proposed. This is due to uv 0.8.1 and newer implementing stronger protections against bypassing ephemeral environments in astral-sh/uv#14790 (following astral-sh/uv#14447 in 0.8.0), which breaks the uv run --isolated ... pattern, as the directory no longer exists after the command as completed execution.
Instead, the UV_TOOL_DIR and UV_TOOL_BIN_DIR environment variables are used to install the required binaries into (task-)local directories. This should not be a significant performance problem, since the (user-level) uv cache will still be utilized to populate the local tool directories. This also further reduced the need for *_binary Bash variables, although it slightly complicated the sequence of commands to obtain preferred build tools due to Windows path handling.
PR description has been updated accordingly.
kevinAlbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I like the newly added patching script, and expect that will ease future upgrades of uv.
|
Bumped uv to 0.8.3 to include a fix (astral-sh/uv#14863) for a regression in 0.8.0 concerning |
vector-of-bool
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor comments.
Do we have an open ticket to migrate away from the VS generators? It would greatly simplify a lot of these scripts, but I don't remember whether those tickets were created.
.evergreen/scripts/test.sh
Outdated
| # Obtain preferred build tools. | ||
| export UV_TOOL_DIR UV_TOOL_BIN_DIR | ||
| if [[ "${OSTYPE:?}" == cygwin ]]; then | ||
| UV_TOOL_DIR="$(cygpath -aw "$(pwd)/uv-tool")" | ||
| UV_TOOL_BIN_DIR="$(cygpath -aw "$(pwd)/uv-bin")" | ||
| else | ||
| UV_TOOL_DIR="$(pwd)/uv-tool" | ||
| UV_TOOL_BIN_DIR="$(pwd)/uv-bin" | ||
| fi | ||
| PATH="${UV_TOOL_BIN_DIR:?}:${UV_INSTALL_DIR:?}:${PATH:-}" | ||
| uv tool install -q cmake | ||
| [[ "${distro_id:?}" == rhel* ]] && PATH="${PATH:-}:/opt/mongodbtoolchain/v4/bin" || uv tool install -q ninja | ||
| PATH="${PATH:-}:/opt/mongodbtoolchain/v4/bin" # For ninja and llvm-symbolizer. | ||
|
|
||
| cmake --version | head -n 1 | ||
| echo "ninja version: $(ninja --version)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This particular incantation is repeated several times. Could it be consolidated into a single script file to be dot-sourced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM. Added an install-build-tools.sh which unconditionally obtains cmake and ninja, but in a way which does not require the distro_id variable to detect RHEL, and also in a way that does not import all of MongoDB Toolchain's binaries by adding them to PATH (instead using a simple symlink to "install" only ninja into the uv tool bin directory).
etc/make_release.py
Outdated
| run_shell_script('cd build;' | ||
| 'echo ' + release_version + ' > VERSION_CURRENT;' | ||
| '${CMAKE} -DCMAKE_BUILD_TYPE=Release -DENABLE_TESTS=ON ' | ||
| 'uvx cmake -DCMAKE_BUILD_TYPE=Release -DENABLE_TESTS=ON ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this cmake version pinned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is not. In this case, the make_release.py script is being run manually by the user performing release instructions. However, your question made me realize this should really be made a dependency of the make_release group in pyproject.toml instead. Updated accordingly. cmake is now pinned to 4.0.3 by the lockfile (for the make_release group).
No, I don't think we do. Filed CXX-3326. |
|
Bumping the uv version once more before PR merge to 0.8.6 to include the following change:
|
* Upgrade uv to 0.8.6 with checksum validation * Consistently use CMAKE_BUILD_PARALLEL_LEVEL (/maxcpucount) for MSBuild
…ixes (#1490) * CXX-3278 update invalid URIs in CSE endpoint tests (#1395) * update test case 4 * replace `.local` with `.invalid` * Migrate EVG tasks from RHEL 7.6 to RHEL 7.9 (#1414) * CXX-3273 revert to using system-installed valgrind (#1400) * CXX-3311 Remove Debian 10 EVG task coverage (#1423) * CXX-3321 Enable CSFLE for sanitizer tasks (#1434) * Reduce sanitizers matrix to static library linkage * Print name of test executable being executed * Pin mongoc_version_minimum to 912209d (CDRIVER-5960) * CXX-3259 bump minimum server version from 4.0 to 4.2 (CDRIVER-5956) * Update MONGOC_VERSION_MINIMUM to better document current practices * Use uvx for CMake and Server Toolchain for Ninja (#1428) * Upgrade uv to 0.8.6 with checksum validation * Consistently use CMAKE_BUILD_PARALLEL_LEVEL (/maxcpucount) for MSBuild * Set ASAN_SYMBOLIZER_PATH to mongodbtoolchain v4 (#1446) * Generate EVG configuration with ASAN_SYMBOLIZER_PATH (#1450) * CXX-3270 remove serverless testing (#1459) * Remove serverless testing EVG configs * Use EVG distro system uv binaries (#1464) * CXX-3352 Revert "CXX-1885 add NPS survey code" (#1466) This reverts commit 3233473. * CXX-3322 remove atlas data lake testing (#1461) * Re-enable distros previously missing system uv binaries (#1472) * Add C++23 to GCC 12+ compile matrix (#1485) * fix: CDRIVER-5710 in validator examples * fix: CDRIVER-5967 with mongo-c-driver<2.0.1 on MacOS * fix: support mongo-c-driver before and after 2.1.2 * Update Invalid URI test for CDRIVER-5983 (#1429) --------- Co-authored-by: Kevin Albertson <[email protected]> Co-authored-by: Connor MacDonald <[email protected]>
Updates the Evergreen configuration and scripts to use
uvto obtain CMake binaries. This removes the need for thefind-cmake-old.sh,find-cmake-latest.sh(C Driver), andfind-cmake-version.sh(C Driver) helper scripts in the C++ Driver.Also updates the EVG configuration to unconditionally use the Ninja generator instead of Unix Makefiles on non-Windows distros. However, instead of using
uv, the Server Toolchain is used instead. The reasons for this are explained below.This PR is intended to be a "simpler" preview of a followup PR which will apply similar changes to the C Driver.
First, this PR updates the
uv-installer.shscript to obtain the current latest release ofuv, bumping theuvversion from 0.5.14 (Jan 2025) to0.8.00.8.2 (July 2025).Related checksums continue to be manually applied to theA newuv-installer.shscript.patch-uv-installer.shscript is used to automatically patch the installer script to enable and verify checksums.To assist with future updates, a Bash script which may be used to obtain the list of checksums is now documented as a comment within the installer script, copied here for reference:Update: the
download_checksumshelper function inpatch-uv-installer.shcan be used to obtain the list of checksums for a given release.Note
Downloading the checksums at runtime would be convenient, but would also defeat the point of the checksum validation as a guard against corrupt or malicious downloads since the checksum values would come from the same potentially-corrupted or potentially-malicious source as the artifacts to be validated.
The
uv.lockfile is also updated with latest package upgrades by runninguv lock -U.The bulk of this PR is updating the Evergreen configuration and scripts to use
cmakebinaries provided byuvinstead of the system-provided binaries (inconsistent availability and versions) or manual download strategies (i.e. viafind-cmake-latest.sh). This is expected to improve the reliability, consistency, and ease of build tool acquisition and usage across EVG distros when the same pattern can be applied to other Python-packaged tools going forward.Update: for compatibility with uv 0.8.1 and newer, which implemented stronger protections against bypassing ephemeral environments in astral-sh/uv#14790 (which is exactly what the
uv run ... "command -v <name>"pattern was doing), this PR now proposes usinguv tool installwith theUV_TOOL_DIRandUV_TOOL_BIN_DIRenvironment variables set to local directories. This should not be a significant performance penalty, as the uv cache directory would still be used to populate the custom tool directories. However, these environment variables unfortunately require theC:\pathform on Windows instead of/cygdrive/c/path(otherwise, no diagnostic or error (?!); instead, ignored in favor of the user's local bin directory 😢).Rather thanuv tool install cmake(which may modify the user's environment, e.g. by installing binaries to$HOME/.local/bin), this PR proposes usinguvx cmakeinstead (uvxis a convenient short-hand foruv tool run). Unlikeuv tool install,uvxdoes not modify the user's environment. When a full path to the binary is required (e.g. for CMake compatibility tests or as an argument to other scripts/tools), the lengthyuv run --no-project --isolated --with cmake bash -c "command -v cmake"command is required. There unfortunately does not yet seem to be a short-handuv toolcommand to obtain a path to the uv-managed executable tool. Note thatuvx bash -c "..."does not work due touvxinterpretingbashas a Python package.Note
Concerning flags to
uv run:--no-projectprevents discovery and installation (uv sync) of a project before running the command, but may still reuse a virtual environment that is active or found in the current or parent directories.--isolatedprevents reuse of any virtual environment by always using an isolated virtual environment, but may discover and install (uv sync) a project within that isolated virtual environment before running the command.Both flags are required to ensure an isolated virtual environment without any project detection and installation is used. However, neither flag prevent use of uv's thread-safe, append-only dependency cache, which is how the same
<name>binary can be safely reused by direct reference as obtained viacommand -v <name>(unless someone unsafely modifies the cache viauv cacheor direct filesystem operations).Although
uvxhas an--isolatedflag, it is currently redundant pending resolution of astral-uv/uv#7186. For now, theuvxcommand always uses an isolated virtual environment.This PR initially attempted to use the same approach to obtain the
ninjabinary. Obtainingninjaviauvis simple; however, this method was greatly frustrated by unexpected integration issues. After much effort, I've given up usinguv-provided Ninja binaries; instead, this PR proposes unconditionally using the binary provided by Server Toolchain v4, which is available on all our currently-used EVG distros.The first complication is that there is no
CMAKE_MAKE_PROGRAMenvironment variable to go alongside theCMAKE_GENERATORenvironment variable. This means the path to the Ninja binary (which requires the lengthyuv runcommand described above) is almost always necessary and has to be explicitly passed as a CLI argument during the CMake configuration step via-D CMAKE_MAKE_PROGRAM=<path/to/ninja>, which is not automatically propogated to subprocess or other tools (e.g.distchecktarget,compile-libmongocrypt.shscript, etc.). This ultimately ends up forcing the addition of theninjabinary's parent directory to thePATHenvironment variable for reliable and consistent behavior.The second and far worse complication is a strange bug which only seems to manifest when all of the following conditions are met:
uv tool)uv tool; version?)clang++(version?)Given these conditions, CMake configuration fails with the following unexpected error for CMake 3.29 or newer:
and with the following unexpected error for CMake 3.28:
I am completely baffled by this issue. My best guess is that there may be an issue with how either CMake or Ninja are being packaged for Python, as system-installed equivalents (for CMake or Ninja) do not appear to exhibit this issue. I could not manage to work out a way to patch this erroneous behavior that didn't end up becoming a variation of this terrible special-casing code across various EVG scripts:
Due to these two issues, I decided to unconditionally use the MongoDB Server Toolchain, as provided by the
/opt/mongodbtoolchain/v4/binpathon all our currently-used non-Windows distroson RHEL distros only:The v4 toolchain is chosen due to currently having better distro availability than v5 (e.g. it is missing on the
rhel7-latestdistro). In spite of prior efforts to avoid depending on the MongoDB Toolchain (due to Server Team not supporting any downstream users beyond themselves), I believe switching from Unix Makefiles to Ninja is nevertheless worth the effort. We are already (knowingly) depending on the Server Toolchain for other tasks (e.g. sanitizers and ccache) despite this lack of downstream support guarantee. I am in favor of actively maintaining compatibility with the Server Toolchain ourselves (as we've already been doing) rather than attempting workarounds like those described above. We can revisit obtainingninjaviauvin followup PRs (i.e. on Windows distros to finally have ccache-friendly MSVC builds).Note
The path to the Server Toolchain is appended rather than prepended to
PATHto avoid having higher precedence when invoking compilers (e.g.g++,clang++, etc.) and other binaries. It is okay if we end up using a systemninja(such as on Ubuntu distros): we just need to guarantee the availability of aninjabinary for use as the CMake generator without needing to also use theCMAKE_MAKE_PROGRAMconfiguration variable.Miscellaneous drive-by fixes and improvements include:
setup_group_can_fail_task(incorrect) ->setup_task_can_fail_task(correct) in theabi-stabilitytask group definition.cmake-compattasks (e.g.cmake~=3.0for "latest v3 minor release",cmake~=3.15.0for "latest v3.15 patch release", etc.).setupEVG function, which have been redundant/obsolete/unused information for a while now.uninstall_check*scripts to avoid the need to handle paths within the scripts themselves (in particular the Windows batch script).UseMultiToolTaskandEnforceProcessCountAcrossBuilds(VS 2019 16.3 and newer) and/maxcpucountCMAKE_BUILD_PARALLEL_LEVEL(CMake 3.26 and newer) incompile.sh.install-c-driver.sh.