Configure coverage and runfiles for sh_library
InstrumentedFilesProvider is already configured for sh_binary and sh_test, it's confusing for this to not be configured the same way. This means that you might get coverage for binaries depended on via a data dependency, even if that dependency is indirect (e.g. an sh_library rule whose sources run the binary provides the data dependency).
sh_library should also gather runfiles from transitive dependencies in the same way as sh_test and sh_binary.
RELNOTES: Configure coverage and runfiles for sh_library.
PiperOrigin-RevId: 319092861
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
index e8d85d1..d29ddee 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
@@ -29,11 +29,9 @@
import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction.LaunchInfo;
import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector;
-import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.InstrumentationSpec;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
-import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -97,7 +95,7 @@
.addNativeDeclaredProvider(
InstrumentedFilesCollector.collectTransitive(
ruleContext,
- new InstrumentationSpec(FileTypeSet.ANY_FILE, "srcs", "deps", "data"),
+ ShCoverage.INSTRUMENTATION_SPEC,
/* reportedToActualSources= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)))
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShCoverage.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShCoverage.java
new file mode 100644
index 0000000..0d2958d
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShCoverage.java
@@ -0,0 +1,26 @@
+// Copyright 2020 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 com.google.devtools.build.lib.bazel.rules.sh;
+
+import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.InstrumentationSpec;
+import com.google.devtools.build.lib.util.FileTypeSet;
+
+/** Common logic for coverage for sh_* rules. */
+final class ShCoverage {
+
+ private ShCoverage() {}
+
+ public static final InstrumentationSpec INSTRUMENTATION_SPEC =
+ new InstrumentationSpec(FileTypeSet.ANY_FILE, "srcs", "deps", "data");
+}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShLibrary.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShLibrary.java
index b36002a..a37d3ac 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShLibrary.java
@@ -22,8 +22,10 @@
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.TransitionMode;
+import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
/**
* Implementation for the sh_library rule.
@@ -39,13 +41,21 @@
.addAll(ruleContext.getPrerequisiteArtifacts("deps", TransitionMode.TARGET).list())
.addAll(ruleContext.getPrerequisiteArtifacts("data", TransitionMode.DONT_CHECK).list())
.build();
- Runfiles runfiles = new Runfiles.Builder(
- ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles())
- .addTransitiveArtifacts(filesToBuild)
- .build();
+ Runfiles runfiles =
+ new Runfiles.Builder(
+ ruleContext.getWorkspaceName(),
+ ruleContext.getConfiguration().legacyExternalRunfiles())
+ .addTransitiveArtifacts(filesToBuild)
+ .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES)
+ .build();
return new RuleConfiguredTargetBuilder(ruleContext)
.setFilesToBuild(filesToBuild)
.addProvider(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
+ .addNativeDeclaredProvider(
+ InstrumentedFilesCollector.collectTransitive(
+ ruleContext,
+ ShCoverage.INSTRUMENTATION_SPEC,
+ /* reportedToActualSources= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)))
.build();
}
}
diff --git a/src/test/shell/bazel/bazel_coverage_sh_test.sh b/src/test/shell/bazel/bazel_coverage_sh_test.sh
index bd7b67a..517f0b6 100755
--- a/src/test/shell/bazel/bazel_coverage_sh_test.sh
+++ b/src/test/shell/bazel/bazel_coverage_sh_test.sh
@@ -72,12 +72,26 @@
echo "$coverage_file_path"
}
-function test_sh_test_coverage() {
+function set_up_sh_test_coverage() {
cat <<EOF > BUILD
sh_test(
name = "orange-sh",
srcs = ["orange-test.sh"],
- data = ["//java/com/google/orange:orange-bin"]
+ data = ["//java/com/google/orange:orange-bin"],
+)
+
+sh_test(
+ name = "orange-sh-indirect",
+ srcs = ["orange-test.sh"],
+ deps = [":orange-sh-lib"],
+)
+
+# This doesn't test all the combinations, it only exercises
+# a deps dependency from sh_test and a data dependency from
+# sh_library, but they use the same InstrumentedFilesSpec.
+sh_library(
+ name = "orange-sh-lib",
+ data = ["//java/com/google/orange:orange-bin"],
)
EOF
cat <<EOF > orange-test.sh
@@ -127,11 +141,7 @@
}
EOF
- bazel coverage --test_output=all //:orange-sh &>$TEST_log || fail "Coverage for //:orange-sh failed"
-
- local coverage_file_path="$( get_coverage_file_path_from_test_log )"
-
- cat <<EOF > result.dat
+ cat <<EOF > expected.dat
SF:java/com/google/orange/orangeBin.java
FN:3,com/google/orange/orangeBin::<init> ()V
FN:5,com/google/orange/orangeBin::main ([Ljava/lang/String;)V
@@ -160,8 +170,22 @@
LF:3
end_of_record
EOF
- diff result.dat "$coverage_file_path" >> $TEST_log
- cmp result.dat "$coverage_file_path" || fail "Coverage output file is different than the expected file"
+}
+
+function test_sh_test_coverage() {
+ set_up_sh_test_coverage
+ bazel coverage --test_output=all //:orange-sh &>$TEST_log || fail "Coverage for //:orange-sh failed"
+ local coverage_file_path="$( get_coverage_file_path_from_test_log )"
+ diff expected.dat "$coverage_file_path" >> $TEST_log
+ cmp expected.dat "$coverage_file_path" || fail "Coverage output file is different than the expected file for data dep of sh_binary"
+}
+
+function test_sh_test_coverage_indirect() {
+ set_up_sh_test_coverage
+ bazel coverage --test_output=all //:orange-sh-indirect &>$TEST_log || fail "Coverage for //:orange-sh-indirect failed"
+ coverage_file_path="$( get_coverage_file_path_from_test_log )"
+ diff expected.dat "$coverage_file_path" >> $TEST_log
+ cmp expected.dat "$coverage_file_path" || fail "Coverage output file is different than the expected file for data dep of sh_library"
}
function test_sh_test_coverage_cc_binary() {
