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: make.bash throws undefined error when building from source #45673

Closed
Dentrax opened this issue Apr 21, 2021 · 19 comments
Closed

cmd/dist: make.bash throws undefined error when building from source #45673

Dentrax opened this issue Apr 21, 2021 · 19 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@Dentrax
Copy link

Dentrax commented Apr 21, 2021

What version of Go are you using (go version)?

go version go1.16.3 darwin/amd64

Does this issue reproduce with the latest release?

It does not related to the version.

What operating system and processor architecture are you using (go env)?

go env Output
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/furkan.turkal/Library/Caches/go-build"
GOENV="/Users/furkan.turkal/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/furkan.turkal/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/furkan.turkal/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/usr/local/Cellar/go/1.16.3/libexec"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.16.3/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/furkan.turkal/src/public/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/nq/vxjjn3311fg4q263qsxrghpcpzgp66/T/go-build71174443=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

  1. $ git clone https://go.googlesource.com/go

  2. $ cd go/src

  3. $ ./make.bash # OK

  4. Play with the source code (i.e. define a new constant variable, and just use it somewhere)

  5. Run the ./make.bash again

What did you expect to see?

It should be built successfully? 🤔

What did you see instead?

  1. See the error:
Building Go cmd/dist using /usr/local/Cellar/go/1.16.3/libexec. (go1.16.3 darwin/amd64)
Building Go toolchain1 using /usr/local/Cellar/go/1.16.3/libexec.
# bootstrap/go/constant
/Users/furkan.turkal/src/public/go/src/go/constant/value.go:1003: undefined: token.TEST
go tool dist: FAILED: /usr/local/Cellar/go/1.16.3/libexec/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

undefined: token.TEST: But it's already defined in the cloned repo. Bootstrap throws this error because it was not defined in /usr/local/Cellar/go/1.16.3/libexec/src/go/token/token.go file, and $GOROOT_BOOTSTRAP lookup that directory.

But I could not figure out how to make the GOROOT_BOOTSTRAP env to look at the source directory when compiling. It uses my /usr/local path which I've installed with Brew.

If I run the following command in ./src:

$ GOROOT_BOOTSTRAP="/Users/furkan.turkal/src/public/go" ./make.bash

It's failing:

Building Go cmd/dist using /Users/furkan.turkal/src/public/go. (devel go1.17-fe26dfadc3 Tue Apr 20 02:42:23 2021 +0000 darwin/amd64)
ERROR: $GOROOT_BOOTSTRAP must not be set to $GOROOT
Set $GOROOT_BOOTSTRAP to a working Go tree >= Go 1.4.

I have followed the exact same steps when I was working with Go 1.16. Is this because Go 1.17 has some kind of behavioral change? Or am I missing something important here?

@ALTree
Copy link
Member

ALTree commented Apr 21, 2021

/Users/furkan.turkal/src/public/go/src/go/constant/value.go:1003: undefined: token.TEST

If you cat /Users/furkan.turkal/src/public/go/src/go/constant/value.go, what's at line 1003?

If I run the following command in ./src:

$ GOROOT_BOOTSTRAP="/Users/furkan.turkal/src/public/go" ./make.bash

It's failing:

That's expected, you can't set GOROOT_BOOTSTRAP to the tip cloned repository you're building.

@Dentrax
Copy link
Author

Dentrax commented Apr 21, 2021

@ALTree Hey,

In file /Users/furkan.turkal/src/public/go/src/go/constant/value.go at line 1003, I just used token.TEST constant variable as a case. Which is already defined in token package.

But ./make.bash looks the /usr/local/Cellar/go/1.16.3/libexec path to get token package instead of cloned repo. 🤔

@WangLeonard
Copy link
Contributor

Try to set GOROOT to the path of the cloned repo path ?

@Dentrax
Copy link
Author

Dentrax commented Apr 21, 2021

@WangLeonard Hey,

I already showed this on the issue but:

