Skip to content
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

crypto/x509: build tag "ios" broken at tip on darwin/amd64 #38710

Open
bradfitz opened this issue Apr 27, 2020 · 6 comments
Open

crypto/x509: build tag "ios" broken at tip on darwin/amd64 #38710

bradfitz opened this issue Apr 27, 2020 · 6 comments

Comments

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Apr 27, 2020

In previous Go releases, the "ios" build tag with darwin/amd64 produced working binaries:

barloga5k:x509 $ go1.14.2 download
...
Downloaded 100.0% (125040726 / 125040726 bytes)
Unpacking /Users/bradfitz/sdk/go1.14.2/go1.14.2.darwin-amd64.tar.gz ...
Success. You may now run 'go1.14.2'
barloga5k:x509 $ GOARCH=amd64 go1.14.2 install --tags=ios crypto/x509
barloga5k:x509 $

(Such a configuration is used when running under the iOS simulator in Xcode)

But at Go tip:

barloga5k:x509 $ go version
go version devel +a7e9e84716 Mon Apr 27 20:20:53 2020 +0000 darwin/amd64
barloga5k:x509 $ go install --tags=ios crypto/x509
# crypto/x509
./cert_pool.go:65:9: undefined: loadSystemRoots
./root.go:21:32: undefined: loadSystemRoots

Looks like https://go-review.googlesource.com/c/go/+/227197 (da8591b, @aclements) dropped the // +build arm arm64 ios disjunction.

Labeling release-blocker in that it's a regression.

@bradfitz bradfitz added this to the Go1.15 milestone Apr 27, 2020
bradfitz added a commit to tailscale/go that referenced this issue Apr 27, 2020
See golang#38710

This fixes our iOS simulator build.
@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Apr 27, 2020

The ios build tag was only used in crypto/x509, and not documented anywhere I could find. Is "like iOS but on amd64" a supported configuration? Is crypto/x509 really the only thing that needs to support it?

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Apr 27, 2020

I have no clue about the history other than it used to work and I stumbled into a project that was depending on it.

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Apr 27, 2020

(fwiw, I was also surprised it ever worked)

@aclements
Copy link
Member

@aclements aclements commented Apr 28, 2020

Probably we just need to drop the !ios build constraint from root_cgo_darwin.go? I guess previously that was meant to be exclusive with root_darwin_armx.go, but I only dropped the positive build tag. That would just make it ignore the ios build tag.

@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Apr 28, 2020

@aclements, that might not work because cgo might be disabled in such an environment.

What I ended up doing for our build was renaming those files from "arm" to "embed" (in tailscale@1f0b2d1) and making the build tags be:

// +build !x509omitbundledroots
// +build darwin
// +build arm64 ios
@aclements
Copy link
Member

@aclements aclements commented May 7, 2020

@FiloSottile , what do you think is the right fix here? It should be easy to fix, I'm just not sure what the right way to do it is. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.