Cut xtrace noise from POSIX-ownership diagnostic steps#2156
Merged
EliahKagan merged 1 commit intoMay 23, 2026
Merged
Conversation
The "Show POSIX file ownership" step in each test workflow looped over a hard-coded path list with one `ls -ld` per iteration. Bash's xtrace -- active throughout (from `~/.bash_profile` on Cygwin and from the `-x` flag in GHA's default shell line on Ubuntu / macOS / Alpine) -- reprints the entire `for` expression's expanded word list at the start of every iteration. For nine paths that's nine long traces of the same word list, drowning out the `ls -ld` output we want to read. Collapse the loop into a single multi-arg `ls -ld --`: xtrace prints the expanded command line once, `ls` produces one line per existing path and a `ls: cannot access '<path>': No such file or directory` line per missing path. `2>&1` merges those missing-path messages into the log stream alongside the existing-path output; `|| true` keeps the step from failing under `set -e` when any path is missing. The format of missing-path reporting changes from `(missing: <path>)` to `ls: cannot access '<path>': No such file or directory`. Both convey the same information; the new form is slightly more verbose per missing path but eliminates the per-iteration trace reprint that dominated the original output. Cosmetic-only; no change to the diagnostic information surfaced. Flagged on PR gitpython-developers#2154 as a follow-up: gitpython-developers#2154 (review) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces CI log noise from set -x tracing in the POSIX-ownership diagnostic steps by replacing per-path loops with a single ls -ld invocation that lists all relevant paths at once (while still keeping the step non-fatal if some paths are missing).
Changes:
- Replace
for p in ...; do ls ...; doneloops with a single multi-argls -ldcall in POSIX ownership diagnostics. - Preserve non-failing behavior for missing paths by keeping the command tolerant of
lsnonzero exit status.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .github/workflows/pythonpackage.yml | Simplifies POSIX ownership diagnostic output to reduce xtrace verbosity. |
| .github/workflows/cygwin-test.yml | Simplifies POSIX ownership diagnostic output in the Cygwin job to reduce xtrace verbosity. |
| .github/workflows/alpine-test.yml | Simplifies POSIX ownership diagnostic output in the Alpine container job to reduce xtrace verbosity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
In #2154 (review) I noted that the new POSIX ownersip diagnostics are hard to read, but I wasn't sure of the best way to fix it. The solution turns out to be simple and doesn't require
set +x.The changes here, and commit message, are generated by Claude, as noted in the trailer. I've reviewed the changes and checked the output.