Skip to content

Commit

Permalink
Migrate InstrumentedFilesProvider to Starlark provider pattern
Browse files Browse the repository at this point in the history
Progress toward #6241

RELNOTES: None.
PiperOrigin-RevId: 222463226
  • Loading branch information
c-parsons authored and Copybara-Service committed Nov 21, 2018
1 parent b6f2ff1 commit b83c5f8
Show file tree
Hide file tree
Showing 26 changed files with 138 additions and 220 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
import com.google.devtools.build.lib.analysis.constraints.TopLevelConstraintSemantics;
import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory;
import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory.CoverageReportActionsWrapper;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand Down Expand Up @@ -528,7 +528,7 @@ private static NestedSet<Artifact> getBaselineCoverageArtifacts(
ArtifactsToOwnerLabels.Builder topLevelArtifactsToOwnerLabels) {
NestedSetBuilder<Artifact> baselineCoverageArtifacts = NestedSetBuilder.stableOrder();
for (ConfiguredTarget target : configuredTargets) {
InstrumentedFilesProvider provider = target.getProvider(InstrumentedFilesProvider.class);
InstrumentedFilesInfo provider = target.get(InstrumentedFilesInfo.SKYLARK_CONSTRUCTOR);
if (provider != null) {
TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
provider.getBaselineCoverageArtifacts(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import com.google.devtools.build.lib.analysis.test.AnalysisTestActionBuilder;
import com.google.devtools.build.lib.analysis.test.AnalysisTestResultInfo;
import com.google.devtools.build.lib.analysis.test.ExecutionInfo;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.analysis.test.TestActionBuilder;
import com.google.devtools.build.lib.analysis.test.TestEnvironmentInfo;
import com.google.devtools.build.lib.analysis.test.TestProvider;
Expand Down Expand Up @@ -269,7 +269,10 @@ private TestProvider initializeTestProvider(FilesToRunProvider filesToRunProvide
}
TestActionBuilder testActionBuilder =
new TestActionBuilder(ruleContext)
.setInstrumentedFiles(providersBuilder.getProvider(InstrumentedFilesProvider.class));
.setInstrumentedFiles(
(InstrumentedFilesInfo)
providersBuilder.getProvider(
InstrumentedFilesInfo.SKYLARK_CONSTRUCTOR.getKey()));

TestEnvironmentInfo environmentProvider =
(TestEnvironmentInfo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
import com.google.devtools.build.lib.analysis.test.TestProvider;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
Expand Down Expand Up @@ -152,8 +152,8 @@ private TargetCompleteEvent(
isTest
? targetAndData.getConfiguredTarget().getProvider(TestProvider.class).getTestParams()
: null;
InstrumentedFilesProvider instrumentedFilesProvider =
targetAndData.getConfiguredTarget().getProvider(InstrumentedFilesProvider.class);
InstrumentedFilesInfo instrumentedFilesProvider =
targetAndData.getConfiguredTarget().get(InstrumentedFilesInfo.SKYLARK_CONSTRUCTOR);
if (instrumentedFilesProvider == null) {
this.baselineCoverageArtifacts = null;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap;
import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMapBuilder;
import com.google.devtools.build.lib.analysis.VisibilityProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand All @@ -33,6 +33,7 @@
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.util.FileType;
import javax.annotation.Nullable;

/**
* A ConfiguredTarget for a source FileTarget. (Generated files use a subclass,
Expand All @@ -47,7 +48,8 @@ public abstract class FileConfiguredTarget extends AbstractConfiguredTarget
Label label,
BuildConfigurationValue.Key configurationKey,
NestedSet<PackageGroupContents> visibility,
Artifact artifact) {
Artifact artifact,
@Nullable InstrumentedFilesInfo instrumentedFilesInfo) {
super(label, configurationKey, visibility);
NestedSet<Artifact> filesToBuild = NestedSetBuilder.create(Order.STABLE_ORDER, artifact);
FileProvider fileProvider = new FileProvider(filesToBuild);
Expand All @@ -59,8 +61,9 @@ public abstract class FileConfiguredTarget extends AbstractConfiguredTarget
.put(LicensesProvider.class, this)
.add(fileProvider)
.add(filesToRunProvider);
if (this instanceof InstrumentedFilesProvider) {
builder.put(InstrumentedFilesProvider.class, this);

if (instrumentedFilesInfo != null) {
builder.put(instrumentedFilesInfo);
}
this.providers = builder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public final class InputFileConfiguredTarget extends FileConfiguredTarget implem
NestedSet<PackageGroupContents> visibility,
SourceArtifact artifact,
NestedSet<TargetLicense> licenses) {
super(label, null, visibility, artifact);
super(label, null, visibility, artifact, null);
this.artifact = artifact;
this.licenses = licenses;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
import com.google.devtools.build.lib.analysis.TargetContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProviderImpl;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.OutputFile;
Expand All @@ -32,12 +31,10 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Instantiator;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.util.Pair;

/** A ConfiguredTarget for an OutputFile. */
@AutoCodec
public class OutputFileConfiguredTarget extends FileConfiguredTarget
implements InstrumentedFilesProvider {
public class OutputFileConfiguredTarget extends FileConfiguredTarget {

private final Artifact artifact;
private final TransitiveInfoCollection generatingRule;
Expand All @@ -62,11 +59,18 @@ public OutputFileConfiguredTarget(
NestedSet<PackageGroupContents> visibility,
Artifact artifact,
TransitiveInfoCollection generatingRule) {
super(label, configurationKey, visibility, artifact);
super(label, configurationKey, visibility, artifact, instrumentedFilesInfo(generatingRule));
this.artifact = artifact;
this.generatingRule = Preconditions.checkNotNull(generatingRule);
}

private static InstrumentedFilesInfo instrumentedFilesInfo(
TransitiveInfoCollection generatingRule) {
Preconditions.checkNotNull(generatingRule);
InstrumentedFilesInfo provider = generatingRule.get(InstrumentedFilesInfo.SKYLARK_CONSTRUCTOR);
return provider == null ? InstrumentedFilesInfo.EMPTY : provider;
}

public TransitiveInfoCollection getGeneratingRule() {
return generatingRule;
}
Expand Down Expand Up @@ -94,49 +98,6 @@ public boolean hasOutputLicenses() {
.hasOutputLicenses();
}


@Override
public NestedSet<Artifact> getInstrumentedFiles() {
return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
.getInstrumentedFiles();
}

@Override
public NestedSet<Artifact> getInstrumentationMetadataFiles() {
return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
.getInstrumentationMetadataFiles();
}

@Override
public NestedSet<Artifact> getBaselineCoverageInstrumentedFiles() {
return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
.getBaselineCoverageInstrumentedFiles();
}

@Override
public NestedSet<Artifact> getBaselineCoverageArtifacts() {
return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
.getBaselineCoverageArtifacts();
}

@Override
public NestedSet<Artifact> getCoverageSupportFiles() {
return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
.getCoverageSupportFiles();
}

@Override
public NestedSet<Pair<String, String>> getCoverageEnvironment() {
return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
.getCoverageEnvironment();
}

@Override
public NestedSet<Pair<String, String>> getReportedToActualSources() {
return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
.getReportedToActualSources();
}

/**
* Returns the corresponding provider from the generating rule, if it is non-null, or {@code
* defaultValue} otherwise.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import com.google.devtools.build.lib.analysis.Whitelist;
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.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
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;
Expand Down Expand Up @@ -260,14 +260,14 @@ private static void addInstrumentedFiles(
new InstrumentationSpec(fileTypeSet)
.withSourceAttributes(sourceAttributes.toArray(new String[0]))
.withDependencyAttributes(dependencyAttributes.toArray(new String[0]));
InstrumentedFilesProvider instrumentedFilesProvider =
InstrumentedFilesInfo instrumentedFilesProvider =
InstrumentedFilesCollector.collect(
ruleContext,
instrumentationSpec,
InstrumentedFilesCollector.NO_METADATA_COLLECTOR,
/* rootFiles= */ Collections.emptySet(),
/* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER));
builder.addProvider(InstrumentedFilesProvider.class, instrumentedFilesProvider);
builder.addNativeDeclaredProvider(instrumentedFilesProvider);
}

public static NestedSet<Artifact> convertToOutputGroupValue(Location loc, String outputGroup,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.stringtemplate.ExpansionException;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Aspect;
Expand Down Expand Up @@ -611,7 +611,7 @@ public boolean instrumentCoverage(Object targetUnchecked) throws EvalException {
return InstrumentedFilesCollector.shouldIncludeLocalSources(ruleContext);
}
TransitiveInfoCollection target = (TransitiveInfoCollection) targetUnchecked;
return (target.getProvider(InstrumentedFilesProvider.class) != null)
return (target.get(InstrumentedFilesInfo.SKYLARK_CONSTRUCTOR) != null)
&& InstrumentedFilesCollector.shouldIncludeLocalSources(config, target);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@
public final class InstrumentedFilesCollector {

/**
* Forwards any instrumented files from the given target's dependencies (as defined in
* {@code dependencyAttributes}) for further export. No files from this target are considered
* Forwards any instrumented files from the given target's dependencies (as defined in {@code
* dependencyAttributes}) for further export. No files from this target are considered
* instrumented.
*
* @return instrumented file provider of all dependencies in {@code dependencyAttributes}
*/
public static InstrumentedFilesProvider forward(
public static InstrumentedFilesInfo forward(
RuleContext ruleContext, String... dependencyAttributes) {
return collect(
ruleContext,
Expand All @@ -58,7 +58,7 @@ public static InstrumentedFilesProvider forward(
/* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER));
}

public static InstrumentedFilesProvider collect(
public static InstrumentedFilesInfo collect(
RuleContext ruleContext,
InstrumentationSpec spec,
LocalMetadataCollector localMetadataCollector,
Expand All @@ -71,7 +71,7 @@ public static InstrumentedFilesProvider collect(
/* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER));
}

public static InstrumentedFilesProvider collect(
public static InstrumentedFilesInfo collect(
RuleContext ruleContext,
InstrumentationSpec spec,
LocalMetadataCollector localMetadataCollector,
Expand All @@ -95,7 +95,7 @@ public static InstrumentedFilesProvider collect(
* coverage actions for the transitive closure of source files (if <code>withBaselineCoverage
* </code> is true).
*/
public static InstrumentedFilesProvider collect(
public static InstrumentedFilesInfo collect(
RuleContext ruleContext,
InstrumentationSpec spec,
LocalMetadataCollector localMetadataCollector,
Expand All @@ -114,7 +114,7 @@ public static InstrumentedFilesProvider collect(
/* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER));
}

public static InstrumentedFilesProvider collect(
public static InstrumentedFilesInfo collect(
RuleContext ruleContext,
InstrumentationSpec spec,
LocalMetadataCollector localMetadataCollector,
Expand All @@ -127,7 +127,7 @@ public static InstrumentedFilesProvider collect(
Preconditions.checkNotNull(spec);

if (!ruleContext.getConfiguration().isCodeCoverageEnabled()) {
return InstrumentedFilesProviderImpl.EMPTY;
return InstrumentedFilesInfo.EMPTY;
}

NestedSetBuilder<Artifact> instrumentedFilesBuilder = NestedSetBuilder.stableOrder();
Expand All @@ -145,7 +145,7 @@ public static InstrumentedFilesProvider collect(
// Transitive instrumentation data.
for (TransitiveInfoCollection dep :
getAllPrerequisites(ruleContext, spec.dependencyAttributes)) {
InstrumentedFilesProvider provider = dep.getProvider(InstrumentedFilesProvider.class);
InstrumentedFilesInfo provider = dep.get(InstrumentedFilesInfo.SKYLARK_CONSTRUCTOR);
if (provider != null) {
instrumentedFilesBuilder.addTransitive(provider.getInstrumentedFiles());
metadataFilesBuilder.addTransitive(provider.getInstrumentationMetadataFiles());
Expand All @@ -162,7 +162,7 @@ public static InstrumentedFilesProvider collect(
NestedSetBuilder<Artifact> localSourcesBuilder = NestedSetBuilder.stableOrder();
for (TransitiveInfoCollection dep :
getAllPrerequisites(ruleContext, spec.sourceAttributes)) {
if (!spec.splitLists && dep.getProvider(InstrumentedFilesProvider.class) != null) {
if (!spec.splitLists && dep.get(InstrumentedFilesInfo.SKYLARK_CONSTRUCTOR) != null) {
continue;
}
for (Artifact artifact : dep.getProvider(FileProvider.class).getFilesToBuild()) {
Expand Down Expand Up @@ -194,7 +194,7 @@ public static InstrumentedFilesProvider collect(
// Create one baseline coverage action per target, but for the transitive closure of files.
NestedSet<Artifact> baselineCoverageArtifacts =
BaselineCoverageAction.create(ruleContext, baselineCoverageFiles);
return new InstrumentedFilesProviderImpl(
return new InstrumentedFilesInfo(
instrumentedFilesBuilder.build(),
metadataFilesBuilder.build(),
baselineCoverageFiles,
Expand Down
Loading

0 comments on commit b83c5f8

Please sign in to comment.