Skip to content

Commit

Permalink
python: port PyInfo to Starlark and enable it.
Browse files Browse the repository at this point in the history
Because providers are based on identity, and there are Java tests that need
to reference the provider, it's hard to split up the changes, so it's all
in a single change.

A side-effect of this is the name used to access the provider in `cquery` commands
changes slightly: from "PyInfo" to "@_builtins//:common/python/providers.bzl%PyInfo". This fully qualified name is considered internal and shouldn't be relied upon; it will change again when
the rules are removed from Bazel and part of rules_python.

The Java class is kept to make usage by the Java tests easier; it isn't
actually used outside of tests.

Work towards bazelbuild#15897

PiperOrigin-RevId: 524054712
Change-Id: I1ab7d299c7b0458e0ee5359babb1fa63f9739498
  • Loading branch information
rickeylev authored and fweikert committed May 25, 2023
1 parent 65d8860 commit edd3fc2
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 362 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@
import com.google.devtools.build.lib.rules.proto.BazelProtoLibraryRule;
import com.google.devtools.build.lib.rules.proto.ProtoConfiguration;
import com.google.devtools.build.lib.rules.proto.ProtoLangToolchainRule;
import com.google.devtools.build.lib.rules.python.PyInfo;
import com.google.devtools.build.lib.rules.python.PyRuleClasses.PySymlink;
import com.google.devtools.build.lib.rules.python.PyRuntimeRule;
import com.google.devtools.build.lib.rules.python.PyStarlarkTransitions;
Expand Down Expand Up @@ -471,12 +470,9 @@ public void init(ConfiguredRuleClassProvider.Builder builder) {
ContextGuardedValue.onlyInAllowedRepos(
Starlark.NONE, PyBootstrap.allowedRepositories));
builder.addStarlarkBuiltinsInternal(BazelPyBuiltins.NAME, new BazelPyBuiltins());

builder.addStarlarkBootstrap(
new PyBootstrap(
PyInfo.PROVIDER,
PyStarlarkTransitions.INSTANCE,
new GoogleLegacyStubs.PyWrapCcHelper()));
PyStarlarkTransitions.INSTANCE, new GoogleLegacyStubs.PyWrapCcHelper()));

builder.addSymlinkDefinition(PySymlink.PY2);
builder.addSymlinkDefinition(PySymlink.PY3);
Expand Down
264 changes: 25 additions & 239 deletions src/main/java/com/google/devtools/build/lib/rules/python/PyInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,277 +14,63 @@

package com.google.devtools.build.lib.rules.python;

import com.google.common.base.Preconditions;
import com.google.common.annotations.VisibleForTesting;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.Depset;
import com.google.devtools.build.lib.collect.nestedset.Depset.TypeException;
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.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.Info;
import com.google.devtools.build.lib.starlarkbuildapi.python.PyInfoApi;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Objects;
import javax.annotation.Nullable;
import com.google.devtools.build.lib.packages.StarlarkInfo;
import com.google.devtools.build.lib.packages.StarlarkProviderWrapper;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.syntax.Location;

