Skip to content

Commit

Permalink
C++: Refactors PyWrapCc to make it easier to migrate to Skylark
Browse files Browse the repository at this point in the history 
Rolls forward bazelbuild@c720152.

This CL includes testing to make sure that linker parameters coming from every possible provider in Python rules is propagated.

RELNOTES:none
PiperOrigin-RevId: 201160720
  • Loading branch information
@oquenchil
oquenchil authored and Copybara-Service committed Jun 19, 2018
1 parent f179ab8 commit 8cf6643
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 62 deletions.
29 changes: 27 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.analysis.ShToolchain;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction;
import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction.LaunchInfo;
Expand All @@ -40,6 +41,10 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.rules.cpp.AbstractCcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo;
import com.google.devtools.build.lib.rules.python.PyCcLinkParamsProvider;
import com.google.devtools.build.lib.rules.python.PyCommon;
import com.google.devtools.build.lib.rules.python.PythonConfiguration;
import com.google.devtools.build.lib.rules.python.PythonSemantics;
Expand Down Expand Up @@ -70,7 +75,10 @@ public void validate(RuleContext ruleContext, PyCommon common) {

@Override
public void collectRunfilesForBinary(
RuleContext ruleContext, Runfiles.Builder builder, PyCommon common) {
RuleContext ruleContext,
Runfiles.Builder builder,
PyCommon common,
CcLinkingInfo ccLinkingInfo) {
addRuntime(ruleContext, builder);
}

Expand Down Expand Up @@ -128,7 +136,7 @@ public Artifact getPythonTemplateMainArtifact(RuleContext ruleContext, Artifact
public Artifact createExecutable(
RuleContext ruleContext,
PyCommon common,
AbstractCcLinkParamsStore ccLinkParamsStore,
CcLinkingInfo ccLinkingInfo,
NestedSet<PathFragment> imports)
throws InterruptedException {
String main = common.determineMainExecutableSource(/*withWorkspaceName=*/ true);
Expand Down Expand Up @@ -354,4 +362,21 @@ private static String getPythonBinary(
return pythonBinary;
}

@Override
public CcLinkingInfo buildCcLinkingInfoProvider(
Iterable<? extends TransitiveInfoCollection> deps) {
CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create();
AbstractCcLinkParamsStore ccLinkParamsStore =
new AbstractCcLinkParamsStore() {
@Override
protected void collect(
CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) {
builder.addTransitiveTargets(
deps, CcLinkParamsStore.TO_LINK_PARAMS, PyCcLinkParamsProvider.TO_LINK_PARAMS);
}
};
// TODO(plf): return empty CcLinkingInfo.
ccLinkingInfoBuilder.setCcLinkParamsStore(new CcLinkParamsStore(ccLinkParamsStore));
return ccLinkingInfoBuilder.build();
}
}
41 changes: 14 additions & 27 deletions src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@
import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.rules.cpp.AbstractCcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -60,7 +57,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
static RuleConfiguredTargetBuilder init(RuleContext ruleContext, PythonSemantics semantics,
PyCommon common) throws InterruptedException {
ruleContext.initConfigurationMakeVariableContext(new CcFlagsSupplier(ruleContext));
AbstractCcLinkParamsStore ccLinkParamsStore = initializeCcLinkParamStore(ruleContext);

List<Artifact> srcs = common.validateSrcs();
List<Artifact> allOutputs =
Expand All @@ -80,9 +76,10 @@ static RuleConfiguredTargetBuilder init(RuleContext ruleContext, PythonSemantics
return null;
}

Artifact realExecutable =
semantics.createExecutable(ruleContext, common, ccLinkParamsStore, imports);
Runfiles commonRunfiles = collectCommonRunfiles(ruleContext, common, semantics);
CcLinkingInfo ccLinkingInfo =
semantics.buildCcLinkingInfoProvider(ruleContext.getPrerequisites("deps", Mode.TARGET));

Runfiles commonRunfiles = collectCommonRunfiles(ruleContext, common, semantics, ccLinkingInfo);

Runfiles.Builder defaultRunfilesBuilder = new Runfiles.Builder(
ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles())
Expand Down Expand Up @@ -124,19 +121,23 @@ static RuleConfiguredTargetBuilder init(RuleContext ruleContext, PythonSemantics

semantics.postInitBinary(ruleContext, runfilesSupport, common);

CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create();
ccLinkingInfoBuilder.setCcLinkParamsStore(new CcLinkParamsStore(ccLinkParamsStore));
Artifact realExecutable =
semantics.createExecutable(ruleContext, common, ccLinkingInfo, imports);

return builder
.setFilesToBuild(common.getFilesToBuild())
.add(RunfilesProvider.class, runfilesProvider)
.setRunfilesSupport(runfilesSupport, realExecutable)
.addNativeDeclaredProvider(ccLinkingInfoBuilder.build())
.addNativeDeclaredProvider(ccLinkingInfo)
.add(PythonImportsProvider.class, new PythonImportsProvider(imports));
}

private static Runfiles collectCommonRunfiles(RuleContext ruleContext, PyCommon common,
PythonSemantics semantics) {
private static Runfiles collectCommonRunfiles(
RuleContext ruleContext,
PyCommon common,
PythonSemantics semantics,
CcLinkingInfo ccLinkingInfo)
throws InterruptedException {
Runfiles.Builder builder = new Runfiles.Builder(
ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles());
builder.addArtifact(common.getExecutable());
Expand All @@ -152,21 +153,7 @@ private static Runfiles collectCommonRunfiles(RuleContext ruleContext, PyCommon
|| ruleContext.attributes().get("legacy_create_init", Type.BOOLEAN)) {
builder.setEmptyFilesSupplier(PythonUtils.GET_INIT_PY_FILES);
}
semantics.collectRunfilesForBinary(ruleContext, builder, common);
semantics.collectRunfilesForBinary(ruleContext, builder, common, ccLinkingInfo);
return builder.build();
}

private static AbstractCcLinkParamsStore initializeCcLinkParamStore(
final RuleContext ruleContext) {
return new AbstractCcLinkParamsStore() {
@Override
protected void collect(
CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) {
builder.addTransitiveTargets(
ruleContext.getPrerequisites("deps", Mode.TARGET),
PyCcLinkParamsProvider.TO_LINK_PARAMS,
CcLinkParamsStore.TO_LINK_PARAMS);
}
};
}
}
14 changes: 7 additions & 7 deletions src/main/java/com/google/devtools/build/lib/rules/python/PyCcLinkParamsProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,26 @@
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.rules.cpp.AbstractCcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;

/** A target that provides C++ libraries to be linked into Python targets. */
@Immutable
@AutoCodec
public final class PyCcLinkParamsProvider implements TransitiveInfoProvider {
private final CcLinkParamsStore store;
private final CcLinkingInfo ccLinkingInfo;

public PyCcLinkParamsProvider(CcLinkParamsStore store) {
this.store = store;
public PyCcLinkParamsProvider(CcLinkingInfo ccLinkingInfo) {
this.ccLinkingInfo = ccLinkingInfo;
}

public AbstractCcLinkParamsStore getLinkParams() {
return store;
public CcLinkingInfo getCcLinkingInfo() {
return ccLinkingInfo;
}

public static final Function<TransitiveInfoCollection, AbstractCcLinkParamsStore> TO_LINK_PARAMS =
input -> {
PyCcLinkParamsProvider provider = input.getProvider(PyCcLinkParamsProvider.class);
return provider == null ? null : provider.getLinkParams();
return provider == null ? null : provider.getCcLinkingInfo().getCcLinkParamsStore();
};
}
22 changes: 2 additions & 20 deletions src/main/java/com/google/devtools/build/lib/rules/python/PyLibrary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@
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.rules.cpp.AbstractCcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -64,18 +60,6 @@ public ConfiguredTarget create(final RuleContext ruleContext)
NestedSetBuilder.wrap(Order.STABLE_ORDER, allOutputs);
common.addPyExtraActionPseudoAction();

AbstractCcLinkParamsStore ccLinkParamsStore =
new AbstractCcLinkParamsStore() {
@Override
protected void collect(
CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) {
builder.addTransitiveTargets(
ruleContext.getPrerequisites("deps", Mode.TARGET),
PyCcLinkParamsProvider.TO_LINK_PARAMS,
CcLinkParamsStore.TO_LINK_PARAMS);
}
};

NestedSet<PathFragment> imports = common.collectImports(ruleContext, semantics);
if (ruleContext.hasErrors()) {
return null;
Expand All @@ -94,13 +78,11 @@ protected void collect(
RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext);
common.addCommonTransitiveInfoProviders(builder, semantics, filesToBuild);

CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create();
ccLinkingInfoBuilder.setCcLinkParamsStore(new CcLinkParamsStore(ccLinkParamsStore));

return builder
.setFilesToBuild(filesToBuild)
.addNativeDeclaredProvider(
semantics.buildCcLinkingInfoProvider(ruleContext.getPrerequisites("deps", Mode.TARGET)))
.add(RunfilesProvider.class, RunfilesProvider.simple(runfilesBuilder.build()))
.addNativeDeclaredProvider(ccLinkingInfoBuilder.build())
.add(PythonImportsProvider.class, new PythonImportsProvider(imports))
.build();
}
Expand Down
18 changes: 12 additions & 6 deletions src/main/java/com/google/devtools/build/lib/rules/python/PythonSemantics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
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.rules.cpp.AbstractCcLinkParamsStore;
import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Collection;
import java.util.List;
Expand All @@ -36,10 +37,13 @@ public interface PythonSemantics {
*/
void validate(RuleContext ruleContext, PyCommon common);

/**
* Extends for the default and data runfiles of {@code py_binary} rules with custom elements.
*/
void collectRunfilesForBinary(RuleContext ruleContext, Runfiles.Builder builder, PyCommon common);
/** Extends for the default and data runfiles of {@code py_binary} rules with custom elements. */
void collectRunfilesForBinary(
RuleContext ruleContext,
Runfiles.Builder builder,
PyCommon common,
CcLinkingInfo ccLinkingInfo)
throws InterruptedException;

/** Extends the default runfiles of {@code py_binary} rules with custom elements. */
void collectDefaultRunfilesForBinary(RuleContext ruleContext, Runfiles.Builder builder)
Expand Down Expand Up @@ -72,7 +76,7 @@ Collection<Artifact> precompiledPythonFiles(
Artifact createExecutable(
RuleContext ruleContext,
PyCommon common,
AbstractCcLinkParamsStore ccLinkParamsStore,
CcLinkingInfo ccLinkingInfo,
NestedSet<PathFragment> imports)
throws InterruptedException;

Expand All @@ -82,4 +86,6 @@ Artifact createExecutable(
*/
void postInitBinary(RuleContext ruleContext, RunfilesSupport runfilesSupport,
PyCommon common) throws InterruptedException;

CcLinkingInfo buildCcLinkingInfoProvider(Iterable<? extends TransitiveInfoCollection> deps);
}

0 comments on commit 8cf6643

Please sign in to comment.