Merged
Conversation
Contributor
Author
|
This change is part of the following stack: Change managed by git-spice. |
|
Is this PR ready to merge? |
prashantv
force-pushed
the
prashant/extract-tracepaths
branch
from
84f304c to
7c72e04
Compare
Fixes #91. Add ability to rewrite packages that don't import errtrace to be rewritten to wrap errors automatically. This would normally cause errors when trying to build the package as we're adding a new dependency to the package, which toolexec doesn't support: golang/go#35204 To workaround this, we use the hack mentioned in the above issue using `go:linkname` to "import" errtrace functionality in a way that doesn't break the package build. Using `go:linkname` requires importing `unsafe`, to the rewriter is updated to: * import the "unsafe" package instead of errtrace, which can be added as a new dependency without impacting the build as it's stdlib. * Add `go:linkname` to import errtrace functions into a package. * Rewrite to use the `go:linkname` errtrace identifiers instead of the errtrace package. `go:linkname` is not compatible with generic functions like `WrapN` so disable `WrapN` when using toolexec. `WrapN` is for `errtrace -w` where source files are directly modified, and formatted with `gofmt` where `WrapN` minimizes rewriting. This isn't necessary for toolexec mode. Unsafe mode still requires one package that's part of the final linked object references `errtrace` somewhere in the binary -- otherwise, the build fails. This seems like a reasonable limitation since users need to manage the dependency.
prashantv
force-pushed
the
prashant/toolexec-unsafe
branch
from
5cd7bdf to
878b94f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #91.
Add ability to rewrite packages that don't import errtrace to be
rewritten to wrap errors automatically. This would normally cause
errors when trying to build the package as we're adding a new
dependency to the package, which toolexec doesn't support:
golang/go#35204
To workaround this, we use the hack mentioned in the above issue
using
go:linknameto "import" errtrace functionality in a waythat doesn't break the package build.
Using
go:linknamerequires importingunsafe, to the rewriteris updated to:
as a new dependency without impacting the build as it's stdlib.
go:linknameto import errtrace functions into a package.go:linknameerrtrace identifiers instead ofthe errtrace package.
go:linknameis not compatible with generic functions likeWrapNsodisable
WrapNwhen using toolexec.WrapNis forerrtrace -wwheresource files are directly modified, and formatted with
gofmtwhereWrapNminimizes rewriting. This isn't necessary for toolexec mode.Unsafe mode still requires one package that's part of the final linked
object references
errtracesomewhere in the binary -- otherwise, thebuild fails. This seems like a reasonable limitation since users need to
manage the dependency.