Skip to content

Bump googletest to v1.18.0 - #4615

Open
d-torrance wants to merge 3 commits into
Macaulay2:developmentfrom
d-torrance:gtest
Open

Bump googletest to v1.18.0#4615
d-torrance wants to merge 3 commits into
Macaulay2:developmentfrom
d-torrance:gtest

Conversation

@d-torrance

@d-torrance d-torrance commented Aug 11, 2026

Copy link
Copy Markdown
Member

Googletest 1.18.0 was just released.

Draft for now to test the builds.

No AI used I asked Claude to diagnose why the macOS cmake build was failing, and it added a couple commits. See below.

@d-torrance d-torrance added the dependencies Pull requests that update a dependency file label Aug 11, 2026
@d-torrance

Copy link
Copy Markdown
Member Author

This comment was written by Claude (Claude Code), at @d-torrance's request, after investigating the cmake-macos-15-brew-clang failure on this branch.

Two CMake-only commits have been added. Neither is a googletest change — the first one fixes a pre-existing bug that the temporary *REMOVE ME* commit exposed, and the second is an unrelated bug found while investigating it. googletest 1.18.0 itself is fine: the autotools-ubuntu-24.04 job built it from the submodule and ran the engine unit tests against it, [ PASSED ] 257 tests.

Why cmake-macos-15-brew-clang failed

It was a link failure, not a test failure:

Undefined symbols for architecture arm64:
  "testing::InitGoogleTest(int*, char**)", referenced from: _main in testMain.cpp.o

M2-unit-tests was linked with no gtest library at all. Macaulay2/e/CMakeLists.txt guards the link with if(GTEST_FOUND), and check-libraries.cmake deliberately unsets GTEST_FOUND when a library is found on the system but listed in BUILD_LIBRARIES, so that build-libraries builds our own copy. Normally the reconfigure that the build-libraries target performs then finds the copy we installed under M2_HOST_PREFIX and the guard passes.

That reconfigure never converged for GTest. memtailor, mathic and mathicgb each run their own find_package(GTest) under if(NOT GTEST_FOUND) — which fires precisely when check-libraries has unset GTEST_FOUND to request a self-build — and that re-caches GTest_DIR pointing at the Homebrew copy after the loop in check-libraries has cleared it. The next configure reads that stale value, classifies it as "on the system, but we want to build it", and unsets GTEST_FOUND again. Permanently one configure behind, so the link was skipped every time.

You can see it in the job logs: the failing configure prints -- Found GTest: twice (Homebrew, then usr-host), where a passing build on development prints it once.

24828375bd clears the stale <Package>_DIR before searching, for every library we intend to build. Verified against the real tree: M2-unit-tests goes from having no gtest on its link line to usr-host/lib/libgtest.a + libgtest_main.a on the reconfigure, and the default (system gtest) path is unchanged.

Three notes for anyone who hits something similar:

  • Only a system googletest of 1.16 or newer triggers this, which is why macOS (Homebrew 1.18.0) broke and Ubuntu (libgtest-dev 1.14.0) did not — there the stale GTest_DIR fails the version check in find_package(GTest 1.16) and CMake carries on searching, finding our own copy by itself.
  • autotools was never affected: Macaulay2/e/unit-tests/Makefile.in puts -lgtest in LOADLIBES unconditionally, with no found-flag gating.
  • cmake-ubuntu-24.04 passed because M2-unit-tests is EXCLUDE_FROM_ALL and the CTest step is macOS-only, so it never linked the target.

The gftables commit

Separate bug, found while checking whether other libraries could hit the same trap (they can't — GTest is the only entry of LIBRARY_OPTIONS that anything else calls find_package on). The copy of factory's gftables into the distribution tree was guarded only on the destination not already existing, so it happened once, on the first configure — when GFTABLESDIR still points at the factory already on the system. With BUILD_LIBRARIES=factory, GFTABLESDIR correctly moves to M2_HOST_PREFIX on the reconfigure, but the destination exists by then, so we distributed the system's tables alongside the factory we had just built.

d3178ebd18 copies again when GFTABLESDIR differs from wherever we last copied from. It also removes the previous copy first: file(COPY) optimizes out files whose timestamps match and compares them at a coarser resolution than it writes them, so it would not reliably replace the tables in place — the first version of the fix re-ran the copy and still shipped the old tables.

Verified with two factory installs carrying distinguishable tables: before, the reconfigure shipped the system's; after, it ships the one we built. A normal build (system factory, no BUILD_LIBRARIES) still copies exactly once and does not re-copy on subsequent configures.

This one is independent of the googletest bump and could reasonably be split into its own PR.

Not addressed

Nothing in CI currently exercises BUILD_LIBRARIES at all, which is why a bug this structural survived — the *REMOVE ME* commit is the first thing to have tried it. Worth considering whether some job should keep doing so.

@d-torrance d-torrance added the AI-generated This PR contains AI-generated code label Aug 12, 2026
d-torrance and others added 2 commits August 12, 2026 22:50
…selves

A cached <Package>_DIR outranks CMAKE_PREFIX_PATH for as long as it still
satisfies the requested version, so once one is set, find_package keeps
returning the copy on the system even after we have installed our own into
M2_HOST_PREFIX, which comes first in the prefix path.

This broke BUILD_LIBRARIES=gtest.  memtailor, mathic and mathicgb each guard
their own find_package(GTest) with "if(NOT GTEST_FOUND)", which fires exactly
when check-libraries has unset GTEST_FOUND to request a self-build, and that
re-caches GTest_DIR pointing at the system copy after the loop below has
cleared it.  check-libraries then reads that stale value at the top of the
next configure and unsets GTEST_FOUND again, so the reconfigure driven by the
build-libraries target never converged: Macaulay2/e/CMakeLists.txt skipped
target_link_libraries(M2-unit-tests GTest::GTest GTest::Main) every time and
M2-unit-tests failed to link with undefined testing:: symbols.

Only builds against a system googletest of 1.16 or newer were affected, which
is why this showed up on macOS (Homebrew has 1.18.0) but not on Ubuntu, where
libgtest-dev is 1.14.0: there the cached GTest_DIR fails the version check in
find_package(GTest 1.16) and cmake carries on searching, finding our own copy
under M2_HOST_PREFIX by itself.

GTest is the only entry of LIBRARY_OPTIONS that anything else calls
find_package on, so it is the only one that could not converge, but clear the
stale entry for every library we intend to build rather than special-casing.

BUILD_LIBRARIES is now folded to upper case once here rather than once per
iteration of the loop below, which is left in place so that it does not depend
on this one having run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The copy was guarded only on the destination not already existing, so it
happened once, on the first configure -- at which point GFTABLESDIR still
points at the factory that was already on the system.  With
BUILD_LIBRARIES=factory, GFTABLESDIR moves to M2_HOST_PREFIX on the
reconfigure that the build-libraries target performs, but the destination
exists by then and the tables we just built were never picked up, so we
distributed the system's tables alongside our own factory.

Copy again when GFTABLESDIR differs from wherever we last copied from, and
remove the previous copy first: file(COPY) optimizes out files whose
timestamps match, and it compares them at a coarser resolution than it
writes them, so it will not reliably replace the earlier tables in place.
Removing first also drops any tables that are no longer present upstream.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@d-torrance

Copy link
Copy Markdown
Member Author

@d-torrance
d-torrance marked this pull request as ready for review August 13, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-generated This PR contains AI-generated code dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant