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

Orders source files before compilation to ensure reproducible output #742

Open
wants to merge 4 commits into
base: master
from

Conversation

@dave
Copy link

@dave dave commented Jan 27, 2018

No description provided.

@dave dave force-pushed the dave:master branch 4 times, most recently from d27c8e0 to 4ff6a48 Jan 30, 2018
Dave Brophy
@dave dave force-pushed the dave:master branch from 4ff6a48 to 3c1ea5c Jan 30, 2018
"go/types"
"testing"

"fmt"

This comment has been minimized.

@hajimehoshi

hajimehoshi Mar 16, 2018
Member

Why is fmt apart from other imports?

This comment has been minimized.

@dave

dave Apr 6, 2018
Author

Fixed.

}

func compile(path string, sourceFiles []source, minify bool) ([]byte, error) {

This comment has been minimized.

@hajimehoshi

hajimehoshi Mar 16, 2018
Member

No need empty line here

This comment has been minimized.

@dave

dave Apr 6, 2018
Author

Fixed.

file, err := parser.ParseFile(fileSet, fullPath, r, parser.ParseComments)
// Files should be uniquely named and in the original package directory in order to be
// ordered correctly
newPath := path.Join(pkg.Dir, "__"+name)

This comment has been minimized.

@hajimehoshi

hajimehoshi Mar 16, 2018
Member

I'm confused: doesn't fullPath already include name?

This comment has been minimized.

@dave

dave Apr 6, 2018
Author

Yes it does, but fullPath is dependant on the goroot of the machine you're building this on... So some machines these files will come before the other files, and some after.

So on Andy's machine the file order would be:

/andys_go_root/src/time/format.go
/andys_go_root/src/time/sleep.go
/andys_go_root/src/time/sys_unix.go
/andys_go_root/src/time/tick.go
/andys_go_root/src/time/time.go
/andys_go_root/src/time/zoneinfo.go
/andys_go_root/src/time/zoneinfo_read.go
/src/time/time.go

... but on Zoe's machine it would be:

/src/time/time.go
/zoes_go_root/src/time/format.go
/zoes_go_root/src/time/sleep.go
/zoes_go_root/src/time/sys_unix.go
/zoes_go_root/src/time/tick.go
/zoes_go_root/src/time/time.go
/zoes_go_root/src/time/zoneinfo.go
/zoes_go_root/src/time/zoneinfo_read.go

See the issue for an explanation.

With this change the order will be identical on any machine because they'll be like this:

/src/time/__format.go
/src/time/__sleep.go
/src/time/__sys_unix.go
/src/time/__tick.go
/src/time/__time.go
/src/time/time.go
/src/time/__zoneinfo.go
/src/time/__zoneinfo_read.go
dave added 2 commits Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.