From 5155ea56879cde2ccfd6a351326073a8ddc54b6b Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sat, 18 Apr 2026 07:32:38 +0800 Subject: [PATCH] fix: address PR review comments for rewriteChartDependencies - Handle non-NotExist errors from st.fs.Stat to surface permission/IO failures - Reword function doc to clarify temp copy is conditional on rewrite being needed - Assert rewrittenPath vs tempDir based on expectModified in test table Signed-off-by: yxxhero --- pkg/state/chart_dependencies_rewrite_test.go | 10 ++++++++++ pkg/state/state.go | 11 +++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pkg/state/chart_dependencies_rewrite_test.go b/pkg/state/chart_dependencies_rewrite_test.go index 343b3dc2..d185b650 100644 --- a/pkg/state/chart_dependencies_rewrite_test.go +++ b/pkg/state/chart_dependencies_rewrite_test.go @@ -209,6 +209,16 @@ extra-field: aaa return } + if tt.expectModified { + if rewrittenPath == tempDir { + t.Errorf("expected rewrittenPath != tempDir when modifications are needed, got same path") + } + } else { + if rewrittenPath != tempDir { + t.Errorf("expected rewrittenPath == tempDir when no modifications are needed, got %q", rewrittenPath) + } + } + if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/state/state.go b/pkg/state/state.go index 06ab0bf6..f51a0dd1 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1410,15 +1410,18 @@ type PrepareChartKey struct { // rewriteChartDependencies rewrites relative file:// dependencies in Chart.yaml to absolute paths // to ensure they can be resolved from chartify's temporary directory. // Instead of modifying the original chart in-place (which causes race conditions when multiple -// releases reference the same local chart), it copies the chart to a temporary directory and -// rewrites the copy. Each call creates a fresh temp copy reflecting the current chart contents, -// so prepare hooks or other steps that mutate the local chart directory are always honored. -// The returned cleanup function removes the temporary directory when called. +// releases reference the same local chart), it creates a temporary copy only when a rewrite is +// needed and rewrites that copy. When a temp copy is created, it reflects the current chart +// contents so prepare hooks or other steps that mutate the local chart directory are honored. +// The returned cleanup function removes the temporary directory when one was created and is +// otherwise a no-op. func (st *HelmState) rewriteChartDependencies(chartPath string) (string, func(), error) { chartYamlPath := filepath.Join(chartPath, "Chart.yaml") if _, err := st.fs.Stat(chartYamlPath); os.IsNotExist(err) { return chartPath, func() {}, nil + } else if err != nil { + return chartPath, func() {}, err } data, err := st.fs.ReadFile(chartYamlPath)