internal/buildgo: handle different layout of go.dev/dl/ tarballs

The genbootstrap-generated bootstrap tarballs placed all files at the
root of the tarball, while the go.dev/dl/ tarballs place them in 'go'.
So when using a tarball from go.dev/dl/ as the bootstrap toolchain, the
previously working GOROOT_BOOTSTRAP=$WORKDIR/go1.4 now ends up needing
a slight change to GOROOT_BOOTSTRAP=$WORKDIR/go1.4/go.

This was missed in CL 520901. Fix it now, so that it's possible to
switch to using the go.dev/dl/ tarballs as originally intended there.

Balance out this new complexity slightly by removing a condition for
leaving out the -force flag. The Go 1.20 release branch has aged out.

Change-Id: I6e598e66020e46cc11d40b21663ddb6bd6792fb2
Reviewed-on: https://go-review.googlesource.com/c/build/+/606835
Reviewed-by: Cherry Mui <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>

Author: dmitshur

cmd/coordinator/buildstatus.go

@@ -599,19 +599,15 @@ func (st *buildStatus) SpanRecord(sp *schedule.Span, err error) *types.SpanRecor
 
 // goBuilder returns a GoBuilder for this buildStatus.
 func (st *buildStatus) goBuilder() buildgo.GoBuilder {
-	forceMake := true
-	if st.RevBranch == "release-branch.go1.20" {
-		// The concept of "broken ports" and -force flag didn't
-		// exist prior to Go 1.21. See go.dev/issue/56679.
-		// TODO: Remove this condition when Go 1.20 is no longer supported.
-		forceMake = false
-	}
+	goDevDLBootstrap := strings.HasPrefix(
+		st.conf.GoBootstrapURL(pool.NewGCEConfiguration().BuildEnv()), "https://go.dev/dl/")
 	return buildgo.GoBuilder{
-		Logger:     st,
-		BuilderRev: st.BuilderRev,
-		Conf:       st.conf,
-		Goroot:     "go",
-		Force:      forceMake,
+		Logger:           st,
+		BuilderRev:       st.BuilderRev,
+		Conf:             st.conf,
+		Goroot:           "go",
+		GoDevDLBootstrap: goDevDLBootstrap,
+		Force:            true,
 	}
 }
 

internal/buildgo/buildgo.go

@@ -98,31 +98,53 @@ type GoBuilder struct {
 	// GorootBootstrap is an optional absolute Unix-style path to the
 	// bootstrap toolchain, overriding the default.
 	GorootBootstrap string
+	// GoDevDLBootstrap is whether the bootstrap comes from go.dev/dl/,
+	// meaning its content is inside a directory named "go" as opposed
+	// to the root of the archive. GorootBootstrap must be empty if so.
+	GoDevDLBootstrap bool
 	// Force controls whether to use the -force flag when building Go.
 	// See go.dev/issue/56679.
 	Force bool
 }
 
-// RunMake builds the tool chain.
-// goroot is relative to the workdir with forward slashes.
+// RunMake builds the toolchain.
 // w is the Writer to send build output to.
 // remoteErr and err are as described at the top of this file.
 func (gb GoBuilder) RunMake(ctx context.Context, bc buildlet.Client, w io.Writer) (remoteErr, err error) {
 	// Build the source code.
 	makeSpan := gb.CreateSpan("make", gb.Conf.MakeScript())
 	env := append(gb.Conf.Env(), "GOBIN=")
 	if gb.GorootBootstrap != "" {
+		if gb.GoDevDLBootstrap {
+			return nil, fmt.Errorf("cannot set a non-empty GorootBootstrap together with GoDevDLBootstrap")
+		}
 		env = append(env, "GOROOT_BOOTSTRAP="+gb.GorootBootstrap)
 	}
 	makeArgs := gb.Conf.MakeScriptArgs()
 	if gb.Force {
 		makeArgs = append(makeArgs, "-force")
 	}
+	var makePath []string
+	if gb.GoDevDLBootstrap {
+		// When using go.dev/dl/ tarballs for bootstrap, their GOROOT ends up being
+		// $WORKDIR/go1.4/go, which differs from the genbootstrap tarballs where it's
+		// $WORKDIR/go1.4 (without the 'go' subdirectory) instead. So, it's necessary
+		// to arrange for it to be found.
+		//
+		// GOROOT_BOOTSTRAP defaults to $WORKDIR/go1.4 via cmd/buildlet, and since
+		// $WORKDIR expansion is supported in $PATH but not in other environment variables,
+		// setting it to a correct value here would need an extra bc.WorkDir remote call.
+		// So it is simpler to clear out GOROOT_BOOTSTRAP and let the bootstrap
+		// toolchain be found by adding its go/bin directory to the front of $PATH.
+		env = append(env, "GOROOT_BOOTSTRAP=")
+		makePath = []string{"$WORKDIR/go1.4/go/bin", "$PATH"}
+	}
 	remoteErr, err = bc.Exec(ctx, path.Join(gb.Goroot, gb.Conf.MakeScript()), buildlet.ExecOpts{
 		Output:   w,
 		ExtraEnv: env,
 		Debug:    true,
 		Args:     makeArgs,
+		Path:     makePath,
 	})
 	if err != nil {
 		makeSpan.Done(err)