Skip to content

x/build/cmd/updatestd: verify that target GOROOT's bin/go is in PATH #44862

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
toothrot opened this issue Mar 8, 2021 · 4 comments
Closed

x/build/cmd/updatestd: verify that target GOROOT's bin/go is in PATH #44862

toothrot opened this issue Mar 8, 2021 · 4 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@toothrot
Copy link
Contributor

toothrot commented Mar 8, 2021

What did you do?

  • Have two checkouts of Go (in my case, ~/development/go, and ~/development/goroot)
  • GOROOT=$HOME/development/goroot
  • PATH=$HOME/development/goroot/bin:$PATH
  • Note that PATH does not include ~/development/go/bin

Run the following command, where --goroot's bin directory is not in PATH:

updatestd --branch master --goroot=$HOME/development/go

What did you expect to see?

git diff src/cmd/go.mod
diff --git a/src/cmd/go.mod b/src/cmd/go.mod
index ef05ca1ad1..983312c64c 100644
--- a/src/cmd/go.mod
+++ b/src/cmd/go.mod
@@ -1,12 +1,13 @@
 module cmd
 
-go 1.16
+go 1.17
 
 require (
        github.com/google/pprof v0.0.0-20201203190320-1bf35d6f28c2
-       golang.org/x/arch v0.0.0-20201008161808-52c3e6f60cff
-       golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897
+       golang.org/x/arch v0.0.0-20210308155006-05f8f0431f72
+       golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83
        golang.org/x/mod v0.4.2-0.20210301144719-c8bb1bd8a2aa
-       golang.org/x/sys v0.0.0-20210218145245-beda7e5e158e // indirect
-       golang.org/x/tools v0.1.1-0.20210220032852-2363391a5b2f
+       golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b // indirect
+       golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
+       golang.org/x/tools v0.1.1-0.20210308142249-c5f5f4bed308

What did you see instead?

Note how every line in src/cmd/go.mod incorrectly has // indirect:

git diff src/cmd/go.mod
diff --git a/src/cmd/go.mod b/src/cmd/go.mod
index ef05ca1ad1..6efb147f2c 100644
--- a/src/cmd/go.mod
+++ b/src/cmd/go.mod
@@ -1,12 +1,13 @@
 module cmd
 
-go 1.16
+go 1.17
 
 require (
-       github.com/google/pprof v0.0.0-20201203190320-1bf35d6f28c2
-       golang.org/x/arch v0.0.0-20201008161808-52c3e6f60cff
-       golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897
-       golang.org/x/mod v0.4.2-0.20210301144719-c8bb1bd8a2aa
-       golang.org/x/sys v0.0.0-20210218145245-beda7e5e158e // indirect
-       golang.org/x/tools v0.1.1-0.20210220032852-2363391a5b2f
+       github.com/google/pprof v0.0.0-20201203190320-1bf35d6f28c2 // indirect
+       golang.org/x/arch v0.0.0-20210308155006-05f8f0431f72 // indirect
+       golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 // indirect
+       golang.org/x/mod v0.4.2-0.20210301144719-c8bb1bd8a2aa // indirect
+       golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b // indirect
+       golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
+       golang.org/x/tools v0.1.1-0.20210308142249-c5f5f4bed308 // indirect
 )

/cc @dmitshur
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 8, 2021
@toothrot toothrot added this to the Unreleased milestone Mar 8, 2021
@toothrot toothrot changed the title x/playground/cmd/updatestd: verify that target GOROOT's bin/go is in PATH x/build/cmd/updatestd: verify that target GOROOT's bin/go is in PATH Mar 8, 2021
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Mar 8, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Mar 8, 2021 •
edited
Loading

The fix is to add code to updatestd that uses exec.LookPath to find what go binary is in PATH, and if it doesn't match filepath.Join(*goroot, "bin", "go") exactly, print a note to stdout (perhaps with a relative path via filepath.Rel) and arrange for the GOROOT env var to be set to *goroot flag value in all invocations of that go binary.

It's also possible to print a fatal error rather than a note, but I think we want to allow the user of updatestd to use a custom go binary rather than being forced to use the in-tree one.

(If we think it's helpful to make sure the in-tree go binary is used most of the time, we can add a bool flag like -allow-out-of-tree-go-binary that needs to be explicitly provided, else it's an error.)

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 8, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Aug 25, 2021 •
edited
Loading

During the 1.18 dev cycle, using the go binary not from the tip checkout can cause go mod tidy step to fail:

> go mod tidy
command failed: exit status 1
go mod tidy: go.mod file indicates go 1.18, but maximum supported version is 1.17
exit status 1

Now that go version includes the major Go version component, we can improve this by also parsing that out of go version output and handling the case if it doesn't match the expected major version.

(CC @cagedmantis.)

@heschi heschi moved this to Planned in Go Release Sep 27, 2022
@dmitshur dmitshur moved this from Planned to In Progress in Go Release Nov 22, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/452767 mentions this issue: cmd/updatestd: use the go command from goroot flag

@cherrymui
Copy link
Member

Running into this today, when updating the vendored directories for 1.20 after the freeze. When the go command and -goroot don't match, it fails with

> go mod tidy
command failed: exit status 1
cmd/addr2line: ambiguous import: found package cmd/addr2line in multiple modules:
	cmd (/Users/cherryyz/src/go-tip/src/cmd/addr2line)
	 (/Users/cherryyz/src/go/src/cmd/addr2line)
(... more similar errors for other packages ...)

Sent CL 452767 to use the go command from -goroot flag.

Repository owner moved this from In Progress to Done in Go Release Nov 22, 2022
@golang golang locked and limited conversation to collaborators Nov 22, 2023
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 NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@toothrot @dmitshur @gopherbot @cherrymui