$ GOROOT="/Users/furkan.turkal/src/public/go" ./make.bash # SAME ERROR

Following attempt does solve either:

$ GOROOT="/usr/local/Cellar/go/1.16.3/libexec" go env -w GOROOT="/Users/furkan.turkal/src/public/go"
$ ./make.bash # SAME ERROR

Set everything to default:

$ unset GOROOT # which is already unset'ed
$ GOROOT="/usr/local/Cellar/go/1.16.3/libexec" go env -w GOROOT="/usr/local/Cellar/go/1.16.3/libexec"
$ ./make.bash # SAME ERROR

@WangLeonard
Copy link
Contributor

You should download a compiled version (for example https://golang.org/dl/go1.16.3.darwin-amd64.tar.gz from https://golang.org/dl/) and unzip it to a folder, and set it to GOROOT_BOOTSTRAP
and the cloned source code is GOROOT,Then you should be able to compile it using make.bash
ps you can download a compiled version by https://golang.org/dl/go1.16.3.darwin-amd64.pkg too, Then install it, just remember the directory(set it to GOROOT_BOOTSTRAP), I prefer the former one

@WangLeonard
Copy link
Contributor

This is the official tutorial : https://golang.org/doc/install/source

@ianlancetaylor
Copy link
Contributor

Closing because this doesn't look like a bug.

For help building Go, please use a forum. See https://golang.org/wiki/Questions. Thanks.

@ALTree
Copy link
Member

ALTree commented Apr 21, 2021

I've looked into this a little bit and I believe the reason you're getting that error on make.bash is that you're trying to change go/constant. Your diff probably looks like this:

diff --git a/src/go/constant/value.go b/src/go/constant/value.go
index 78cb3f896f..4a4af7426b 100644
--- a/src/go/constant/value.go
+++ b/src/go/constant/value.go
@@ -1000,6 +1000,10 @@ func UnaryOp(op token.Token, y Value, prec uint) Value {
                case boolVal:
                        return !y
                }
+
+       case token.TEST:
+               goto Error
+
        }
 
 Error:
diff --git a/src/go/token/token.go b/src/go/token/token.go
index 96a1079ec3..bc16352e5a 100644
--- a/src/go/token/token.go
+++ b/src/go/token/token.go
@@ -124,6 +124,7 @@ const (
        SWITCH
        TYPE
        VAR
+       TEST
        keyword_end
 )

The problem is that go/constant is a bootstrap package. From /src/cmd/dist/buildtool.go:

// bootstrapDirs is a list of directories holding code that must be
// compiled with a Go 1.4 toolchain to produce the bootstrapTargets.
// All directories in this list are relative to and must be below $GOROOT/src.
var bootstrapDirs = []string{
...
	"go/constant",
...
}

This means that go/constant needs to be compilable by the bootstrapping toolchain (probably a go1.16 in your case, but we need to support down to go1.4). But since you added a case token.TEST: to value.go, go/constant is no longer compilable with an older toolchain, and the bootstrapping process fails.

@ALTree
Copy link
Member

ALTree commented Apr 21, 2021

And yet the same patch I posted above doesn't cause early boostrapping errors when applied to go1.16.

Did something change in the bootstrapping code that caused a regression?

@Dentrax
Copy link
Author

Dentrax commented Apr 21, 2021

Thanks @WangLeonard, I tried that instruction (installed from .pkg) and it seems still does not work:

$ GOROOT="/Users/furkan.turkal/src/public/go" GOROOT_BOOTSTRAP="/usr/local/go" ./make.bash # SAME ERROR

Thanks, @ALTree. I applied your patch before did this.

I posted above doesn't cause early boostrapping errors when applied to go1.16.

What were your GOROOT and GOROOT_BOOTSTRAP values when bootstrapping the code successfully?

Did something change in the bootstrapping code that caused a regression?

I don't quite understand this statement, on which commit did you able to compile?

