fix: resolve git config failures during testing#347
Open
patrick-stephens wants to merge 19 commits into
Open
Conversation
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
patrick-stephens
added
the
build-linux
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
Author
|
/build linux=centos/6,almalinux/8,ubuntu/22.04 |
patrick-stephens
removed
the
build-linux
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Contributor
Author
|
/build linux=centos/6,almalinux/8,ubuntu/22.04,amazonlinux/2,centos/7,centos/8,rockylinux/8 |
There was a problem hiding this comment.
Pull request overview
This PR addresses cross-distro test/build failures for the git_config plugin by standardizing the test shell behavior, making the functional tests compatible with older git, enforcing a minimum libgit2 version at configure time, and adding a dedicated Amazon Linux 2 build path that compiles required dependencies from source.
Changes:
- Standardize test container shell execution (bash) and fix git branch renaming for older
gitbehavior. - Enforce
libgit2 >= 0.27.0via pkg-config and a compile-time header check to fail fast on unsupported versions. - Update RPM-based distro build images to use newer
libgit2dev packages and add an Amazon Linux 2 builder that builds OpenSSL/libssh2/libgit2 from source.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/Dockerfile.bats | Forces bash as the default shell to make package-manager detection logic reliable on Ubuntu/dash. |
| testing/bats/tests/functional/plugins/git_config/verify-git-config-plugin.bats | Renames the default branch after the initial commit to support older git on empty repos. |
| testing/bats/run-package-functional-tests.sh | Adds an option to wipe the local download directory before functional test runs. |
| source/packaging/distros/rockylinux/Dockerfile | Switches to libgit2_1.7-devel to avoid older libgit2 crashes. |
| source/packaging/distros/centos/Dockerfile | Enables EPEL and switches to libgit2_1.7-devel on CentOS 8-based targets. |
| source/packaging/distros/centos/7/Dockerfile | Adjusts install ordering and switches to libgit2_1.7-devel on CentOS 7. |
| source/packaging/distros/amazonlinux/Dockerfile | Updates top-level documentation/comments now that Amazon Linux 2 is handled separately. |
| source/packaging/distros/amazonlinux/2/Dockerfile | Adds a dedicated Amazon Linux 2 builder that compiles OpenSSL/libssh2/libgit2 from source and wires static-link flags. |
| source/packaging/distros/almalinux/Dockerfile | Enables EPEL and switches to libgit2_1.7-devel on AlmaLinux 8. |
| source/cmake/libgit2.cmake | Enforces a minimum libgit2 version (>= 0.27.0) and errors out early if unmet. |
| .github/workflows/call-get-metadata.yaml | Minor formatting cleanup to satisfy workflow/YAML linters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Contributor
Author
|
/build linux=centos/6,almalinux/8,ubuntu/22.04,amazonlinux/2,centos/7,centos/8,rockylinux/8 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Contributor
Author
|
/build linux=centos/6,almalinux/8,ubuntu/22.04,amazonlinux/2,centos/7,centos/8,rockylinux/8 |
Coverage Report for CI Build 29104661980Coverage decreased (-0.001%) to 57.874%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions3 previously-covered lines in 2 files lost coverage.
Coverage Stats
💛 - Coveralls |
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Contributor
Author
|
/build linux=centos/6,almalinux/8,ubuntu/22.04,amazonlinux/2,centos/7,centos/8,rockylinux/8 |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="source/packaging/distros/centos/7/Dockerfile">
<violation number="1" location="source/packaging/distros/centos/7/Dockerfile:112">
P1: The source-built shared libgit2 may cause RPM installation or runtime failures on target CentOS 7 hosts. Because `CPACK_RPM_PACKAGE_AUTOREQPROV` is enabled, `cpack` will automatically detect the `libgit2.so` SONAME and add it as an RPM `Requires`, but CentOS 7 repositories do not provide this source-built version. The shared library is also not explicitly bundled into the RPM, so the installed binary may fail to load it at runtime. Consider building libgit2 statically by setting `-DBUILD_SHARED_LIBS=OFF` and ensuring the Fluent Bit build links against the static archive instead.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
There was a problem hiding this comment.
2 issues found across 15 files
Confidence score: 3/5
source/packaging/distros/centos/7/Dockerfilecan still fail CentOS 7 package builds at final link time because CMake appears to keep using non-staticLIBGIT2_LIBRARIESwhile onlylibssh2.ais added, so merging as-is risks broken package artifacts for that distro — switch the libgit2 CMake linkage to the static library path before merging.testing/bats/run-package-functional-tests.shmay invoke Docker forBUILD_PACKAGE=1even whenCONTAINER_RUNTIMEis set to Podman, which can cause local functional package tests to run in the wrong runtime and hide environment-specific failures — pass the selected runtime throughFLB_DOCKER_CLIso test/build behavior stays aligned.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="testing/bats/run-package-functional-tests.sh">
<violation number="1" location="testing/bats/run-package-functional-tests.sh:72">
P2: `BUILD_PACKAGE=1` runs the package build with Docker even when this test runner is configured for Podman via `CONTAINER_RUNTIME`. Passing the selected runtime through `FLB_DOCKER_CLI` keeps local package builds aligned with the container runtime used for the test image.</violation>
</file>
<file name="source/packaging/distros/centos/7/Dockerfile">
<violation number="1" location="source/packaging/distros/centos/7/Dockerfile:126">
P1: CentOS 7 package builds can fail at final link once only `libgit2.a` is available, because CMake still uses non-static `LIBGIT2_LIBRARIES` and this Dockerfile only adds `libssh2.a`. Consider switching the libgit2 CMake lookup to static pkg-config/link flags for this build or explicitly passing all private deps required by the static libgit2 archive.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| tar xf v${LIBGIT2_VER}.tar.gz | ||
| WORKDIR /tmp/libgit2-${LIBGIT2_VER} | ||
| RUN cmake -B build -DCMAKE_BUILD_TYPE=Release \ | ||
| -DBUILD_SHARED_LIBS=OFF \ |
There was a problem hiding this comment.
P1: CentOS 7 package builds can fail at final link once only libgit2.a is available, because CMake still uses non-static LIBGIT2_LIBRARIES and this Dockerfile only adds libssh2.a. Consider switching the libgit2 CMake lookup to static pkg-config/link flags for this build or explicitly passing all private deps required by the static libgit2 archive.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At source/packaging/distros/centos/7/Dockerfile, line 126:
<comment>CentOS 7 package builds can fail at final link once only `libgit2.a` is available, because CMake still uses non-static `LIBGIT2_LIBRARIES` and this Dockerfile only adds `libssh2.a`. Consider switching the libgit2 CMake lookup to static pkg-config/link flags for this build or explicitly passing all private deps required by the static libgit2 archive.</comment>
<file context>
@@ -102,6 +102,37 @@ RUN tar -xzvf /openssl/openssl-3.5.2.tar.gz -C /openssl
+ tar xf v${LIBGIT2_VER}.tar.gz
+WORKDIR /tmp/libgit2-${LIBGIT2_VER}
+RUN cmake -B build -DCMAKE_BUILD_TYPE=Release \
+ -DBUILD_SHARED_LIBS=OFF \
+ -DLINK_WITH_STATIC_LIBRARIES=ON \
+ -DUSE_SSH=ON \
</file context>
| if [[ -n "${BUILD_PACKAGE:-}" ]]; then | ||
| echo "INFO: Building package for $DISTRO as BUILD_PACKAGE is set" | ||
| # If BUILD_PACKAGE is set, build the package first | ||
| "$REPO_ROOT/build-package.sh" -d "$DISTRO" |
There was a problem hiding this comment.
P2: BUILD_PACKAGE=1 runs the package build with Docker even when this test runner is configured for Podman via CONTAINER_RUNTIME. Passing the selected runtime through FLB_DOCKER_CLI keeps local package builds aligned with the container runtime used for the test image.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At testing/bats/run-package-functional-tests.sh, line 72:
<comment>`BUILD_PACKAGE=1` runs the package build with Docker even when this test runner is configured for Podman via `CONTAINER_RUNTIME`. Passing the selected runtime through `FLB_DOCKER_CLI` keeps local package builds aligned with the container runtime used for the test image.</comment>
<file context>
@@ -61,8 +64,21 @@ if [[ $FOUND_FILES == false ]]; then
+ if [[ -n "${BUILD_PACKAGE:-}" ]]; then
+ echo "INFO: Building package for $DISTRO as BUILD_PACKAGE is set"
+ # If BUILD_PACKAGE is set, build the package first
+ "$REPO_ROOT/build-package.sh" -d "$DISTRO"
+ echo "INFO: Package built for $DISTRO, copying to $DOWNLOAD_DIR for test container to find it"
+ # Need to copy the package to the download directory for the test container to find it
</file context>
Suggested change
| "$REPO_ROOT/build-package.sh" -d "$DISTRO" | |
| FLB_DOCKER_CLI="$CONTAINER_RUNTIME" "$REPO_ROOT/build-package.sh" -d "$DISTRO" |
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
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.
Resolves #341 by fixing various issues found during testing of the git_config plugin:
git branchcommands on an empty repoyum, etc. failedSummary by cubic
Fixes
git_configplugin test failures across distros by standardizing the test shell, supporting oldergit, aligninglibgit2/libssh2/opensslversions, improving the test runner, and handlingsystemd-analyze/CI edge cases. Resolves #341; AlmaLinux 8libgit2crash remains under investigation.Bug Fixes
/bin/bash -cin the test image so package manager detection works on Ubuntu.git branch -M mainto support oldergiton CentOS 6.BUILD_PACKAGE,CLEAN_DOWNLOAD, use$REPO_ROOT, set Docker build context reliably, and guard against root deletion.systemd-analyzeenvironments without a manager and detect the correctfluent-bit.servicepath; CI now auto-includesubuntu/24.04when explicitlinux=...targets are requested to satisfy integration tests.Dependencies
CMakelibgit2minimum-version check to avoid false failures on older distros.libgit2_1.7-develon CentOS 8, AlmaLinux 8, and Rocky Linux 8 (enableepel-release).openssl3.5.2,libssh21.11.1, andlibgit21.9.1 from source as static libs and link viaCMake(explicit staticlibssh2linkage).amazonlinux/2builder that compilesopenssl3.5.2,libssh21.11.1, andlibgit21.9.1 statically; wireCMakeflags to link these.Written for commit 1dd2414. Summary will update on new commits.