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

Issues building multi-OS / multi-arch platforms #14

Open
ns-cweber opened this issue Jul 28, 2019 · 3 comments
Open

Issues building multi-OS / multi-arch platforms #14

ns-cweber opened this issue Jul 28, 2019 · 3 comments
Labels
Bug 🐛 Something isn't working Go 🐹 Issues pertaining to the Go plugin

Comments

@ns-cweber
Copy link
Contributor

ns-cweber commented Jul 28, 2019

When running builder build 3rdParty/golang:sys-unix, if I modify plugins/golang/compile.go to pass only .go files in the specified source directory, then I get this error

/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc64.go:14:6: Major redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc.go:14:24
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc64.go:19:6: Minor redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc.go:19:24
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc64.go:25:6: Mkdev redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc.go:25:33
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_darwin.go:11:6: Major redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc64.go:14:24
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_darwin.go:16:6: Minor redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc64.go:19:24
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_darwin.go:22:6: Mkdev redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_aix_ppc64.go:25:33
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_dragonfly.go:17:6: Major redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_darwin.go:11:24
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_dragonfly.go:22:6: Minor redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_darwin.go:16:24
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_dragonfly.go:28:6: Mkdev redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_darwin.go:22:33
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_freebsd.go:17:6: Major redeclared in this block
        previous declaration at /tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_dragonfly.go:17:24
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/dev_freebsd.go:17:6: too many errors

If I also pass .s files, I get this:

/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/asm_aix_ppc64.s:7:1: invalid character U+0023 '#'
/tmp/cache/packages/3rdParty/golang/targets/sys-unix_sources/786236713/unix/asm_aix_ppc64.s:7:2: syntax error: package statement must be first

It doesn't appear that go tool compile respects GOOS/GOARCH, so we'll have to build that filtering functionality into the plugin or use go build directly.

@weberc2 weberc2 added the Go 🐹 Issues pertaining to the Go plugin label Jul 28, 2019
@weberc2 weberc2 added this to the Builder is self-hosting milestone Jul 28, 2019
@weberc2
Copy link
Owner

weberc2 commented Jul 28, 2019

This blocks https://github.com/weberc2/builder/milestone/1 because github.com/fatih/color depends on golang.org/x/sys which suffers this bug.

@weberc2 weberc2 added the Bug 🐛 Something isn't working label Jul 28, 2019
@weberc2
Copy link
Owner

weberc2 commented Jul 30, 2019

Redditors pointed me to the go/build package to locate the relevant files to pass to the go tool compile command, but looking at go/build#Package, I'm concerned that go tool compile might just be about building .go files, while the go build command is about comprehensively building Go, assembly, and the various CGo files. If this is true, it's probably worthwhile to stop going down the go tool compile path and start supporting go build directly (there are no compelling reasons to prefer go tool compile over go build and CGo support and GOOS/GOARCH handling are pretty compelling reasons to prefer go build over go tool compile).

@ns-cweber
Copy link
Contributor Author

ns-cweber commented Sep 16, 2019

This comment is a brain dump:

After experimenting with using go tool compile vs go build, the tradeoffs are becoming clearer.

go tool compile is lower level and requires a lot of understanding about compiling and linking together .a files that I don't have, especially with regard to edge cases like .s files.

go build (after 1.12) requires all source code to be present and appears to not respect pre-built .a files. The issue here is that the same libraries are being rebuilt for each target (or at least each toplevel target if we decide not to try to build libraries at all) that depends on them--or more likely, the Go compiler is managing its own cache of precompiled libraries which is probably okay insofar as we can probably trust the Go compiler to build reproducibly (i.e., it will rebuild when rebuilding is necessary)? One downside is that builder aspires to distribute builds across clusters of machines by way of a distributed cache, but the Go toolchain's cache is not distributed--although it's probably faster to rebuild on each node rather than to pull dependencies over a network.

The reason the Go team deprecated binary-only packages is because it is difficult to make sure that the same versions of dependencies are used throughout. If the lower-level command is used, the "protection" against the differing-versions-of-same-dependency issue is that users should only use one version of a dependency in their whole source tree. Users could abuse this by building dependency foo with transitive dependency errors_0_1_0 and dependency bar with transitive dependency errors_1_0_0, which would cause the issues the Go team aims to avoid. It's unclear to me at the moment whether or not this is acceptable.

If we are going to lean on the Go toolchain to cache intermediate artifacts, perhaps we should lean on it to manage dependencies as well, such that we don't have go_library() targets at all, but only go_binary() targets that each manage their own dependencies (ideally via go.mod files)? Is there any reason for builder to care about individual Go packages or modules?

  • If builder is aware of the dependency tree, it can determine whether or not tests need to be rerun
    • go test already does this, albeit like the build cache, the test cache is not distributed, so tests would need to be rerun on every host (or we would need to figure out how to distribute its cache--not sure if this is possible/feasible). It would also mean that there is no way to test dependencies of the toplevel module.
      • go test ./... is probably fine if
        • Users are content to have one go.mod for the whole repo and
        • Users trust that upstream maintainers have properly tested their dependencies and
        • The local-vs-distributed-cache issue is solved or not actually a real problem in practice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Something isn't working Go 🐹 Issues pertaining to the Go plugin
Projects
None yet
Development

No branches or pull requests

2 participants