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

x/build: add test coverage for netgo and osusergo build tags #24845

Closed
bradfitz opened this issue Apr 13, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@bradfitz
Copy link
Member

commented Apr 13, 2018

In #24841 we learned that the osusergo build tag resulted in an os/user package that didn't even compile.

We should make sure we have some test or builder coverage for those build tags.

@gopherbot gopherbot added this to the Unreleased milestone Apr 13, 2018

@gopherbot gopherbot added the Builders label Apr 13, 2018

@gopherbot

This comment has been minimized.

Copy link

commented Apr 13, 2018

Change https://golang.org/cl/106837 mentions this issue: os/user: fix build with "osusergo" build tag

gopherbot pushed a commit that referenced this issue Apr 13, 2018

os/user: fix build with "osusergo" build tag
Fixes #24841
Updates #24845

Change-Id: I4a5c05f4cbf9692bd6cab48baf3cc51fa43fe5a9
Reviewed-on: https://go-review.googlesource.com/106837
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2018

The osusergo tag was added in 62f0127 (@kolyshkin)

And it turns out it never worked, even when it was first committed:

bradfitz@gdev:~/go/src$ git reset --hard 62f0127d8104d8266d9a3fb5a87e2f09ec8b6f5b
HEAD is now at 62f0127 os/user: add a way to enforce pure Go implementation

bradfitz@gdev:~/go/src$ ./make.bash 
Building Go cmd/dist using /home/bradfitz/go1.4.
Building Go toolchain1 using /home/bradfitz/go1.4.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/amd64.
---
Installed Go for linux/amd64 in /home/bradfitz/go
Installed commands in /home/bradfitz/go/bin

bradfitz@gdev:~/go/src$ go install --tags=osusergo os/user
# os/user
os/user/getgrouplist_unix.go:32: undefined: maxGroups
os/user/getgrouplist_unix.go:33: undefined: maxGroups
@gopherbot

This comment has been minimized.

Copy link

commented Apr 16, 2018

Change https://golang.org/cl/107299 mentions this issue: os/user: fix build on darwin with "osusergo" build tag

@gopherbot

This comment has been minimized.

Copy link

commented Apr 16, 2018

Change https://golang.org/cl/107300 mentions this issue: cmd/dist: test os/user in osusergo mode as well

@kolyshkin

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2018

I am not sure what would be an elegant way of implementing this, but this is what I came up with:

kir@kd:~/git/golang/go/src$ cat os/user/build_test.go 
// Copyright 2018 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package user_test

import (
	"internal/testenv"
	"os"
	"os/exec"
	"testing"
)

// TestBuild checks if the package can be built with the "osusergo" tag set
func TestBuild(t *testing.T) {
	testenv.MustHaveGoBuild(t)

	cmd := exec.Command(testenv.GoToolPath(t), "build", "-tags", "osusergo", ".")
	run(cmd, t)
}

func run(c *exec.Cmd, t *testing.T) {
	t.Helper()
	c.Stdout = os.Stdout
	c.Stderr = os.Stderr
	err := c.Run()
	if err != nil {
		t.Fatal(err)
	}
}

Let me know if this looks decent, and I'll add the same for netgo

@kolyshkin

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2018

And yes, the above test catches the #24841 when I revert the patch manually:

kir@kd:~/git/golang/go/src$ ./race.bash 
Building Go cmd/dist using /usr/lib/go-1.10.
Building Go toolchain1 using /usr/lib/go-1.10.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/amd64.


##### Testing packages.
ok  	archive/tar	1.202s
ok  	archive/zip	1.910s
...skipped...
ok  	os	9.767s
ok  	os/exec	22.864s
ok  	os/signal	18.517s
# os/user
./getgrouplist_unix.go:32: undefined: maxGroups
./getgrouplist_unix.go:33: undefined: maxGroups
--- FAIL: TestBuild (0.22s)
	osusergo_build_test.go:19: exit status 2
FAIL
FAIL	os/user	0.224s
ok  	path	1.007s
ok  	path/filepath	1.021s
...skipped...
ok  	vendor/golang_org/x/text/unicode/norm	1.007s
2018/04/16 10:50:24 Failed: exit status 1
@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 16, 2018

gopherbot pushed a commit that referenced this issue Apr 16, 2018

os/user: fix build on darwin with "osusergo" build tag
Fixes #24841
Updates #24845

Change-Id: Ia7e2deefe64c12ee8a76ce6ed9f9e003e912b161
Reviewed-on: https://go-review.googlesource.com/107299
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

gopherbot pushed a commit that referenced this issue Apr 16, 2018

cmd/dist, os/user: test os/user in osusergo mode as well, fix plan9 &…
… windows

Would've caught two regressions so far, and found two more.

Updates #24841
Updates #24845 (package net remains)

Change-Id: I57ad06eb54e04b8c99b5d2e7f24c77ad865224e8
Reviewed-on: https://go-review.googlesource.com/107300
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot

This comment has been minimized.

Copy link

commented Apr 16, 2018

Change https://golang.org/cl/107304 mentions this issue: os/user: fix osusergo build on Solaris

gopherbot pushed a commit that referenced this issue Apr 16, 2018

os/user: fix osusergo build on Solaris
Verified that on on Linux, with:

CGO_ENABLED=1 GOOS=solaris go install --tags=osusergo

... it builds now.

Updates #24841
Updates #24845

Change-Id: I49f40532bc2a13a9d282771592fc8d7f116b1902
Reviewed-on: https://go-review.googlesource.com/107304
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Oct 24, 2018

Closing as fixed enough. The netgo build tag is covered by the nocgo builder at least. That's only Linux, but it's something.

@bradfitz bradfitz closed this Oct 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.