fix: replace "github.com/pkg/errors" with stdlib "errors" package #13460

justenstall · 2024-11-18T17:57:03Z

What this PR does / why we need it:

Replaces the archived "github.com/pkg/errors" package with the Go standard library equivalents, and updates error handling to use more recent standard library features.

Special notes for your reviewer:

Updates are generally:

Replace errors.Wrap(err, "msg") and errors.Wrapf(err, "msg") with fmt.Errorf("msg: %w", err)
Replace errors.Errorf("msg", args) with fmt.Errorf("msg", args)
Replace uses of []string and strings.Join for error messages with []error and errors.Join
- Used a simple wrapper type (example) if the error messages were being joined with any string other than "\n" so they will render the same as before
Replace os.IsNotExist(err) with errors.Is(err, fs.ErrNotExist)

If applicable:

this PR contains user facing changes (the docs needed label should be applied if so)
this PR contains unit tests
this PR has been tested for backwards compatibility

Signed-off-by: Justen Stall <[email protected]>

- replace os.IsNotExist with errors.Is and fs.ErrNotExist - use %w directive Signed-off-by: Justen Stall <[email protected]>

Signed-off-by: Justen Stall <[email protected]>

gjenkins8

Can we address the if s.showResources && s.release.Info.Resources != nil change

Otherwise most comments fairly minor.

Thanks for the effort here!

cmd/helm/status.go

gjenkins8 · 2024-11-19T02:15:28Z

cmd/helm/dependency_update_test.go

 	// Make sure tmpcharts-x is deleted
 	tmpPath := filepath.Join(dir(chartname), fmt.Sprintf("tmpcharts-%d", os.Getpid()))