@ALTree
Copy link
Member

ALTree commented Apr 21, 2021

I've bisected using this procedure:

  1. First, git apply this patch, which adds a new TOKEN in go/token, and uses it in go/constant:
diff --git a/src/go/constant/value.go b/src/go/constant/value.go
index 46414423f2..13be429771 100644
--- a/src/go/constant/value.go
+++ b/src/go/constant/value.go
@@ -974,6 +974,10 @@ func UnaryOp(op token.Token, y Value, prec uint) Value {
 		case boolVal:
 			return !y
 		}
+
+	case token.TEST:
+		goto Error
+
 	}
 
 Error:
diff --git a/src/go/token/token.go b/src/go/token/token.go
index 96a1079ec3..b7a3c26dbb 100644
--- a/src/go/token/token.go
+++ b/src/go/token/token.go
@@ -124,6 +124,7 @@ const (
 	SWITCH
 	TYPE
 	VAR
+	TEST
 	keyword_end
 )
 
@@ -225,6 +226,7 @@ var tokens = [...]string{
 	SWITCH: "switch",
 	TYPE:   "type",
 	VAR:    "var",
+	TEST:   "ZZZZZZZZZZZZZZZZZZZZZZZZZZZ",
 }
 
 // String returns the string corresponding to the token tok.
  1. Then run make.bash.

This worked in go1.16, and it worked on tip up to the parent of commit 742c05e. At commit 742c05e, make.bash started failing with:

Building Go cmd/dist using /usr/lib/golang. (go1.15.8 linux/arm64)
Building Go toolchain1 using /usr/lib/golang.
# bootstrap/go/constant
/home/ec2-user/gotip/src/go/constant/value.go:986: undefined: token.TEST
go tool dist: FAILED: /usr/lib/golang/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

The question is, is this an expected side effect of that commit? Unfortunately I can't tell...

cc @mdempsky

@ALTree ALTree reopened this Apr 21, 2021
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 21, 2021
@ianlancetaylor
Copy link
Contributor

If you are applying patches like that you probably need to add "go/token" to bootstrapDirs in cmd/dist/buildtool.go.

@ALTree
Copy link
Member

ALTree commented Apr 21, 2021

Tried that too, but unfortunately:

