-
Notifications
You must be signed in to change notification settings - Fork 9.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove three redundant error-related dependencies #34794
Conversation
This functionality is now provided by the "errors" package in the standard library, so the HashiCorp-specific library is obsolete. This also removed our one direct use of github.com/pkg/errors, which we were using in a way that was totally redundant with the stdlib package errors. However, it lives on as an indirect dependency through the Consul backend's dependency on the Consul SDK, which uses that library.
HashiCorp needed support for wrapping multiple errors into a single error and for wrapping one error inside another long before that was in the standard library, and so built its own libraries for those needs. However, the standard library now has its own answers for both and so the HashiCorp libraries are redundant. We've already eliminated all use of both elsewhere in Terraform (except for some legacy packages that predate tfdiags anyway), so we no longer need to support them in package tfdiags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from the perspective of the gcs
backend. Thanks for cleaning this up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
S3 backend changes LGTM 👍
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
In the early days the Go standard library lacked various useful helpers for wrapping and combining errors and so various parties, including HashiCorp itself, wrote separate libraries to fill those gaps.
However, recent improvements to the standard library have made those all redundant. There's now sufficient functionality in the standard library to support all of Terraform's error wrapping and combining needs.
This therefore removes all of the uses of the following three modules from our non-legacy packages:
github.com/pkg/errors
github.com/hashicorp/go-multierror
github.com/hashicorp/errwrap
The latter two still have callers in the packages under our
legacy
directory, but we'll hopefully eventually take care of those with a combination of #34793 and #34772.The first has no remaining callers in this repository, but unfortunately it's still used by the Consul SDK that we depend on for the
consul
remote state backend. Hopefully a future version of that will switch to using the standard library functions instead too, if that hasn't already happened.This is motivated just by minimizing the number of external dependencies Terraform has. We prefer to use standard library functionality instead of third-party libraries whenever possible.
As a bonus, this also fills two prior gaps in the unit testing in
package tfdiags
.