From d0e01478f07865ba1a1b48d427f25d200d33e41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Mon, 10 Aug 2020 19:29:20 +0200 Subject: [PATCH] keep build flags when calling 'go list' Otherwise any build flags like -tags won't be used, and we might easily end up with errors or incorrect packages. The common case with -tags is covered by one of the integration test scripts. On top of that, we add a table-driven unit test to cover all edge cases, since there are many we can do quickly in a unit test. Fixes #82. --- main.go | 70 ++++++++++++++++++++++++++++++++---- main_test.go | 37 +++++++++++++++++++ testdata/scripts/imports.txt | 21 +++++++++-- 3 files changed, 118 insertions(+), 10 deletions(-) diff --git a/main.go b/main.go index 0d5cf80..c3d2cb7 100644 --- a/main.go +++ b/main.go @@ -102,11 +102,9 @@ var ( seed []byte ) -func saveListedPackages(w io.Writer, test bool, patterns ...string) error { +func saveListedPackages(w io.Writer, flags, patterns []string) error { args := []string{"list", "-json", "-deps", "-export"} - if test { - args = append(args, "-test") - } + args = append(args, flags...) args = append(args, patterns...) cmd := exec.Command("go", args...) @@ -295,9 +293,14 @@ func mainErr(args []string) error { return err } defer os.Remove(f.Name()) - // TODO: Pass along flags that 'go list' understands too, such - // as -mod or -modfile. - if err := saveListedPackages(f, cmd == "test", args...); err != nil { + + // Note that we also need to pass build flags to 'go list', such + // as -tags. + listFlags := filterBuildFlags(flags) + if cmd == "test" { + listFlags = append(listFlags, "-test") + } + if err := saveListedPackages(f, listFlags, args); err != nil { return err } os.Setenv("GARBLE_LISTPKGS", f.Name()) @@ -959,6 +962,35 @@ func splitFlagsFromArgs(all []string) (flags, args []string) { return all, nil } +// buildFlags is obtained from 'go help build' as of Go 1.15. +var buildFlags = map[string]bool{ + "-a": true, + "-n": true, + "-p": true, + "-race": true, + "-msan": true, + "-v": true, + "-work": true, + "-x": true, + "-asmflags": true, + "-buildmode": true, + "-compiler": true, + "-gccgoflags": true, + "-gcflags": true, + "-installsuffix": true, + "-ldflags": true, + "-linkshared": true, + "-mod": true, + "-modcacherw": true, + "-modfile": true, + "-pkgdir": true, + "-tags": true, + "-trimpath": true, + "-toolexec": true, +} + +// booleanFlags is obtained from 'go help build' and 'go help testflag' as of Go +// 1.15. var booleanFlags = map[string]bool{ // Shared build flags. "-a": true, @@ -981,6 +1013,30 @@ var booleanFlags = map[string]bool{ "-benchmem": true, } +func filterBuildFlags(flags []string) (filtered []string) { + for i := 0; i < len(flags); i++ { + arg := flags[i] + name := arg + if i := strings.IndexByte(arg, '='); i > 0 { + name = arg[:i] + } + + buildFlag := buildFlags[name] + if buildFlag { + filtered = append(filtered, arg) + } + if booleanFlags[arg] || strings.Contains(arg, "=") { + // Either "-bool" or "-name=value". + continue + } + // "-name value", so the next arg is part of this flag. + if i++; i < len(flags) { + filtered = append(filtered, flags[i]) + } + } + return filtered +} + // splitFlagsFromFiles splits args into a list of flag and file arguments. Since // we can't rely on "--" being present, and we don't parse all flags upfront, we // rely on finding the first argument that doesn't begin with "-" and that has diff --git a/main_test.go b/main_test.go index c8b75e0..59a3bce 100644 --- a/main_test.go +++ b/main_test.go @@ -354,6 +354,11 @@ func TestSplitFlagsFromArgs(t *testing.T) { []string{"-race", "pkg"}, [2][]string{{"-race"}, {"pkg"}}, }, + { + "ExplicitBoolFlag", + []string{"-race=true", "pkg"}, + [2][]string{{"-race=true"}, {"pkg"}}, + }, } for _, test := range tests { test := test @@ -369,6 +374,38 @@ func TestSplitFlagsFromArgs(t *testing.T) { } } +func TestFilterBuildFlags(t *testing.T) { + t.Parallel() + tests := []struct { + name string + flags []string + want []string + }{ + {"Empty", []string{}, nil}, + { + "NoBuild", + []string{"-short", "-json"}, + nil, + }, + { + "Mixed", + []string{"-short", "-tags", "foo", "-mod=readonly", "-json"}, + []string{"-tags", "foo", "-mod=readonly"}, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + got := filterBuildFlags(test.flags) + + if diff := cmp.Diff(test.want, got); diff != "" { + t.Fatalf("filterBuildFlags(%q) mismatch (-want +got):\n%s", test.flags, diff) + } + }) + } +} + func TestFlagValue(t *testing.T) { t.Parallel() tests := []struct { diff --git a/testdata/scripts/imports.txt b/testdata/scripts/imports.txt index 63b800c..108d01f 100644 --- a/testdata/scripts/imports.txt +++ b/testdata/scripts/imports.txt @@ -1,4 +1,4 @@ -garble build +garble build -tags buildtag exec ./main cmp stdout main.stdout @@ -9,10 +9,10 @@ cmp stdout main.stdout # Also check that the binary is reproducible when many imports are involved. cp main$exe main_old$exe rm main$exe -garble build +garble build -tags buildtag bincmp main$exe main_old$exe -go build +go build -tags buildtag exec ./main cmp stdout main.stdout @@ -55,6 +55,20 @@ func main() { linkedPrintln(nil) fmt.Println(quote.Go()) } +-- notag_fail.go -- +// +build !buildtag + +package main + +var foo int = "should be omitted by -tags" +-- withtag_success.go -- +// +build buildtag + +package main + +import "fmt" + +func init() { fmt.Println("buildtag init func") } -- imported/imported.go -- package imported @@ -92,6 +106,7 @@ var _ = reflect.TypeOf(ReflectValueOfVar) type ImportedType int -- main.stdout -- +buildtag init func imported var value imported const value x