crypto: fix build when CGO_ENABLED=0 #19121
Merged
Conversation
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
Package crypto works with or without cgo, which is great. However, to make it work without cgo required setting the build tag `nocgo`. It's common to disable cgo by instead just setting the environment variable `CGO_ENABLED=0`. Setting this environment variable does _not_ implicitly set the build tag `nocgo`. So projects that try to build the crypto package with `CGO_ENABLED=0` will fail. I have done this myself several times. Until today, I had just assumed that this meant that this package requires cgo. But a small build tag change will make this case work. Instead of using `nocgo` and `!nocgo`, we can use `!cgo` and `cgo`, respectively. The `cgo` build tag is automatically set if cgo is enabled, and unset if it is disabled. You can test this change by running: CGO_ENABLED=0 go test ./crypto Before this change, that test would fail. After this change, it passes.
Member
|
The problem is that there is no stable pure Go implementation of secp256k1. @whyrusleeping tried to use the same one we have for no-cgo at some point in time, even fixed a few issues, but I think they gave up in in end. As long as it's not stable, I don't think it's a good idea to make it simple for people to use it. It's mostly there for some gopherjs testing AFAIK. Ping @fjl. |
Contributor
|
I disagree. The nocgo implementation of secp256k1 is fine and passes all consensus tests. It's slower than libsecp256k1, but that's expected. |
Contributor
|
The btcsuite authors addressed the remaining incompatibilities in May 2018 (test cases added here). |
Member
|
@fjl Your call. |
fjl
approved these changes
Feb 19, 2019
|
Yeah, i think we were using a version before the may 2018 fixes. |
19 tasks
gzliudan
added a commit
to gzliudan/XDPoSChain
that referenced
this pull request
Dec 9, 2024
Package crypto works with or without cgo, which is great. However, to make it work without cgo required setting the build tag `nocgo`. It's common to disable cgo by instead just setting the environment variable `CGO_ENABLED=0`. Setting this environment variable does _not_ implicitly set the build tag `nocgo`. So projects that try to build the crypto package with `CGO_ENABLED=0` will fail. I have done this myself several times. Until today, I had just assumed that this meant that this package requires cgo. But a small build tag change will make this case work. Instead of using `nocgo` and `!nocgo`, we can use `!cgo` and `cgo`, respectively. The `cgo` build tag is automatically set if cgo is enabled, and unset if it is disabled.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Package crypto works with or without cgo, which is great. However, to make it
work without cgo required setting the build tag
nocgo. It's common to disablecgo by instead just setting the environment variable
CGO_ENABLED=0. Settingthis environment variable does not implicitly set the build tag
nocgo. Soprojects that try to build the crypto package with
CGO_ENABLED=0will fail. Ihave done this myself several times. Until today, I had just assumed that this
meant that this package requires cgo.
But a small build tag change will make this case work. Instead of using
nocgoand
!nocgo, we can use!cgoandcgo, respectively. Thecgobuild tag isautomatically set if cgo is enabled, and unset if it is disabled.
You can test this change by running:
Before this change, that test would fail. After this change, it passes.