Fix link error when go_binary is instrumented for coverage

go/private/actions/link.bzl

@@ -12,17 +12,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-load(
-    "@bazel_skylib//lib:sets.bzl",
-    "sets",
-)
 load(
     "@bazel_skylib//lib:shell.bzl",
     "shell",
 )
 load(
     "@io_bazel_rules_go//go/private:common.bzl",
     "as_iterable",
+    "as_set",
     "has_shared_lib_extension",
 )
 load(
@@ -100,6 +97,8 @@ def emit_link(
         map_each = _map_archive,
     )
     builder_args.add_all(test_archives, before_each = "-arc", map_each = _format_archive)
+    if go.coverage_enabled and go.coverdata:
+        builder_args.add("-arc", _format_archive(go.coverdata.data))
     builder_args.add("-package_list", go.package_list)
 
     # Build a list of rpaths for dynamic libraries we need to find.
@@ -152,16 +151,20 @@ def emit_link(
         tool_args.add("-w")
     tool_args.add_joined("-extldflags", extldflags, join_with = " ")
 
+    inputs_direct = stamp_inputs + [go.sdk.package_list]
+    if go.coverage_enabled:
+        inputs_direct.append(go.coverdata.data.file)
+    inputs_transitive = [
+        archive.libs,
+        archive.cgo_deps,
+        as_set(go.crosstool),
+        as_set(go.sdk.tools),
+        as_set(go.stdlib.libs),
+    ]
+    inputs = depset(direct = inputs_direct, transitive = inputs_transitive)
+
     go.actions.run(
-        inputs = sets.union(
-            archive.libs,
-            archive.cgo_deps,
-            go.crosstool,
-            stamp_inputs,
-            go.sdk.tools,
-            [go.sdk.package_list],
-            go.stdlib.libs,
-        ),
+        inputs = inputs,
         outputs = [executable],
         mnemonic = "GoLink",
         executable = go.toolchain._builder,

go/tools/builders/compilepkg.go

@@ -243,7 +243,11 @@ func compileArchive(
 				srcName = path.Join(importPath, filepath.Base(origSrc))
 			}
 
-			coverVar := fmt.Sprintf("Cover_%s_%s", sanitizePathForIdentifier(importPath), sanitizePathForIdentifier(origSrc[:len(origSrc)-len(".go")]))
+			stem := filepath.Base(origSrc)
+			if ext := filepath.Ext(stem); ext != "" {
+				stem = stem[:len(stem)-len(ext)]
+			}
+			coverVar := fmt.Sprintf("Cover_%s_%d_%s", sanitizePathForIdentifier(importPath), i, sanitizePathForIdentifier(stem))
 			coverSrc := filepath.Join(workDir, fmt.Sprintf("cover_%d.go", i))
 			if err := instrumentForCoverage(goenv, origSrc, srcName, coverVar, coverMode, coverSrc); err != nil {
 				return err
@@ -448,6 +452,13 @@ func runNogo(ctx context.Context, nogoPath string, srcs []string, deps []archive
 }
 
 func sanitizePathForIdentifier(path string) string {
-	r := strings.NewReplacer("/", "_", "-", "_", ".", "_")
-	return r.Replace(path)
+	return strings.Map(func(r rune) rune {
+		if 'A' <= r && r <= 'Z' ||
+			'a' <= r && r <= 'z' ||
+			'0' <= r && r <= '9' ||
+			r == '_' {
+			return r
+		}
+		return '_'
+	}, path)
 }

tests/core/coverage/BUILD.bazel

@@ -1,5 +1,6 @@
 load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")
 load("@io_bazel_rules_go//tests:bazel_tests.bzl", "bazel_test")
+load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test")
 
 bazel_test(
     name = "coverage_test_test",
@@ -98,3 +99,8 @@ go_test(
     pure = "on",
     tags = ["manual"],
 )
+
+go_bazel_test(
+    name = "binary_coverage_test",
+    srcs = ["binary_coverage_test.go"],
+)

tests/core/coverage/README.rst

@@ -1,3 +1,5 @@
+.. _#2127: https://github.com/bazelbuild/rules_go/issues/2127
+
 coverage functionality
 ======================
 
@@ -14,3 +16,14 @@ coverdata_aspect_test_test
 
 Checks that the ``coverdata`` library is compiled in the same mode as the
 test that depends on it.
+
+binary_coverage_test
+--------------------
+
+Checks that ``bazel build --collect_code_coverage`` can instrument a
+``go_binary``. ``bazel coverage`` should also work, though it should fail
+with status 4 since the binary is not a test.
+
+This functionality isn't really complete. The generate test main package
+gathers and writes coverage data, and that's not present. This is just
+a regression test for a link error (`#2127`_).

tests/core/coverage/binary_coverage_test.go

@@ -0,0 +1,75 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package binary_coverage_test
+
+import (
+	"testing"
+
+	"github.com/bazelbuild/rules_go/go/tools/bazel_testing"
+)
+
+func TestMain(m *testing.M) {
+	bazel_testing.TestMain(m, bazel_testing.Args{
+		Main: `
+-- BUILD.bazel --
+load("@io_bazel_rules_go//go:def.bzl", "go_binary")
+
+go_binary(
+    name = "hello",
+    srcs = ["hello.go"],
+    out = "hello",
+)
+-- hello.go --
+package main
+
+import "fmt"
+
+func main() {
+	fmt.Println(A())
+}
+
+func A() int { return 12 }
+
+func B() int { return 34 }
+`,
+	})
+}
+
+func Test(t *testing.T) {
+	// Check that we can build a binary with coverage instrumentation enabled.
+	args := []string{
+		"build",
+		"--collect_code_coverage",
+		"--instrumentation_filter=.*",
+		"//:hello",
+	}
+	if err := bazel_testing.RunBazel(args...); err != nil {
+		t.Fatal(err)
+	}
+
+	// Check that we can build with `bazel coverage`. It will fail because
+	// there are no tests.
+	args = []string{
+		"coverage",
+		"//:hello",
+	}
+	if err := bazel_testing.RunBazel(args...); err == nil {
+		t.Fatal("got success; want failure")
+	} else if bErr, ok := err.(*bazel_testing.StderrExitError); !ok {
+		t.Fatalf("got %v; want StderrExitError", err)
+	} else if code := bErr.Err.ExitCode(); code != 4 {
+		t.Fatalf("got code %d; want code 4 (no tests found)", code)
+	}
+}