Skip to content

go1.9: net/http is broken on macOS due to new "os/user" import. #664

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.

Sign up for GitHub

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

Jump to bottom
Closed
dmitshur opened this issue Jul 27, 2017 · 1 comment
Closed

go1.9: net/http is broken on macOS due to new "os/user" import. #664

dmitshur opened this issue Jul 27, 2017 · 1 comment
Labels
bug

Comments

@dmitshur
Copy link
Member

This is a go1.9 specific issue. Creating an issue so I can elaborate on the details. I have a fix that I'll apply to go1.9 branch soon.

In Go 1.9, the crypto/x509 package, on macOS, now imports os/user package:

https://github.com/golang/go/blob/go1.9rc1/src/crypto/x509/root_darwin.go#L19

It didn't do so in Go 1.8:

https://github.com/golang/go/blob/go1.8.3/src/crypto/x509/root_darwin.go#L18

This causes GopherJS on the go1.9 branch at commit cca79e8 to fail when building net/http with:

$ gopherjs test net/http
os/user: importing "C" is not supported by GopherJS

We don't run net/http tests in CI right now, because even though the HTTP client works, there are too many tests in HTTP that rely on the server also working, which isn't supported in GopherJS. So we'd have to disable many tests. Maybe we should add a gopherjs build net/http line instead, to ensure that the package at least builds.

However, that's not the reason CI didn't catch it. The real reason is that this is a macOS-specific issue, it doesn't occur on Linux, because on Linux, crypto/x509 still doesn't import os/user package. The import is in root_darwin.go, which is constrained to GOOS=darwin because of _darwin suffix in filename.

This issue demonstrates a pretty significant limitation of the current compiler/natives mechanism to override standard library packages. We're able to override individual identifiers, but if the package happens to import some (no longer needed) packages, that can't be stopped. /cc @neelance However, dealing with that is out of scope and can be considered later.

For now, my fix will be to get os/user package to build without errors.
@dmitshur dmitshur added the bug label Jul 27, 2017
dmitshur added a commit that referenced this issue Jul 27, 2017
@dmitshur
build: Don't use cgo for "os/user", "crypto/x509".
e9fdce4 
This changes fixes GopherJS ability to build os/user package. In turn,
this change also fixes crypto/x509 and net/http on darwin. The goal is
to ensure that os/user builds without errors, because it may end up
being imported by some stdlib packages (e.g., crypto/x509 on darwin
in Go 1.9).

This is accomplished by disabling cgo for these standard library
packages. Normally, GopherJS build context enables cgo for the purpose
of catching when a user package uses cgo and reporting that:

	CgoEnabled: true, // detect `import "C"` to throw proper error
	...
	if len(pkg.CgoFiles) > 0 {
		return nil, &ImportCError{path}
	}

But we don't want to use cgo for standard library packages. They have
support for building without cgo by using !cgo build constraints, which
we want to activate. Setting bctxt.CgoEnabled to false achieves that.

Add to CI a test of os/user package for gopherjs, as well as ensure
net/http builds successfully. We can't run its tests because there
are too many and they require the HTTP server, but only client is
implemented in GopherJS.

Fixes #664.
@dmitshur
Copy link
Member Author

This Go 1.9-specific issue has been resolved by e9fdce4.

(The issue was just a place to post more information about what went wrong.)

dmitshur added a commit that referenced this issue Aug 25, 2017
@dmitshur
build: Don't use cgo for "os/user", "crypto/x509".
00a3767 
This changes fixes GopherJS ability to build os/user package. In turn,
this change also fixes crypto/x509 and net/http on darwin. The goal is
to ensure that os/user builds without errors, because it may end up
being imported by some stdlib packages (e.g., crypto/x509 on darwin
in Go 1.9).

This is accomplished by disabling cgo for these standard library
packages. Normally, GopherJS build context enables cgo for the purpose
of catching when a user package uses cgo and reporting that:

	CgoEnabled: true, // detect `import "C"` to throw proper error
	...
	if len(pkg.CgoFiles) > 0 {
		return nil, &ImportCError{path}
	}

But we don't want to use cgo for standard library packages. They have
support for building without cgo by using !cgo build constraints, which
we want to activate. Setting bctxt.CgoEnabled to false achieves that.

Add to CI a test of os/user package for gopherjs, as well as ensure
net/http builds successfully. We can't run its tests because there
are too many and they require the HTTP server, but only client is
implemented in GopherJS.

Fixes #664.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dmitshur