$ time GOGC=off ./make.bash 
Building Go cmd/dist using /usr/local/go. (go1.16.3 linux/amd64)
Building Go toolchain1 using /usr/local/go.
# bootstrap/cmd/cgo
/home/alberto/go/src/cmd/cgo/ast.go:20: cannot use fset (type *"bootstrap/go/token".FileSet) as type *"go/token".FileSet in argument to parser.ParseFile
/home/alberto/go/src/cmd/cgo/ast.go:38: cannot use n.Pos() (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to fset.Position
/home/alberto/go/src/cmd/cgo/ast.go:76: cannot use s.Path.Pos() (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to error_
/home/alberto/go/src/cmd/cgo/ast.go:90: cannot use ast1.Package (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to error_
/home/alberto/go/src/cmd/cgo/ast.go:130: cannot use s.Path.Pos() (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to f.offset
/home/alberto/go/src/cmd/cgo/ast.go:130: cannot use s.Path.End() (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to f.offset
/home/alberto/go/src/cmd/cgo/ast.go:180: cannot use x.Pos() (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to error_
/home/alberto/go/src/cmd/cgo/ast.go:213: cannot use sel.Pos() (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to error_
/home/alberto/go/src/cmd/cgo/ast.go:217: cannot use sel.Pos() (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to error_
/home/alberto/go/src/cmd/cgo/ast.go:221: cannot use sel.Pos() (type "go/token".Pos) as type "bootstrap/go/token".Pos in argument to error_
/home/alberto/go/src/cmd/cgo/ast.go:221: too many errors
go tool dist: FAILED: /usr/local/go/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

@ianlancetaylor
Copy link
Contributor

Add "go/parser" also.

@ALTree
Copy link
Member

ALTree commented Apr 21, 2021

Adding go/parser gives

$ ./make.bash 
Building Go cmd/dist using /usr/local/go. (go1.16.3 linux/amd64)
Building Go toolchain1 using /usr/local/go.
../../../../src/go/parser/parser.go:22: package go/internal/typeparams is not in GOROOT (/usr/local/go/src/go/internal/typeparams)
go tool dist: FAILED: /usr/local/go/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 1

and adding go/internal/typeparams too then gives:

Building Go cmd/dist using /usr/local/go. (go1.16.3 linux/amd64)
Building Go toolchain1 using /usr/local/go.
# bootstrap/go/parser
/home/alberto/go/src/go/parser/parser.go:71: cannot use pos (type "bootstrap/go/token".Position) as type "go/token".Position in argument to p.errors.Add
/home/alberto/go/src/go/parser/parser.go:72: cannot use p.file (type *"bootstrap/go/token".File) as type *"go/token".File in argument to p.scanner.Init
/home/alberto/go/src/go/parser/parser.go:72: cannot use eh (type func("bootstrap/go/token".Position, string)) as type scanner.ErrorHandler in argument to p.scanner.Init
/home/alberto/go/src/go/parser/parser.go:131: cannot assign "go/token".Pos to p.pos (type "bootstrap/go/token".Pos) in multiple assignment
/home/alberto/go/src/go/parser/parser.go:131: cannot assign "go/token".Token to p.tok (type "bootstrap/go/token".Token) in multiple assignment
/home/alberto/go/src/go/parser/parser.go:148: cannot use p.pos (type "bootstrap/go/token".Pos) as type "go/token".Pos in field value
/home/alberto/go/src/go/parser/parser.go:248: cannot use epos (type "bootstrap/go/token".Position) as type "go/token".Position in argument to p.errors.Add
/home/alberto/go/src/go/parser/parser.go:433: cannot use pos (type "bootstrap/go/token".Pos) as type "go/token".Pos in field value
/home/alberto/go/src/go/parser/parser.go:490: cannot use pos (type "bootstrap/go/token".Pos) as type "go/token".Pos in field value
/home/alberto/go/src/go/parser/parser.go:490: cannot use p.pos (type "bootstrap/go/token".Pos) as type "go/token".Pos in field value
/home/alberto/go/src/go/parser/parser.go:490: too many errors
go tool dist: FAILED: /usr/local/go/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status

Clearly I'm missing something. Still, I can reproduce the behaviour in the OP (patch working on 1.16 breaks bootstrapping on tip), so it wasn't just them having troubles with building tip. Of course maybe it's just that their patch has some missing pieces.

@ianlancetaylor
Copy link
Contributor

I can get past that problem, but I have to admit that I ran into what seems to be an unsurmountable problem: I eventually need to add the "time" package to bootstrapDirs, but Go 1.4 can't compile that package.

Supporting this kind of change while bootstrapping may require waiting for #44505.

@ALTree
Copy link
Member

ALTree commented Apr 21, 2021

I see, thanks. Feel free to re-close this if you think there's nothing else to be done.

@ianlancetaylor
Copy link
Contributor

I agree that it's a bit awkward that we've locked ourselves into a place where it's hard to add a new parsing token that is referenced by go/constant, but that seems to be where we are. Everything does work, even if you can't make certain kinds of changes. And I think that #44505 does give us a path to fix it, fairly soon. So I don't think there is anything to do here.

@mdempsky
Copy link
Member

Some packages support build tags for building simplified variants during bootstrap. E.g., math/big has math_big_pure_go and strconv and math/bits have compiler_bootstrap. It might be possible to do something similar for go/constant, if you're particularly pressed for this to work today.

But I think like @ianlancetaylor points out, waiting for #44505 seems like the simplest/surest solution.

@golang golang locked and limited conversation to collaborators Apr 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants