Skip to content

fix(tool): preserve filenames in changed chart detection - #885

Merged
cpanato merged 1 commit into
helm:mainfrom
ViktorBystrov72:fix/preserve-changed-chart-paths
Nov 10, 2026
Merged

cpanato merged 1 commit into
helm:mainfrom
ViktorBystrov72:fix/preserve-changed-chart-paths

Conversation

@ViktorBystrov72

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Changed-chart detection consumes newline-separated git diff --name-only output as literal paths. Git quotes Unicode and control characters by default, so paths such as charts/café/values.yaml no longer name files in the checkout and can be skipped. The output helper also trims leading whitespace from the first path.

Request NUL-delimited paths and read stdout without trimming it. Split only on NUL boundaries, preserving Unicode, quotes, tabs, newlines and leading spaces in filenames. Empty diffs and command errors keep their existing behavior.

Special notes for your reviewer:

The regression test uses a temporary Git repository with core.quotepath=true and compares against the empty tree. On unmodified main, six of the eight fixture paths are corrupted; with the fix the returned list matches all filenames exactly. It also covers an empty diff and an invalid revision. The fixture creates no commits.

Validation with Go 1.27.3 and golangci-lint 2.14.0:

go test ./... -count=1
golangci-lint run ./...
git diff --check

All passed.

Read NUL-delimited git diff output without trimming whitespace or mixing stderr into paths. Cover Unicode, whitespace and quoted filenames in a temporary Git repository.

Signed-off-by: Виктор Быстров <anatema@bk.ru>

@cpanato cpanato left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@cpanato
cpanato merged commit e6ce62b into helm:main Nov 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants