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

os: package os should not depend on cgo for darwin/arm #10455

Closed
josharian opened this issue Apr 14, 2015 · 6 comments
Closed

os: package os should not depend on cgo for darwin/arm #10455

josharian opened this issue Apr 14, 2015 · 6 comments
Milestone

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Apr 14, 2015

$ go test go/build
--- FAIL: TestDependencies (5.80s)
    deps_test.go:426: darwin/arm/cgo=false unexpected dependency: os imports [C]
    deps_test.go:426: darwin/arm/cgo=true unexpected dependency: os imports [C]
FAIL
FAIL    go/build    5.943s
$ go test -short go/build
ok      go/build    0.089s

/cc @minux @crawshaw

@minux
Copy link
Member

@minux minux commented Apr 15, 2015

@josharian
Copy link
Contributor Author

@josharian josharian commented Apr 15, 2015

I don't know. But IIRC, darwin/arm needed cgo to function correctly. There's a line in misc/ios/README about import "_ runtime/cgo". Maybe that got fixed somewhere by faking the C import, which has broken this test in turn? I'd need to dig.

@minux
Copy link
Member

@minux minux commented Apr 16, 2015

@crawshaw
Copy link
Contributor

@crawshaw crawshaw commented Apr 16, 2015

The data structure holding the environment variables is owned by the syscall package. I suppose linker tricks can be used to work around the test.

There are many options, but I'm not sure what the right solution is.

@ianlancetaylor ianlancetaylor modified the milestones: Unreleased, Go1.5Maybe Jun 3, 2015
@ianlancetaylor ianlancetaylor changed the title build: TestDependencies fails for darwin/arm in non-short mode go/build: TestDependencies fails for darwin/arm in non-short mode Jun 3, 2015
@rsc rsc modified the milestones: Go1.5, Go1.5Maybe Jul 23, 2015
@rsc rsc changed the title go/build: TestDependencies fails for darwin/arm in non-short mode os: depends on cgo in darwin/arm build Jul 23, 2015
@rsc rsc changed the title os: depends on cgo in darwin/arm build os: package os should not depend on cgo for darwin/arm Jul 23, 2015
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 23, 2015

CL https://golang.org/cl/12576 mentions this issue.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 23, 2015

CL https://golang.org/cl/12574 mentions this issue.

@rsc rsc closed this in f6fb549 Jul 27, 2015
rsc added a commit that referenced this issue Jul 27, 2015
We used to use build.Import to get the dependencies, but that meant
we had to repeat the check for every possible GOOS/GOARCH/cgo
combination, which took too long. So we made the test in short mode
only check the current GOOS/GOARCH/cgo combination.
But some combinations can't run the test at all. For example darwin/arm64
does not run tests with a full source file systems, so it cannot test itself,
so nothing was testing darwin/arm64. This led to bugs like #10455
not being caught.

Rewrite the test to read the imports out of the source files ourselves,
so that we can look at all source files in a directory in one pass,
regardless of which GOOS/GOARCH/cgo/etc they require.
This one complete pass runs in the same amount of time as the
old single combination check ran, so we can now test all systems,
even in short mode.

Change-Id: Ie216303c2515bbf1b6fb717d530a0636e271cb6d
Reviewed-on: https://go-review.googlesource.com/12576
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Aug 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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