Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy expand absolute path to avoid including them in the output #4009

Merged
merged 4 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go/tools/builders/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ filegroup(
"ar.go",
"asm.go",
"builder.go",
"cc.go",
"cgo2.go",
"compilepkg.go",
"constants.go",
Expand Down
2 changes: 2 additions & 0 deletions go/tools/builders/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ func main() {
action = stdlib
case "stdliblist":
action = stdliblist
case "cc":
action = cc
default:
log.Fatalf("unknown action: %s", verb)
}
Expand Down
47 changes: 47 additions & 0 deletions go/tools/builders/cc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package main

import (
"errors"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"syscall"
)

func cc(args []string) error {
cc := os.Getenv("GO_CC")
if cc == "" {
errors.New("GO_CC environment variable not set")
}
ccroot := os.Getenv("GO_CC_ROOT")
if ccroot == "" {
errors.New("GO_CC_ROOT environment variable not set")
}
normalized := []string{cc}
normalized = append(normalized, args...)
transformArgs(normalized, cgoAbsEnvFlags, func(s string) string {
if strings.HasPrefix(s, cgoAbsPlaceholder) {
trimmed := strings.TrimPrefix(s, cgoAbsPlaceholder)
abspath := filepath.Join(ccroot, trimmed)
if _, err := os.Stat(abspath); err == nil {
// Only return the abspath if it exists, otherwise it
// means that either it won't have any effect or the original
// value was not a relpath (e.g. a path with a XCODE placehold from
// macos cc_wrapper)
return abspath
}
return trimmed
}
return s
})
if runtime.GOOS == "windows" {
cmd := exec.Command(normalized[0], normalized[1:]...)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return cmd.Run()
} else {
return syscall.Exec(normalized[0], normalized, os.Environ())
}
}
39 changes: 35 additions & 4 deletions go/tools/builders/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ var (
cgoEnvVars = []string{"CGO_CFLAGS", "CGO_CXXFLAGS", "CGO_CPPFLAGS", "CGO_LDFLAGS"}
// cgoAbsEnvFlags are all the flags that need absolute path in cgoEnvVars
cgoAbsEnvFlags = []string{"-I", "-L", "-isysroot", "-isystem", "-iquote", "-include", "-gcc-toolchain", "--sysroot", "-resource-dir", "-fsanitize-blacklist", "-fsanitize-ignorelist"}
// cgoAbsPlaceholder is placed in front of flag values that must be absolutized
cgoAbsPlaceholder = "__GO_BAZEL_CC_PLACEHOLDER__"
)

// env holds a small amount of Go environment and toolchain information
Expand Down Expand Up @@ -160,10 +162,33 @@ func (e *env) runCommandToFile(out, err io.Writer, args []string) error {
return runAndLogCommand(cmd, e.verbose)
}

func absEnv(envNameList []string, argList []string) error {
// absCCCompiler modifies CGO flags to workaround relative paths.
// Because go is having its own sandbox, all CGO flags should use
// absolute paths. However, CGO flags are embedded in the output
// so we cannot use absolute paths directly. Instead, use a placeholder
// for the absolute path and we replace CC with this builder so that
// we can expand the placeholder later.
func absCCCompiler(envNameList []string, argList []string) error {
err := os.Setenv("GO_CC", os.Getenv("CC"))
if err != nil {
return err
}
err = os.Setenv("GO_CC_ROOT", abs("."))
if err != nil {
return err
}
err = os.Setenv("CC", abs(os.Args[0])+" cc")
if err != nil {
return err
}
for _, envName := range envNameList {
splitedEnv := strings.Fields(os.Getenv(envName))
absArgs(splitedEnv, argList)
transformArgs(splitedEnv, argList, func(s string) string {
if filepath.IsAbs(s) {
return s
}
return cgoAbsPlaceholder + s
})
if err := os.Setenv(envName, strings.Join(splitedEnv, " ")); err != nil {
return err
}
Expand Down Expand Up @@ -320,10 +345,16 @@ func abs(path string) string {
// absArgs applies abs to strings that appear in args. Only paths that are
// part of options named by flags are modified.
func absArgs(args []string, flags []string) {
transformArgs(args, flags, abs)
}

// transformArgs applies fn to strings that appear in args. Only paths that are
// part of options named by flags are modified.
func transformArgs(args []string, flags []string, fn func(string) string) {
absNext := false
for i := range args {
if absNext {
args[i] = abs(args[i])
args[i] = fn(args[i])
absNext = false
continue
}
Expand All @@ -341,7 +372,7 @@ func absArgs(args []string, flags []string) {
possibleValue = possibleValue[1:]
separator = "="
}
args[i] = fmt.Sprintf("%s%s%s", f, separator, abs(possibleValue))
args[i] = fmt.Sprintf("%s%s%s", f, separator, fn(possibleValue))
break
}
}
Expand Down
4 changes: 1 addition & 3 deletions go/tools/builders/stdlib.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,7 @@ You may need to use the flags --cpu=x64_windows --compiler=mingw-gcc.`)
installArgs = append(installArgs, "-ldflags="+allSlug+strings.Join(ldflags, " "))
installArgs = append(installArgs, "-asmflags="+allSlug+strings.Join(asmflags, " "))

// Modify CGO flags to use only absolute path
// because go is having its own sandbox, all CGO flags must use absolute path
if err := absEnv(cgoEnvVars, cgoAbsEnvFlags); err != nil {
if err := absCCCompiler(cgoEnvVars, cgoAbsEnvFlags); err != nil {
return fmt.Errorf("error modifying cgo environment to absolute path: %v", err)
}

Expand Down
4 changes: 1 addition & 3 deletions go/tools/builders/stdliblist.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,7 @@ func stdliblist(args []string) error {
}
os.Setenv("CC", quotePathIfNeeded(abs(ccEnv)))

// Modify CGO flags to use only absolute path
// because go is having its own sandbox, all CGO flags must use absolute path
if err := absEnv(cgoEnvVars, cgoAbsEnvFlags); err != nil {
if err := absCCCompiler(cgoEnvVars, cgoAbsEnvFlags); err != nil {
return fmt.Errorf("error modifying cgo environment to absolute path: %v", err)
}

Expand Down
131 changes: 92 additions & 39 deletions tests/integration/reproducibility/reproducibility_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ func Test(t *testing.T) {
defer wg.Done()
cmd := bazel_testing.BazelCmd("build",
"//:all",
"--copt=-Irelative/path/does/not/exist",
"--linkopt=-Lrelative/path/does/not/exist",
"@io_bazel_rules_go//go/tools/builders:go_path",
"@go_sdk//:builder",
)
Expand All @@ -200,49 +202,92 @@ func Test(t *testing.T) {
}
wg.Wait()

// Hash files in each bazel-bin directory.
dirHashes := make([][]fileHash, len(dirs))
errs := make([]error, len(dirs))
wg.Add(len(dirs))
for i := range dirs {
go func(i int) {
defer wg.Done()
dirHashes[i], errs[i] = hashFiles(filepath.Join(dirs[i], "bazel-bin"))
}(i)
}
wg.Wait()
for _, err := range errs {
if err != nil {
t.Run("Check Hashes", func(t *testing.T) {
// Hash files in each bazel-bin directory.
dirHashes := make([][]fileHash, len(dirs))
errs := make([]error, len(dirs))
wg.Add(len(dirs))
for i := range dirs {
go func(i int) {
defer wg.Done()
dirHashes[i], errs[i] = hashFiles(filepath.Join(dirs[i], "bazel-out/"), func(root, path string) bool {
// Hash everything but actions stdout/stderr and the volatile-status.txt file
// as they are expected to change
return strings.HasPrefix(path, filepath.Join(root, "_tmp")) ||
path == filepath.Join(root, "volatile-status.txt")
})
}(i)
}
wg.Wait()
for _, err := range errs {
if err != nil {
t.Fatal(err)
}
}

// Compare dir0 and dir1. They should be identical.
if err := compareHashes(dirHashes[0], dirHashes[1]); err != nil {
t.Fatal(err)
}
}

// Compare dir0 and dir1. They should be identical.
if err := compareHashes(dirHashes[0], dirHashes[1]); err != nil {
t.Fatal(err)
}
// Compare dir0 and dir2. They should be different.
if err := compareHashes(dirHashes[0], dirHashes[2]); err == nil {
t.Fatalf("dir0 and dir2 are the same)", len(dirHashes[0]))
}
})

// Compare dir0 and dir2. They should be different.
if err := compareHashes(dirHashes[0], dirHashes[2]); err == nil {
t.Fatalf("dir0 and dir2 are the same)", len(dirHashes[0]))
}
t.Run("Check builder", func(t *testing.T) {
// Check that the go_sdk path doesn't appear in the builder binary. This path is different
// nominally different per workspace (but in these tests, the go_sdk paths are all set to the same
// path in WORKSPACE) -- so if this path is in the builder binary, then builds between workspaces
// would be partially non cacheable.
builder_file, err := os.Open(filepath.Join(dirs[0], "bazel-bin", "external", "go_sdk", "builder"))
if err != nil {
t.Fatal(err)
}
defer builder_file.Close()
builder_data, err := ioutil.ReadAll(builder_file)
if err != nil {
t.Fatal(err)
}
if bytes.Index(builder_data, []byte("go_sdk")) != -1 {
t.Fatalf("Found go_sdk path in builder binary, builder tool won't be reproducible")
}
})

// Check that the go_sdk path doesn't appear in the builder binary. This path is different
// nominally different per workspace (but in these tests, the go_sdk paths are all set to the same
// path in WORKSPACE) -- so if this path is in the builder binary, then builds between workspaces
// would be partially non cacheable.
builder_file, err := os.Open(filepath.Join(dirs[0], "bazel-bin", "external", "go_sdk", "builder"))
if err != nil {
t.Fatal(err)
}
defer builder_file.Close()
builder_data, err := ioutil.ReadAll(builder_file)
if err != nil {
t.Fatal(err)
}
if bytes.Index(builder_data, []byte("go_sdk")) != -1 {
t.Fatalf("Found go_sdk path in builder binary, builder tool won't be reproducible")
}
t.Run("Check stdlib", func(t *testing.T) {
for _, dir := range dirs {
found := false
root, err := os.Readlink(filepath.Join(dir, "bazel-out/"))
if err != nil {
t.Fatal(err)
}

filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if !strings.Contains(path, "stdlib_/pkg") {
return nil
}
if !strings.HasSuffix(path, ".a") {
return nil
}
data, err := ioutil.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if bytes.Index(data, []byte("__GO_BAZEL_CC_PLACEHOLDER__")) == -1 {
found = true
return filepath.SkipAll
}
return nil
})
if !found {
t.Fatal("Placeholder not found in stdlib of " + dir)
}
}
})
}

func copyTree(dstRoot, srcRoot string) error {
Expand Down Expand Up @@ -311,7 +356,7 @@ type fileHash struct {
rel, hash string
}

func hashFiles(dir string) ([]fileHash, error) {
func hashFiles(dir string, exclude func(root, path string) bool) ([]fileHash, error) {
// Follow top-level symbolic link
root := dir
for {
Expand Down Expand Up @@ -348,7 +393,15 @@ func hashFiles(dir string) ([]fileHash, error) {
return err
}
}

if info.IsDir() {
if exclude(root, path) {
return filepath.SkipDir
}
return nil
}

if exclude(root, path) {
return nil
}

Expand Down