/** Instance of the provider type for the Python rules. */
public final class PyInfo implements Info, PyInfoApi<Artifact> {

public static final String STARLARK_NAME = "PyInfo";

@VisibleForTesting
public final class PyInfo {
public static final PyInfoProvider PROVIDER = new PyInfoProvider();

/**
* Returns true if the given depset has the given content type and order compatible with the given
* order.
*/
private static boolean depsetHasTypeAndCompatibleOrder(
Depset depset, Depset.ElementType type, Order order) {
// Work around #7266 by special-casing the empty set in the type check.
boolean typeOk = depset.isEmpty() || depset.getElementType().equals(type);
boolean orderOk = depset.getOrder().isCompatible(order);
return typeOk && orderOk;
}
private final StarlarkInfo info;

/**
* Returns the type name of a value and possibly additional description.
*
* <p>For depsets, this includes its content type and order.
*/
private static String describeType(Object value) {
if (value instanceof Depset) {
Depset depset = (Depset) value;
return depset.getOrder().getStarlarkName()
+ "-ordered depset of "
+ depset.getElementType()
+ "s";
}
return Starlark.type(value);
private PyInfo(StarlarkInfo info) {
this.info = info;
}

private final Location location;
// Verified on initialization to contain Artifact.
private final Depset transitiveSources;
private final boolean usesSharedLibraries;
// Verified on initialization to contain String.
private final Depset imports;
private final boolean hasPy2OnlySources;
private final boolean hasPy3OnlySources;

private PyInfo(
@Nullable Location location,
Depset transitiveSources,
boolean usesSharedLibraries,
Depset imports,
boolean hasPy2OnlySources,
boolean hasPy3OnlySources) {
Preconditions.checkArgument(
depsetHasTypeAndCompatibleOrder(transitiveSources, Artifact.TYPE, Order.COMPILE_ORDER));
// TODO(brandjon): PyCommon currently requires COMPILE_ORDER, but we'll probably want to change
// that to NAIVE_LINK (preorder). In the meantime, order isn't an invariant of the provider
// itself, so we use STABLE here to accept any order.
Preconditions.checkArgument(
depsetHasTypeAndCompatibleOrder(imports, Depset.ElementType.STRING, Order.STABLE_ORDER));
this.location = location != null ? location : Location.BUILTIN;
this.transitiveSources = transitiveSources;
this.usesSharedLibraries = usesSharedLibraries;
this.imports = imports;
this.hasPy2OnlySources = hasPy2OnlySources;
this.hasPy3OnlySources = hasPy3OnlySources;
}

@Override
public PyInfoProvider getProvider() {
return PROVIDER;
}

@Override
public Location getCreationLocation() {
return location;
public NestedSet<Artifact> getTransitiveSourcesSet() throws EvalException {
Object value = info.getValue("transitive_sources");
return Depset.cast(value, Artifact.class, "transitive_sources");
}

@Override
public boolean equals(Object other) {
// PyInfo implements value equality, but note that it contains identity-equality fields
// (depsets), so you generally shouldn't rely on equality comparisons.
if (!(other instanceof PyInfo)) {
return false;
}
PyInfo otherInfo = (PyInfo) other;
return (this.transitiveSources.equals(otherInfo.transitiveSources)
&& this.usesSharedLibraries == otherInfo.usesSharedLibraries
&& this.imports.equals(otherInfo.imports)
&& this.hasPy2OnlySources == otherInfo.hasPy2OnlySources
&& this.hasPy3OnlySources == otherInfo.hasPy3OnlySources);
public boolean getUsesSharedLibraries() throws EvalException {
return info.getValue("uses_shared_libraries", Boolean.class);
}

@Override
public int hashCode() {
return Objects.hash(
PyInfo.class,
transitiveSources,
usesSharedLibraries,
imports,
hasPy2OnlySources,
hasPy3OnlySources);
public NestedSet<String> getImportsSet() throws EvalException {
Object value = info.getValue("imports");
return Depset.cast(value, String.class, "imports");
}

@Override
public Depset getTransitiveSources() {
return transitiveSources;
}

public NestedSet<Artifact> getTransitiveSourcesSet() {
try {
return transitiveSources.getSet(Artifact.class);
} catch (TypeException e) {
throw new IllegalStateException(
"'transitiveSources' depset was found to be invalid type " + imports.getElementType(), e);
}
public boolean getHasPy2OnlySources() throws EvalException {
return info.getValue("has_py2_only_sources", Boolean.class);
}

@Override
public boolean getUsesSharedLibraries() {
return usesSharedLibraries;
}

@Override
public Depset getImports() {
return imports;
}

public NestedSet<String> getImportsSet() {
try {
return imports.getSet(String.class);
} catch (TypeException e) {
throw new IllegalStateException(
"'imports' depset was found to be invalid type " + imports.getElementType(), e);
}
}

@Override
public boolean getHasPy2OnlySources() {
return hasPy2OnlySources;
}

@Override
public boolean getHasPy3OnlySources() {
return hasPy3OnlySources;
public boolean getHasPy3OnlySources() throws EvalException {
return info.getValue("has_py3_only_sources", Boolean.class);
}

/** The singular PyInfo provider type object. */
public static class PyInfoProvider extends BuiltinProvider<PyInfo>
implements PyInfoApi.PyInfoProviderApi {
public static class PyInfoProvider extends StarlarkProviderWrapper<PyInfo> {

private PyInfoProvider() {
super(STARLARK_NAME, PyInfo.class);
super(Label.parseCanonicalUnchecked("@_builtins//:common/python/providers.bzl"), "PyInfo");
}

@Override
public PyInfo constructor(
Depset transitiveSources,
boolean usesSharedLibraries,
Object importsUncast,
boolean hasPy2OnlySources,
boolean hasPy3OnlySources,
StarlarkThread thread)
throws EvalException {
Depset imports =
importsUncast.equals(Starlark.UNBOUND)
? Depset.of(String.class, NestedSetBuilder.emptySet(Order.COMPILE_ORDER))
: (Depset) importsUncast;

if (!depsetHasTypeAndCompatibleOrder(transitiveSources, Artifact.TYPE, Order.COMPILE_ORDER)) {
throw Starlark.errorf(
"'transitive_sources' field should be a postorder-compatible depset of Files (got a"
+ " '%s')",
describeType(transitiveSources));
}
if (!depsetHasTypeAndCompatibleOrder(
imports, Depset.ElementType.STRING, Order.STABLE_ORDER)) {
throw Starlark.errorf(
"'imports' field should be a depset of strings (got a '%s')", describeType(imports));
}
// Validate depset parameters
Depset.cast(transitiveSources, Artifact.class, "transitive_sources");
Depset.cast(imports, String.class, "imports");

return new PyInfo(
thread.getCallerLocation(),
transitiveSources,
usesSharedLibraries,
imports,
hasPy2OnlySources,
hasPy3OnlySources);
}
}

public static Builder builder() {
return new Builder();
}

/** Builder for {@link PyInfo}. */
public static class Builder {
Location location = null;
NestedSet<Artifact> transitiveSources = NestedSetBuilder.emptySet(Order.COMPILE_ORDER);
boolean usesSharedLibraries = false;
NestedSet<String> imports = NestedSetBuilder.emptySet(Order.COMPILE_ORDER);
boolean hasPy2OnlySources = false;
boolean hasPy3OnlySources = false;

// Use the static builder() method instead.
private Builder() {}

@CanIgnoreReturnValue
public Builder setLocation(Location location) {
this.location = location;
return this;
}

@CanIgnoreReturnValue
public Builder setTransitiveSources(NestedSet<Artifact> transitiveSources) {
this.transitiveSources = transitiveSources;
return this;
}

@CanIgnoreReturnValue
public Builder setUsesSharedLibraries(boolean usesSharedLibraries) {
this.usesSharedLibraries = usesSharedLibraries;
return this;
}

@CanIgnoreReturnValue
public Builder setImports(NestedSet<String> imports) {
this.imports = imports;
return this;
}

@CanIgnoreReturnValue
public Builder setHasPy2OnlySources(boolean hasPy2OnlySources) {
this.hasPy2OnlySources = hasPy2OnlySources;
return this;
}

@CanIgnoreReturnValue
public Builder setHasPy3OnlySources(boolean hasPy3OnlySources) {
this.hasPy3OnlySources = hasPy3OnlySources;
return this;
}

public PyInfo build() {
Preconditions.checkNotNull(transitiveSources);
return new PyInfo(
location,
Depset.of(Artifact.class, transitiveSources),
usesSharedLibraries,
Depset.of(String.class, imports),
hasPy2OnlySources,
hasPy3OnlySources);
public PyInfo wrap(Info value) {
return new PyInfo((StarlarkInfo) value);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.devtools.build.lib.starlarkbuildapi.core.Bootstrap;
import com.google.devtools.build.lib.starlarkbuildapi.core.ContextAndFlagGuardedValue;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.PyWrapCcHelperApi;
import com.google.devtools.build.lib.starlarkbuildapi.python.PyInfoApi.PyInfoProviderApi;
import com.google.devtools.build.lib.starlarkbuildapi.stubs.ProviderStub;
import net.starlark.java.eval.FlagGuardedValue;

Expand All @@ -34,15 +33,12 @@ public class PyBootstrap implements Bootstrap {
PackageIdentifier.createUnchecked("rules_python", ""),
PackageIdentifier.createUnchecked("", "tools/build_defs/python"));

private final PyInfoProviderApi pyInfoProviderApi;
private final PyStarlarkTransitionsApi pyStarlarkTransitionsApi;
private final PyWrapCcHelperApi<?, ?, ?, ?, ?, ?, ?, ?, ?> pyWrapCcHelper;

public PyBootstrap(
PyInfoProviderApi pyInfoProviderApi,
PyStarlarkTransitionsApi pyStarlarkTransitionsApi,
PyWrapCcHelperApi<?, ?, ?, ?, ?, ?, ?, ?, ?> pyWrapCcHelper) {
this.pyInfoProviderApi = pyInfoProviderApi;
this.pyStarlarkTransitionsApi = pyStarlarkTransitionsApi;
this.pyWrapCcHelper = pyWrapCcHelper;
}
Expand All @@ -53,7 +49,8 @@ public void addBindingsToBuilder(ImmutableMap.Builder<String, Object> builder) {
"PyInfo",
ContextAndFlagGuardedValue.onlyInAllowedReposOrWhenIncompatibleFlagIsFalse(
BuildLanguageOptions.INCOMPATIBLE_STOP_EXPORTING_LANGUAGE_MODULES,
pyInfoProviderApi,
// Workaround for https://github.com/bazelbuild/bazel/issues/17713
new ProviderStub(),
allowedRepositories));
builder.put(
"PyRuntimeInfo",
Expand Down
10 changes: 6 additions & 4 deletions src/main/starlark/builtins_bzl/common/python/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,14 @@ def _PyInfo_init(
has_py2_only_sources = False,
has_py3_only_sources = False):
_check_arg_type("transitive_sources", "depset", transitive_sources)

# Verify it's postorder compatible, but retain is original ordering.
depset(transitive = [transitive_sources], order = "postorder")

_check_arg_type("uses_shared_libraries", "bool", uses_shared_libraries)
_check_arg_type("imports", "depset", imports)
_check_arg_type("has_py2_only_sources", "bool", has_py2_only_sources)
_check_arg_type("has_py2_only_sources", "bool", has_py3_only_sources)
_check_arg_type("has_py3_only_sources", "bool", has_py3_only_sources)
return {
"transitive_sources": transitive_sources,
"imports": imports,
Expand All @@ -166,7 +170,7 @@ def _PyInfo_init(
"has_py3_only_sources": has_py2_only_sources,
}

StarlarkPyInfo, _unused_raw_py_info_ctor = provider(
PyInfo, _unused_raw_py_info_ctor = provider(
"Encapsulates information provided by the Python rules.",
init = _PyInfo_init,
fields = {
Expand All @@ -191,8 +195,6 @@ is recommended to use `default` order (the default).
},
)

PyInfo = _builtins.toplevel.PyInfo

def _PyCcLinkParamsProvider_init(cc_info):
return {
"cc_info": _CcInfo(linking_context = cc_info.linking_context),
Expand Down
Loading

0 comments on commit edd3fc2

Please sign in to comment.