[playground] all: support Go 1.16 in the Go Playground

93 views
Skip to first unread message

Alexander Rakoczy (Gerrit)

unread,
Feb 23, 2021, 2:22:56 PM2/23/21
to Carlos Amedee, Dmitri Shuralyov, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Carlos Amedee, Dmitri Shuralyov.

Alexander Rakoczy would like Carlos Amedee and Dmitri Shuralyov to review this change.

View Change

all: support Go 1.16 in the Go Playground

This makes a number of changes in order to support modules being on by
default in Go 1.16.

- Change default Go version in Dockerfile to go1.16
- Change default Go version in sandbox/Dockerfile to go1.16
- Build playground web server with GO_VERSION instead of
GO_BOOTSTRAP_VERSION (for golang/go#40319)
- Drop code related to non-module codepaths. This should keep us
future-proof for later versions of Go.

This works for me testing locally with gVisor's runsc.

Fixes golang/go#44389
Fixes golang/go#40319

Change-Id: Ib99510e662658b75c401567314c3e8ed612002a0
---
M Dockerfile
M sandbox.go
M sandbox/Dockerfile
M server_test.go
M tests.go
M vet.go
6 files changed, 53 insertions(+), 138 deletions(-)

diff --git a/Dockerfile b/Dockerfile
index 5a314c1..5e3396a 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -2,8 +2,18 @@
 # Use of this source code is governed by a BSD-style
 # license that can be found in the LICENSE file.
 
-ARG GO_VERSION=go1.14.1
+# The playground builds Go from a bootstrap version for two reasons:
+# - The plground deployment is triggered before the artifacts are
+#   published for the latest version of go.
+# - The sandbox builds the Go standard library with a custom build
+#   flag called faketime.
 
+# GO_VERSION is provided by Cloud Build, and is set to the latest
+# version of Go. See the configuration in the deploy directory.
+ARG GO_VERSION=go1.16
+
+############################################################################
+# Build Go at GO_VERSION, and build faketime standard library.
 FROM debian:buster AS go-faketime
 LABEL maintainer="golan...@googlegroups.com"
 
@@ -12,11 +22,13 @@
 
 ENV GOPATH /go
 ENV PATH /usr/local/go/bin:$GOPATH/bin:$PATH
-ENV GO_BOOTSTRAP_VERSION go1.14.1
+ENV GO_BOOTSTRAP_VERSION go1.16
 ARG GO_VERSION
 ENV GO_VERSION ${GO_VERSION}
 
-# Get a version of Go for building the playground
+# Get a bootstrap version of Go for building GO_VERSION. At the time
+# of this Dockerfile being built, GO_VERSION's artifacts may not yet
+# be published.
 RUN curl -sSL https://dl.google.com/go/$GO_BOOTSTRAP_VERSION.linux-amd64.tar.gz -o /tmp/go.tar.gz
 RUN curl -sSL https://dl.google.com/go/$GO_BOOTSTRAP_VERSION.linux-amd64.tar.gz.sha256 -o /tmp/go.tar.gz.sha256
 RUN echo "$(cat /tmp/go.tar.gz.sha256) /tmp/go.tar.gz" | sha256sum -c -
@@ -28,22 +40,35 @@
 ENV GO111MODULE on
 ENV GOPROXY=https://proxy.golang.org
 
-# Compile Go at target sandbox version and install standard library with --tags=faketime.
+# Compile Go at target version.
 WORKDIR /usr/local
-RUN git clone https://go.googlesource.com/go go-faketime && cd go-faketime && git reset --hard $GO_VERSION
-WORKDIR /usr/local/go-faketime/src
+RUN git clone https://go.googlesource.com/go go-latest && cd go-latest && git reset --hard $GO_VERSION
+WORKDIR /usr/local/go-latest/src
 RUN ./make.bash
+
+# Install standard library with --tags=faketime.
+RUN cp -R /usr/local/go-latest /usr/local/go-faketime
 ENV GOROOT /usr/local/go-faketime
+WORKDIR /usr/local/go-faketime/src
 RUN ../bin/go install --tags=faketime std
 
-FROM golang:1.14 as build-playground
+############################################################################
+# Build playground web server.
+FROM debian:buster as build-playground
 
+RUN apt-get update && apt-get install -y ca-certificates --no-install-recommends
+# Build playground from Go built at GO_VERSION.
+COPY --from=go-faketime /usr/local/go-latest /usr/local/go
+ENV GOROOT /usr/local/go
+ENV GOPATH /go
+ENV PATH="/go/bin:/usr/local/go/bin:${PATH}"
+# Cache dependencies for effecient Dockerfile building.
 COPY go.mod /go/src/playground/go.mod
 COPY go.sum /go/src/playground/go.sum
 WORKDIR /go/src/playground
 RUN go mod download
 
-# Add and compile playground daemon
+# Add and compile playground daemon.
 COPY . /go/src/playground/
 RUN go install
 
@@ -60,26 +85,6 @@
 ENV GOPATH /go
 ENV PATH /usr/local/go-faketime/bin:$GOPATH/bin:$PATH
 
-# Add and compile tour packages
-RUN go get \
-    golang.org/x/tour/pic \
-    golang.org/x/tour/reader \
-    golang.org/x/tour/tree \
-    golang.org/x/tour/wc \
-    golang.org/x/talks/content/2016/applicative/google && \
-    rm -rf $GOPATH/src/golang.org/x/tour/.git && \
-    rm -rf $GOPATH/src/golang.org/x/talks/.git
-
-# Add tour packages under their old import paths (so old snippets still work)
-RUN mkdir -p $GOPATH/src/code.google.com/p/go-tour && \
-    cp -R $GOPATH/src/golang.org/x/tour/* $GOPATH/src/code.google.com/p/go-tour/ && \
-    sed -i 's_// import_// public import_' $(find $GOPATH/src/code.google.com/p/go-tour/ -name *.go) && \
-    go install \
-    code.google.com/p/go-tour/pic \
-    code.google.com/p/go-tour/reader \
-    code.google.com/p/go-tour/tree \
-    code.google.com/p/go-tour/wc
-
 RUN mkdir /app
 
 COPY --from=build-playground /go/bin/playground /app
@@ -88,8 +93,5 @@
 COPY examples /app/examples
 WORKDIR /app
 
-# Whether we allow third-party imports via proxy.golang.org:
-ENV ALLOW_PLAY_MODULE_DOWNLOADS true
-
 EXPOSE 8080
 ENTRYPOINT ["/app/playground"]
diff --git a/sandbox.go b/sandbox.go
index 8931100..9a8e8a3 100644
--- a/sandbox.go
+++ b/sandbox.go
@@ -396,8 +396,6 @@
 	goPath string
 	// exePath is the path to the built binary.
 	exePath string
-	// useModules is true if the binary was built with module support.
-	useModules bool
 	// testParam is set if tests should be run when running the binary.
 	testParam string
 	// errorMessage is an error message string to be returned to the user.
@@ -408,7 +406,7 @@
 
 // cleanup cleans up the temporary goPath created when building with module support.
 func (b *buildResult) cleanup() error {
-	if b.useModules && b.goPath != "" {
+	if b.goPath != "" {
 		return os.RemoveAll(b.goPath)
 	}
 	return nil
@@ -435,8 +433,7 @@
 		}
 	}
 
-	br.useModules = allowModuleDownloads(files)
-	if !files.Contains("go.mod") && br.useModules {
+	if !files.Contains("go.mod") {
 		files.AddFile("go.mod", []byte("module play\n"))
 	}
 
@@ -471,20 +468,16 @@
 	cmd.Dir = tmpDir
 	cmd.Env = []string{"GOOS=linux", "GOARCH=amd64", "GOROOT=/usr/local/go-faketime"}
 	cmd.Env = append(cmd.Env, "GOCACHE="+goCache)
-	if br.useModules {
-		// Create a GOPATH just for modules to be downloaded
-		// into GOPATH/pkg/mod.
-		cmd.Args = append(cmd.Args, "-modcacherw")
-		br.goPath, err = ioutil.TempDir("", "gopath")
-		if err != nil {
-			log.Printf("error creating temp directory: %v", err)
-			return nil, fmt.Errorf("error creating temp directory: %v", err)
-		}
-		cmd.Env = append(cmd.Env, "GO111MODULE=on", "GOPROXY="+playgroundGoproxy())
-	} else {
-		br.goPath = os.Getenv("GOPATH")              // contains old code.google.com/p/go-tour, etc
-		cmd.Env = append(cmd.Env, "GO111MODULE=off") // in case it becomes on by default later
+	// Create a GOPATH just for modules to be downloaded
+	// into GOPATH/pkg/mod.
+	cmd.Args = append(cmd.Args, "-modcacherw")
+	cmd.Args = append(cmd.Args, "-mod=mod")
+	br.goPath, err = ioutil.TempDir("", "gopath")
+	if err != nil {
+		log.Printf("error creating temp directory: %v", err)
+		return nil, fmt.Errorf("error creating temp directory: %v", err)
 	}
+	cmd.Env = append(cmd.Env, "GO111MODULE=on", "GOPROXY="+playgroundGoproxy())
 	cmd.Args = append(cmd.Args, buildPkgArg)
 	cmd.Env = append(cmd.Env, "GOPATH="+br.goPath)
 	out := &bytes.Buffer{}
@@ -521,7 +514,7 @@
 	}
 	if vet {
 		// TODO: do this concurrently with the execution to reduce latency.
-		br.vetOut, err = vetCheckInDir(tmpDir, br.goPath, br.useModules)
+		br.vetOut, err = vetCheckInDir(tmpDir, br.goPath)
 		if err != nil {
 			return nil, fmt.Errorf("running vet: %v", err)
 		}
@@ -567,21 +560,6 @@
 	return execRes, nil
 }
 
-// allowModuleDownloads reports whether the code snippet in src should be allowed
-// to download modules.
-func allowModuleDownloads(files *fileSet) bool {
-	if files.Num() == 1 && bytes.Contains(files.Data(progName), []byte(`"code.google.com/p/go-tour/`)) {
-		// This domain doesn't exist anymore but we want old snippets using
-		// these packages to still run, so the Dockerfile adds these packages
-		// at this name in $GOPATH. Any snippets using this old name wouldn't
-		// have expected (or been able to use) third-party packages anyway,
-		// so disabling modules and proxy fetches is acceptable.
-		return false
-	}
-	v, _ := strconv.ParseBool(os.Getenv("ALLOW_PLAY_MODULE_DOWNLOADS"))
-	return v
-}
-
 // playgroundGoproxy returns the GOPROXY environment config the playground should use.
 // It is fetched from the environment variable PLAY_GOPROXY. A missing or empty
 // value for PLAY_GOPROXY returns the default value of https://proxy.golang.org.
diff --git a/sandbox/Dockerfile b/sandbox/Dockerfile
index bd32b1e..5a19a1b 100644
--- a/sandbox/Dockerfile
+++ b/sandbox/Dockerfile
@@ -4,7 +4,7 @@
 # environment so the play-sandbox server can connect to the host's
 # docker daemon, which has the gvisor "runsc" runtime available.
 
-FROM golang:1.14 AS build
+FROM golang:1.16 AS build
 
 COPY go.mod /go/src/playground/go.mod
 COPY go.sum /go/src/playground/go.sum
diff --git a/server_test.go b/server_test.go
index 7b6f77b..ae3a9cd 100644
--- a/server_test.go
+++ b/server_test.go
@@ -299,37 +299,6 @@
 	}
 }
 
-func TestAllowModuleDownloads(t *testing.T) {
-	const envKey = "ALLOW_PLAY_MODULE_DOWNLOADS"
-	defer func(old string) { os.Setenv(envKey, old) }(os.Getenv(envKey))
-
-	tests := []struct {
-		src  string
-		env  string
-		want bool
-	}{
-		{src: "package main", want: true},
-		{src: "package main", env: "false", want: false},
-		{src: `import "code.google.com/p/go-tour/"`, want: false},
-	}
-	for i, tt := range tests {
-		if tt.env != "" {
-			os.Setenv(envKey, tt.env)
-		} else {
-			os.Setenv(envKey, "true")
-		}
-		files, err := splitFiles([]byte(tt.src))
-		if err != nil {
-			t.Errorf("%d. splitFiles = %v", i, err)
-			continue
-		}
-		got := allowModuleDownloads(files)
-		if got != tt.want {
-			t.Errorf("%d. allow = %v; want %v; files:\n%s", i, got, tt.want, filesAsString(files))
-		}
-	}
-}
-
 func TestPlaygroundGoproxy(t *testing.T) {
 	const envKey = "PLAY_GOPROXY"
 	defer os.Setenv(envKey, os.Getenv(envKey))
diff --git a/tests.go b/tests.go
index 6c60e62..760c971 100644
--- a/tests.go
+++ b/tests.go
@@ -41,10 +41,6 @@
 		stdlog.Fatal(err)
 	}
 
-	// Enable module downloads for testing:
-	defer func(old string) { os.Setenv("ALLOW_PLAY_MODULE_DOWNLOADS", old) }(os.Getenv("ALLOW_PLAY_MODULE_DOWNLOADS"))
-	os.Setenv("ALLOW_PLAY_MODULE_DOWNLOADS", "true")
-
 	failed := false
 	for i, t := range tests {
 		stdlog.Printf("testing case %d (%q)...\n", i, t.name)
@@ -178,30 +174,6 @@
 	}
 }
 `, want: "timers fired as expected"},
-
-	{
-		name: "old_tour_pkgs_in_gopath",
-		prog: `
-package main
-
-import (
-	"code.google.com/p/go-tour/pic"
-	"code.google.com/p/go-tour/reader"
-	"code.google.com/p/go-tour/tree"
-	"code.google.com/p/go-tour/wc"
-)
-
-var (
-	_ = pic.Show
-	_ = reader.Validate
-	_ = tree.New
-	_ = wc.Test
-)
-
-func main() {
-	println("ok")
-}
-`, want: "ok"},
 	{
 		name: "must_be_package_main",
 		prog: `
diff --git a/vet.go b/vet.go
index 032a5aa..4288adc 100644
--- a/vet.go
+++ b/vet.go
@@ -33,8 +33,7 @@
 	if err := ioutil.WriteFile(in, []byte(req.Body), 0400); err != nil {
 		return nil, fmt.Errorf("error creating temp file %q: %v", in, err)
 	}
-	const useModules = false // legacy handler; no modules (see func comment)
-	vetOutput, err := vetCheckInDir(tmpDir, os.Getenv("GOPATH"), useModules)
+	vetOutput, err := vetCheckInDir(tmpDir, os.Getenv("GOPATH"))
 	if err != nil {
 		// This is about errors running vet, not vet returning output.
 		return nil, err
@@ -48,22 +47,17 @@
 // whether vet reported problem. The returned value is ("", nil) if
 // vet successfully found nothing, and (non-empty, nil) if vet ran and
 // found issues.
-func vetCheckInDir(dir, goPath string, modules bool) (output string, execErr error) {
-	cmd := exec.Command("go", "vet")
-	if !modules {
-		cmd.Args = append(cmd.Args, progName)
-	}
+func vetCheckInDir(dir, goPath string) (output string, execErr error) {
+	cmd := exec.Command("go", "vet", progName)
 	cmd.Dir = dir
 	// Linux go binary is not built with CGO_ENABLED=0.
 	// Prevent vet to compile packages in cgo mode.
 	// See #26307.
 	cmd.Env = append(os.Environ(), "CGO_ENABLED=0", "GOPATH="+goPath)
-	if modules {
-		cmd.Env = append(cmd.Env,
-			"GO111MODULE=on",
-			"GOPROXY="+playgroundGoproxy(),
-		)
-	}
+	cmd.Env = append(cmd.Env,
+		"GO111MODULE=on",
+		"GOPROXY="+playgroundGoproxy(),
+	)
 	out, err := cmd.CombinedOutput()
 	if err == nil {
 		return "", nil

To view, visit change 295649. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: playground
Gerrit-Branch: master
Gerrit-Change-Id: Ib99510e662658b75c401567314c3e8ed612002a0
Gerrit-Change-Number: 295649
Gerrit-PatchSet: 1
Gerrit-Owner: Alexander Rakoczy <al...@golang.org>
Gerrit-Reviewer: Alexander Rakoczy <al...@golang.org>
Gerrit-Reviewer: Carlos Amedee <car...@golang.org>
Gerrit-Reviewer: Dmitri Shuralyov <dmit...@golang.org>
Gerrit-Attention: Carlos Amedee <car...@golang.org>
Gerrit-Attention: Dmitri Shuralyov <dmit...@golang.org>
Gerrit-MessageType: newchange

Dmitri Shuralyov (Gerrit)

unread,
Feb 23, 2021, 6:16:11 PM2/23/21
to Alexander Rakoczy, goph...@pubsubhelper.golang.org, Dmitri Shuralyov, Takeshi Yaegashi, Go Bot, Carlos Amedee, golang-co...@googlegroups.com

Attention is currently required from: Alexander Rakoczy, Carlos Amedee.

Patch set 1:Code-Review +2

View Change

8 comments:

  • File Dockerfile:

    • Patch Set #1, Line 6: plground

      Typo here.

    • Patch Set #1, Line 7: go

      s/go/Go/

    • Patch Set #1, Line 17: go-faketime

      As of this CL, this image now contains both a normal and faketime Go installation, and what they have in common is they're both built at GO_VERSION version.

      Optionally, if it's not too difficult, consider renaming this stage from go-faketime to perhaps go-latest, as it seems that'd make things more readable. (Also see related comment below.)

    • Patch Set #1, Line 43: # Compile Go at target version.

      For this heading and the one on line 49, I think it'd help to make the location on disk and type of Go installation more visible. How about something like:

        # Compile a normal Go at target version in /usr/local/go-realtime.
        [...]
        # Make a copy in /usr/local/go-faketime where the
        # standard library is installed with -tags=faketime.
        [...]

      A path named 'go-latest' may be somewhat misleading, since it's really the version in $GO_VERSION, and whether that's latest or not depends on the caller.

      Naming it go-realtime makes things more clear on line 61, where a normal Go installation is being copied "--from=go-faketime" (also see a related comment above).

    • Patch Set #1, Line 64: ENV PATH="/go/bin:/usr/local/go/bin:${PATH}"

      In the final stage, the order of paths is different:

          ENV PATH="/go/bin:/usr/local/go/bin:${PATH}"           (line 64)
          ENV PATH /usr/local/go-faketime/bin:$GOPATH/bin:$PATH  (line 86)

      Unless they need to be different, perhaps make them the same.

    • Patch Set #1, Line 65: effecient

      s/effecient/efficient/

  • File vet.go:

    • Patch Set #1, Line 45: and whether modules are enabled

      This part of the comment is made obsolete by the change in this CL.

    • Patch Set #1, Line 51: progName

      This parameter was previously added only in legacy GOPATH mode, and as I understand this function is being updated to always run in module mode. So, it should no be added.

      (If it's added, go vet will only check prog.go, rather than all .go files that may be present in a multi-file command.)

To view, visit change 295649. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: playground
Gerrit-Branch: master
Gerrit-Change-Id: Ib99510e662658b75c401567314c3e8ed612002a0
Gerrit-Change-Number: 295649
Gerrit-PatchSet: 1
Gerrit-Owner: Alexander Rakoczy <al...@golang.org>
Gerrit-Reviewer: Alexander Rakoczy <al...@golang.org>
Gerrit-Reviewer: Carlos Amedee <car...@golang.org>
Gerrit-Reviewer: Dmitri Shuralyov <dmit...@golang.org>
Gerrit-Reviewer: Go Bot <go...@golang.org>
Gerrit-CC: Takeshi Yaegashi <yaeg...@gmail.com>
Gerrit-Attention: Alexander Rakoczy <al...@golang.org>
Gerrit-Attention: Carlos Amedee <car...@golang.org>
Gerrit-Comment-Date: Tue, 23 Feb 2021 23:16:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Alexander Rakoczy (Gerrit)

unread,
Feb 24, 2021, 4:17:45 PM2/24/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Alexander Rakoczy, Carlos Amedee.

Alexander Rakoczy uploaded patch set #2 to this change.

View Change

all: support Go 1.16 in the Go Playground

This makes a number of changes in order to support modules being on by
default in Go 1.16.

- Change default Go version in Dockerfile to go1.16
- Change default Go version in sandbox/Dockerfile to go1.16
- Build playground web server with GO_VERSION instead of
GO_BOOTSTRAP_VERSION (for golang/go#40319)
- Drop code related to non-module codepaths. This should keep us
future-proof for later versions of Go.

This works for me testing locally with gVisor's runsc.

Fixes golang/go#44389
Fixes golang/go#40319

Change-Id: Ib99510e662658b75c401567314c3e8ed612002a0
---
M Dockerfile
M sandbox.go
M sandbox/Dockerfile
M server_test.go
M tests.go
M vet.go
6 files changed, 64 insertions(+), 150 deletions(-)

To view, visit change 295649. To unsubscribe, or for help writing mail filters, visit settings.



 Gerrit-Project: playground 


 Gerrit-Branch: master 


 Gerrit-Change-Id: Ib99510e662658b75c401567314c3e8ed612002a0 


 Gerrit-Change-Number: 295649 


 Gerrit-PatchSet: 2 


 Gerrit-Owner: Alexander Rakoczy <al...@golang.org> 


 Gerrit-Reviewer: Alexander Rakoczy <al...@golang.org> 


 Gerrit-Reviewer: Carlos Amedee <car...@golang.org> 


 Gerrit-Reviewer: Dmitri Shuralyov <dmit...@golang.org> 


 Gerrit-Reviewer: Go Bot <go...@golang.org> 


 Gerrit-CC: Takeshi Yaegashi <yaeg...@gmail.com> 


 Gerrit-Attention: Alexander Rakoczy <al...@golang.org> 


 Gerrit-Attention: Carlos Amedee <car...@golang.org> 


 Gerrit-MessageType: newpatchset 



Alexander Rakoczy (Gerrit)
unread,
Feb 24, 2021, 4:20:14 PM2/24/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
 Attention is currently required from: Alexander Rakoczy, Carlos Amedee. 


Alexander Rakoczy uploaded patch set #3 to this change.
 Gerrit-PatchSet: 3 
Alexander Rakoczy (Gerrit)
unread,
Feb 24, 2021, 4:20:20 PM2/24/21
to goph...@pubsubhelper.golang.org, Go Bot, Dmitri Shuralyov, Takeshi Yaegashi, Carlos Amedee, golang-co...@googlegroups.com
 Attention is currently required from: Carlos Amedee. 


View Change
8 comments:
  • File Dockerfile:
    • Done
    • Done
    • As of this CL, this image now contains both a normal and faketime Go installation, and what they hav […]
      Done
    • For this heading and the one on line 49, I think it'd help to make the location on disk and type of  […]
      This is great feedback. go-realtime was confusing to me, as realtime means something different to me than "don't fake time".
      I've opted to rename the bootstrap Go to /usr/local/go-bootstrap, and then our target Go version to /usr/local/go.
    • In the final stage, the order of paths is different: […]
      Done
    • Done. I don't know why my spellchecker isn't working in Dockerfiles.
  • File vet.go:
    • Patch Set #1, Line 45: and whether modules are enabled
      This part of the comment is made obsolete by the change in this CL.
    • Done
    • This parameter was previously added only in legacy GOPATH mode, and as I understand this function is […]
      Done
To view, visit change 295649. To unsubscribe, or for help writing mail filters, visit settings.



 Gerrit-Project: playground 


 Gerrit-Branch: master 


 Gerrit-Change-Id: Ib99510e662658b75c401567314c3e8ed612002a0 


 Gerrit-Change-Number: 295649 


 Gerrit-PatchSet: 3 


 Gerrit-Owner: Alexander Rakoczy <al...@golang.org> 


 Gerrit-Reviewer: Alexander Rakoczy <al...@golang.org> 


 Gerrit-Reviewer: Carlos Amedee <car...@golang.org> 


 Gerrit-Reviewer: Dmitri Shuralyov <dmit...@golang.org> 


 Gerrit-Reviewer: Go Bot <go...@golang.org> 


 Gerrit-CC: Takeshi Yaegashi <yaeg...@gmail.com> 


 Gerrit-Attention: Carlos Amedee <car...@golang.org> 


 Gerrit-Comment-Date: Wed, 24 Feb 2021 21:20:17 +0000 


 Gerrit-HasComments: Yes 


 Gerrit-Has-Labels: No 


 Comment-In-Reply-To: Dmitri Shuralyov <dmit...@golang.org> 


 Gerrit-MessageType: comment 



Alexander Rakoczy (Gerrit)
unread,
Feb 24, 2021, 5:11:33 PM2/24/21
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go Bot, Dmitri Shuralyov, Takeshi Yaegashi, Carlos Amedee, golang-co...@googlegroups.com
Alexander Rakoczy submitted this change.
View Change
Approvals:
  Dmitri Shuralyov: Looks good to me, approved
  Alexander Rakoczy: Trusted; Run TryBots
  Go Bot: TryBots succeeded


all: support Go 1.16 in the Go Playground

This makes a number of changes in order to support modules being on by
default in Go 1.16.

- Change default Go version in Dockerfile to go1.16
- Change default Go version in sandbox/Dockerfile to go1.16
- Build playground web server with GO_VERSION instead of
GO_BOOTSTRAP_VERSION (for golang/go#40319)
- Drop code related to non-module codepaths. This should keep us
future-proof for later versions of Go.

This works for me testing locally with gVisor's runsc.

Fixes golang/go#44389
Fixes golang/go#40319

Change-Id: Ib99510e662658b75c401567314c3e8ed612002a0
Reviewed-on: https://go-review.googlesource.com/c/playground/+/295649
Trust: Alexander Rakoczy <al...@golang.org>
Run-TryBot: Alexander Rakoczy <al...@golang.org>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Dmitri Shuralyov <dmit...@golang.org>

---
M Dockerfile
M sandbox.go
M sandbox/Dockerfile
M server_test.go
M tests.go
M vet.go
6 files changed, 64 insertions(+), 150 deletions(-)

diff --git a/Dockerfile b/Dockerfile
index 5a314c1..f779131 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -2,48 +2,74 @@

 # Use of this source code is governed by a BSD-style
 # license that can be found in the LICENSE file.
 
-ARG GO_VERSION=go1.14.1
+# The playground builds Go from a bootstrap version for two reasons:
+# - The playground deployment is triggered before the artifacts are
+#   published for the latest version of Go.

+# - The sandbox builds the Go standard library with a custom build
+#   flag called faketime.
 
-FROM debian:buster AS go-faketime

+# GO_VERSION is provided by Cloud Build, and is set to the latest
+# version of Go. See the configuration in the deploy directory.
+ARG GO_VERSION=go1.16
+
+############################################################################
+# Build Go at GO_VERSION, and build faketime standard library.
+FROM debian:buster AS build-go
 LABEL maintainer="golan...@googlegroups.com"
 
 ENV BUILD_DEPS 'curl git gcc patch libc6-dev ca-certificates'
 RUN apt-get update && apt-get install -y ${BUILD_DEPS} --no-install-recommends
 
 ENV GOPATH /go
-ENV PATH /usr/local/go/bin:$GOPATH/bin:$PATH
-ENV GO_BOOTSTRAP_VERSION go1.14.1
+ENV GOROOT_BOOTSTRAP=/usr/local/go-bootstrap

+ENV GO_BOOTSTRAP_VERSION go1.16
 ARG GO_VERSION
 ENV GO_VERSION ${GO_VERSION}
 
-# Get a version of Go for building the playground
+# Get a bootstrap version of Go for building GO_VERSION. At the time
+# of this Dockerfile being built, GO_VERSION's artifacts may not yet
+# be published.
 RUN curl -sSL https://dl.google.com/go/$GO_BOOTSTRAP_VERSION.linux-amd64.tar.gz -o /tmp/go.tar.gz
 RUN curl -sSL https://dl.google.com/go/$GO_BOOTSTRAP_VERSION.linux-amd64.tar.gz.sha256 -o /tmp/go.tar.gz.sha256
 RUN echo "$(cat /tmp/go.tar.gz.sha256) /tmp/go.tar.gz" | sha256sum -c -
-RUN mkdir -p /usr/local/go
-RUN tar --strip=1 -C /usr/local/go -vxzf /tmp/go.tar.gz
+RUN mkdir -p $GOROOT_BOOTSTRAP
+RUN tar --strip=1 -C $GOROOT_BOOTSTRAP -vxzf /tmp/go.tar.gz
 
 RUN mkdir /gocache
 ENV GOCACHE /gocache

 ENV GO111MODULE on
 ENV GOPROXY=https://proxy.golang.org
 
-# Compile Go at target sandbox version and install standard library with --tags=faketime.
+# Compile Go at target version in /usr/local/go.

 WORKDIR /usr/local
-RUN git clone https://go.googlesource.com/go go-faketime && cd go-faketime && git reset --hard $GO_VERSION
-WORKDIR /usr/local/go-faketime/src
+RUN git clone https://go.googlesource.com/go go && cd go && git reset --hard $GO_VERSION
+WORKDIR /usr/local/go/src
 RUN ./make.bash
+
+# Make a copy in /usr/local/go-faketime where the standard library
+# is installed with -tags=faketime.
+RUN cp -R /usr/local/go /usr/local/go-faketime

 ENV GOROOT /usr/local/go-faketime
+WORKDIR /usr/local/go-faketime/src
 RUN ../bin/go install --tags=faketime std
 
-FROM golang:1.14 as build-playground
+############################################################################
+# Build playground web server.
+FROM debian:buster as build-playground
 
+RUN apt-get update && apt-get install -y ca-certificates --no-install-recommends
+# Build playground from Go built at GO_VERSION.
+COPY --from=build-go /usr/local/go /usr/local/go

+ENV GOROOT /usr/local/go
+ENV GOPATH /go
+ENV PATH="/go/bin:/usr/local/go/bin:${PATH}"
+# Cache dependencies for efficient Dockerfile building.

 COPY go.mod /go/src/playground/go.mod
 COPY go.sum /go/src/playground/go.sum
 WORKDIR /go/src/playground
 RUN go mod download
 
-# Add and compile playground daemon
+# Add and compile playground daemon.
 COPY . /go/src/playground/
 RUN go install
 
@@ -53,43 +79,19 @@
 
 RUN apt-get update && apt-get install -y git ca-certificates --no-install-recommends
 
-COPY --from=go-faketime /usr/local/go-faketime /usr/local/go-faketime
+COPY --from=build-go /usr/local/go-faketime /usr/local/go-faketime

 
 ARG GO_VERSION
 ENV GO_VERSION ${GO_VERSION}
 ENV GOPATH /go
-ENV PATH /usr/local/go-faketime/bin:$GOPATH/bin:$PATH
-

-# Add and compile tour packages
-RUN go get \
-    golang.org/x/tour/pic \
-    golang.org/x/tour/reader \
-    golang.org/x/tour/tree \
-    golang.org/x/tour/wc \
-    golang.org/x/talks/content/2016/applicative/google && \
-    rm -rf $GOPATH/src/golang.org/x/tour/.git && \
-    rm -rf $GOPATH/src/golang.org/x/talks/.git
-
-# Add tour packages under their old import paths (so old snippets still work)
-RUN mkdir -p $GOPATH/src/code.google.com/p/go-tour && \
-    cp -R $GOPATH/src/golang.org/x/tour/* $GOPATH/src/code.google.com/p/go-tour/ && \
-    sed -i 's_// import_// public import_' $(find $GOPATH/src/code.google.com/p/go-tour/ -name *.go) && \
-    go install \
-    code.google.com/p/go-tour/pic \
-    code.google.com/p/go-tour/reader \
-    code.google.com/p/go-tour/tree \
-    code.google.com/p/go-tour/wc
+ENV PATH="/go/bin:/usr/local/go-faketime/bin:${PATH}"
 
 RUN mkdir /app
-
 COPY --from=build-playground /go/bin/playground /app
 COPY edit.html /app
 COPY static /app/static
index 032a5aa..905c03a 100644
--- a/vet.go
+++ b/vet.go
@@ -21,7 +21,7 @@
 // Deprecated: this is the handler for the legacy /vet endpoint; use
 // the /compile (compileAndRun) handler instead with the WithVet
 // boolean set. This code path doesn't support modules and only exists
-// as a temporary compatiblity bridge to older javascript clients.
+// as a temporary compatibility bridge to older javascript clients.
 func vetCheck(ctx context.Context, req *request) (*response, error) {
 	tmpDir, err := ioutil.TempDir("", "vet")
 	if err != nil {

@@ -33,8 +33,7 @@
 	if err := ioutil.WriteFile(in, []byte(req.Body), 0400); err != nil {
 		return nil, fmt.Errorf("error creating temp file %q: %v", in, err)
 	}
-	const useModules = false // legacy handler; no modules (see func comment)
-	vetOutput, err := vetCheckInDir(tmpDir, os.Getenv("GOPATH"), useModules)
+	vetOutput, err := vetCheckInDir(tmpDir, os.Getenv("GOPATH"))
 	if err != nil {
 		// This is about errors running vet, not vet returning output.
 		return nil, err
@@ -43,27 +42,21 @@
 }
 
 // vetCheckInDir runs go vet in the provided directory, using the
-// provided GOPATH value, and whether modules are enabled. The
-// returned error is only about whether go vet was able to run, not
-// whether vet reported problem. The returned value is ("", nil) if
-// vet successfully found nothing, and (non-empty, nil) if vet ran and
-// found issues.

-func vetCheckInDir(dir, goPath string, modules bool) (output string, execErr error) {
+// provided GOPATH value. The returned error is only about whether
+// go vet was able to run, not whether vet reported problem. The
+// returned value is ("", nil) if vet successfully found nothing,
+// and (non-empty, nil) if vet ran and found issues.

+func vetCheckInDir(dir, goPath string) (output string, execErr error) {
 	cmd := exec.Command("go", "vet")
-	if !modules {
-		cmd.Args = append(cmd.Args, progName)
-	}
 	cmd.Dir = dir
 	// Linux go binary is not built with CGO_ENABLED=0.
 	// Prevent vet to compile packages in cgo mode.
 	// See #26307.
 	cmd.Env = append(os.Environ(), "CGO_ENABLED=0", "GOPATH="+goPath)
-	if modules {
-		cmd.Env = append(cmd.Env,
-			"GO111MODULE=on",
-			"GOPROXY="+playgroundGoproxy(),
-		)
-	}
+	cmd.Env = append(cmd.Env,
+		"GO111MODULE=on",
+		"GOPROXY="+playgroundGoproxy(),
+	)
 	out, err := cmd.CombinedOutput()
 	if err == nil {
 		return "", nil



1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

The name of the file: Dockerfile
Insertions: 16, Deletions: 16.

```
@@ -5:7, +5:7 @@
-  # - The plground deployment is triggered before the artifacts are
-  #   published for the latest version of go.
+  # - The playground deployment is triggered before the artifacts are
+  #   published for the latest version of Go.

@@ -16:17, +16:17 @@
-  FROM debian:buster AS go-faketime
+  FROM debian:buster AS build-go

@@ -23:24, +23:24 @@
-  ENV PATH /usr/local/go/bin:$GOPATH/bin:$PATH
+  ENV GOROOT_BOOTSTRAP=/usr/local/go-bootstrap

@@ -34:36, +34:36 @@
-  RUN mkdir -p /usr/local/go
-  RUN tar --strip=1 -C /usr/local/go -vxzf /tmp/go.tar.gz
+  RUN mkdir -p $GOROOT_BOOTSTRAP
+  RUN tar --strip=1 -C $GOROOT_BOOTSTRAP -vxzf /tmp/go.tar.gz

@@ -42:43, +42:43 @@
-  # Compile Go at target version.
+  # Compile Go at target version in /usr/local/go.

@@ -44:46, +44:46 @@
-  RUN git clone https://go.googlesource.com/go go-latest && cd go-latest && git reset --hard $GO_VERSION
-  WORKDIR /usr/local/go-latest/src
+  RUN git clone https://go.googlesource.com/go go && cd go && git reset --hard $GO_VERSION
+  WORKDIR /usr/local/go/src

@@ -48:50, +48:51 @@
-  # Install standard library with --tags=faketime.
-  RUN cp -R /usr/local/go-latest /usr/local/go-faketime
+  # Make a copy in /usr/local/go-faketime where the standard library
+  # is installed with -tags=faketime.
+  RUN cp -R /usr/local/go /usr/local/go-faketime

@@ -60:61, +61:62 @@
-  COPY --from=go-faketime /usr/local/go-latest /usr/local/go
+  COPY --from=build-go /usr/local/go /usr/local/go

@@ -64:65, +65:66 @@
-  # Cache dependencies for effecient Dockerfile building.
+  # Cache dependencies for efficient Dockerfile building.

@@ -80:81, +81:82 @@
-  COPY --from=go-faketime /usr/local/go-faketime /usr/local/go-faketime
+  COPY --from=build-go /usr/local/go-faketime /usr/local/go-faketime

@@ -85:86, +86:87 @@
-  ENV PATH /usr/local/go-faketime/bin:$GOPATH/bin:$PATH
+  ENV PATH="/go/bin:/usr/local/go-faketime/bin:${PATH}"

@@ -88:89 @@
-  

```
The name of the file: vet.go
Insertions: 6, Deletions: 7.

```
@@ -23:24, +23:24 @@
-  // as a temporary compatiblity bridge to older javascript clients.
+  // as a temporary compatibility bridge to older javascript clients.

@@ -44:51, +44:50 @@
-  // provided GOPATH value, and whether modules are enabled. The
-  // returned error is only about whether go vet was able to run, not
-  // whether vet reported problem. The returned value is ("", nil) if
-  // vet successfully found nothing, and (non-empty, nil) if vet ran and
-  // found issues.
-  func vetCheckInDir(dir, goPath string) (output string, execErr error) {
-  	cmd := exec.Command("go", "vet", progName)
+  // provided GOPATH value. The returned error is only about whether
+  // go vet was able to run, not whether vet reported problem. The
+  // returned value is ("", nil) if vet successfully found nothing,
+  // and (non-empty, nil) if vet ran and found issues.
+  func vetCheckInDir(dir, goPath string) (output string, execErr error) {
+  	cmd := exec.Command("go", "vet")

```

To view, visit change 295649. To unsubscribe, or for help writing mail filters, visit settings.



 Gerrit-Project: playground 


 Gerrit-Branch: master 


 Gerrit-Change-Id: Ib99510e662658b75c401567314c3e8ed612002a0 


 Gerrit-Change-Number: 295649 


 Gerrit-PatchSet: 4 


 Gerrit-Owner: Alexander Rakoczy <al...@golang.org> 


 Gerrit-Reviewer: Alexander Rakoczy <al...@golang.org> 


 Gerrit-Reviewer: Carlos Amedee <car...@golang.org> 


 Gerrit-Reviewer: Dmitri Shuralyov <dmit...@golang.org> 


 Gerrit-Reviewer: Go Bot <go...@golang.org> 


 Gerrit-CC: Takeshi Yaegashi <yaeg...@gmail.com> 


 Gerrit-MessageType: merged 



Reply all
Reply to author
Forward
0 new messages