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

cmd/dist: "moved GOROOT" test failing on plan9/386 builder #21016

Closed
0intro opened this issue Jul 15, 2017 · 6 comments
Closed

cmd/dist: "moved GOROOT" test failing on plan9/386 builder #21016

0intro opened this issue Jul 15, 2017 · 6 comments

Comments

@0intro
Copy link
Member

@0intro 0intro commented Jul 15, 2017

CL 48550 added "moved GOROOT" test, which is failing on the plan9/386 builder. The plan9/386 builder is running as a buildlet on GCE, while the other builders are old-style builders.

##### moved GOROOT
go: cannot find GOROOT directory: /tmp/workdir/go

See https://build.golang.org/log/6fc6155f74eeff35e8a05ce78ab0531a4f9018bf

@0intro 0intro added this to the Go1.9 milestone Jul 15, 2017
@0intro 0intro self-assigned this Jul 15, 2017
@0intro
Copy link
Member Author

@0intro 0intro commented Jul 15, 2017

The test is passing on the old-style builders because the GO_BUILDER_NAME environment variable hasn't been set on these builders, so the "moved GOROOT" test is skipped.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 15, 2017

I will disable the test, but ideally someone with a plan9 environment should figure out why it is failing. The error suggests that on Plan 9 the default GOROOT is overriding the one computed on the basis of where the cmd/go executable is found.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 15, 2017

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

@0intro
Copy link
Member Author

@0intro 0intro commented Jul 15, 2017

Thanks. I'm currently investigating the issue on my side.

gopherbot pushed a commit that referenced this issue Jul 15, 2017
Fails on iOS because CC_FOR_TARGET points to clangwrap.sh in the
original GOROOT. We could fix that but it doesn't seem worth it.

Fails on Android with "exec format error". I'm not sure why but I
doubt it is interesting.

Fails on Plan 9 because the original GOROOT is being preserved in some
unknown way. This is issue #21016.

Updates #21016

Change-Id: I4e7115d734fc7bf21e5a2ba18fb6ad0bfa31c735
Reviewed-on: https://go-review.googlesource.com/48650
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Reviewed-by: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@0intro
Copy link
Member Author

@0intro 0intro commented Jul 15, 2017

I think I figured out the issue.

In findGOROOT, the os.Executable function returns /bin/go. This is not the command being executed by exec.Command (/tmp/workdir_moved/bin/go), but since I did bind -a /tmp/workdir/bin /bin, this is also a valid path for the go tool (returned by fd2path on /proc/<pid>/text). However, it's not possible to recover the GOROOT from this path.

Since the go tool is called directly from the tests, it should be accessible from the $path environment variable. On Plan 9, $path usually only contains "/bin" and "." and the directories containing binaries are bound to the /bin directory.

@broady broady added the Testing label Jul 17, 2017
@broady broady modified the milestones: Go1.9Maybe, Go1.9 Jul 17, 2017
@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.10 Jul 20, 2017
@rsc
Copy link
Contributor

@rsc rsc commented Nov 29, 2017

I think it would be hard to argue that os.Executable is misbehaving here. /bin/go was run, and it returned /bin/go. findGOROOT is just making assumptions about program lookup that only hold on non-Plan 9 systems. It seems like findGOROOT just fundamentally can't work on Plan 9. Of course if the default builds used some canonical path like /sys/go or even /go, then users who want a different Go could bind their Go onto the canonical path. That might be the way forward if we want to support semi-movable GOROOTs on Plan 9. But I'm going to close this issue - the test failing - as working as intended.

@rsc rsc closed this Nov 29, 2017
@golang golang locked and limited conversation to collaborators Nov 29, 2018
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.