Skip to content

Commit

Permalink
Remove FilesToCompileProvider and CompilationPrerequisitesProvider an…
Browse files Browse the repository at this point in the history
…d replace them with output groups.

--
MOS_MIGRATED_REVID=87038548
  • Loading branch information
lberki authored and hanwen committed Feb 24, 2015
1 parent ff03154 commit 3f4d4e9
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,9 @@ private void addExtraActionsIfRequested(BuildView.Options viewOptions,
private static void scheduleTestsIfRequested(Collection<ConfiguredTarget> targetsToTest,
Collection<ConfiguredTarget> targetsToTestExclusive, TopLevelArtifactContext topLevelOptions,
Collection<ConfiguredTarget> allTestTargets) {
if (!topLevelOptions.compileOnly() && !topLevelOptions.compilationPrerequisitesOnly()
Set<String> outputGroups = topLevelOptions.outputGroups();
if (!outputGroups.contains(TopLevelArtifactProvider.FILES_TO_COMPILE)
&& !outputGroups.contains(TopLevelArtifactProvider.COMPILATION_PREREQUISITES)
&& allTestTargets != null) {
scheduleTests(targetsToTest, targetsToTestExclusive, allTestTargets,
topLevelOptions.runTestsExclusively());
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.TreeMap;

/**
* Builder class for analyzed rule instances (i.e., instances of {@link ConfiguredTarget}).
Expand All @@ -65,8 +66,7 @@ public final class RuleConfiguredTargetBuilder {
private final Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> providers =
new LinkedHashMap<>();
private final ImmutableMap.Builder<String, Object> skylarkProviders = ImmutableMap.builder();
private final ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups =
ImmutableMap.builder();
private final Map<String, NestedSetBuilder<Artifact>> outputGroupBuilders = new TreeMap<>();

/** These are supported by all configured targets and need to be specially handled. */
private NestedSet<Artifact> filesToBuild = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
Expand All @@ -92,9 +92,13 @@ public ConfiguredTarget build() {
return null;
}

ImmutableMap<String, NestedSet<Artifact>> outputGroupMap = outputGroups.build();
if (!outputGroupMap.isEmpty()) {
add(TopLevelArtifactProvider.class, new TopLevelArtifactProvider(outputGroupMap));
if (!outputGroupBuilders.isEmpty()) {
ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups = ImmutableMap.builder();
for (Map.Entry<String, NestedSetBuilder<Artifact>> entry : outputGroupBuilders.entrySet()) {
outputGroups.put(entry.getKey(), entry.getValue().build());
}

add(TopLevelArtifactProvider.class, new TopLevelArtifactProvider(outputGroups.build()));
}

FilesToRunProvider filesToRunProvider = new FilesToRunProvider(ruleContext.getLabel(),
Expand Down Expand Up @@ -401,19 +405,41 @@ public RuleConfiguredTargetBuilder setFilesToBuild(NestedSet<Artifact> filesToBu
return this;
}

private NestedSetBuilder<Artifact> getOutputGroupBuilder(String name) {
NestedSetBuilder<Artifact> result = outputGroupBuilders.get(name);
if (result != null) {
return result;
}

result = NestedSetBuilder.<Artifact>stableOrder();
outputGroupBuilders.put(name, result);
return result;
}

/**
* Add an output group.
* Adds a set of files to an output group.
*/
public RuleConfiguredTargetBuilder addOutputGroup(String name, NestedSet<Artifact> artifacts) {
outputGroups.put(name, artifacts);
getOutputGroupBuilder(name).addTransitive(artifacts);
return this;
}

/**
* Add an output group.
* Adds a file to an output group.
*/
public RuleConfiguredTargetBuilder addOutputGroup(String name, Artifact artifact) {
outputGroups.put(name, NestedSetBuilder.create(Order.STABLE_ORDER, artifact));
getOutputGroupBuilder(name).add(artifact);
return this;
}

/**
* Adds multiple output groups.
*/
public RuleConfiguredTargetBuilder addOutputGroups(Map<String, NestedSet<Artifact>> groups) {
for (Map.Entry<String, NestedSet<Artifact>> group : groups.entrySet()) {
getOutputGroupBuilder(group.getKey()).addTransitive(group.getValue());
}

return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,18 @@
public final class TopLevelArtifactContext {

public static final TopLevelArtifactContext DEFAULT = new TopLevelArtifactContext(
"", /*compileOnly=*/false, /*compilationPrerequisitesOnly*/false,
/*buildDefaultArtifacts=*/true, /*runTestsExclusively=*/false,
"", /*buildDefaultArtifacts=*/true, /*runTestsExclusively=*/false,
/*outputGroups=*/ImmutableSet.<String>of(), /*shouldRunTests=*/false);

private final String buildCommand;
private final boolean compileOnly;
private final boolean compilationPrerequisitesOnly;
private final boolean buildDefaultArtifacts;
private final boolean runTestsExclusively;
private final ImmutableSet<String> outputGroups;
private final boolean shouldRunTests;

public TopLevelArtifactContext(String buildCommand, boolean compileOnly,
boolean compilationPrerequisitesOnly, boolean filesToRun, boolean runTestsExclusively,
ImmutableSet<String> outputGroups, boolean shouldRunTests) {
public TopLevelArtifactContext(String buildCommand, boolean filesToRun,
boolean runTestsExclusively, ImmutableSet<String> outputGroups, boolean shouldRunTests) {
this.buildCommand = buildCommand;
this.compileOnly = compileOnly;
this.compilationPrerequisitesOnly = compilationPrerequisitesOnly;
this.buildDefaultArtifacts = filesToRun;
this.runTestsExclusively = runTestsExclusively;
this.outputGroups = outputGroups;
Expand All @@ -56,16 +50,6 @@ public String buildCommand() {
return buildCommand;
}

/** Returns the value of the --compile_only flag. */
public boolean compileOnly() {
return compileOnly;
}

/** Returns the value of the --compilation_prerequisites_only flag. */
public boolean compilationPrerequisitesOnly() {
return compilationPrerequisitesOnly;
}

/** Returns the value of the (undocumented) --build_default_artifacts flag. */
public boolean buildDefaultArtifacts() {
return buildDefaultArtifacts;
Expand Down Expand Up @@ -93,8 +77,6 @@ public boolean equals(Object other) {
if (other instanceof TopLevelArtifactContext) {
TopLevelArtifactContext otherContext = (TopLevelArtifactContext) other;
return buildCommand.equals(otherContext.buildCommand)
&& compileOnly == otherContext.compileOnly
&& compilationPrerequisitesOnly == otherContext.compilationPrerequisitesOnly
&& buildDefaultArtifacts == otherContext.buildDefaultArtifacts
&& runTestsExclusively == otherContext.runTestsExclusively
&& outputGroups.equals(otherContext.outputGroups)
Expand All @@ -106,7 +88,7 @@ public boolean equals(Object other) {

@Override
public int hashCode() {
return Objects.hash(buildCommand, compileOnly, compilationPrerequisitesOnly,
buildDefaultArtifacts, runTestsExclusively, outputGroups, shouldRunTests);
return Objects.hash(
buildCommand, buildDefaultArtifacts, runTestsExclusively, outputGroups, shouldRunTests);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,18 +128,9 @@ public static ArtifactsToBuild getAllArtifactsToBuild(TransitiveInfoCollection t
}
}

if (context.compileOnly()) {
FilesToCompileProvider provider = target.getProvider(FilesToCompileProvider.class);
if (provider != null) {
importantBuilder.addAll(provider.getFilesToCompile());
}
} else if (context.compilationPrerequisitesOnly()) {
CompilationPrerequisitesProvider provider =
target.getProvider(CompilationPrerequisitesProvider.class);
if (provider != null) {
importantBuilder.addTransitive(provider.getCompilationPrerequisites());
}
} else if (context.buildDefaultArtifacts()) {
if (context.buildDefaultArtifacts()
&& !context.outputGroups().contains(TopLevelArtifactProvider.COMPILATION_PREREQUISITES)
&& !context.outputGroups().contains(TopLevelArtifactProvider.FILES_TO_COMPILE)) {
FilesToRunProvider filesToRunProvider = target.getProvider(FilesToRunProvider.class);
boolean hasRunfilesSupport = false;
if (filesToRunProvider != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
public final class TopLevelArtifactProvider implements TransitiveInfoProvider {
public static final String HIDDEN_OUTPUT_GROUP_PREFIX = "_";
public static final String BASELINE_COVERAGE = HIDDEN_OUTPUT_GROUP_PREFIX + "_baseline_coverage";
public static final String FILES_TO_COMPILE = "files_to_compile";
public static final String COMPILATION_PREREQUISITES = "compilation_prerequisites";

private final ImmutableMap<String, NestedSet<Artifact>> outputGroups;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,6 @@ public List<String> validateOptions() throws InvalidConfigurationException {
/** Creates a new TopLevelArtifactContext from this build request. */
public TopLevelArtifactContext getTopLevelArtifactContext() {
return new TopLevelArtifactContext(getCommandName(),
getBuildOptions().compileOnly, getBuildOptions().compilationPrerequisitesOnly,
getBuildOptions().buildDefaultArtifacts,
getOptions(ExecutionOptions.class).testStrategy.equals("exclusive"),
determineOutputGroups(), shouldRunTests());
Expand All @@ -529,6 +528,14 @@ private ImmutableSet<String> determineOutputGroups() {
current.add(TopLevelArtifactProvider.BASELINE_COVERAGE);
}

if (getBuildOptions().compileOnly) {
current.add(TopLevelArtifactProvider.FILES_TO_COMPILE);
}

if (getBuildOptions().compilationPrerequisitesOnly) {
current.add(TopLevelArtifactProvider.COMPILATION_PREREQUISITES);
}

return ImmutableSet.copyOf(current);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.CompilationPrerequisitesProvider;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToCompileProvider;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TempsProvider;
import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand Down Expand Up @@ -594,7 +593,7 @@ private List<PathFragment> getIncludeDirsFromIncludesAttribute() {
/**
* Collects compilation prerequisite artifacts.
*/
static CompilationPrerequisitesProvider collectCompilationPrerequisites(
static NestedSet<Artifact> collectCompilationPrerequisites(
RuleContext ruleContext, CppCompilationContext context) {
// TODO(bazel-team): Use context.getCompilationPrerequisites() instead.
NestedSetBuilder<Artifact> prerequisites = NestedSetBuilder.stableOrder();
Expand All @@ -605,7 +604,7 @@ static CompilationPrerequisitesProvider collectCompilationPrerequisites(
}
}
prerequisites.addTransitive(context.getDeclaredIncludeSrcs());
return new CompilationPrerequisitesProvider(prerequisites.build());
return prerequisites.build();
}

/**
Expand Down Expand Up @@ -711,13 +710,13 @@ public void addTransitiveInfoProviders(RuleConfiguredTargetBuilder builder,
collectTransitiveCcNativeLibraries(ruleContext, linkingOutputs.getDynamicLibraries())))
.add(InstrumentedFilesProvider.class, getInstrumentedFilesProvider(
instrumentedObjectFiles))
.add(FilesToCompileProvider.class, new FilesToCompileProvider(
getFilesToCompile(ccCompilationOutputs)))
.add(CompilationPrerequisitesProvider.class,
collectCompilationPrerequisites(ruleContext, cppCompilationContext))
.add(TempsProvider.class, new TempsProvider(getTemps(ccCompilationOutputs)))
.add(CppDebugFileProvider.class, new CppDebugFileProvider(
dwoArtifacts.getDwoArtifacts(),
dwoArtifacts.getPicDwoArtifacts()));
dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts()))
.addOutputGroup(TopLevelArtifactProvider.FILES_TO_COMPILE,
NestedSetBuilder.wrap(Order.STABLE_ORDER, getFilesToCompile(ccCompilationOutputs)))
.addOutputGroup(TopLevelArtifactProvider.COMPILATION_PREREQUISITES,
collectCompilationPrerequisites(ruleContext, cppCompilationContext));

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ public LibraryToLink apply(Artifact library) {
targetBuilder
.setFilesToBuild(filesToBuild)
.addProviders(info.getProviders())
.addOutputGroups(info.getOutputGroups())
.add(InstrumentedFilesProvider.class, instrumentedFilesProvider)
.add(RunfilesProvider.class, RunfilesProvider.withData(staticRunfiles, sharedRunfiles))
// Remove this?
Expand Down
Loading

0 comments on commit 3f4d4e9

Please sign in to comment.