From: Russ Cox <rsc@golang.org>
Date: Wed, 24 Apr 2024 16:59:02 +0000 (-0400)
Subject: cmd/dist: reject accidental use of internal packages from bootstrap toolchain
X-Git-Tag: go1.23rc1~414
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=805f6b3f5db714ce8f7dae2776748f6df96f288b;p=gostls13.git

cmd/dist: reject accidental use of internal packages from bootstrap toolchain

The compiler was accidentally using internal/godebug from
the Go 1.20 bootstrap toolchain and didn't get the behavior
it expected. Generalizing, we should never assume we know
the behavior of an internal package from an earlier bootstrap
toolchain, so disallow that case in cmd/dist.

Change-Id: I41e079f6120f4081124619bbe2b30069c96b9f29
Reviewed-on: https://go-review.googlesource.com/c/go/+/581496
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
---

diff --git a/src/cmd/dist/buildtool.go b/src/cmd/dist/buildtool.go
index 1141981329..a47b7f90da 100644
--- a/src/cmd/dist/buildtool.go
+++ b/src/cmd/dist/buildtool.go
@@ -300,6 +300,11 @@ func rewriteBlock%s(b *Block) bool { panic("unused during bootstrap") }
 	return bootstrapFixImports(srcFile)
 }
 
+var (
+	importRE      = regexp.MustCompile(`\Aimport\s+(\.|[A-Za-z0-9_]+)?\s*"([^"]+)"\s*(//.*)?\n\z`)
+	importBlockRE = regexp.MustCompile(`\A\s*(?:(\.|[A-Za-z0-9_]+)?\s*"([^"]+)")?\s*(//.*)?\n\z`)
+)
+
 func bootstrapFixImports(srcFile string) string {
 	text := readfile(srcFile)
 	if !strings.Contains(srcFile, "/cmd/") && !strings.Contains(srcFile, `\cmd\`) {
@@ -307,7 +312,17 @@ func bootstrapFixImports(srcFile string) string {
 	}
 	lines := strings.SplitAfter(text, "\n")
 	inBlock := false
+	inComment := false
 	for i, line := range lines {
+		if strings.HasSuffix(line, "*/\n") {
+			inComment = false
+		}
+		if strings.HasSuffix(line, "/*\n") {
+			inComment = true
+		}
+		if inComment {
+			continue
+		}
 		if strings.HasPrefix(line, "import (") {
 			inBlock = true
 			continue
@@ -316,15 +331,56 @@ func bootstrapFixImports(srcFile string) string {
 			inBlock = false
 			continue
 		}
-		if strings.HasPrefix(line, `import `) || inBlock {
-			line = strings.Replace(line, `"cmd/`, `"bootstrap/cmd/`, -1)
+
+		var m []string
+		if !inBlock {
+			if !strings.HasPrefix(line, "import ") {
+				continue
+			}
+			m = importRE.FindStringSubmatch(line)
+			if m == nil {
+				fatalf("%s:%d: invalid import declaration: %q", srcFile, i+1, line)
+			}
+		} else {
+			m = importBlockRE.FindStringSubmatch(line)
+			if m == nil {
+				fatalf("%s:%d: invalid import block line", srcFile, i+1)
+			}
+			if m[2] == "" {
+				continue
+			}
+		}
+
+		path := m[2]
+		if strings.HasPrefix(path, "cmd/") {
+			path = "bootstrap/" + path
+		} else {
 			for _, dir := range bootstrapDirs {
-				if strings.HasPrefix(dir, "cmd/") {
-					continue
+				if path == dir {
+					path = "bootstrap/" + dir
+					break
 				}
-				line = strings.Replace(line, `"`+dir+`"`, `"bootstrap/`+dir+`"`, -1)
 			}
-			lines[i] = line
+		}
+
+		// Rewrite use of internal/reflectlite to be plain reflect.
+		if path == "internal/reflectlite" {
+			lines[i] = strings.ReplaceAll(line, `"reflect"`, `reflectlite "reflect"`)
+			continue
+		}
+
+		// Otherwise, reject direct imports of internal packages,
+		// since that implies knowledge of internal details that might
+		// change from one bootstrap toolchain to the next.
+		// There are many internal packages that are listed in
+		// bootstrapDirs and made into bootstrap copies based on the
+		// current repo's source code. Those are fine; this is catching
+		// references to internal packages in the older bootstrap toolchain.
+		if strings.HasPrefix(path, "internal/") {
+			fatalf("%s:%d: bootstrap-copied source file cannot import %s", srcFile, i+1, path)
+		}
+		if path != m[2] {
+			lines[i] = strings.ReplaceAll(line, `"`+m[2]+`"`, `"`+path+`"`)
 		}
 	}