feat: add config example validation script and workflow - #2627
feat: add config example validation script and workflow#2627patrick-stephens wants to merge 26 commits into
Conversation
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
📝 WalkthroughWalkthroughAdded configuration extraction and dry-run validation scripts, integrated them into pull-request workflow checks, and corrected configuration examples across installation and pipeline documentation. ChangesConfiguration validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Pull request
participant GitHub Actions
participant test-config.sh
participant Fluent Bit container
Pull request->>GitHub Actions: change Markdown files
GitHub Actions->>test-config.sh: validate each changed file
test-config.sh->>Fluent Bit container: run configuration dry-run
Fluent Bit container-->>test-config.sh: return validation status
test-config.sh-->>GitHub Actions: report failures or success
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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>
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>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
…in TOML format Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
Signed-off-by: Patrick Stephens <pat@telemetryforge.io>
ba1f42c to
56630f1
Compare
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>
Resolve all Vale errors and suggestions in the config example validation PR, plus markdownlint errors in the new scripts README. - scripts/README.md: replace "e.g.," with "for example," (3 instances), convert 5 headings to sentence case, use "aren't" contraction, and fix markdownlint MD031/MD032/MD040 by adding blank lines around fences and lists and switching nested Markdown examples to 4-backtick outer fences - installation/downloads/docker.md: spell out "K8s" as "Kubernetes" - pipeline/inputs/tail.md: use "`inode` numbers" so the term is in code font and skipped by the spelling rule - pipeline/filters/parser.md: use "shouldn't" contraction - pipeline/outputs/kafka.md: use "isn't" contraction - pipeline/parsers.md: remove leading ellipsis and rewrite as a complete sentence Signed-off-by: Eric D. Schabell <eric@schabell.org>
eschabell
left a comment
There was a problem hiding this comment.
@patrick-stephens did some cleanup work but it now is good to go, thanks for this! Please merge this when you are ready?
| Configuration examples should be formatted using Gitbook-style tabs: | ||
|
|
||
| ```markdown | ||
| ````markdown |
|
Once merged we can see how things go and look to extend it in the future with checks for case, etc. |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
scripts/test-config.sh (2)
119-128: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid rerunning the container just to capture failure output.
On failure, the container is run twice: once with output discarded (Line 120) to check the exit status, and again (Line 125) purely to surface output on stderr. Capture combined output on the first run instead, so a failing validation doesn't double the container startup/dry-run cost.
♻️ Proposed fix
- if ! $CONTAINER_RUNTIME run --rm -t -v "$OUTPUT_FILE":"$OUTPUT_FILE":ro "$VALIDATION_IMAGE" fluent-bit --dry-run --config="$OUTPUT_FILE" &>/dev/null; then + VALIDATION_OUTPUT=$($CONTAINER_RUNTIME run --rm -v "$OUTPUT_FILE":"$OUTPUT_FILE":ro "$VALIDATION_IMAGE" fluent-bit --dry-run --config="$OUTPUT_FILE" 2>&1) && VALIDATION_STATUS=0 || VALIDATION_STATUS=$? + if [ "$VALIDATION_STATUS" -ne 0 ]; then FAILED_VALIDATIONS+=("$LANGUAGE example $EXAMPLE_INDEX") - # Provide the configuration and failure output for debugging purposes on stderr echo "ERROR: Validation failed for $LANGUAGE example $EXAMPLE_INDEX in $FILE" >&2 cat "$OUTPUT_FILE" >&2 - $CONTAINER_RUNTIME run --rm -t -v "$OUTPUT_FILE":"$OUTPUT_FILE":ro "$VALIDATION_IMAGE" fluent-bit --dry-run --config="$OUTPUT_FILE" >&2 || true + echo "$VALIDATION_OUTPUT" >&2 else🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test-config.sh` around lines 119 - 128, Update the validation command in the configuration-checking flow to capture its combined output during the initial container run while preserving the exit status. In the failure branch, print the captured output instead of invoking $CONTAINER_RUNTIME a second time; keep the existing configuration and failure-context diagnostics unchanged.
73-81: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winImage is pulled once per invocation, i.e., once per changed file per PR run.
The workflow invokes this script once per changed Markdown file (see
pr-example-validation.yaml, Line 58-61). Each invocation independently pulls$VALIDATION_IMAGE(Line 78), adding a network round-trip per file even when the image is already present locally. For PRs touching several documentation files, this adds up.Consider checking for a local image and only pulling when absent, or moving the pull into a one-time setup step.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test-config.sh` around lines 73 - 81, Update the image setup around CONTAINER_RUNTIME and VALIDATION_IMAGE so the script checks whether VALIDATION_IMAGE is already available locally before pulling it. Invoke the runtime’s image-inspection command, pull only when the image is absent, and preserve the existing error message and exit behavior when a required pull fails.scripts/extract-config.sh (1)
146-171: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSilent success when the tab title is never found.
In the
ENDblock, the error branch only fires whenfound_tab && !found_fence(Line 166). Ifwanted_titlenever matches anywhere in the file,found_tabstays0, so neither the success branch nor the error branch runs. The script exits0with empty output.
test-config.shmasks this because count mode already returns0and the caller skips extraction in that case. Butscripts/README.mddocuments this script for direct standalone use ("can also be called directly"). A typo'd tab title passed directly would silently succeed with no output instead of reporting an error.♻️ Proposed fix
} else if (found_tab && !found_fence) { printf "ERROR: %s code fence #%d not found in tab: %s\n", wanted_language, target_index, wanted_title > "/dev/stderr" exit 1 + } else if (!found_tab) { + printf "ERROR: tab not found: %s\n", wanted_title > "/dev/stderr" + exit 1 }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/extract-config.sh` around lines 146 - 171, Update the extract-mode validation in the END block so a missing tab title reports an error and exits nonzero instead of silently succeeding. Extend the existing found_tab/found_fence handling around wanted_title, while preserving count mode and the current missing-fence and missing-code-fence errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-example-validation.yaml:
- Line 43: Update the changed_md grep filter in the workflow to use a literal
dot for the filename/path match, ensuring Markdown files at the repository root
and in subdirectories are detected while retaining the existing .md suffix
requirement.
- Line 43: Update the changed_md command in the PR validation workflow to use
git diff with the existing base and HEAD revision range, or first compute the
merge base before invoking git diff-tree. Preserve the current name and
diff-filter options so only added, modified, copied, or renamed Markdown files
are selected.
- Around line 42-46: Update the output-writing logic in the Markdown
change-detection step so the newline-separated value from changed_md uses GitHub
Actions’ multiline output delimiter syntax instead of echoing it as a plain
list= entry. Preserve the existing output name list and ensure the
delimiter-wrapped value is written safely to GITHUB_OUTPUT for multiple Markdown
files.
In `@pipeline/filters/parser.md`:
- Line 30: Update scripts/test-config.sh to discover each per-file tab title and
pass that title to extract-config.sh, rather than counting only
fluent-bit.yaml/fluent-bit.conf; ensure every renamed example is validated.
Apply this discovery behavior to the tab declarations at
pipeline/filters/parser.md:30-30 and :40-40, and
pipeline/outputs/kafka.md:110-110 and :157-157, using each declaration’s title
as the corresponding configuration name.
In `@pipeline/inputs/kafka.md`:
- Around line 145-156: Update the introductory paragraph to state that each
message is processed by the inline modify_kafka_message function rather than
kafka.lua, and revise the sentence structure so it clearly describes sending the
result back to the fb-sink topic on the same broker.
In `@pipeline/parsers.md`:
- Around line 68-72: Make both standalone YAML examples self-contained: in
pipeline/parsers.md, define custom_parser1 inline or reference a concrete
parser-file fixture before it is used; in pipeline/filters/kubernetes.md,
replace the legacy undefined parsers_file reference with the existing inline
custom-tag definition or another concrete parser-file fixture. Ensure each
extracted YAML tab validates independently.
In `@pipeline/router.md`:
- Line 210: Update the routing.yaml tab title in the routing documentation
around the routing.yaml example so scripts/test-config.sh recognizes and
extracts the routes configuration snippet. Use a supported extractor tab title
while preserving the existing routes syntax example.
In `@scripts/test-config.sh`:
- Around line 56-71: In the suppressed-file loop, replace the three redundant
conditions with one path-anchored match that accepts only an exact path or a
file beneath the suppressed entry as a directory. Preserve the informational
message and exit behavior, while ensuring names embedded in unrelated filenames
or extensions do not suppress validation.
- Around line 96-102: Update the Stage 1 counting flow around extract-config.sh
so genuine command failures remain visible and cause the validation to fail,
rather than being converted into EXAMPLE_COUNT=0. Preserve the legitimate
zero-example path that continues without validation, but capture and report the
extraction error separately using the existing script error-handling
conventions.
---
Nitpick comments:
In `@scripts/extract-config.sh`:
- Around line 146-171: Update the extract-mode validation in the END block so a
missing tab title reports an error and exits nonzero instead of silently
succeeding. Extend the existing found_tab/found_fence handling around
wanted_title, while preserving count mode and the current missing-fence and
missing-code-fence errors.
In `@scripts/test-config.sh`:
- Around line 119-128: Update the validation command in the
configuration-checking flow to capture its combined output during the initial
container run while preserving the exit status. In the failure branch, print the
captured output instead of invoking $CONTAINER_RUNTIME a second time; keep the
existing configuration and failure-context diagnostics unchanged.
- Around line 73-81: Update the image setup around CONTAINER_RUNTIME and
VALIDATION_IMAGE so the script checks whether VALIDATION_IMAGE is already
available locally before pulling it. Invoke the runtime’s image-inspection
command, pull only when the image is absent, and preserve the existing error
message and exit behavior when a required pull fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 526f19af-3dd1-4b14-80b6-0c240eb98fc1
📒 Files selected for processing (22)
.github/workflows/pr-example-validation.yamlinstallation/downloads/docker.mdpipeline/buffering.mdpipeline/filters/geoip2-filter.mdpipeline/filters/kubernetes.mdpipeline/filters/parser.mdpipeline/inputs/blob.mdpipeline/inputs/cpu-metrics.mdpipeline/inputs/kafka.mdpipeline/inputs/tail.mdpipeline/outputs/dynatrace.mdpipeline/outputs/gelf.mdpipeline/outputs/kafka.mdpipeline/outputs/loki.mdpipeline/outputs/s3.mdpipeline/parsers.mdpipeline/processors/conditional-processing.mdpipeline/processors/sql.mdpipeline/router.mdscripts/README.mdscripts/extract-config.shscripts/test-config.sh
💤 Files with no reviewable changes (2)
- pipeline/processors/conditional-processing.md
- pipeline/outputs/gelf.md
| run: | | ||
| changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep './.*\.md$' || true) | ||
| echo "Changed Markdown files: $changed_md" | ||
| echo "list=${changed_md}" >> $GITHUB_OUTPUT | ||
| shell: bash |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Multi-line output breaks when more than one Markdown file changes.
echo "list=${changed_md}" >> $GITHUB_OUTPUT (Line 45) writes $changed_md as a single key=value line. $changed_md is newline-separated whenever more than one Markdown file changed (the common case for documentation PRs). GitHub's $GITHUB_OUTPUT file is newline-delimited, so embedding raw newlines inside a plain name=value entry breaks the file format; GitHub Actions reports this with an error message: "Error: Unable to process file command 'output' successfully" and "Error: Invalid format" for the offending lines. This step will fail, or produce truncated steps.changed_files.outputs.list, whenever a PR touches multiple .md files.
Use the delimiter syntax for multi-line outputs instead.
🐛 Proposed fix
changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep './.*\.md$' || true)
echo "Changed Markdown files: $changed_md"
- echo "list=${changed_md}" >> $GITHUB_OUTPUT
+ {
+ echo "list<<CHANGED_MD_EOF"
+ echo "$changed_md"
+ echo "CHANGED_MD_EOF"
+ } >> "$GITHUB_OUTPUT"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: | | |
| changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep './.*\.md$' || true) | |
| echo "Changed Markdown files: $changed_md" | |
| echo "list=${changed_md}" >> $GITHUB_OUTPUT | |
| shell: bash | |
| run: | | |
| changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep './.*\.md$' || true) | |
| echo "Changed Markdown files: $changed_md" | |
| { | |
| echo "list<<CHANGED_MD_EOF" | |
| echo "$changed_md" | |
| echo "CHANGED_MD_EOF" | |
| } >> "$GITHUB_OUTPUT" | |
| shell: bash |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-example-validation.yaml around lines 42 - 46, Update
the output-writing logic in the Markdown change-detection step so the
newline-separated value from changed_md uses GitHub Actions’ multiline output
delimiter syntax instead of echoing it as a plain list= entry. Preserve the
existing output name list and ensure the delimiter-wrapped value is written
safely to GITHUB_OUTPUT for multiple Markdown files.
| # We grep for *.md files you are interested in. | ||
| # The 'docs/' prefix is a common convention but adjust as necessary. | ||
| run: | | ||
| changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep './.*\.md$' || true) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unescaped . in the grep filter changes its intended meaning.
grep './.*\.md$' uses an unescaped . as the second pattern character, which acts as "any character" rather than a literal dot. The effective pattern only requires some character, then /, then anything, then a literal .md at the end — so it silently excludes any changed Markdown file that has no / before it (for example, a repository-root file). The accompanying comment says "The 'docs/' prefix is a common convention but adjust as necessary," suggesting the intent was closer to matching by directory, but the current pattern accomplishes that only by accident. Escape the dot (or simplify) so the filter matches what's intended.
🛠️ Proposed fix
- changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep './.*\.md$' || true)
+ changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep '\.md$' || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep './.*\.md$' || true) | |
| changed_md=$(git diff-tree --no-commit-id --name-only --diff-filter=AMCR ${{ github.event.pull_request.base.sha }}...HEAD | grep '\.md$' || true) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-example-validation.yaml at line 43, Update the
changed_md grep filter in the workflow to use a literal dot for the
filename/path match, ensuring Markdown files at the repository root and in
subdirectories are detected while retaining the existing .md suffix requirement.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does "git diff-tree" support the triple-dot A...B merge-base range the same way "git diff" does, or does it treat it as a revision-walk range like "git log"?
💡 Result:
No, git diff-tree does not support the triple-dot (A...B) syntax for revision ranges in the same way that git diff does [1][2][3]. Git diff-tree is designed to compare two tree objects directly or to compare a commit with its parents [1][3]. Its primary arguments are expected to be objects [1][2]. While git diff interprets the A...B syntax as a shorthand for comparing the merge base of A and B against B [4][5][6], git diff-tree does not perform this interpretation and will not accept the range notation to calculate a diff between those points [1][3]. Furthermore, git diff-tree does not treat the triple-dot as a revision-walk range (like git log does) [1][3]. In git log, A...B represents a symmetric difference (the set of commits reachable from either A or B, but not both). Because git diff-tree is a low-level command focused on comparing specific trees, it does not process revision-walk ranges [7][3]. If you wish to achieve the equivalent of git diff A...B using git diff-tree, you must manually provide the merge base as one of the tree arguments [8]: git diff-tree -r $(git merge-base A B) B Alternatively, if you are working with higher-level diff commands, git diff remains the correct tool for interpreting the triple-dot range syntax [4][5][6].
Citations:
- 1: https://git-scm.com/docs/git-diff-tree
- 2: https://git.github.io/htmldocs/git-diff-tree.html
- 3: https://github.com/git/git/blob/3d09c22869a7bd47f1683031937af0ed93b2fa3b/Documentation/git-diff-tree.txt
- 4: https://git-scm.com/docs/git-diff
- 5: https://git.github.io/htmldocs/git-diff.html
- 6: https://cdn.kernel.org/pub/software/scm/git/docs/git-diff.html
- 7: https://git-scm.com/docs/gitdiffcore
- 8: https://git-scm.com/docs/git-merge-base
🌐 Web query:
git diff --name-only --diff-filter=AMCR base HEAD documentation triple dot behavior
💡 Result:
In Git, the triple-dot syntax (...) used with git diff performs a specific operation to compare a branch against its common ancestor with another branch, rather than comparing two arbitrary points directly [1][2]. When you execute the command git diff base...HEAD: 1. Triple-dot behavior: Git calculates the merge base (the most recent common ancestor) of the two provided commits [1][2]. The command git diff A...B is effectively treated as git diff $(git merge-base A B) B [1][3]. This shows you the changes introduced in the HEAD branch since it diverged from the base branch, effectively ignoring any new changes that may have been committed to the base branch in the meantime [3][2]. 2. --name-only: This flag restricts the output to only display the names (paths) of the files that have changed, rather than showing the patch content (the diff itself) [4][5]. 3. --diff-filter=AMCR: This flag filters the list of changed files based on their status [4][5]. It restricts the output to files that have been: - A: Added - M: Modified - C: Copied - R: Renamed By combining these, git diff --name-only --diff-filter=AMCR base...HEAD provides a concise list of files that were added, modified, copied, or renamed in your current branch (HEAD) relative to the common ancestor shared with the base branch [6]. This is commonly used in CI/CD pipelines to identify which files have been changed in a feature branch without being affected by subsequent commits on the main/base branch [3][6]. If one side of the triple-dot is omitted (e.g., git diff...HEAD or git diff base...), Git treats the omitted side as HEAD [1].
Citations:
- 1: https://github.com/git/git/blob/89afd5f5ad920d1b5f01548bf5c8aeca8e56e899/Documentation/git-diff.txt
- 2: https://stackoverflow.com/questions/7251477/what-are-the-differences-between-double-dot-and-triple-dot-in-git-dif
- 3: https://ipwnponies.github.io/programming/2022/11/26/git-double-triple-dot-notation.html
- 4: https://git-scm.com/docs/diff-options
- 5: https://www.man7.org/linux/man-pages/man1/git-diff.1.html
- 6: https://latchkey.dev/learn/command-reference/git-diff-name-only
🌐 Web query:
git diff-tree documentation --no-commit-id --name-only --diff-filter merge base syntax
💡 Result:
The command git diff-tree is used to compare the content and mode of the blobs found via two tree objects [1][2]. When using the syntax requested, the command operates as follows: git diff-tree --no-commit-id --name-only --diff-filter=[FILTER] [TREE-ISH] [TREE-ISH] --no-commit-id: Suppresses the output of the commit ID that typically appears when git diff-tree processes commits [1][3]. --name-only: Limits the output to showing only the filenames of changed files, omitting the actual diff content [1][4]. --diff-filter=[FILTER]: Selects only files that match specific change types [1][2]. The [FILTER] argument consists of one or more characters representing change categories, such as Added (A), Copied (C), Deleted (D), Modified (M), Renamed (R), Type changed (T), Unmerged (U), Unknown (X), or Broken (B) [1][4]. You can combine these characters or use lowercase versions to exclude specific types (e.g., --diff-filter=d excludes deleted files) [1][2]. [TREE-ISH] [TREE-ISH]: These are the two objects (typically commits, tags, or tree objects) being compared [1][5]. If comparing two commits, the command shows the changes between them [1][6]. For example, to list only the names of modified files between two commits while suppressing commit IDs, you would use: git diff-tree --no-commit-id --name-only --diff-filter=M
Citations:
- 1: https://git-scm.com/docs/git-diff-tree
- 2: https://www.kernel.org/pub/software/scm/git/docs/git-diff-tree.html
- 3: https://github.com/git/git/blob/3d09c22869a7bd47f1683031937af0ed93b2fa3b/Documentation/git-diff-tree.txt
- 4: https://man.archlinux.org/man/extra/git/git-diff-tree.1.en
- 5: https://code.googlesource.com/git/+/HEAD/Documentation/git-diff-tree.adoc
- 6: https://github.com/git/git/blob/6d64ea965bc2894ef0aee16ac23a5e47f20eb824/builtin-diff-tree.c
🌐 Web query:
git diff-tree A...B argument triple dot exit code usage tree-ish tree-ish
💡 Result:
In Git, the triple-dot syntax (A...B) is generally used to specify a range of commits or to compare a branch against its common ancestor with another branch [1][2][3]. When used with commands like git diff, A...B is equivalent to comparing the common ancestor of A and B (the merge base) with B [2][3]. For git-diff-tree, however, the command is a plumbing command that operates primarily on tree objects [4][5]. Its standard usage compares two tree objects (e.g., git diff-tree ) [4][6]. While git-diff supports the triple-dot syntax as a shorthand for merge-base logic [2][3], git-diff-tree does not natively expand the triple-dot shorthand in the same way. When providing A...B to git-diff-tree, the command may not interpret it as a range or merge-base operation, and it is standard practice to explicitly provide the two tree objects you wish to compare [4][7]. Regarding exit codes, git-diff-tree does not use standard diff exit codes (0 for no difference, 1 for differences) by default [8][9]. To achieve this behavior, you must explicitly pass the --exit-code option [8][10]. When --exit-code is provided: - It exits with 0 if there are no differences [8][9]. - It exits with 1 if there are differences [8][10]. Additionally, the --quiet option can be used, which implies --exit-code and suppresses all output [8][9][10]. Note that the --check option, which identifies conflict markers or whitespace errors, is not compatible with --exit-code [9][10].
Citations:
- 1: https://www.baeldung.com/ops/git-double-vs-triple-dot
- 2: https://github.com/git/git/blob/eb448631fb541239d91709d812b92bf2b57bc733/Documentation/git-diff.txt
- 3: https://cdn.kernel.org/pub/software/scm/git/docs/git-diff.html
- 4: https://git-scm.com/docs/git-diff-tree
- 5: https://www.kernel.org/pub/software/scm/git/docs/git-diff-tree.html
- 6: https://git.github.io/htmldocs/git-diff-tree.html
- 7: https://code.googlesource.com/git/+/HEAD/Documentation/git-diff-tree.adoc
- 8: https://man7.org/linux/man-pages/man1/git-diff-tree.1.html
- 9: https://git-scm.com/docs/git-diff-tree/2.55.0
- 10: https://man.archlinux.org/man/extra/git/git-diff-tree.1.en
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository git version:"
git --version
echo
echo "Relevant workflow lines:"
sed -n '30,55p' .github/workflows/pr-example-validation.yaml
echo
echo "git diff-tree argument handling probe:"
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
cd "$tmp"
git init -q
echo base >/tmp/base.txt
echo head >/tmp/head.txt
git add base.txt
git commit -q -m base
git tag base
echo head >/tmp/head.txt
git add head.txt
git commit -q -m head
git tag head
commands=(
'git diff-tree --no-commit-id --name-only --diff-filter=AMCR base...head || true'
'git diff --name-only --diff-filter=AMCR base...head || true'
'git diff-tree --no-commit-id --name-only --diff-filter=AMCR base head || true'
'git merge-base base head'
'git diff --name-only --diff-filter=AMCR $(git merge-base base head) head || true'
)
for cmd in "${commands[@]}"; do
echo "--- $cmd"
eval "$cmd"
echo "exit=$?"
done
echo
echo "git help snippet for diff-tree usage:"
git help -w diff-tree >/tmp/git-help-diff-tree.html 2>&1 || true
if [ -f /tmp/git-help-diff-tree.html ]; then
sed -n '/<h2 id="SYNOPSIS">/,/<\/h2>/p' /tmp/git-help-diff-tree.html | sed 's/<[^>]*>//g' | head -20
fiRepository: fluent/fluent-bit-docs
Length of output: 1788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
cd "$tmp"
git init -q
git config user.email test@example.com
git config user.name test
echo base >/tmp/base.txt
echo head >/tmp/head.txt
git add base.txt
git commit -q -m base
git tag base
echo head >/tmp/head.txt
git add head.txt
git commit -q -m head
git tag head
commands=(
'git diff-tree --no-commit-id --name-only --diff-filter=AMCR base...head || true'
'git diff --name-only --diff-filter=AMCR base...head || true'
'git diff-tree --no-commit-id --name-only --diff-filter=AMCR base head || true'
'git merges-base base head || true'
'git merge-base base head'
'git diff --name-only --diff-filter=AMCR $(git merge-base base head) head || true'
)
for cmd in "${commands[@]}"; do
echo "--- $cmd"
eval "$cmd"
echo "exit=$?"
doneRepository: fluent/fluent-bit-docs
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository status and workflow around changed_files:"
git status --short --branch
sed -n '1,90p' .github/workflows/pr-example-validation.yaml
echo
echo "Search for git commands in workflow:"
rg -n "git (diff|diff-tree|diff-index|ls-files|merge-base|show)" .github/workflows/pr-example-validation.yaml || true
echo
echo "Check for docs directory and example markdown files:"
if [ -d docs ]; then
find docs -maxdepth 3 -type f -name '*.md' | head -20 || true
find docs -maxdepth 3 -type f -path '*examples*' | head -20 || true
else
echo "docs directory not present"
fi
echo
echo "Workflow diff context:"
git diff -- .github/workflows/pr-example-validation.yaml | sed -n '1,160p' || true
sed -n '/on:/,/on:/p' .github/workflows/pr-example-validation.yaml | head -60 || trueRepository: fluent/fluent-bit-docs
Length of output: 3689
🌐 Web query:
"git diff-tree A...B" "fatal: pathspec"
💡 Result:
The error "fatal: pathspec... did not match any files" in Git occurs when the provided pathspec (a pattern used to limit the scope of a command) does not successfully match any files within the specified context [1][2]. When using a command like git diff-tree A...B -- , Git attempts to find files matching that pathspec within the trees being compared [1][3]. If no files or directories match that pattern, Git terminates with this fatal error [1][2]. Key reasons for this error include: 1. Typographical Errors: The most common cause is a simple spelling mistake in the filename or directory path [1]. 2. Path Scope: The pathspec may be correct but does not exist within the specific tree objects or commit range (A...B) you are inspecting [1][2]. 3. Unsupported Wildcards: Unlike some other Git commands, git diff-tree (and the broader diff-tree family) does not support globbing (e.g., *.txt) in the same way higher-level commands might [3]. Because diff-tree walks the tree objects in parallel, it requires literal paths or specific directory structures rather than shell-style wildcards [3]. 4. Incorrect Syntax: The A...B syntax (symmetric difference) compares the changes on the branch up to the second commit, starting from their common ancestor [4][5]. If the path provided does not exist in any of those relevant versions, the match will fail [1]. To resolve this, verify that the path exists in the commits or trees being compared by running a command like git ls-tree to confirm the path's presence [6]. If you are attempting to use wildcards, note that git diff-tree may not interpret them as expected; you may need to use a different command or provide literal paths [3].
Citations:
- 1: https://public-inbox.org/git/51B10236.2030009@ensimag.fr/t/
- 2: https://public-inbox.org/git/CANd8ic+6AtWgbK3S7DMkaOz7K9WJYdq8ozP9mesc-vJRhxLjqg@mail.gmail.com/t/
- 3: http://public-inbox.org/git/7vwrx56tet.fsf@alter.siamese.dyndns.org/T/
- 4: https://git-scm.com/docs/git-diff
- 5: https://www.kernel.org/pub/software/scm/git/docs/git-diff.html
- 6: https://public-inbox.org/git/20200528231622.GB1267915@coredump.intra.peff.net/T/
🌐 Web query:
site:stackoverflow.com "git diff-tree" "A...B"
💡 Result:
The command git diff-tree A...B is not a standard or documented shortcut in Git's command-line interface [1][2][3]. While git diff A...B is a well-known command that shows the changes between the second commit (B) and the merge base of the two commits (the common ancestor of A and B) [1][3], this triple-dot syntax does not behave the same way with git diff-tree [1][4]. Key distinctions to understand: 1. git diff vs. git diff-tree: git diff is a porcelain command designed for human-readable output, whereas git diff-tree is a plumbing command intended for scripts [5][4]. Plumbing commands typically do not interpret complex revision range syntaxes—like the triple-dot merge-base shortcut—that are provided by higher-level commands [1][4]. 2. How git diff-tree interprets arguments: When you provide two arguments to git diff-tree (e.g., git diff-tree A B), it compares the tree of A against the tree of B directly. It does not automatically calculate a merge base [4]. If you intend to use the merge base functionality of the triple-dot syntax with git diff-tree, you must calculate the merge base explicitly [1][6]. You can achieve this by using command substitution to find the base first: git diff-tree -r $(git merge-base A B) B This command sequence explicitly identifies the common ancestor and then compares it against the tip of branch B [1][6]. If you run git diff-tree with the literal string A...B as arguments, it will likely fail or produce unexpected results because it will treat the literal strings as invalid tree-ish identifiers or attempt to parse them incorrectly depending on your shell and Git version [7].
Citations:
- 1: https://stackoverflow.com/questions/66415892/how-do-i-get-a-list-of-merged-files-git
- 2: https://stackoverflow.com/questions/72867771/what-is-the-base-of-a-git-diff
- 3: https://stackoverflow.com/questions/462974/what-are-the-differences-between-double-dot-and-triple-dot-in-git-com
- 4: https://stackoverflow.com/questions/51963396/git-how-can-i-list-the-changed-files-in-a-commit-including-the-sha-1-hash-for
- 5: https://stackoverflow.com/questions/42458005/need-to-get-all-file-differences-added-modified-renamed-between-two-git-comm
- 6: https://stackoverflow.com/questions/47141241/git-diff-merge-base-with-working-tree-using-notation
- 7: https://stackoverflow.com/questions/13092854/what-is-the-difference-between-git-diff-origin-master-origin-branch-and-git
🌐 Web query:
"git diff" "git diff-tree" syntax "tree-ish" "A...B" documentation
💡 Result:
In Git, the syntax A...B is used by the git diff command to compare the changes on a branch up to the second commit (B), starting from the common ancestor of both commits (A and B) [1][2]. Specifically, the command: git diff A...B is equivalent to: git diff $(git merge-base A B) B This syntax is distinct from the range notation (A..B) and is not a range in the context of Git's revision range specifications (which describe a set of commits) [3][2]. Instead, for git diff, this "three-dot" syntax identifies a specific common ancestor to use as the "before" point for the comparison [1][2]. You can omit either A or B, which defaults the omitted value to HEAD [1][2]. Regarding git diff-tree, the command documentation uses the term tree-ish to describe its arguments [4][5]. A tree-ish is defined in the Git glossary as a tree object or any object that can be recursively dereferenced to a tree object, such as a commit object or a tag object [6]. git diff-tree compares the content and mode of blobs found in these tree objects [4][7]. When a single commit is provided to git diff-tree, it compares that commit with its parents; when two trees are provided, it compares them directly [4][5].
Citations:
- 1: https://git-scm.com/docs/git-diff
- 2: http://staroceans.org.s3-website-us-east-1.amazonaws.com/man7.org/linux/man-pages/man1/git-diff.1.html
- 3: https://manpages.debian.org/jessie-backports/git-man/git-diff.1.en.html
- 4: https://git-scm.com/docs/git-diff-tree
- 5: https://www.kernel.org/pub/software/scm/git/docs/git-diff-tree.html
- 6: https://git-scm.com/docs/gitglossary
- 7: https://git-scm.com/docs/git-diff-tree/2.54.0
🌐 Web query:
"git diff-tree" "revision range" "diff-tree" "SYNOPSIS"
💡 Result:
The git diff-tree command is used to compare the content and mode of blobs found via tree objects [1][2]. SYNOPSIS git diff-tree [--stdin] [-m] [-s] [-v] [--no-commit-id] [--pretty] [-t] [-r] [-c | --cc] [--combined-all-paths] [--root] [--merge-base] [] [] [...] [1][2] REVISION RANGE AND USAGE While the synopsis lists arguments, it is important to note that git diff-tree does not accept "revision ranges" (such as A..B or A...B) in the same way commands like git log do [3]. Instead, the command treats its arguments as specific tree or commit objects to compare [1][4]. - When two arguments are provided, git diff-tree compares the first tree with the second [2][4]. - When a single (usually a commit) is provided, it compares that commit with its parents [1][2]. - If additional commits are provided alongside a single commit, they are treated as if they were parents of the first commit [2][4]. - When the --stdin option is used, the command reads tree or commit objects from standard input instead of taking arguments from the command line [1][2].
Citations:
- 1: https://git-scm.com/docs/git-diff-tree
- 2: https://www.kernel.org/pub/software/scm/git/docs/git-diff-tree.html
- 3: https://git-scm.com/docs/git-diff
- 4: https://oracle.hydrogen.sagittarius.connect.product.adaptavist.com/cd/E36784_01/html/E36870/git-diff-tree-1.html
Use git diff or compute the merge base before calling git diff-tree.
git diff-tree takes explicit tree-ish arguments such as <BASE> <HEAD>, while BASE...HEAD is the git diff merge-base shorthand. With git diff-tree, this will not reliably produce changed files between the PR base and HEAD.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-example-validation.yaml at line 43, Update the
changed_md command in the PR validation workflow to use git diff with the
existing base and HEAD revision range, or first compute the merge base before
invoking git diff-tree. Preserve the current name and diff-filter options so
only added, modified, copied, or renamed Markdown files are selected.
| for suppressed_file in "${SUPPRESSED_FILES[@]}"; do | ||
| if [[ "$FILE" == "$suppressed_file" ]]; then | ||
| echo "INFO: Skipping validation for suppressed file $FILE" | ||
| exit 0 | ||
| fi | ||
| # Use a wildcard match to check if the file path matches any of the suppressed files. This allows for more flexible matching. | ||
| if [[ "$FILE" == *"$suppressed_file"* ]]; then | ||
| echo "INFO: Skipping validation for suppressed file $FILE" | ||
| exit 0 | ||
| fi | ||
| # Check if the file is in a subdirectory of any suppressed file. This allows for suppressing entire directories of files if needed. | ||
| if [[ "$FILE" == */"$suppressed_file" || "$FILE" == */"$suppressed_file"/* ]]; then | ||
| echo "INFO: Skipping validation for suppressed file $FILE" | ||
| exit 0 | ||
| fi | ||
| done |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Overly permissive suppression matching risks silently skipping unrelated files.
The wildcard check [[ "$FILE" == *"$suppressed_file"* ]] (Line 62) is a strict superset of the exact-match check (Line 57) and the subdirectory check (Line 67-70), making those two dead code. Worse, unrestricted substring matching does not respect path boundaries: a suppressed entry like "CONTRIBUTING.md" would also match an unrelated file such as NOT_CONTRIBUTING.md or old/CONTRIBUTING.md.bak, since the suppressed string appears anywhere inside the actual filename. This silently skips validation for files that were never meant to be suppressed.
Replace the three checks with a single path-anchored match.
🛠️ Proposed fix
for suppressed_file in "${SUPPRESSED_FILES[@]}"; do
- if [[ "$FILE" == "$suppressed_file" ]]; then
- echo "INFO: Skipping validation for suppressed file $FILE"
- exit 0
- fi
- # Use a wildcard match to check if the file path matches any of the suppressed files. This allows for more flexible matching.
- if [[ "$FILE" == *"$suppressed_file"* ]]; then
- echo "INFO: Skipping validation for suppressed file $FILE"
- exit 0
- fi
- # Check if the file is in a subdirectory of any suppressed file. This allows for suppressing entire directories of files if needed.
- if [[ "$FILE" == */"$suppressed_file" || "$FILE" == */"$suppressed_file"/* ]]; then
+ if [[ "$FILE" == "$suppressed_file" || "$FILE" == */"$suppressed_file" || "$FILE" == "$suppressed_file"/* || "$FILE" == */"$suppressed_file"/* ]]; then
echo "INFO: Skipping validation for suppressed file $FILE"
exit 0
fi
done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for suppressed_file in "${SUPPRESSED_FILES[@]}"; do | |
| if [[ "$FILE" == "$suppressed_file" ]]; then | |
| echo "INFO: Skipping validation for suppressed file $FILE" | |
| exit 0 | |
| fi | |
| # Use a wildcard match to check if the file path matches any of the suppressed files. This allows for more flexible matching. | |
| if [[ "$FILE" == *"$suppressed_file"* ]]; then | |
| echo "INFO: Skipping validation for suppressed file $FILE" | |
| exit 0 | |
| fi | |
| # Check if the file is in a subdirectory of any suppressed file. This allows for suppressing entire directories of files if needed. | |
| if [[ "$FILE" == */"$suppressed_file" || "$FILE" == */"$suppressed_file"/* ]]; then | |
| echo "INFO: Skipping validation for suppressed file $FILE" | |
| exit 0 | |
| fi | |
| done | |
| for suppressed_file in "${SUPPRESSED_FILES[@]}"; do | |
| if [[ "$FILE" == "$suppressed_file" || "$FILE" == */"$suppressed_file" || "$FILE" == "$suppressed_file"/* || "$FILE" == */"$suppressed_file"/* ]]; then | |
| echo "INFO: Skipping validation for suppressed file $FILE" | |
| exit 0 | |
| fi | |
| done |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/test-config.sh` around lines 56 - 71, In the suppressed-file loop,
replace the three redundant conditions with one path-anchored match that accepts
only an exact path or a file beneath the suppressed entry as a directory.
Preserve the informational message and exit behavior, while ensuring names
embedded in unrelated filenames or extensions do not suppress validation.
| # Stage 1: Count how many examples exist for this language in the file | ||
| # We do this so we know we should have X examples to validate, and we can report if any are missing or fail validation or extraction. | ||
| EXAMPLE_COUNT=$("$SCRIPT_DIR/extract-config.sh" "$FILE" "$TAB_TITLE" "$LANGUAGE" count 2>/dev/null || echo 0) | ||
|
|
||
| if [ "$EXAMPLE_COUNT" -eq 0 ]; then | ||
| continue | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Genuine script errors are silently reported as "no examples found."
2>/dev/null || echo 0 (Line 98) discards extract-config.sh's stderr and treats any non-zero exit (missing gawk, permission errors, unexpected corruption) the same as a legitimate zero-count result. When EXAMPLE_COUNT is 0, the loop continues (Line 100-102) without validating anything, and the file is silently treated as if it has no examples, rather than causing the overall check to fail loudly.
🛠️ Proposed fix
- EXAMPLE_COUNT=$("$SCRIPT_DIR/extract-config.sh" "$FILE" "$TAB_TITLE" "$LANGUAGE" count 2>/dev/null || echo 0)
+ if ! EXAMPLE_COUNT=$("$SCRIPT_DIR/extract-config.sh" "$FILE" "$TAB_TITLE" "$LANGUAGE" count 2>&1); then
+ echo "ERROR: Failed to count $LANGUAGE examples in $FILE: $EXAMPLE_COUNT" >&2
+ FAILED_VALIDATIONS+=("$LANGUAGE count")
+ continue
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Stage 1: Count how many examples exist for this language in the file | |
| # We do this so we know we should have X examples to validate, and we can report if any are missing or fail validation or extraction. | |
| EXAMPLE_COUNT=$("$SCRIPT_DIR/extract-config.sh" "$FILE" "$TAB_TITLE" "$LANGUAGE" count 2>/dev/null || echo 0) | |
| if [ "$EXAMPLE_COUNT" -eq 0 ]; then | |
| continue | |
| fi | |
| # Stage 1: Count how many examples exist for this language in the file | |
| # We do this so we know we should have X examples to validate, and we can report if any are missing or fail validation or extraction. | |
| if ! EXAMPLE_COUNT=$("$SCRIPT_DIR/extract-config.sh" "$FILE" "$TAB_TITLE" "$LANGUAGE" count 2>&1); then | |
| echo "ERROR: Failed to count $LANGUAGE examples in $FILE: $EXAMPLE_COUNT" >&2 | |
| FAILED_VALIDATIONS+=("$LANGUAGE count") | |
| continue | |
| fi | |
| if [ "$EXAMPLE_COUNT" -eq 0 ]; then | |
| continue | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/test-config.sh` around lines 96 - 102, Update the Stage 1 counting
flow around extract-config.sh so genuine command failures remain visible and
cause the validation to fail, rather than being converted into EXAMPLE_COUNT=0.
Preserve the legitimate zero-example path that continues without validation, but
capture and report the extraction error separately using the existing script
error-handling conventions.
Resolves #2459 by providing a simple AWK based approach to validating configuration examples:
--dry-runthe configuration and report success/failureA local script is provided that can be run for any file in the repo or all of them:
A workflow is provided to run this for any files changed in a PR so if an update is made to documentation it should check it is valid.
There is a basic suppression approach by file as there are some valid reasons for this:
execare not part of the container imageAs part of these changes we also found failures in existing files that were resolved.
Some tweaks were also required, e.g. parser definition must be in a separate file for legacy TOML config so it was updated to be a comment for the examples (which would be rejected anyway otherwise).
There are options to use something more complex like markdown-tree or similar to build an AST from the Markdown file to then pull out the bits we need but this may not work with the specific Gitbook format anyway and requires a whole load of extra dependencies.
Currently there is an upstream failure with certain plugins triggering a segmentation fault for
--dry-run: fluent/fluent-bit#12113This is resolved by plugins: fix dry-run segmentation faults fluent-bit#12114 so waiting on that to merge.
Summary by CodeRabbit
Documentation
Validation