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 
Rolling forward 6afc2eb. This CL was only rolled back to make it easier to
rollback d0982b9.

RELNOTES:none
PiperOrigin-RevId: 199779304
  • Loading branch information
@oquenchil
oquenchil authored and Copybara-Service committed Jun 8, 2018
1 parent 2b86bee commit 6c87715
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 62 deletions.
27 changes: 25 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,9 @@
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.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 +74,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 +135,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 +361,20 @@ 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);
}
};
// TODO(plf): return empty CcLinkingInfo.
ccLinkingInfoBuilder.setCcLinkParamsStore(new CcLinkParamsStore(ccLinkParamsStore));
return ccLinkingInfoBuilder.build();
}
}
40 changes: 13 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,22 @@ 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())
.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 +152,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 6c87715

Please sign in to comment.