From d9e2e4152d4123fb9b1ee8d37b4d60277fa0bb0c Mon Sep 17 00:00:00 2001 From: Claudia Hardman Date: Mon, 27 Sep 2021 00:33:47 -0400 Subject: [PATCH] fix: deterministic compiled mainfile (#348) * fix: determanistic compiled mainfile sort template data so resulting mainfile is consistantly reproducible, which is important to leverage build cache and results in a consistent build id on output (given identical build parameters, e.g. ldflags, output file name, etc.) Result of `mage -compile ./mage_static && go tool buildid ./mage_static` should now always be identical on repeated invocation. Results in a probably imperceptible build time quickening, but solves a minor peave of mine, and adds a test as protection from regression :) * fix: poorly spelled test Co-authored-by: Nate Finch * Update parse/parse.go got caught out not using vim's spellcheck Co-authored-by: Paul Burlumi * Update mage/main.go got caught out not using vim's spellcheck Co-authored-by: Paul Burlumi * fix spelling Co-authored-by: Paul Burlumi Co-authored-by: Nate Finch Co-authored-by: Paul Burlumi Co-authored-by: Nate Finch --- mage/main.go | 12 +++++--- mage/main_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++--- parse/parse.go | 42 ++++++++++++++++++++++++-- 3 files changed, 119 insertions(+), 11 deletions(-) diff --git a/mage/main.go b/mage/main.go index 04dc94c1..f57fdc69 100644 --- a/mage/main.go +++ b/mage/main.go @@ -59,8 +59,10 @@ var mainfileTemplate = template.Must(template.New("").Funcs(map[string]interface }).Parse(mageMainfileTplString)) var initOutput = template.Must(template.New("").Parse(mageTpl)) -const mainfile = "mage_output_file.go" -const initFile = "magefile.go" +const ( + mainfile = "mage_output_file.go" + initFile = "magefile.go" +) var debug = log.New(ioutil.Discard, "DEBUG: ", log.Ltime|log.Lmicroseconds) @@ -261,7 +263,6 @@ Options: if fs.NArg() > 0 { // Temporary dupe of below check until we refactor the other commands to use this check return inv, cmd, errors.New("-h, -init, -clean, -compile and -version cannot be used simultaneously") - } } if inv.Help { @@ -383,6 +384,10 @@ func Invoke(inv Invocation) int { return 1 } + // reproducible output for deterministic builds + sort.Sort(info.Funcs) + sort.Sort(info.Imports) + main := filepath.Join(inv.Dir, mainfile) binaryName := "mage" if inv.CompileOut != "" { @@ -696,5 +701,4 @@ func removeContents(dir string) error { } } return nil - } diff --git a/mage/main_test.go b/mage/main_test.go index abee250a..d0b43d84 100644 --- a/mage/main_test.go +++ b/mage/main_test.go @@ -2,8 +2,10 @@ package mage import ( "bytes" + "crypto/sha256" "debug/macho" "debug/pe" + "encoding/hex" "flag" "fmt" "go/build" @@ -404,6 +406,7 @@ func TestVerboseEnv(t *testing.T) { t.Fatalf("expected %t, but got %t ", expected, inv.Verbose) } } + func TestVerboseFalseEnv(t *testing.T) { os.Setenv("MAGEFILE_VERBOSE", "0") defer os.Unsetenv("MAGEFILE_VERBOSE") @@ -868,7 +871,6 @@ func TestParse(t *testing.T) { if s := buf.String(); s != "" { t.Fatalf("expected no stdout output but got %q", s) } - } func TestSetDir(t *testing.T) { @@ -935,6 +937,7 @@ func TestTimeout(t *testing.T) { t.Fatalf("expected %q, but got %q", expected, actual) } } + func TestParseHelp(t *testing.T) { buf := &bytes.Buffer{} _, _, err := Parse(ioutil.Discard, buf, []string{"-h"}) @@ -1320,6 +1323,70 @@ func TestCompiledVerboseFlag(t *testing.T) { } } +func TestCompiledDeterministic(t *testing.T) { + dir := "./testdata/compiled" + compileDir, err := ioutil.TempDir(dir, "") + if err != nil { + t.Fatal(err) + } + + var exp string + outFile := filepath.Join(dir, mainfile) + + // compile a couple times to be sure + for i, run := range []string{"one", "two", "three", "four"} { + run := run + t.Run(run, func(t *testing.T) { + // probably don't run this parallel + filename := filepath.Join(compileDir, "mage_out") + if runtime.GOOS == "windows" { + filename += ".exe" + } + + // The CompileOut directory is relative to the + // invocation directory, so chop off the invocation dir. + outName := "./" + filename[len(dir)-1:] + defer os.RemoveAll(compileDir) + defer os.Remove(outFile) + + inv := Invocation{ + Stderr: os.Stderr, + Stdout: os.Stdout, + Verbose: true, + Keep: true, + Dir: dir, + CompileOut: outName, + } + + code := Invoke(inv) + if code != 0 { + t.Errorf("expected to exit with code 0, but got %v", code) + } + + f, err := os.Open(outFile) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + hasher := sha256.New() + if _, err := io.Copy(hasher, f); err != nil { + t.Fatal(err) + } + + got := hex.EncodeToString(hasher.Sum(nil)) + // set exp on first iteration, subsequent iterations prove the compiled file is identical + if i == 0 { + exp = got + } + + if i > 0 && got != exp { + t.Errorf("unexpected sha256 hash of %s; wanted %s, got %s", outFile, exp, got) + } + }) + } +} + func TestClean(t *testing.T) { if err := os.RemoveAll(mg.CacheDir()); err != nil { t.Error("error removing cache dir:", err) @@ -1528,7 +1595,6 @@ func TestNamespaceDefault(t *testing.T) { } func TestAliasToImport(t *testing.T) { - } func TestWrongDependency(t *testing.T) { @@ -1551,8 +1617,10 @@ func TestWrongDependency(t *testing.T) { /// This code liberally borrowed from https://github.com/rsc/goversion/blob/master/version/exe.go -type exeType int -type archSize int +type ( + exeType int + archSize int +) const ( winExe exeType = iota diff --git a/parse/parse.go b/parse/parse.go index 472a3dce..5ab0070f 100644 --- a/parse/parse.go +++ b/parse/parse.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "log" "os" + "sort" "strings" "time" @@ -31,10 +32,10 @@ type PkgInfo struct { AstPkg *ast.Package DocPkg *doc.Package Description string - Funcs []*Function + Funcs Functions DefaultFunc *Function Aliases map[string]*Function - Imports []*Import + Imports Imports } // Function represented a job function from a mage file @@ -51,6 +52,24 @@ type Function struct { Args []Arg } +var _ sort.Interface = (Functions)(nil) + +// Functions implements sort interface to optimize compiled output with +// deterministic generated mainfile. +type Functions []*Function + +func (s Functions) Len() int { + return len(s) +} + +func (s Functions) Less(i, j int) bool { + return s[i].TargetName() < s[j].TargetName() +} + +func (s Functions) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + // Arg is an argument to a Function. type Arg struct { Name, Type string @@ -302,6 +321,24 @@ type Import struct { Info PkgInfo } +var _ sort.Interface = (Imports)(nil) + +// Imports implements sort interface to optimize compiled output with +// deterministic generated mainfile. +type Imports []*Import + +func (s Imports) Len() int { + return len(s) +} + +func (s Imports) Less(i, j int) bool { + return s[i].UniqueName < s[j].UniqueName +} + +func (s Imports) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + func setFuncs(pi *PkgInfo) { for _, f := range pi.DocPkg.Funcs { if f.Recv != "" { @@ -586,7 +623,6 @@ func setAliases(pi *PkgInfo) { } func getFunction(exp ast.Expr, pi *PkgInfo) (*Function, error) { - // selector expressions are in LIFO format. // So, in foo.bar.baz the first selector.Name is // actually "baz", the second is "bar", and the last is "foo"