From ef06a5f44a46dfaf601ca79717ffb00b3591d297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Tue, 8 Feb 2022 14:49:30 +0000 Subject: [PATCH] misc/reboot: don't use symlinks when copying GOROOT/src MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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í Trust: Bryan Mills Run-TryBot: Bryan Mills Reviewed-by: Bryan Mills TryBot-Result: Gopher Robot --- misc/reboot/overlaydir_test.go | 15 +++++++++++---- misc/reboot/reboot_test.go | 9 ++++----- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/misc/reboot/overlaydir_test.go b/misc/reboot/overlaydir_test.go index c446d0891c..71faf0936b 100644 --- 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 ef164d3232..a134affbc2 100644 --- 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)