misc/reboot: don't use symlinks when copying GOROOT/src

go:embed disallows using symlinked files by design.
crypto/elliptic is the first std package to use it as of CL 380475,
and unfortunately that broke the TestRepeatBootstrap long test.

The reason it uses symlinks is for speed; it wants to copy GOROOT/src,
but regular files aren't going to be modified in any way,
so a symlink, if supported, means not needing to copy the contents.

Replace the symlink attempt with hard links,
which will mean regular files remain as such, fixing go:embed.
It's worth noting that on many systems hard links won't work,
as the temporary filesystem tends to be separate,
but it doesn't hurt to try.

In my system, where /tmp is tmpfs, the test now copies more bytes.
With the added Logf, I can see overlayDir goes from ~30ms to ~100ms.
This makes sense, as GOROOT/src currently weighs around 100MiB.
To alleviate that slow-down, stop copying testdata directories,
as they currently weigh around 20MiB and aren't needed for the test.
This gets overlayDir on my system down to an acceptable ~70ms.

I briefly considered teaching overlayDir what files can be symlinks,
but that seemed fairly complex long-term, as any file could be embedded.

While here, start using testing.T.TempDir and fs.WalkDir.

For #50995.

Change-Id: I17947e6bdee96237e1ca0606ad0b95e7c5987bc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/383995
Trust: Daniel Martí <mvdan@mvdan.cc>
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/misc/reboot/overlaydir_test.go b/misc/reboot/overlaydir_test.go
index c446d0891cf0b56af85cfc67844ca916b3181803..71faf0936ba19af6e961f158deb8ffd488b5fb28 100644 (file)
--- a/misc/reboot/overlaydir_test.go
+++ b/misc/reboot/overlaydir_test.go
@@ -6,6 +6,7 @@ package reboot_test
 
 import (
        "io"
+       "io/fs"
        "os"
        "path/filepath"
        "strings"
@@ -26,10 +27,14 @@ func overlayDir(dstRoot, srcRoot string) error {
                return err
        }
 
-       return filepath.Walk(srcRoot, func(srcPath string, info os.FileInfo, err error) error {
+       return filepath.WalkDir(srcRoot, func(srcPath string, entry fs.DirEntry, err error) error {
                if err != nil || srcPath == srcRoot {
                        return err
                }
+               if filepath.Base(srcPath) == "testdata" {
+                       // We're just building, so no need to copy those.
+                       return fs.SkipDir
+               }
 
                suffix := strings.TrimPrefix(srcPath, srcRoot)
                for len(suffix) > 0 && suffix[0] == filepath.Separator {
@@ -37,6 +42,7 @@ func overlayDir(dstRoot, srcRoot string) error {
                }
                dstPath := filepath.Join(dstRoot, suffix)
 
+               info, err := entry.Info()
                perm := info.Mode() & os.ModePerm
                if info.Mode()&os.ModeSymlink != 0 {
                        info, err = os.Stat(srcPath)
@@ -46,14 +52,15 @@ func overlayDir(dstRoot, srcRoot string) error {
                        perm = info.Mode() & os.ModePerm
                }
 
-               // Always copy directories (don't symlink them).
+               // Always make copies of directories.
                // If we add a file in the overlay, we don't want to add it in the original.
                if info.IsDir() {
                        return os.MkdirAll(dstPath, perm|0200)
                }
 
-               // If the OS supports symlinks, use them instead of copying bytes.
-               if err := os.Symlink(srcPath, dstPath); err == nil {
+               // If we can use a hard link, do that instead of copying bytes.
+               // Go builds don't like symlinks in some cases, such as go:embed.
+               if err := os.Link(srcPath, dstPath); err == nil {
                        return nil
                }
 

diff --git a/misc/reboot/reboot_test.go b/misc/reboot/reboot_test.go
index ef164d32327bec41cf719d4b5bc39ed2238fb367..a134affbc244558bb1dd83ee34baab60ce28b1c1 100644 (file)
--- a/misc/reboot/reboot_test.go
+++ b/misc/reboot/reboot_test.go
@@ -12,6 +12,7 @@ import (
        "path/filepath"
        "runtime"
        "testing"
+       "time"
 )
 
 func TestRepeatBootstrap(t *testing.T) {
@@ -19,16 +20,14 @@ func TestRepeatBootstrap(t *testing.T) {
                t.Skipf("skipping test that rebuilds the entire toolchain")
        }
 
-       goroot, err := os.MkdirTemp("", "reboot-goroot")
-       if err != nil {
-               t.Fatal(err)
-       }
-       defer os.RemoveAll(goroot)
+       goroot := t.TempDir()
 
        gorootSrc := filepath.Join(goroot, "src")
+       overlayStart := time.Now()
        if err := overlayDir(gorootSrc, filepath.Join(runtime.GOROOT(), "src")); err != nil {
                t.Fatal(err)
        }
+       t.Logf("GOROOT/src overlay set up in %s", time.Since(overlayStart))
 
        if err := os.WriteFile(filepath.Join(goroot, "VERSION"), []byte(runtime.Version()), 0666); err != nil {
                t.Fatal(err)