ci: cross-platform test matrix with git version testing#1034
Open
clemlesne wants to merge 11 commits intosourcegraph:mainfrom
Open
ci: cross-platform test matrix with git version testing#1034clemlesne wants to merge 11 commits intosourcegraph:mainfrom
clemlesne wants to merge 11 commits intosourcegraph:mainfrom
Conversation
…sting Add build-git + test job split to test across all 4 supported platforms (x86_64-linux, aarch64-linux, aarch64-darwin, x86_64-darwin) and multiple git versions (2.34.1, 2.39.5, 2.47.3, 2.53.0) to catch regressions like the cat-file --filter breakage on older git (issue sourcegraph#1029).
clemlesne
force-pushed
the
ci/cross-platform-test-matrix
branch
3 times, most recently
from
60f13ce to
a7cb7fa
Compare
Each version maps to a real distro's stock git package: - 2.34.1: Ubuntu 22.04 LTS - 2.39.5: Debian 12, reported failing in sourcegraph#1029 - 2.47.3: Debian 13 + Alpine 3.21, also failing in sourcegraph#1029 - 2.53.0: latest stable, first with cat-file --batch --filter= (added in 2.50.0)
Define runners and git-versions once in an init job, then reference via fromJSON() in both build-git and test matrices. Avoids duplication and keeps version justification comments in a single place.
Build universal-ctags from source alongside git, using the same pattern (cache + artifact + per-runner build). Two versions: - p5.9.20210829.0: Ubuntu 22.04/24.04 LTS default (2021 snapshot) - v6.2.1: latest stable, matches Homebrew and Alpine Edge Uses --program-prefix=universal- and --enable-json to match the existing install-ctags-alpine.sh flags. Test matrix is now 8 runners × 4 git versions × 2 ctags versions = 64 combinations.
clemlesne
force-pushed
the
ci/cross-platform-test-matrix
branch
from
a7cb7fa to
965d839
Compare
gettext is keg-only on Homebrew, so libintl.h is not in the default include path. Add it to brew install and CFLAGS/LDFLAGS.
The verify step runs unconditionally but dynamically-linked binaries need their runtime deps present. Removing the cache-miss condition from dep install steps ensures dylibs are always available. brew/apt install are idempotent and fast when packages are already present.
clemlesne
force-pushed
the
ci/cross-platform-test-matrix
branch
from
ee2a502 to
36d7705
Compare
Git's config.mak.uname already auto-detects Homebrew prefix and finds gettext, libiconv, openssl paths. Manual CFLAGS/LDFLAGS were redundant and could conflict with the build system's own detection.
clemlesne
force-pushed
the
ci/cross-platform-test-matrix
branch
from
0c55791 to
00ddf88
Compare
This reverts commit 00ddf88. Git's config.mak.uname Homebrew auto-detection for gettext was added after git 2.34.1, so older versions fail with 'libintl.h' not found on macOS. Restore explicit CFLAGS/LDFLAGS to support all git versions in the matrix.
clemlesne
force-pushed
the
ci/cross-platform-test-matrix
branch
from
affb64f to
3069942
Compare
Git's Makefile sets CFLAGS = -g -O2 -Wall which overrides the env var. CPPFLAGS is preserved via ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS), so keg-only Homebrew include paths (gettext, openssl, etc.) are actually picked up by the compiler.
clemlesne
force-pushed
the
ci/cross-platform-test-matrix
branch
from
9e2fa73 to
21b8da5
Compare
Contributor
Author
|
Sorry @keegancsmith need to get your approval each time I fix the Actions. This is a security policy you enforced on the repo config. Achieving a standard cross-version git build on both Linux and macOS is a bit tricky but I'll solve that. |
Git's Makefile overrides both CFLAGS and LDFLAGS env vars with its own assignments. Only command-line arguments to make take precedence over Makefile assignments. Previous fix solved the compile step (CPPFLAGS for headers) but the linker still couldn't find libintl because LDFLAGS was being overridden.
clemlesne
force-pushed
the
ci/cross-platform-test-matrix
branch
2 times, most recently
from
97165ef to
430511c
Compare
Shell quoting with make command-line variables is fragile — spaces in CPPFLAGS/LDFLAGS caused make to interpret paths as options. Git's Makefile auto-includes config.mak, so writing the paths there avoids all quoting issues. Uses += to append to existing flags. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
clemlesne
force-pushed
the
ci/cross-platform-test-matrix
branch
from
430511c to
030ba7e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
testjob with three decoupled jobs:build-git,build-ctags, andtestx86_64-linux,aarch64-linux,aarch64-darwin,x86_64-darwin(8 runners)2.34.1,2.39.5,2.47.3,2.53.0p5.9.20210829.0,v6.2.1initjob, consumed viafromJSON()— single source of truthMotivation
Issue #1029 revealed that the CI only tested on Alpine Edge (bleeding-edge git + ctags), masking a compatibility regression with
git cat-file --batch --filter=on older git versions (Debian 12/13, Ubuntu 22.04). More broadly, the CI only covered a single platform/arch despite zoekt officially supporting four.This PR strengthens the QA posture by testing across the full platform matrix and multiple dependency versions, catching system-level integration issues before they reach users.
Design
initbuild-gitbuild-ctags--program-prefix=universal- --enable-jsonmatching existinginstall-ctags-alpine.shtestgo test ./...OS-specific steps use
if: runner.os == 'Linux'/if: runner.os == 'macOS'conditionals.Git versions
--batch --filter=2.34.11:2.34.1-1ubuntu12.39.51:2.39.5-0+deb12u3, reported failing in #10292.47.31:2.47.3-0+deb13u1, also failing in #10292.53.0Universal-ctags versions
p5.9.20210829.05.9.20210829.0-1(2021 snapshot)v6.2.1Test plan
build-gitjobs compile successfullybuild-ctagsjobs compile successfullytestjobs on git 2.53.0 pass —--filteris supportedtestjobs on git 2.34.1 / 2.39.5 / 2.47.3 surface expectedcat-filefailures — confirms the CI would have caught suddenly no content in repos #1029universal-ctagsbinaries with+interactiveRelated to #1029
🤖 Generated with Claude Code