Skip to content

Commit

Permalink
Make it possible to include a FilesToRunProvider in the coverage supp…
Browse files Browse the repository at this point in the history
…ort files when constructing an InstrumentedFileInfo.

This makes it possible for coverage to depend on tools that have accompanying runfiles. The motivating use case is to allow using a binary with runfiles as the singlejar implementation for the Java toolchain, to make experiments with cross-platform action sharing easier.

PiperOrigin-RevId: 492259136
Change-Id: I75d9bf33faa0638f4ec44fab39929efc5e35c4ab
  • Loading branch information
tjgq authored and copybara-github committed Dec 1, 2022
1 parent f86b76a commit 062b83f
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
Expand Down Expand Up @@ -51,7 +52,8 @@ public InstrumentedFilesInfoApi instrumentedFilesInfo(
StarlarkRuleContext starlarkRuleContext,
Sequence<?> sourceAttributes, // <String>
Sequence<?> dependencyAttributes, // <String>
Object supportFiles, // Depset or Sequence of <Artifact>
Object
supportFiles, // Depset<Artifact>|Sequence<Artifact|Depset<Artifact>|FilesToRunProvider>
Dict<?, ?> environment, // <String, String>
Object extensions,
StarlarkThread thread)
Expand All @@ -65,12 +67,31 @@ public InstrumentedFilesInfoApi instrumentedFilesInfo(
.map(entry -> new Pair<>(entry.getKey(), entry.getValue()))
.collect(Collectors.toList());
NestedSetBuilder<Artifact> supportFilesBuilder = NestedSetBuilder.stableOrder();
if (supportFiles instanceof Sequence) {
supportFilesBuilder.addAll(
Sequence.cast(supportFiles, Artifact.class, "coverage_support_files"));
} else {
if (supportFiles instanceof Depset) {
supportFilesBuilder.addTransitive(
Depset.cast(supportFiles, Artifact.class, "coverage_support_files"));
} else if (supportFiles instanceof Sequence) {
Sequence<?> supportFilesSequence = (Sequence<?>) supportFiles;
for (int i = 0; i < supportFilesSequence.size(); i++) {
Object supportFilesElement = supportFilesSequence.get(i);
if (supportFilesElement instanceof Depset) {
supportFilesBuilder.addTransitive(
Depset.cast(supportFilesElement, Artifact.class, "coverage_support_files"));
} else if (supportFilesElement instanceof Artifact) {
supportFilesBuilder.add((Artifact) supportFilesElement);
} else if (supportFilesElement instanceof FilesToRunProvider) {
supportFilesBuilder.addTransitive(
((FilesToRunProvider) supportFilesElement).getFilesToRun());
} else {
throw Starlark.errorf(
"at index %d of coverage_support_files, got element of type %s, want one of depset,"
+ " File or FilesToRunProvider",
i, Starlark.type(supportFilesElement));
}
}
} else {
// Should have been verified by Starlark before this function is called
throw new IllegalStateException();
}
if (!supportFilesBuilder.isEmpty() || !environmentPairs.isEmpty()) {
BuiltinRestriction.throwIfNotBuiltinUsage(thread);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ public interface CoverageCommonApi<
@Param(
name = "coverage_support_files",
allowedTypes = {
@ParamType(type = Sequence.class, generic1 = FileApi.class),
// TODO(#13365): improve the @ParamType annotation once it can support multiple
// contained types.
@ParamType(type = Sequence.class),
@ParamType(type = Depset.class, generic1 = FileApi.class)
},
documented = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,98 @@ public void testInstrumentedFilesInfo_coverageEnabled() throws Exception {
"dict_dep.cc");
}

@Test
public void testInstrumentedFilesInfo_coverageSupportFiles_depset() throws Exception {
// Only builtins can pass coverage_support_files to coverage_common.instrumented_files_info.
// Override extra_action since builtins can only be injected over preexisting native symbols.
setBuildLanguageOptions("--experimental_builtins_bzl_path=tools/builtins");
scratch.file(
"tools/builtins/exports.bzl",
"",
"coverage_common = _builtins.toplevel.coverage_common",
"",
"def _impl(ctx):",
" file1 = ctx.actions.declare_file(ctx.label.name + '.file1')",
" ctx.actions.write(file1, '')",
" file2 = ctx.actions.declare_file(ctx.label.name + '.file2')",
" ctx.actions.write(file2, '')",
" return coverage_common.instrumented_files_info(",
" ctx,",
" coverage_support_files = depset([file1, file2]),",
" )",
"",
"overridden_extra_action = rule(implementation = _impl)",
"",
"exported_toplevels = {}",
"exported_rules = {'+extra_action': overridden_extra_action}",
"exported_to_java = {}");
scratch.file("test/starlark/BUILD", "extra_action(name = 'foo')");

useConfiguration("--collect_code_coverage");

assertThat(
ActionsTestUtil.baseArtifactNames(
getConfiguredTarget("//test/starlark:foo")
.get(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR)
.getCoverageSupportFiles()))
.containsExactly("foo.file1", "foo.file2");
}

@Test
public void testInstrumentedFilesInfo_coverageSupportFiles_sequence() throws Exception {
// Only builtins can pass coverage_support_files to coverage_common.instrumented_files_info.
// Override extra_action since builtins can only be injected over preexisting native symbols.
setBuildLanguageOptions("--experimental_builtins_bzl_path=tools/builtins");
scratch.file(
"tools/builtins/exports.bzl",
"",
"coverage_common = _builtins.toplevel.coverage_common",
"",
"def _impl(ctx):",
" file1 = ctx.actions.declare_file(ctx.label.name + '.file1')",
" ctx.actions.write(file1, '')",
" file2 = ctx.actions.declare_file(ctx.label.name + '.file2')",
" ctx.actions.write(file2, '')",
" return coverage_common.instrumented_files_info(",
" ctx,",
" coverage_support_files = [depset([file1]), file2, ctx.attr.tool.files_to_run],",
" )",
"",
"overridden_extra_action = rule(",
" implementation = _impl,",
" attrs = {'tool': attr.label(cfg = 'exec', executable = True)},",
")",
"",
"exported_toplevels = {}",
"exported_rules = {'+extra_action': overridden_extra_action}",
"exported_to_java = {}");
scratch.file(
"test/starlark/tool_with_runfiles.bzl",
"def _impl(ctx):",
" exe = ctx.actions.declare_file(ctx.label.name)",
" ctx.actions.write(exe, '', is_executable = True)",
" data = ctx.actions.declare_file(ctx.label.name + '.data')",
" ctx.actions.write(data, '')",
" return DefaultInfo(executable = exe, runfiles = ctx.runfiles(files = [data]))",
"",
"tool_with_runfiles = rule(implementation = _impl)");
scratch.file(
"test/starlark/BUILD",
"load(':tool_with_runfiles.bzl', 'tool_with_runfiles')",
"tool_with_runfiles(name = 'tool')",
"extra_action(name = 'foo', tool = ':tool')");
scratch.file("test/starlark/bin.sh", "");

useConfiguration("--collect_code_coverage");

assertThat(
ActionsTestUtil.baseArtifactNames(
getConfiguredTarget("//test/starlark:foo")
.get(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR)
.getCoverageSupportFiles()))
.containsExactly("foo.file1", "foo.file2", "tool", "test_Sstarlark_Stool-runfiles");
}

@Test
public void testInstrumentedFilesInfo_coverageSupportAndEnvVarsArePrivateAPI() throws Exception {
// Arrange
Expand Down

0 comments on commit 062b83f

Please sign in to comment.