From faac9ea2e63987f73e3673f214f8893f852c329e Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 31 Jul 2026 22:32:04 +0200 Subject: [PATCH 1/2] build: fetch the test data instead of vendoring it as submodules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01VYR3UqA1asTaTTNRm29csV --- .github/workflows/build_test.yml | 24 +++- .gitignore | 3 + .gitmodules | 12 -- AGENTS.md | 8 +- CMakeLists.txt | 1 + cmake/test_data.cmake | 151 +++++++++++++++++++++++++ test/CMakeLists.txt | 14 +++ test/data.cmake | 25 ++++ test/data/input/odr-private | 1 - test/data/input/odr-public | 1 - test/data/reference-output/odr-private | 1 - test/data/reference-output/odr-public | 1 - 12 files changed, 221 insertions(+), 21 deletions(-) create mode 100644 cmake/test_data.cmake create mode 100644 test/data.cmake delete mode 160000 test/data/input/odr-private delete mode 160000 test/data/input/odr-public delete mode 160000 test/data/reference-output/odr-private delete mode 160000 test/data/reference-output/odr-public diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index e9fb4b36f..4197a55e2 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -147,7 +147,8 @@ jobs: # Both suites run in seconds, so they stay in this job rather than paying # an artifact round-trip. The heavy `odr_test` still runs in `test`, which - # needs firefox and the reference-output submodule. + # needs firefox and the reference output — this job compiles `odr_test` + # but never runs it, so it never fetches the data either. - name: junit if: matrix.bindings run: ctest --test-dir build/jni --output-on-failure @@ -244,12 +245,27 @@ jobs: # Running main() from C:\Users\runneradmin\.conan2\p\b\gtestdd9407d368b89\b\src\googletest\src\gtest_main.cc # [ FATAL ] C:/Users/runneradmin/.conan2/p/gtest28fa6787e7f6e/p/include\gtest/internal/gtest-param-util.h(585):: Condition IsValidParamName(param_name) failed. Parameterized test name 'odr_private\docx\03_smpldap_docx' is invalid, in D:\a\OpenDocument.core\OpenDocument.core\test\src\html_output_test.cpp line 129 #- { os: windows-2022, host_profile: windows-2022-msvc-1940 } + env: + # read by the git credential helper, on every step that runs git + GH_TOKEN: ${{ secrets.PAT_ANDIWAND }} steps: - name: checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - with: - token: ${{ secrets.PAT_ANDIWAND }} - submodules: true + + # The test data stopped being submodules (see cmake/test_data.cmake), so + # the PAT is no longer the checkout token — it authenticates the two + # private clones instead. A credential helper rather than a url rewrite, + # so the token never lands in a git config file. + # + # `GH_TOKEN` is on the job rather than this step: the helper + # `gh auth setup-git` installs shells back out to `gh auth git-credential`, + # which reads the token from the environment at the moment git asks it. + # Set only here, the fetch below falls through to prompting and dies with + # "could not read Username for 'https://github.com'". + - name: authenticate to the private test data + run: gh auth setup-git + - name: fetch test data + run: cmake -DODR_TEST_DATA_UPDATE=ON -P cmake/test_data.cmake - name: ubuntu install tidy if: runner.os == 'Linux' diff --git a/.gitignore b/.gitignore index 140a129c4..8dec485a8 100644 --- a/.gitignore +++ b/.gitignore @@ -51,6 +51,9 @@ CMakeUserPresets.json .vscode/.env offline/ +## Test inputs and reference output (fetched by cmake/test_data.cmake) +test/data/input/odr-*/ +test/data/reference-output/odr-*/ ## PDF CJK CMap input data (fetched by tools/pdf/generate_cid_data.py) tools/pdf/cmap-resources/ ## Adobe Glyph List (fetched by tools/pdf/generate_encoding_data.py) diff --git a/.gitmodules b/.gitmodules index f60e96a52..97d2b5bd3 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,15 +1,3 @@ -[submodule "test/data/input/odr-public"] - path = test/data/input/odr-public - url = https://github.com/opendocument-app/OpenDocument.test.git -[submodule "test/data/reference-output/odr-public"] - path = test/data/reference-output/odr-public - url = https://github.com/opendocument-app/OpenDocument.test.output.git -[submodule "test/data/input/odr-private"] - path = test/data/input/odr-private - url = git@github.com:opendocument-app/OpenDocument.test-private.git -[submodule "test/data/reference-output/odr-private"] - path = test/data/reference-output/odr-private - url = git@github.com:opendocument-app/OpenDocument.test-private.output.git [submodule "conan-odr-index"] path = conan-odr-index url = https://github.com/opendocument-app/conan-odr-index.git diff --git a/AGENTS.md b/AGENTS.md index 6ce4f70d4..923f49f84 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,7 +90,13 @@ cmake --build cmake-build-relwithdebinfo --target translate # CLI: file → HTM - CMake options (`CMakeLists.txt`): `ODR_TEST`, `ODR_CLI`, `ODR_WITH_LIBMAGIC`, `ODR_PYTHON`, `ODR_CLANG_TIDY`. A new `.cpp` must be added to `ODR_SOURCE_FILES`. -- **Test data lives in git submodules** under `test/data/`. +- **Test data is fetched, not vendored**, and opt in: `-DODR_TEST_FETCH_DATA=ON` + makes `cmake/test_data.cmake` clone the repositories pinned in + `test/data.cmake` into `test/data/` (they were submodules until then; read + that file's header for why). Off by default — building `odr_test` does not + need the data, and several gigabytes should not arrive because someone turned + tests on. The `update_test_data` target moves existing checkouts onto the + pins. The two private repositories need credentials. ## Conventions diff --git a/CMakeLists.txt b/CMakeLists.txt index d7bc0c0e5..7d58eaf05 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,6 +12,7 @@ set(CMAKE_CXX_EXTENSIONS OFF) option(BUILD_SHARED_LIBS "Build using shared libraries" ON) option(ODR_TEST "enable tests" OFF) +option(ODR_TEST_FETCH_DATA "Fetch the test data pinned in test/data.cmake" OFF) option(ODR_CLI "enable command line interface" ON) option(ODR_WITH_HTTP_SERVER "Build the HTTP server (requires cpp-httplib)" ON) option(ODR_CLANG_TIDY "Run clang-tidy static analysis" OFF) diff --git a/cmake/test_data.cmake b/cmake/test_data.cmake new file mode 100644 index 000000000..b572e6afe --- /dev/null +++ b/cmake/test_data.cmake @@ -0,0 +1,151 @@ +# Fetches the test-data repositories pinned in `test/data.cmake` into +# `test/data/`. They were git submodules until this file existed. +# +# Why they stopped being submodules: SwiftPM runs `git submodule update --init +# --recursive` on a package repository at *every consumer's* checkout, so the +# root `Package.swift` that ships the Apple framework would have made everyone +# clone 4.5 GB of test data — and then fail outright on the two private +# remotes. Fetching here keeps that cost on the people who run the tests. +# +# The checkouts live in the source tree, not the build tree, on purpose: the +# path is baked into `test_info.cpp`, the three usual build directories would +# otherwise hold three copies, and `reference-output/*` has to stay a working +# copy the regeneration workflow can commit from. +# +# Two modes: +# +# include() configure time. Clones what is missing and reports what has +# drifted. Never moves an existing checkout — during a +# reference-output regeneration the local revision is ahead of +# the pin on purpose, and silently resetting it would throw the +# work away. +# cmake -P the `update_test_data` target. Moves clean checkouts onto their +# pinned revisions. +# +# This shells out to plain `git`, so authentication is git's business and not +# CMake's: the two private repositories need whatever your git is already set +# up with. A credential helper (`gh auth setup-git` is the one-liner, and what +# CI does), or ssh, in which case: +# +# git config --global url."git@github.com:".insteadOf "https://github.com/" + +# Only in script mode: calling this from an include() would reset the policy +# defaults of the including directory. +if (CMAKE_SCRIPT_MODE_FILE) + cmake_minimum_required(VERSION 3.15) +endif () + +find_package(Git REQUIRED) + +# Both overridable, so a downstream can keep the data elsewhere or pin a +# different set. +if (NOT DEFINED ODR_TEST_DATA_ROOT) + set(ODR_TEST_DATA_ROOT "${CMAKE_CURRENT_LIST_DIR}/../test/data") +endif () +if (NOT DEFINED ODR_TEST_DATA_PINS) + set(ODR_TEST_DATA_PINS "${CMAKE_CURRENT_LIST_DIR}/../test/data.cmake") +endif () + +# Clone at a single revision. `--depth 1` of a bare sha needs the server to +# allow it; GitHub does, but a mirror may not, so fall back to a full fetch +# rather than failing. +function(odr_test_data_clone directory url revision) + message(STATUS "test data: cloning ${url} at ${revision}") + + file(MAKE_DIRECTORY "${directory}") + execute_process( + COMMAND "${GIT_EXECUTABLE}" init --quiet + COMMAND_ERROR_IS_FATAL ANY + WORKING_DIRECTORY "${directory}") + execute_process( + COMMAND "${GIT_EXECUTABLE}" remote add origin "${url}" + COMMAND_ERROR_IS_FATAL ANY + WORKING_DIRECTORY "${directory}") + + execute_process( + COMMAND "${GIT_EXECUTABLE}" fetch --depth 1 origin "${revision}" + RESULT_VARIABLE shallow + WORKING_DIRECTORY "${directory}") + if (NOT shallow EQUAL 0) + execute_process( + COMMAND "${GIT_EXECUTABLE}" fetch origin + COMMAND_ERROR_IS_FATAL ANY + WORKING_DIRECTORY "${directory}") + endif () + + execute_process( + COMMAND "${GIT_EXECUTABLE}" checkout --quiet --detach "${revision}" + COMMAND_ERROR_IS_FATAL ANY + WORKING_DIRECTORY "${directory}") +endfunction() + +# Move an existing clean checkout onto `revision`. Refuses on a dirty tree. +function(odr_test_data_update directory revision) + execute_process( + COMMAND "${GIT_EXECUTABLE}" status --porcelain + OUTPUT_VARIABLE dirty + OUTPUT_STRIP_TRAILING_WHITESPACE + COMMAND_ERROR_IS_FATAL ANY + WORKING_DIRECTORY "${directory}") + if (NOT dirty STREQUAL "") + message(FATAL_ERROR + "${directory} has uncommitted changes. Commit or stash them " + "before updating the test data.") + endif () + + execute_process( + COMMAND "${GIT_EXECUTABLE}" fetch --depth 1 origin "${revision}" + RESULT_VARIABLE shallow + WORKING_DIRECTORY "${directory}") + if (NOT shallow EQUAL 0) + execute_process( + COMMAND "${GIT_EXECUTABLE}" fetch origin + COMMAND_ERROR_IS_FATAL ANY + WORKING_DIRECTORY "${directory}") + endif () + + message(STATUS "test data: ${directory} -> ${revision}") + execute_process( + COMMAND "${GIT_EXECUTABLE}" checkout --quiet --detach "${revision}" + COMMAND_ERROR_IS_FATAL ANY + WORKING_DIRECTORY "${directory}") +endfunction() + +# One test-data repository. Called from `test/data.cmake`. +# +# PATH below `test/data`, also the name the tests refer to it by +# URL clone url +# REVISION the pinned commit; advance it here after regenerating output +function(odr_test_data) + cmake_parse_arguments(PARSE_ARGV 0 ARG "" "PATH;URL;REVISION" "") + set(directory "${ODR_TEST_DATA_ROOT}/${ARG_PATH}") + + # a submodule checkout carries `.git` as a file, a plain clone as a + # directory, so test for either + if (NOT EXISTS "${directory}/.git") + odr_test_data_clone("${directory}" "${ARG_URL}" "${ARG_REVISION}") + return() + endif () + + execute_process( + COMMAND "${GIT_EXECUTABLE}" rev-parse HEAD + OUTPUT_VARIABLE head + OUTPUT_STRIP_TRAILING_WHITESPACE + COMMAND_ERROR_IS_FATAL ANY + WORKING_DIRECTORY "${directory}") + if (head STREQUAL ARG_REVISION) + return() + endif () + + if (ODR_TEST_DATA_UPDATE) + odr_test_data_update("${directory}" "${ARG_REVISION}") + return() + endif () + + message(WARNING + "test/data/${ARG_PATH} is at ${head}, but test/data.cmake pins " + "${ARG_REVISION}. Left untouched — if this is not a reference-output " + "regeneration in progress, run the `update_test_data` target.") +endfunction() + +include("${ODR_TEST_DATA_PINS}") diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ad4ff6120..8f0a23bbf 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -2,6 +2,20 @@ find_package(GTest REQUIRED) include(GoogleTest REQUIRED) +# The inputs and the reference output, pinned in `data.cmake` and fetched by +# `../cmake/test_data.cmake` — they used to be git submodules, see that file. +# +# Opt in: `ODR_TEST_FETCH_DATA` is off by default, because building `odr_test` +# does not require the data and several gigabytes is not something to download +# because someone turned tests on. +set(ODR_TEST_DATA_SCRIPT "${CMAKE_CURRENT_LIST_DIR}/../cmake/test_data.cmake") +if (ODR_TEST_FETCH_DATA) + include("${ODR_TEST_DATA_SCRIPT}") +endif () +add_custom_target(update_test_data + COMMAND "${CMAKE_COMMAND}" -DODR_TEST_DATA_UPDATE=ON -P "${ODR_TEST_DATA_SCRIPT}" + COMMENT "Moving test/data onto the revisions pinned in test/data.cmake") + set(ODR_TEST_DATA_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}/data") configure_file("src/test_info.cpp.in" "src/test_info.cpp") diff --git a/test/data.cmake b/test/data.cmake new file mode 100644 index 000000000..1c111198e --- /dev/null +++ b/test/data.cmake @@ -0,0 +1,25 @@ +# The test-data repositories and the revisions the suite is pinned to. +# Fetched by `cmake/test_data.cmake`; these were git submodules before. +# +# Advancing a pin is the replacement for `git add`-ing a submodule: after +# regenerating and pushing reference output, put the new commit here. + +odr_test_data( + PATH "input/odr-public" + URL "https://github.com/opendocument-app/OpenDocument.test.git" + REVISION "42694a9ab07659ac1f9b937670385b6bc287455b") + +odr_test_data( + PATH "input/odr-private" + URL "https://github.com/opendocument-app/OpenDocument.test-private.git" + REVISION "753b7e5c313c23c0524cb9ed678ac294508f3e21") + +odr_test_data( + PATH "reference-output/odr-public" + URL "https://github.com/opendocument-app/OpenDocument.test.output.git" + REVISION "e838097581d832a8071bafa20981cc7f8632b45e") + +odr_test_data( + PATH "reference-output/odr-private" + URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" + REVISION "ff7961051a535d518bb23cb62cc8a77d89c90389") diff --git a/test/data/input/odr-private b/test/data/input/odr-private deleted file mode 160000 index 753b7e5c3..000000000 --- a/test/data/input/odr-private +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 753b7e5c313c23c0524cb9ed678ac294508f3e21 diff --git a/test/data/input/odr-public b/test/data/input/odr-public deleted file mode 160000 index 42694a9ab..000000000 --- a/test/data/input/odr-public +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 42694a9ab07659ac1f9b937670385b6bc287455b diff --git a/test/data/reference-output/odr-private b/test/data/reference-output/odr-private deleted file mode 160000 index ff7961051..000000000 --- a/test/data/reference-output/odr-private +++ /dev/null @@ -1 +0,0 @@ -Subproject commit ff7961051a535d518bb23cb62cc8a77d89c90389 diff --git a/test/data/reference-output/odr-public b/test/data/reference-output/odr-public deleted file mode 160000 index e83809758..000000000 --- a/test/data/reference-output/odr-public +++ /dev/null @@ -1 +0,0 @@ -Subproject commit e838097581d832a8071bafa20981cc7f8632b45e From f8e5cfa172fcb5de911ae341678c9f40ff2505a7 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 31 Jul 2026 23:59:21 +0200 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cmake/test_data.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/test_data.cmake b/cmake/test_data.cmake index b572e6afe..d73da2830 100644 --- a/cmake/test_data.cmake +++ b/cmake/test_data.cmake @@ -133,7 +133,7 @@ function(odr_test_data) OUTPUT_STRIP_TRAILING_WHITESPACE COMMAND_ERROR_IS_FATAL ANY WORKING_DIRECTORY "${directory}") - if (head STREQUAL ARG_REVISION) + if ("${head}" STREQUAL "${ARG_REVISION}") return() endif ()