From f9d99190d29ea731be611e0c6cf33f212b0ab0da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Tue, 6 Sep 2022 16:44:55 +0100 Subject: [PATCH] use -toolexec="garble toolexec" This way, the child process knows that it's running a toolchain command via -toolexec without having to guess via filepath.IsAbs. While here, improve the docs and tests a bit. --- main.go | 98 ++++++++++++++++++++----------------- shared.go | 2 +- testdata/script/basic.txtar | 4 +- testdata/script/help.txtar | 6 +-- testdata/script/tiny.txtar | 1 + 5 files changed, 61 insertions(+), 50 deletions(-) diff --git a/main.go b/main.go index fce82a0..4a8915d 100644 --- a/main.go +++ b/main.go @@ -299,8 +299,14 @@ func goVersionOK() bool { } func mainErr(args []string) error { - // If we recognize an argument, we're not running within -toolexec. - switch command, args := args[0], args[1:]; command { + command, args := args[0], args[1:] + + // Catch users reaching for `go build -toolexec=garble`. + if command != "toolexec" && len(args) == 1 && args[0] == "-V=full" { + return fmt.Errorf(`did you run "go [command] -toolexec=garble" instead of "garble [command]"?`) + } + + switch command { case "help": if hasHelpFlag(args) || len(args) > 1 { fmt.Fprintf(os.Stderr, "usage: garble help [command]\n") @@ -387,56 +393,51 @@ func mainErr(args []string) error { cmd.Stderr = os.Stderr log.Printf("calling via toolexec: %s", cmd) return cmd.Run() - } - - if !filepath.IsAbs(args[0]) { - // -toolexec gives us an absolute path to the tool binary to - // run, so this is most likely misuse of garble by a user. - return fmt.Errorf("unknown command: %q", args[0]) - } - - // We're in a toolexec sub-process, not directly called by the user. - // Load the shared data and wrap the tool, like the compiler or linker. - - if err := loadSharedCache(); err != nil { - return err - } - _, tool := filepath.Split(args[0]) - if runtime.GOOS == "windows" { - tool = strings.TrimSuffix(tool, ".exe") - } - if len(args) == 2 && args[1] == "-V=full" { - return alterToolVersion(tool, args) - } + case "toolexec": + // We're in a toolexec sub-process, not directly called by the user. + // Load the shared data and wrap the tool, like the compiler or linker. + if err := loadSharedCache(); err != nil { + return err + } - toolexecImportPath := os.Getenv("TOOLEXEC_IMPORTPATH") + _, tool := filepath.Split(args[0]) + if runtime.GOOS == "windows" { + tool = strings.TrimSuffix(tool, ".exe") + } + if len(args) == 2 && args[1] == "-V=full" { + return alterToolVersion(tool, args) + } - curPkg = cache.ListedPackages[toolexecImportPath] - if curPkg == nil { - return fmt.Errorf("TOOLEXEC_IMPORTPATH not found in listed packages: %s", toolexecImportPath) - } + toolexecImportPath := os.Getenv("TOOLEXEC_IMPORTPATH") + curPkg = cache.ListedPackages[toolexecImportPath] + if curPkg == nil { + return fmt.Errorf("TOOLEXEC_IMPORTPATH not found in listed packages: %s", toolexecImportPath) + } - transform := transformFuncs[tool] - transformed := args[1:] - if transform != nil { - startTime := time.Now() - log.Printf("transforming %s with args: %s", tool, strings.Join(transformed, " ")) - var err error - if transformed, err = transform(transformed); err != nil { + transform := transformFuncs[tool] + transformed := args[1:] + if transform != nil { + startTime := time.Now() + log.Printf("transforming %s with args: %s", tool, strings.Join(transformed, " ")) + var err error + if transformed, err = transform(transformed); err != nil { + return err + } + log.Printf("transformed args for %s in %s: %s", tool, debugSince(startTime), strings.Join(transformed, " ")) + } else { + log.Printf("skipping transform on %s with args: %s", tool, strings.Join(transformed, " ")) + } + cmd := exec.Command(args[0], transformed...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := cmd.Run(); err != nil { return err } - log.Printf("transformed args for %s in %s: %s", tool, debugSince(startTime), strings.Join(transformed, " ")) - } else { - log.Printf("skipping transform on %s with args: %s", tool, strings.Join(transformed, " ")) - } - cmd := exec.Command(args[0], transformed...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { - return err + return nil + default: + return fmt.Errorf("unknown command: %q", command) } - return nil } func hasHelpFlag(flags []string) bool { @@ -541,6 +542,12 @@ This command wraps "go %s". Below is its help: // Pass the garble flags down to each toolexec invocation. // This way, all garble processes see the same flag values. + // Note that we can end up with a single argument to `go` in the form of: + // + // -toolexec='/binary dir/garble' -tiny toolexec + // + // We quote the absolute path to garble if it contains spaces. + // We can add extra flags to the end of the same -toolexec argument. var toolexecFlag strings.Builder toolexecFlag.WriteString("-toolexec=") quotedExecPath, err := cmdgoQuotedJoin([]string{cache.ExecPath}) @@ -551,6 +558,7 @@ This command wraps "go %s". Below is its help: } toolexecFlag.WriteString(quotedExecPath) appendFlags(&toolexecFlag, false) + toolexecFlag.WriteString(" toolexec") goArgs = append(goArgs, toolexecFlag.String()) if flagDebugDir != "" { diff --git a/shared.go b/shared.go index 0b6b96a..b2ae248 100644 --- a/shared.go +++ b/shared.go @@ -65,7 +65,7 @@ func loadSharedCache() error { startTime := time.Now() f, err := os.Open(filepath.Join(sharedTempDir, "main-cache.gob")) if err != nil { - return fmt.Errorf(`cannot open shared file: %v; did you run "garble [command]"?`, err) + return fmt.Errorf(`cannot open shared file: %v\ndid you run "go [command] -toolexec=garble" instead of "garble [command]"?`, err) } defer func() { log.Printf("shared cache loaded in %s from %s", debugSince(startTime), f.Name()) diff --git a/testdata/script/basic.txtar b/testdata/script/basic.txtar index c6ec453..678e8d5 100644 --- a/testdata/script/basic.txtar +++ b/testdata/script/basic.txtar @@ -29,7 +29,9 @@ stdout 'unknown' # Check that we fail if the user used "go build -toolexec garble" instead of "garble build" ! go build -toolexec=garble -o=main$exe garble_main.go -stderr 'run "garble \[command\]"' +stderr '^did you run.*instead of "garble \[command\]"' +! go build -toolexec='garble toolexec' -o=main$exe garble_main.go +stderr 'cannot open shared file.*did you run.*instead of "garble \[command\]"' # Also check that the binary is reproducible. # No packages should be rebuilt either, thanks to the build cache. diff --git a/testdata/script/help.txtar b/testdata/script/help.txtar index 1d185ff..b9b830a 100644 --- a/testdata/script/help.txtar +++ b/testdata/script/help.txtar @@ -75,9 +75,9 @@ stderr 'must precede command, like: garble -literals build \./pkg' stderr 'must precede command, like: garble -seed=random build \./pkg' ! stdout . -[!windows] ! garble /does/not/exist/compile -[windows] ! garble C:\does\not\exist\compile -stderr 'run "garble \[command\]"' +[!windows] ! garble toolexec /does/not/exist/compile +[windows] ! garble toolexec C:\does\not\exist\compile +stderr 'did you run.*instead of "garble \[command\]"' ! garble build badpackage stderr 'package badpackage is not in GOROOT' diff --git a/testdata/script/tiny.txtar b/testdata/script/tiny.txtar index b3942a6..07568d9 100644 --- a/testdata/script/tiny.txtar +++ b/testdata/script/tiny.txtar @@ -11,6 +11,7 @@ stderr '^\(0x[\d\w]{4,8},0x[\d\w]{4,8}\)' # interfaces/pointers print correctly # Unfortunately, line comment directives don't allow erasing line numbers entirely. stderr '^caller: \?\? 1$' # position info is removed stderr '^recovered: ya like jazz?' +! stderr '^init runtime' # GODEBUG prints are hidden, like inittrace=1 ! stderr 'panic: oh noes' # panics are hidden