Skip to content

Conversation

@dave
Copy link
Contributor

@dave dave commented Jan 27, 2018

No description provided.

@dave dave force-pushed the master branch 4 times, most recently from d27c8e0 to 4ff6a48 Compare January 30, 2018 10:48
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@dave dave Apr 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@flimzy
Copy link
Member

flimzy commented Jan 3, 2022

I'm closing this PR, as #1100 supercedes it.

@flimzy flimzy closed this Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants