build: fetch the test data instead of vendoring it as submodules - #642
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
Pull request overview
This PR replaces the large test/data/* git submodules with a CMake-driven fetch mechanism (pinned via commit SHAs) to avoid forcing SwiftPM consumers to clone multi-GB test data and to handle private test-data repos without baking credentials into checkout.
Changes:
- Add
test/data.cmakepin set and a newcmake/test_data.cmakefetch/update script for test inputs + reference output. - Wire fetching into the test build via
ODR_FETCH_TEST_DATAand provide anupdate_test_datatarget for pin-aligned updates. - Update repo metadata/CI to remove test-data submodules, ignore fetched folders, and fetch data explicitly in the
testjob.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
test/data.cmake |
Introduces the pinned test-data repo list (URL + revision per checkout). |
test/CMakeLists.txt |
Includes the fetch script during test configure (optional) and adds update_test_data target. |
CMakeLists.txt |
Adds ODR_FETCH_TEST_DATA option to control configure-time fetching. |
cmake/test_data.cmake |
Implements clone/update logic for pinned repositories under test/data/. |
AGENTS.md |
Updates contributor/build documentation to reflect fetched (not submodule) test data. |
.gitmodules |
Removes the four test-data submodules, leaving only conan-odr-index. |
.gitignore |
Ignores fetched test/data/input/odr-* and test/data/reference-output/odr-* checkouts. |
.github/workflows/build_test.yml |
Stops using submodules for test data; authenticates via gh auth setup-git and fetches pinned data in the test job; disables fetching in build jobs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The four test-data repositories under `test/data/` become a CMake fetch pinned in `test/data.cmake`, driven by `cmake/test_data.cmake` behind `ODR_TEST_FETCH_DATA`. Only `conan-odr-index` stays a submodule. This is groundwork for shipping the Apple framework over Swift Package Manager. SwiftPM runs `git submodule update --init --recursive` on a package repository at every consumer's checkout, so a root `Package.swift` would have made everyone clone 4.5 GB of test data and then fail outright on the two private remotes. Core's own history is 55.8 MiB; the 910 MB `.git` is almost entirely `.git/modules/`. Fetching is opt in. Building `odr_test` does not need the data, and several gigabytes should not arrive because someone turned tests on. The checkouts stay in the source tree rather than the build tree: the path is baked into `test_info.cpp`, three build directories would otherwise hold three copies, and `reference-output/*` has to remain a working copy the regeneration workflow can commit from. Advancing a pin replaces `git add`-ing a submodule, and shows up as a readable diff. Configure time only clones what is missing and warns about drift — it never moves an existing checkout, because during a regeneration the local revision is ahead of the pin on purpose. The `update_test_data` target does move them, refusing on a dirty tree. Authentication stays git's business: the script shells out to plain `git`. The `test` job authenticates with `gh auth setup-git` instead of carrying the PAT as its checkout token. `GH_TOKEN` sits on the job rather than that one step, because the helper it installs shells back out to `gh auth git-credential`, which reads the token from the environment at the moment git asks for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VYR3UqA1asTaTTNRm29csV
c21fb92 to
faac9ea
Compare
|
Force-pushed: the
Also, per review: the option is renamed |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
🤖 Generated with Claude Code
Groundwork for shipping the Apple framework over Swift Package Manager, but worth landing on its own.
The four test-data repositories under
test/data/become a CMake fetch pinned intest/data.cmakeand driven bycmake/test_data.cmakewhenODR_TESTis on. Onlyconan-odr-indexstays a submodule.Why
SwiftPM runs
git submodule update --init --recursiveon a package repository at every consumer's checkout. A rootPackage.swiftwould therefore make everyone clone 4.5 GB of test data and then fail outright on the two private remotes. This repo's own history is 55.8 MiB; the 910 MB.gitis almost entirely.git/modules/.It also matches how the repo already treats external data —
tools/pdf/cmap-resources,glyphlist.txtandafm/are fetched into gitignored paths, and odr.js already comes down viaFetchContent. The test data was the outlier.Behaviour
update_test_datamoves checkouts onto the pins, refusing on a dirty tree.test_info.cpp, three build directories would otherwise hold three copies, andreference-output/*has to remain a working copy the regeneration workflow can commit from.git add-ing a submodule, and shows up as a readable diff.Authentication stays git's business — the script shells out to plain
git. Thetestjob authenticates withgh auth setup-gitinstead of carrying the PAT as its checkout token, so the token never lands in a git config file. Thebuildjob passesODR_FETCH_TEST_DATA=OFFbecause it compilesodr_testwithout running it.Verification
All five paths exercised against a real repository: clone, silent no-op, drift warning that leaves the checkout alone, dirty-tree refusal, and clean update.
odr_testbuilds and the data-reading suites pass.