opa
opa copied to clipboard
Consider dropping use of github.com/pkg/errors package
Golang 1.13 introduced new features to the errors package that largely match the capabilities provided by the github.com/pkg/errors package. It would be good to remove this unnecessary dependency to a thirdparty library.
There was some discussion around requiring go 1.13 in OPA over in https://github.com/open-policy-agent/opa/pull/1969#discussion_r382850242
As-is 1.13 crept in as a requirement to cmd
and tester
, if we make changes to use the newer functionality it would become much more widely spread (probably everything).
Hey, @patrick-east I can take this up. Do you have any suggestions? I am thinking to replace github.com/pkg/errors package with errors standard package from go
Most of our use cases with github.com/pkg/errors
are around errors.wrap()
which isn't offered by standard errors package from go. But the official go docs say to use fmt.Errorf()
instead of wrap()
. which does a similar job.
@patrick-east if we are okay with dropping github.com/pkg/errors
I can take this up.
golang 1.13 is only 8 months old, though. Not sure if we should give it a bit more time before we require 1.13+ for OPA.
Lets bring it up as a topic for the next OPA community meeting, for now lets hold off on making any changes.
👍 I agree with @patrick-east here. We need to figure out what packages are impacted and then make the call. If it impacts the rego package (or any of it's dependencies), we'll probably want to hold for a while. EDIT: Of course we can also look at usages and see if they'd be easy to port to just use fmt.Errorf or similar without breaking compatibility in a significant way.
Of all the vendored dependencies, it looks like at the moment github.com/yashtewari/glob-intersection is the only one with a dependency to pkg/errors. I prepared a PR to fix that: https://github.com/yashtewari/glob-intersection/pull/2 After that, it's only a few internal uses of the errors package we have left. They can be removed as we already have parts of the codebase dependning on the 1.13 "%w" formatting.
ℹ️ This also blocks using TinyGo with the code base, due to its use of the runtime
stdlib package.
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
github.com/yashtewari/glob-intersection has removed its dependency to pkg/errors.
thanks for the heads-up. I'll see if dependabot picks it up or if it's time for a manual update.
I'm afraid with github.com/yashtewari/glob-intersection gone, we've still got more third party stuff using github.com/pkg/errors since this issue was opened: github.com/dgraph-io/{badger,ristretto}
But I think getting rid of this is still worthwhile; if we'd desire to make parts of OPA (notably rego|ast|topdown) buildable with tinygo (compiling to wasm), we wouldn't want to build the storage/disk
pieces anyways.
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
Even ignoring vendored dependencies, there are quite a lot of direct dependencies on pkg/errors in the codebase:
$ git grep -l github.com/pkg/errors | grep \.go$ | grep -v vendor
ast/location/location.go
ast/parser.go
ast/parser_ext.go
ast/term.go
bundle/bundle.go
bundle/bundle_test.go
bundle/file.go
bundle/verify.go
cmd/deps.go
cmd/sign.go
compile/compile.go
download/download.go
internal/compiler/wasm/wasm.go
internal/jwx/buffer/buffer.go
internal/jwx/jwa/key_type.go
internal/jwx/jwa/signature.go
internal/jwx/jwk/ecdsa.go
internal/jwx/jwk/headers.go
internal/jwx/jwk/jwk.go
internal/jwx/jwk/key_ops.go
internal/jwx/jwk/rsa.go
internal/jwx/jwk/symmetric.go
internal/jwx/jws/headers.go
internal/jwx/jws/jws.go
internal/jwx/jws/sign/ecdsa.go
internal/jwx/jws/sign/hmac.go
internal/jwx/jws/sign/rsa.go
internal/jwx/jws/sign/sign.go
internal/jwx/jws/verify/ecdsa.go
internal/jwx/jws/verify/hmac.go
internal/jwx/jws/verify/rsa.go
internal/jwx/jws/verify/verify.go
internal/runtime/init/init.go
internal/wasm/encoding/reader.go
plugins/bundle/status.go
plugins/status/plugin.go
server/server.go
server/server_test.go
test/wasm/cmd/wasm-rego-testgen/main.go
topdown/tokens.go
If there's interest in cleaning these up I can send a PR.
@imjasonh Thanks a bunch! I've got the (not so) secret mission of running the compiler and maybe even the evaluator in Wasm via tinygo; removing this package from our code will go a long way towards that. The compiler should have little external dependencies; the evaluator might have a few (due to builtin functions... of which we could also not support a few when running under Wasm)
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.