-	if _, err := os.Stat(tmpPath); !os.IsNotExist(err) {
+	if _, err := os.Stat(tmpPath); !errors.Is(err, fs.ErrNotExist) {


👍 details (for this substitution and others):

This function predates errors.Is. It only supports errors returned by the os package. New code should use errors.Is(err, fs.ErrNotExist).

https://pkg.go.dev/os#IsNotExist

internal/third_party/dep/fs/fs.go

pkg/action/hooks.go

pkg/kube/wait.go

pkg/registry/util.go

gjenkins8 · 2024-11-19T02:33:31Z

pkg/storage/driver/secrets_test.go


 	// fetch the deleted release
 	_, err = secrets.Get(key)
-	if !reflect.DeepEqual(ErrReleaseNotFound, err) {


I guess strictly, reflect.DeepEqual does more than errors.Is. But should be equivalent in this case

Lmk if I should revert

gjenkins8 · 2024-11-19T02:36:28Z

(kicked of CI, will find out if any tests rely on error formatting)

Signed-off-by: Justen Stall <[email protected]>

Co-authored-by: George Jenkins <[email protected]> Signed-off-by: Justen Stall <[email protected]>

Signed-off-by: Justen Stall <[email protected]>

gjenkins8 · 2024-11-19T04:20:47Z

(Marked as Needs v3 backport for backport back to dev-v3 branch, as this commit touches enough files that it merge conflicts will likely eventuate when backporting other changes)

justenstall · 2024-11-19T14:35:36Z

(Marked as needs v3 fix for backport back to dev-v3 branch, as this commit touches enough files that it merge conflicts will likely eventuate when backporting other changes)

@gjenkins8 Does this mean I should rebase this on the dev-v3 branch?

gjenkins8 · 2024-11-19T20:17:27Z

(Marked as needs v3 fix for backport back to dev-v3 branch, as this commit touches enough files that it merge conflicts will likely eventuate when backporting other changes)

@gjenkins8 Does this mean I should rebase this on the dev-v3 branch?

No, please (continue to) target the main branch. We would cherry-pick these commits to dev-v3 branch to backport to Helm v3. This avoids e.g. bugfixes, refactors like this being applied to Helm 3 only. And being "forgotten" to be merged into Helm 4.

justenstall · 2024-11-25T16:16:50Z

@gjenkins8 Is there anything I can do to help push this along?

robertsirc · 2025-04-18T19:52:16Z

Hello, thank you for your PR, this branch does look like it has some conflicts if you could please address and we can work on getting this in.

scottrigby · 2025-04-21T15:19:24Z

Hello, thank you for your PR, this branch does look like it has some conflicts if you could please address and we can work on getting this in.

@justenstall I saw you responded 👍 to my request about this in #helm-dev Slack channel. Feel free to ping there when this is complete for a faster response

Signed-off-by: Justen Stall <[email protected]>

justenstall · 2025-04-21T16:56:30Z

@scottrigby I opened a child PR with fixes for the lint issues, let me know if they are acceptable and I will add them to this PR: justenstall#1

scottrigby · 2025-04-21T19:25:01Z

@scottrigby I opened a child PR with fixes for the lint issues, let me know if they are acceptable and I will add them to this PR: justenstall#1

Yes that looks great. Thanks for fixing everywhere 👍

Fix lint issues

Signed-off-by: Justen Stall <[email protected]>

scottrigby

This PR looks great, apart from one question.

pkg/kube/client.go

Signed-off-by: Justen Stall <[email protected]>

scottrigby

@justenstall thanks for fixing that.

This PR also needs another rebase again. This is because we just merged another PR that modified an overlapping error—that one was focused on removing a different package and was worth merging separately because it added a golangci-lint depguard rule scoped for that package.

Approving this PR in advance of resolving that new small conflict.

Question: I'm thinking it would be good to add a similar rule for "github.com/pkg/errors" (see #30781 (review)). Are you in the place to add that to this PR as well? If not, that's ok we could do as a fast-follow to your PR, please just let us know.

This PR is ready for another maintainer review either way.

Signed-off-by: Justen Stall <[email protected]>

justenstall · 2025-04-22T17:33:49Z

Question: I'm thinking it would be good to add a similar rule for "github.com/pkg/errors" (see #30781 (review)). Are you in the place to add that to this PR as well? If not, that's ok we could do as a fast-follow to your PR, please just let us know.

That's a good idea, I'll add it here.

Signed-off-by: Justen Stall <[email protected]>

scottrigby

Nice work. LGTM 👍

The package github.com/pkg/errors was removed via the pull request helm#13460. This change did not correctly handle the case in the windows code and CI did not exercise this to find the error. Signed-off-by: Matt Farina <[email protected]>

justenstall added 4 commits November 18, 2024 11:35

fix: replace "github.com/pkg/errors" with stdlib "errors" package

63cf42a

Signed-off-by: Justen Stall <[email protected]>

fix incorrect error return

b0944e8

Signed-off-by: Justen Stall <[email protected]>

more error wrapping uses

6aa19b8

- replace os.IsNotExist with errors.Is and fs.ErrNotExist - use %w directive Signed-off-by: Justen Stall <[email protected]>

restore error check in secrets.go

62b5bdc

Signed-off-by: Justen Stall <[email protected]>

pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 18, 2024

justenstall mentioned this pull request Nov 18, 2024

fix: replace "github.com/pkg/errors" with stdlib "errors" package #13456

Closed

3 tasks

gjenkins8 requested changes Nov 19, 2024

View reviewed changes

justenstall and others added 3 commits November 18, 2024 22:28

restore --show-resources check in cmd/helm/status.go

cff32ff

Signed-off-by: Justen Stall <[email protected]>

Update internal/third_party/dep/fs/fs.go

8549a25

Co-authored-by: George Jenkins <[email protected]> Signed-off-by: Justen Stall <[email protected]>

revert duplicate slice conditions

7df6902

Signed-off-by: Justen Stall <[email protected]>

gjenkins8 previously approved these changes Nov 19, 2024

View reviewed changes

gjenkins8 added Has One Approval This PR has one approval. It still needs a second approval to be merged. Needs v3 backport Label PRs for v4/main, which are still applicable to v3 so need a separate backport PR labels Nov 19, 2024

AustinAbro321 mentioned this pull request Dec 19, 2024

use "errors" package over deprecated "github/pkg/errors" #13543

Closed

3 tasks

gjenkins8 removed the Needs v3 backport Label PRs for v4/main, which are still applicable to v3 so need a separate backport PR label Jan 5, 2025

justenstall added 2 commits April 21, 2025 12:13

Merge branch 'main' into stdlib-errors-2

280a9dd

Signed-off-by: Justen Stall <[email protected]>

updates after merge conflict resolution

065e2eb

Signed-off-by: Justen Stall <[email protected]>

justenstall dismissed gjenkins8’s stale review via 065e2eb April 21, 2025 16:17

fix golangci-lint issues

3877ec9

Signed-off-by: Justen Stall <[email protected]>

justenstall added 2 commits April 21, 2025 15:28

Merge pull request #1 from justenstall/stdlib-errors-2-lint-issues

6c3fc1d

Fix lint issues

update expected error message in install test

7a316c8

Signed-off-by: Justen Stall <[email protected]>

justenstall added 2 commits April 21, 2025 16:04

update another test output

4a6092b

Signed-off-by: Justen Stall <[email protected]>

one more test output

73545f9

Signed-off-by: Justen Stall <[email protected]>

scottrigby added the refactor label Apr 21, 2025

scottrigby reviewed Apr 21, 2025

View reviewed changes

pkg/kube/client.go Outdated Show resolved Hide resolved

remove WaitAndGetCompletedPodPhase function

fc6c5e5

Signed-off-by: Justen Stall <[email protected]>

scottrigby mentioned this pull request Apr 22, 2025

chore: remove github.com/hashicorp/go-multierror dependency #30781

Merged

3 tasks

scottrigby previously approved these changes Apr 22, 2025

View reviewed changes

Merge branch 'main' into stdlib-errors-2

18aae5a

Signed-off-by: Justen Stall <[email protected]>

justenstall dismissed scottrigby’s stale review via 18aae5a April 22, 2025 17:24

chore: add depguard rule for github.com/pkg/errors

700103d

Signed-off-by: Justen Stall <[email protected]>

scottrigby added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2025

Merge branch 'main' into stdlib-errors-2

4905a61

Signed-off-by: Justen Stall <[email protected]>

scottrigby removed Has One Approval This PR has one approval. It still needs a second approval to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 23, 2025

scottrigby approved these changes Apr 23, 2025

View reviewed changes

scottrigby added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Apr 23, 2025

gjenkins8 approved these changes Apr 23, 2025

View reviewed changes

gjenkins8 merged commit ecf1730 into helm:main Apr 23, 2025
5 checks passed

mattfarina mentioned this pull request Apr 25, 2025

Fixing windows build #30803

Merged

3 tasks

hosmir mentioned this pull request May 8, 2025

Deprecate github.com/pkg/errors in favor of stdlib #11994

Closed

scottrigby mentioned this pull request Aug 22, 2025

replace hashicorp multierror with core errors #12298

Closed

3 tasks

fix: replace "github.com/pkg/errors" with stdlib "errors" package #13460

fix: replace "github.com/pkg/errors" with stdlib "errors" package #13460

Uh oh!

Conversation

justenstall commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gjenkins8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gjenkins8 Nov 19, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gjenkins8 Nov 19, 2024

Choose a reason for hiding this comment

Uh oh!

justenstall Nov 19, 2024

Choose a reason for hiding this comment

Uh oh!

gjenkins8 commented Nov 19, 2024

Uh oh!

gjenkins8 commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justenstall commented Nov 19, 2024

Uh oh!

gjenkins8 commented Nov 19, 2024

Uh oh!

justenstall commented Nov 25, 2024

Uh oh!

robertsirc commented Apr 18, 2025

Uh oh!

scottrigby commented Apr 21, 2025

Uh oh!

justenstall commented Apr 21, 2025

Uh oh!

scottrigby commented Apr 21, 2025

Uh oh!

scottrigby left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scottrigby left a comment

Choose a reason for hiding this comment

Uh oh!

justenstall commented Apr 22, 2025

Uh oh!

scottrigby left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

justenstall commented Nov 18, 2024 •

edited

Loading

gjenkins8 commented Nov 19, 2024 •

edited

Loading