Skip to content

Commit

Permalink
fix: deterministic compiled mainfile (#348)
Browse files Browse the repository at this point in the history
* 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 <nate.finch@gmail.com>

* Update parse/parse.go

got caught out not using vim's spellcheck

Co-authored-by: Paul Burlumi <paul@burlumi.com>

* Update mage/main.go

got caught out not using vim's spellcheck

Co-authored-by: Paul Burlumi <paul@burlumi.com>

* fix spelling

Co-authored-by: Paul Burlumi <paul@burlumi.com>

Co-authored-by: Nate Finch <nate.finch@gmail.com>
Co-authored-by: Paul Burlumi <paul@burlumi.com>
Co-authored-by: Nate Finch <natefinch@github.com>
  • Loading branch information
4 people committed Sep 27, 2021
1 parent 4cf3cfc commit d9e2e41
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 11 deletions.
12 changes: 8 additions & 4 deletions mage/main.go
Expand Up @@ -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)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -696,5 +701,4 @@ func removeContents(dir string) error {
}
}
return nil

}
76 changes: 72 additions & 4 deletions mage/main_test.go
Expand Up @@ -2,8 +2,10 @@ package mage

import (
"bytes"
"crypto/sha256"
"debug/macho"
"debug/pe"
"encoding/hex"
"flag"
"fmt"
"go/build"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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"})
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1528,7 +1595,6 @@ func TestNamespaceDefault(t *testing.T) {
}

func TestAliasToImport(t *testing.T) {

}

func TestWrongDependency(t *testing.T) {
Expand All @@ -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
Expand Down
42 changes: 39 additions & 3 deletions parse/parse.go
Expand Up @@ -10,6 +10,7 @@ import (
"io/ioutil"
"log"
"os"
"sort"
"strings"
"time"

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit d9e2e41

Please sign in to comment.