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

proposal: x/build: add a CI runner with CC="zig cc" for linux/amd64 #59666

Open
motiejus opened this issue Apr 17, 2023 · 2 comments
Open

proposal: x/build: add a CI runner with CC="zig cc" for linux/amd64 #59666

motiejus opened this issue Apr 17, 2023 · 2 comments
Labels
Builders x/build issues (builders, bots, dashboards) new-builder Proposal
Milestone

Comments

@motiejus
Copy link
Contributor

motiejus commented Apr 17, 2023

Short Summary

Add a CI worker that runs Go's unit tests with CC="zig cc" on linux/amd64.

About zig cc

On Linux, zig cc is a wrapper on top of LLVM and LLD. Main benefits compared to other popular compilers:

  • Cross-compilation out of the box via a command-line flag.
  • Can dynamically link to a specified glibc version via a command-line flag (i.e. it generates a libc.so.6 on the fly before linking, no need to ship one).

For example:

CC="zig cc -target aarch64-linux-gnu.2.17" go build .

Will (cross-)compile the current package for linux/arm64 and link to glibc 2.17. Read this to learn more about zig cc.

Zig also has it's own Mach-O linker, making it possible to cross-compile to OS X. This is not in scope for this proposal, but may be considered as a next step. cc @kubkon who expressed interest in this.

Logistics

Initial changes:

  1. Add a linux/amd64 worker that lack any C/C++ compiler (i.e. make sure gcc/ld are not installed).
  2. Install zig v0.11 (to be released in May) to the worker.
  3. Run all Golang's unit tests with CC="zig cc".

Upkeep:

  1. The CI worker will have ReleaseSafe+Debug build, so, in case Zig crashes or trips an assertion, there will be decent information about the crash right in the logs. Refs Proposal: change nightlies from ReleaseFast to ReleaseSafe+debug ziglang/zig#15194.
  2. The worker should have Zig upgraded when a new Zig version is released. This can be coordinated by Uber's Go team (cc @linzhp @sywhang) or Zig folks (cc @kubkon @andrewrk).

Why is this important?

As of writing a few companies are known to use zig cc to compile C/C++ (via this toolchain) and bundle it with Go code. My employer Uber uses zig cc to compile+link it's Go Monorepo for linux/amd64 and linux/arm64. During the last 4 months that we have been using zig cc in production (which included an upgrade to Go 1.20) we have encountered some issues in a released Go version that would have been caught in dev versions should zig cc worker be present:

Since Go unit tests is a decent "integration" test, we believe it is a worth-while exercise to have this in CI.

@gopherbot gopherbot added this to the Proposal milestone Apr 17, 2023
@seankhliao seankhliao changed the title proposal: add a CI runner with CC="zig cc" for linux/amd64 proposal: x/build: add a CI runner with CC="zig cc" for linux/amd64 Apr 17, 2023
@seankhliao
Copy link
Member

cc @golang/release

@motiejus
Copy link
Contributor Author

motiejus commented May 18, 2023

For the record, Go unit tests pass on linux/amd64 with the patch below.

How to run the unit tests (click here)

zig-cc

#!/bin/sh
export ZIG_GLOBAL_CACHE_DIR=/tmp/zig-global-cache
exec /usr/local/bin/zig cc -fno-sanitize=undefined "$@"

And this patch:

diff --git a/src/cmd/cgo/internal/testshared/shared_test.go b/src/cmd/cgo/internal/testshared/shared_test.go
index 34cf3f3141..5e5dbaea13 100644
--- a/src/cmd/cgo/internal/testshared/shared_test.go
+++ b/src/cmd/cgo/internal/testshared/shared_test.go
@@ -12,6 +12,7 @@ import (
        "flag"
        "fmt"
        "go/build"
+       "internal/testenv"
        "io"
        "log"
        "os"
@@ -682,6 +683,11 @@ func TestNotes(t *testing.T) {
 // runtime, another package (dep2) that links against the first, and an
 // executable that links against dep2.
 func TestTwoGopathShlibs(t *testing.T) {
+       // -buildmode=shared is to be removed (#47788). Does not work with linux zig-cc.
+       b := testenv.Builder()
+       if strings.HasPrefix(b, "linux-") && strings.HasSuffix(b, "-zig-cc") {
+               t.Skipf("skipping on linux zig-cc")
+       }
        goCmd(t, "install", "-buildmode=shared", "-linkshared", "./depBase")
        goCmd(t, "install", "-buildmode=shared", "-linkshared", "./dep2")
        goCmd(t, "install", "-linkshared", "./exe2")
@@ -689,6 +695,11 @@ func TestTwoGopathShlibs(t *testing.T) {
 }
 
 func TestThreeGopathShlibs(t *testing.T) {
+       // -buildmode=shared is to be removed (#47788). Does not work with linux zig-cc.
+       b := testenv.Builder()
+       if strings.HasPrefix(b, "linux-") && strings.HasSuffix(b, "-zig-cc") {
+               t.Skipf("skipping on linux zig-cc")
+       }
        goCmd(t, "install", "-buildmode=shared", "-linkshared", "./depBase")
        goCmd(t, "install", "-buildmode=shared", "-linkshared", "./dep2")
        goCmd(t, "install", "-buildmode=shared", "-linkshared", "./dep3")
@@ -1085,6 +1096,11 @@ func TestPackageOrder(t *testing.T) {
 // Test that GC data are generated correctly by the linker when it needs a type defined in
 // a shared library. See issue 39927.
 func TestGCData(t *testing.T) {
+       // -buildmode=shared is to be removed (#47788). Does not work with linux zig-cc.
+       b := testenv.Builder()
+       if strings.HasPrefix(b, "linux-") && strings.HasSuffix(b, "-zig-cc") {
+               t.Skipf("skipping on linux zig-cc")
+       }
        goCmd(t, "install", "-buildmode=shared", "-linkshared", "./gcdata/p")
        goCmd(t, "build", "-linkshared", "./gcdata/main")
        runWithEnv(t, "running gcdata/main", []string{"GODEBUG=clobberfree=1"}, "./main")
diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index 04dfd22f88..1aebf7bded 100644
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -1113,6 +1113,14 @@ func (t *tester) internalLink() bool {
                // linkmode=internal isn't supported.
                return false
        }
+
+       // internal linkmode does not always work with zig-cc due to Golang's implicit
+       // dependency on gcc_s. See issue 59735.
+       b := os.Getenv("GO_BUILDER_NAME")
+       if strings.HasPrefix(b, "linux-") && strings.HasSuffix(b, "-zig-cc") {
+               return false
+       }
+
        return true
 }
 

Run like this:

src$ GO_BUILDER_NAME=linux-amd64-zig-cc CC=zig-cc ./run.bash

Also in https://github.com/motiejus/go/blob/zig-cc-tests/README-zig-cc.md

On linux/arm64 I am getting a very similar error to #59466. I will punt linux/arm64 for now; let's get linux/amd64 going first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) new-builder Proposal
Projects
Status: Incoming
Development

No branches or pull requests

4 participants