Skip to content

Commit

Permalink
Only install runtime/cgo with pure = False (bazelbuild#3492)
Browse files Browse the repository at this point in the history
We don't know why installing `runtime/cgo` even if cgo is disabled
doesn't seem to break anything, so we shouldn't do it.
  • Loading branch information
fmeum authored and jacqueline.lee committed Jul 19, 2023
1 parent ec143e6 commit d12e8f7
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 22 deletions.
3 changes: 3 additions & 0 deletions go/private/actions/stdlib.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ def _build_stdlib(go):
if go.mode.race:
args.add("-race")
args.add_all(go.sdk.experiments, before_each = "-experiment")
args.add("-package", "std")
if not go.mode.pure:
args.add("-package", "runtime/cgo")
args.add_all(link_mode_args(go.mode))
env = go.env
if go.mode.pure:
Expand Down
5 changes: 3 additions & 2 deletions go/tools/builders/stdlib.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ func stdlib(args []string) error {
race := flags.Bool("race", false, "Build in race mode")
shared := flags.Bool("shared", false, "Build in shared mode")
dynlink := flags.Bool("dynlink", false, "Build in dynlink mode")
var packages multiFlag
flags.Var(&packages, "package", "Packages to build")
var experiments multiFlag
flags.Var(&experiments, "experiment", "Go experiments to enable via GOEXPERIMENT")
if err := flags.Parse(args); err != nil {
Expand Down Expand Up @@ -164,8 +166,7 @@ You may need to use the flags --cpu=x64_windows --compiler=mingw-gcc.`)
return fmt.Errorf("error modifying cgo environment to absolute path: %v", err)
}

// TODO(#1885): don't install runtime/cgo in pure mode.
installArgs = append(installArgs, "std", "runtime/cgo")
installArgs = append(installArgs, packages...)
if err := goenv.runCommand(installArgs); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion tests/core/stdlib/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go_test(
name = "buildid_test",
srcs = ["buildid_test.go"],
data = [":stdlib_files"],
rundir = ".",
deps = ["//go/runfiles"],
)

stdlib_files(name = "stdlib_files")
18 changes: 8 additions & 10 deletions tests/core/stdlib/buildid_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build go1.10
// +build go1.10

/* Copyright 2018 The Bazel Authors. All rights reserved.
Expand All @@ -24,6 +25,8 @@ import (
"os/exec"
"path/filepath"
"testing"

"github.com/bazelbuild/rules_go/go/runfiles"
)

func TestEmptyBuildID(t *testing.T) {
Expand All @@ -39,20 +42,17 @@ func TestEmptyBuildID(t *testing.T) {
"aes.a": "",
"cgo.a": "",
}
stdlibPkgDir, err := runfiles.Rlocation("io_bazel_rules_go/stdlib_/pkg")
if err != nil {
t.Fatal(err)
}
n := len(pkgPaths)
done := errors.New("done")
var visit filepath.WalkFunc
visit = func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if (info.Mode() & os.ModeType) == os.ModeSymlink {
path, err = filepath.EvalSymlinks(path)
if err != nil {
return err
}
return filepath.Walk(path, visit)
}
if filepath.Base(path) == "buildid" && (info.Mode()&0111) != 0 {
buildidPath = path
}
Expand All @@ -67,9 +67,7 @@ func TestEmptyBuildID(t *testing.T) {
}
return nil
}
if err := filepath.Walk(".", visit); err == nil {
t.Fatal("could not locate stdlib ROOT file")
} else if err != done {
if err = filepath.Walk(stdlibPkgDir, visit); err != nil && err != done {
t.Fatal(err)
}
if buildidPath == "" {
Expand Down
18 changes: 9 additions & 9 deletions tests/core/stdlib/stdlib_files.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@

load("//go/private:providers.bzl", "GoStdLib")

def _pure_transition_impl(settings, attr):
return {"//go/config:pure": True}
def _force_rebuild_transition_impl(settings, attr):
return {"//go/config:race": True}

pure_transition = transition(
implementation = _pure_transition_impl,
inputs = ["//go/config:pure"],
outputs = ["//go/config:pure"],
force_rebuild_transition = transition(
implementation = _force_rebuild_transition_impl,
inputs = ["//go/config:race"],
outputs = ["//go/config:race"],
)

def _stdlib_files_impl(ctx):
# When a transition is used, ctx.attr._stdlib is a list of Target instead
# of a Target. Possibly a bug?
# When an outgoing transition (aka split transition) is used,
# ctx.attr._stdlib is a list of Target.
stdlib = ctx.attr._stdlib[0][GoStdLib]
libs = stdlib.libs
runfiles = ctx.runfiles(files = libs)
Expand All @@ -40,7 +40,7 @@ stdlib_files = rule(
"_stdlib": attr.label(
default = "@io_bazel_rules_go//:stdlib",
providers = [GoStdLib],
cfg = pure_transition, # force recompilation
cfg = force_rebuild_transition,
),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
Expand Down

0 comments on commit d12e8f7

Please sign in to comment.