Skip to content

fix(build): gate libgit2 allocator support#829

Merged
DeusData merged 3 commits into
DeusData:mainfrom
SyntaxSawdust:codex/issue-801-libgit2-floor
Jul 4, 2026
Merged

fix(build): gate libgit2 allocator support#829
DeusData merged 3 commits into
DeusData:mainfrom
SyntaxSawdust:codex/issue-801-libgit2-floor

Conversation

@SyntaxSawdust

Copy link
Copy Markdown
Contributor

Summary

  • Require libgit2 >= 1.7.0 before enabling the optional HAVE_LIBGIT2 build path.
  • Add REQUIRE_LIBGIT2=1 so CI/proof builds fail clearly if the intended libgit2 path is missing or too old.
  • Add a defensive header-version guard before binding git_allocator.
  • Run one Ubuntu PR test matrix leg with libgit2-dev installed and required.

Why

Fixes #801.

The allocator binding uses the libgit2 1.7+ shape, but the build previously enabled HAVE_LIBGIT2 for any libgit2 pkg-config result. PR CI also did not intentionally cover the optional libgit2 path.

Tests

  • make -f Makefile.cbm cbm REQUIRE_LIBGIT2=1
  • make -f Makefile.cbm build/c/test-runner REQUIRE_LIBGIT2=1
  • Missing pkg-config fallback probe with PKG_CONFIG=/bin/false
  • Fake libgit2 1.6.9 pkg-config probe: default fallback works, required mode fails clearly
  • ruby -e 'require "yaml"; YAML.load_file(".github/workflows/_test.yml")'
  • git diff --check

Scope

  • Keeps missing or too-old libgit2 as a normal fallback unless REQUIRE_LIBGIT2=1 is set.
  • Does not vendor or build libgit2 from source.
  • Does not expand every OS matrix leg.
  • Does not change git-history parsing behavior.

Caveats

  • Full scripts/test.sh, make -f Makefile.cbm test, and make -f Makefile.cbm lint-ci were not run locally.
  • actionlint was unavailable locally, so workflow validation used YAML parsing plus manual review.
  • Old-version behavior is proven with pkg-config simulation and header-macro compatibility review, not by compiling against a real old libgit2 package.

@SyntaxSawdust SyntaxSawdust marked this pull request as ready for review July 3, 2026 23:27
@SyntaxSawdust SyntaxSawdust requested a review from DeusData as a code owner July 3, 2026 23:27
Signed-off-by: Dustin Persek <dustin.persek@protonmail.com>
@DeusData DeusData added bug Something isn't working github_actions Pull requests that update GitHub Actions code priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jul 4, 2026
@DeusData

DeusData commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Thanks for the libgit2 build-gating fix for #801. Triage: build/CI correctness PR.

Because this touches Makefile.cbm and .github/workflows/_test.yml, review needs the usual build-infra caution: verify fallback behavior when libgit2 is absent/old, required-mode failure when requested, and that the new CI leg proves the intended optional path without making all platforms depend on libgit2.

@DeusData

DeusData commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Thank you — this closes #801 more completely than the issue asked: the header-level guard correctly handles a future libgit2 2.x (the issue's own suggestion would have rejected it), and making the CI leg loud via REQUIRE_LIBGIT2=1 means silent fallback can never fake coverage again. The exact regression class that let the missing include ship (#722's backstory) is now structurally impossible. Merging.

@DeusData DeusData merged commit 9770803 into DeusData:main Jul 4, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working github_actions Pull requests that update GitHub Actions code priority/normal Standard review queue; useful PR with ordinary maintainer urgency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build: libgit2 version floor (git_allocator needs >=1.7) + libgit2-enabled CI leg

2 participants