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

Jump to bottom

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

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

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

bradfitz opened this issue Apr 13, 2018 · 9 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge
Milestone
Unreleased

Comments

@bradfitz
Copy link
Contributor

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 x/build issues (builders, bots, dashboards) label Apr 13, 2018
@gopherbot
Copy link

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
@sbinet @bradfitz
os/user: fix build with "osusergo" build tag
98dfd40 
Fixes #24841
Updates #24845

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

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
Copy link

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

@gopherbot
Copy link

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

@kolyshkin
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

@kolyshkin, I had already sent https://golang.org/cl/107300

gopherbot pushed a commit that referenced this issue Apr 16, 2018
@kolyshkin @bradfitz
os/user: fix build on darwin with "osusergo" build tag
04a27be 
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
@bradfitz
cmd/dist, os/user: test os/user in osusergo mode as well, fix plan9 &…
f613a7b 
… 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
Copy link

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
@bradfitz
os/user: fix osusergo build on Solaris
a3df9c4 
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>
@ianlancetaylor ianlancetaylor mentioned this issue Apr 27, 2018
Closed
@bradfitz
Copy link
Contributor Author

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

@golang golang locked and limited conversation to collaborators Oct 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@bradfitz @kolyshkin @gopherbot