-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/go: default to CGO_ENABLED=0 when CC not set and default compiler not found #47251
Comments
With this approach, does this mean that if one makes a local changes to package If so, this sounds bad, since it leads to programs being built that don't match the user's expectation, and the user has no way of knowing this is happening. |
Interesting. I think that at this point in the release cycle we have to roll back the fix for #40042. |
@nishanths If there are local changes to |
Rolling back the fix for #40042 would be unfortunate, since it can lead to subtle problems in other ways (for example, linking against cached packages that were affected by subsequently-fixed bugs, or benchmarking cgo calls that do not accurately reflect the current C compiler). As a middle ground, would it make sense to omit the (Then we could consider a more holistic fix, such as the one proposed in this issue, for Go 1.18.) |
@ianlancetaylor That may be the right thing to do, though @bcmills' suggestion sounds like a good alternative, keeping most of the benefit of that fix. We've been considering removing pre-compiled |
Skipping the compiler ID for standard library packages occurred to me also. Let's do that for 1.17. |
Change https://golang.org/cl/335409 mentions this issue: |
After some internal discussion, I'd like to withdraw this issue. Changing the default of Two issues motivated this:
|
No test because a real test requires installing two different compilers. For #40042 For #47251 Change-Id: Iefddd67830d242a119378b7ce20be481904806e4 Reviewed-on: https://go-review.googlesource.com/c/go/+/335409 Trust: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> Reviewed-by: Jay Conrod <[email protected]>
This was implemented after all in CL 450739. |
When we fixed #40042, we started including version information about the C compiler in cache action keys that use it. It looks like this:
This means that if the C compiler changes, Go packages in the cache and in
GOROOT/pkg
built with the old version will not be used.Consequently,
runtime/cgo
,net
,os/user
, and other packages instd
that use cgo by default are stale if the user's C compiler is different than the one used to build the Go binary distribution. It almost always is.If the user does not have a C compiler installed and has not explicitly set
CGO_ENABLED=0
,go build
will fail to build any package that depends on these packages, even if nothing else uses cgo. This is common when building inside a Docker container and in CI. I'm worried Go 1.17 will break a lot of builds without some mitigation. For example #47215 is caused by this.One possibility is to default
CGO_ENABLED
to0
ifCC
is not explicitly set, and the default C compiler (usuallygcc
) is not inPATH
.std
packages would be stale, but everything could be rebuilt without a C compiler.This may cause subtle changes in behavior. The cgo versions of
net
andos/user
use libc code to resolve DNS queries and find users. The pure Go versions don't use libc. Also, any package, even a pure Go package may use thecgo
build tag for conditional compilation, and that would change.This is a big behavioral change late in the release cycle, though I expect the diff will be small.
cc @rsc @ianlancetaylor @bcmills @matloob
The text was updated successfully, but these errors were encountered: