Skip to content

Commit

Permalink
Rename the PyProvider helper class to PyStructUtils
Browse files Browse the repository at this point in the history 
In follow-up CLs, we'll introduce a new modern-style provider named PyInfo, and a new helper class PyProviderUtils to give a unified view for accessing either provider. The old name PyProvider would have been confusing.

Work toward bazelbuild#7010.

RELNOTES: None
PiperOrigin-RevId: 230759883
  • Loading branch information
@brandjon @weixiao-huang
brandjon authored and weixiao-huang committed Jan 31, 2019
1 parent 5c71a67 commit d75e7ea
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 84 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.TriState;
import com.google.devtools.build.lib.rules.python.PyCommon;
import com.google.devtools.build.lib.rules.python.PyProvider;
import com.google.devtools.build.lib.rules.python.PyRuleClasses;
import com.google.devtools.build.lib.rules.python.PyStructUtils;
import com.google.devtools.build.lib.rules.python.PythonVersion;
import com.google.devtools.build.lib.util.FileType;

Expand Down Expand Up @@ -70,7 +70,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
.override(
builder
.copy("deps")
.legacyMandatoryProviders(PyProvider.PROVIDER_NAME)
.legacyMandatoryProviders(PyStructUtils.PROVIDER_NAME)
.allowedFileTypes())
/* <!-- #BLAZE_RULE($base_py).ATTRIBUTE(imports) -->
List of import directories to be added to the <code>PYTHONPATH</code>.
Expand Down
26 changes: 13 additions & 13 deletions src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,14 @@ private static NestedSet<Artifact> initTransitivePythonSources(RuleContext ruleC
private static void collectTransitivePythonSourcesFromDeps(
RuleContext ruleContext, NestedSetBuilder<Artifact> builder) {
for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
if (PyProvider.hasProvider(dep)) {
if (PyStructUtils.hasProvider(dep)) {
try {
StructImpl info = PyProvider.getProvider(dep);
NestedSet<Artifact> sources = PyProvider.getTransitiveSources(info);
StructImpl info = PyStructUtils.getProvider(dep);
NestedSet<Artifact> sources = PyStructUtils.getTransitiveSources(info);
if (!builder.getOrder().isCompatible(sources.getOrder())) {
ruleContext.ruleError(
getOrderErrorMessage(
PyProvider.TRANSITIVE_SOURCES, builder.getOrder(), sources.getOrder()));
PyStructUtils.TRANSITIVE_SOURCES, builder.getOrder(), sources.getOrder()));
} else {
builder.addTransitive(sources);
}
Expand Down Expand Up @@ -289,8 +289,8 @@ private static boolean initUsesSharedLibraries(RuleContext ruleContext) {

private static boolean checkForSharedLibraries(TransitiveInfoCollection target)
throws EvalException {
if (PyProvider.hasProvider(target)) {
return PyProvider.getUsesSharedLibraries(PyProvider.getProvider(target));
if (PyStructUtils.hasProvider(target)) {
return PyStructUtils.getUsesSharedLibraries(PyStructUtils.getProvider(target));
} else {
NestedSet<Artifact> files = target.getProvider(FileProvider.class).getFilesToBuild();
return FileType.contains(files, CppFileTypes.SHARED_LIBRARY);
Expand All @@ -309,7 +309,7 @@ private static NestedSet<String> initImports(RuleContext ruleContext, PythonSema
// TODO(brandjon): Add test case for this error, once we replace PythonImportsProvider
// with the normal Python provider and once we clean up our provider merge logic.
ruleContext.ruleError(
getOrderErrorMessage(PyProvider.IMPORTS, builder.getOrder(), imports.getOrder()));
getOrderErrorMessage(PyStructUtils.IMPORTS, builder.getOrder(), imports.getOrder()));
} else {
builder.addTransitive(imports);
}
Expand All @@ -330,9 +330,9 @@ private static boolean initHasPy2OnlySources(
return true;
}
for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
if (PyProvider.hasProvider(dep)) {
if (PyStructUtils.hasProvider(dep)) {
try {
if (PyProvider.getHasPy2OnlySources(PyProvider.getProvider(dep))) {
if (PyStructUtils.getHasPy2OnlySources(PyStructUtils.getProvider(dep))) {
return true;
}
} catch (EvalException e) {
Expand All @@ -353,9 +353,9 @@ private static boolean initHasPy3OnlySources(
return true;
}
for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
if (PyProvider.hasProvider(dep)) {
if (PyStructUtils.hasProvider(dep)) {
try {
if (PyProvider.getHasPy3OnlySources(PyProvider.getProvider(dep))) {
if (PyStructUtils.getHasPy3OnlySources(PyStructUtils.getProvider(dep))) {
return true;
}
} catch (EvalException e) {
Expand Down Expand Up @@ -604,8 +604,8 @@ public void addCommonTransitiveInfoProviders(
filesToBuild,
/* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER)))
.addSkylarkTransitiveInfo(
PyProvider.PROVIDER_NAME,
PyProvider.builder()
PyStructUtils.PROVIDER_NAME,
PyStructUtils.builder()
.setTransitiveSources(transitivePythonSources)
.setUsesSharedLibraries(usesSharedLibraries)
.setImports(imports)
Expand Down
8 changes: 3 additions & 5 deletions ...ls/build/lib/rules/python/PyProvider.java → ...build/lib/rules/python/PyStructUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,12 @@
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import com.google.devtools.build.lib.syntax.SkylarkType;

/**
* Static helper class for managing the "py" struct provider returned and consumed by Python rules.
*/
/** Static helper class for creating and accessing instances of the "py" legacy struct provider. */
// TODO(#7010): Replace this with a real provider.
public class PyProvider {
public class PyStructUtils {

// Disable construction.
private PyProvider() {}
private PyStructUtils() {}

/** Name of the Python provider in Starlark code (as a field of a {@code Target}. */
public static final String PROVIDER_NAME = "py";
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/com/google/devtools/build/lib/rules/python/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ test_suite(
":BazelPythonConfigurationTest",
":PyBinaryConfiguredTargetTest",
":PyLibraryConfiguredTargetTest",
":PyProviderTest",
":PyStructUtilsTest",
":PyTestConfiguredTargetTest",
":PythonConfigurationTest",
":PythonStarlarkApiTest",
Expand Down Expand Up @@ -156,8 +156,8 @@ java_test(
)

java_test(
name = "PyProviderTest",
srcs = ["PyProviderTest.java"],
name = "PyStructUtilsTest",
srcs = ["PyStructUtilsTest.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib:build-base",
"//src/main/java/com/google/devtools/build/lib:packages-internal",
Expand Down
110 changes: 56 additions & 54 deletions ...uild/lib/rules/python/PyProviderTest.java → ...d/lib/rules/python/PyStructUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,30 +35,30 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/** Tests for {@link PyProvider}. */
/** Tests for {@link PyStructUtils}. */
@RunWith(JUnit4.class)
public class PyProviderTest extends BuildViewTestCase {
public class PyStructUtilsTest extends BuildViewTestCase {

/**
* Constructs a py provider struct with the given field values and with default values for any
* field not specified.
*
* <p>The struct is constructed directly, rather than using {@link PyProvider.Builder}, so that
* the resulting instance is suitable for asserting on {@code PyProvider}'s operations over
* <p>The struct is constructed directly, rather than using {@link PyStructUtils.Builder}, so that
* the resulting instance is suitable for asserting on {@code PyStructUtils}'s operations over
* structs with known contents. {@code overrides} is applied directly without validating the
* fields' names or types.
*/
private StructImpl makeStruct(Map<String, Object> overrides) {
Map<String, Object> fields = new LinkedHashMap<>();
fields.put(
PyProvider.TRANSITIVE_SOURCES,
PyStructUtils.TRANSITIVE_SOURCES,
SkylarkNestedSet.of(Artifact.class, NestedSetBuilder.emptySet(Order.COMPILE_ORDER)));
fields.put(PyProvider.USES_SHARED_LIBRARIES, false);
fields.put(PyStructUtils.USES_SHARED_LIBRARIES, false);
fields.put(
PyProvider.IMPORTS,
PyStructUtils.IMPORTS,
SkylarkNestedSet.of(String.class, NestedSetBuilder.emptySet(Order.COMPILE_ORDER)));
fields.put(PyProvider.HAS_PY2_ONLY_SOURCES, false);
fields.put(PyProvider.HAS_PY3_ONLY_SOURCES, false);
fields.put(PyStructUtils.HAS_PY2_ONLY_SOURCES, false);
fields.put(PyStructUtils.HAS_PY3_ONLY_SOURCES, false);
fields.putAll(overrides);
return StructProvider.STRUCT.create(fields, "No such attribute '%s'");
}
Expand Down Expand Up @@ -126,19 +126,19 @@ private void defineDummyTarget() throws IOException {
@Test
public void hasProvider_True() throws Exception {
definePyTarget();
assertThat(PyProvider.hasProvider(getConfiguredTarget("//pytarget"))).isTrue();
assertThat(PyStructUtils.hasProvider(getConfiguredTarget("//pytarget"))).isTrue();
}

@Test
public void hasProvider_False() throws Exception {
defineDummyTarget();
assertThat(PyProvider.hasProvider(getConfiguredTarget("//dummytarget"))).isFalse();
assertThat(PyStructUtils.hasProvider(getConfiguredTarget("//dummytarget"))).isFalse();
}

@Test
public void getProvider_Present() throws Exception {
definePyTarget();
StructImpl info = PyProvider.getProvider(getConfiguredTarget("//pytarget"));
StructImpl info = PyStructUtils.getProvider(getConfiguredTarget("//pytarget"));
// If we got this far, it's present. getProvider() should never be null, but check just in case.
assertThat(info).isNotNull();
}
Expand All @@ -149,7 +149,7 @@ public void getProvider_Absent() throws Exception {
EvalException ex =
assertThrows(
EvalException.class,
() -> PyProvider.getProvider(getConfiguredTarget("//dummytarget")));
() -> PyStructUtils.getProvider(getConfiguredTarget("//dummytarget")));
assertThat(ex).hasMessageThat().contains("Target does not have 'py' provider");
}

Expand All @@ -175,7 +175,7 @@ public void getProvider_WrongType() throws Exception {
EvalException ex =
assertThrows(
EvalException.class,
() -> PyProvider.getProvider(getConfiguredTarget("//badtypetarget")));
() -> PyStructUtils.getProvider(getConfiguredTarget("//badtypetarget")));
assertThat(ex).hasMessageThat().contains("'py' provider should be a struct");
}

Expand Down Expand Up @@ -211,21 +211,22 @@ public void getTransitiveSources_Good() throws Exception {
StructImpl info =
makeStruct(
ImmutableMap.of(
PyProvider.TRANSITIVE_SOURCES, SkylarkNestedSet.of(Artifact.class, sources)));
assertThat(PyProvider.getTransitiveSources(info)).isSameAs(sources);
PyStructUtils.TRANSITIVE_SOURCES, SkylarkNestedSet.of(Artifact.class, sources)));
assertThat(PyStructUtils.getTransitiveSources(info)).isSameAs(sources);
}

@Test
public void getTransitiveSources_Missing() {
StructImpl info = StructProvider.STRUCT.createEmpty(null);
assertHasMissingFieldMessage(() -> PyProvider.getTransitiveSources(info), "transitive_sources");
assertHasMissingFieldMessage(
() -> PyStructUtils.getTransitiveSources(info), "transitive_sources");
}

@Test
public void getTransitiveSources_WrongType() {
StructImpl info = makeStruct(ImmutableMap.of(PyProvider.TRANSITIVE_SOURCES, 123));
StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.TRANSITIVE_SOURCES, 123));
assertHasWrongTypeMessage(
() -> PyProvider.getTransitiveSources(info), "transitive_sources", "depset of Files");
() -> PyStructUtils.getTransitiveSources(info), "transitive_sources", "depset of Files");
}

@Test
Expand Down Expand Up @@ -267,79 +268,80 @@ public void getTransitiveSources_OrderMismatch() throws Exception {

@Test
public void getUsesSharedLibraries_Good() throws Exception {
StructImpl info = makeStruct(ImmutableMap.of(PyProvider.USES_SHARED_LIBRARIES, true));
assertThat(PyProvider.getUsesSharedLibraries(info)).isTrue();
StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.USES_SHARED_LIBRARIES, true));
assertThat(PyStructUtils.getUsesSharedLibraries(info)).isTrue();
}

@Test
public void getUsesSharedLibraries_Missing() throws Exception {
StructImpl info = StructProvider.STRUCT.createEmpty(null);
assertThat(PyProvider.getUsesSharedLibraries(info)).isFalse();
assertThat(PyStructUtils.getUsesSharedLibraries(info)).isFalse();
}

@Test
public void getUsesSharedLibraries_WrongType() {
StructImpl info = makeStruct(ImmutableMap.of(PyProvider.USES_SHARED_LIBRARIES, 123));
StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.USES_SHARED_LIBRARIES, 123));
assertHasWrongTypeMessage(
() -> PyProvider.getUsesSharedLibraries(info), "uses_shared_libraries", "boolean");
() -> PyStructUtils.getUsesSharedLibraries(info), "uses_shared_libraries", "boolean");
}

@Test
public void getImports_Good() throws Exception {
NestedSet<String> imports = NestedSetBuilder.create(Order.COMPILE_ORDER, "abc");
StructImpl info =
makeStruct(ImmutableMap.of(PyProvider.IMPORTS, SkylarkNestedSet.of(String.class, imports)));
assertThat(PyProvider.getImports(info)).isSameAs(imports);
makeStruct(
ImmutableMap.of(PyStructUtils.IMPORTS, SkylarkNestedSet.of(String.class, imports)));
assertThat(PyStructUtils.getImports(info)).isSameAs(imports);
}

@Test
public void getImports_Missing() throws Exception {
StructImpl info = StructProvider.STRUCT.createEmpty(null);
assertHasOrderAndContainsExactly(PyProvider.getImports(info), Order.COMPILE_ORDER);
assertHasOrderAndContainsExactly(PyStructUtils.getImports(info), Order.COMPILE_ORDER);
}

@Test
public void getImports_WrongType() {
StructImpl info = makeStruct(ImmutableMap.of(PyProvider.IMPORTS, 123));
assertHasWrongTypeMessage(() -> PyProvider.getImports(info), "imports", "depset of strings");
StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.IMPORTS, 123));
assertHasWrongTypeMessage(() -> PyStructUtils.getImports(info), "imports", "depset of strings");
}

@Test
public void getHasPy2OnlySources_Good() throws Exception {
StructImpl info = makeStruct(ImmutableMap.of(PyProvider.HAS_PY2_ONLY_SOURCES, true));
assertThat(PyProvider.getHasPy2OnlySources(info)).isTrue();
StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.HAS_PY2_ONLY_SOURCES, true));
assertThat(PyStructUtils.getHasPy2OnlySources(info)).isTrue();
}

@Test
public void getHasPy2OnlySources_Missing() throws Exception {
StructImpl info = StructProvider.STRUCT.createEmpty(null);
assertThat(PyProvider.getHasPy2OnlySources(info)).isFalse();
assertThat(PyStructUtils.getHasPy2OnlySources(info)).isFalse();
}

@Test
public void getHasPy2OnlySources_WrongType() {
StructImpl info = makeStruct(ImmutableMap.of(PyProvider.HAS_PY2_ONLY_SOURCES, 123));
StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.HAS_PY2_ONLY_SOURCES, 123));
assertHasWrongTypeMessage(
() -> PyProvider.getHasPy2OnlySources(info), "has_py2_only_sources", "boolean");
() -> PyStructUtils.getHasPy2OnlySources(info), "has_py2_only_sources", "boolean");
}

@Test
public void getHasPy3OnlySources_Good() throws Exception {
StructImpl info = makeStruct(ImmutableMap.of(PyProvider.HAS_PY3_ONLY_SOURCES, true));
assertThat(PyProvider.getHasPy3OnlySources(info)).isTrue();
StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.HAS_PY3_ONLY_SOURCES, true));
assertThat(PyStructUtils.getHasPy3OnlySources(info)).isTrue();
}

@Test
public void getHasPy3OnlySources_Missing() throws Exception {
StructImpl info = StructProvider.STRUCT.createEmpty(null);
assertThat(PyProvider.getHasPy3OnlySources(info)).isFalse();
assertThat(PyStructUtils.getHasPy3OnlySources(info)).isFalse();
}

@Test
public void getHasPy3OnlySources_WrongType() {
StructImpl info = makeStruct(ImmutableMap.of(PyProvider.HAS_PY3_ONLY_SOURCES, 123));
StructImpl info = makeStruct(ImmutableMap.of(PyStructUtils.HAS_PY3_ONLY_SOURCES, 123));
assertHasWrongTypeMessage(
() -> PyProvider.getHasPy3OnlySources(info), "has_py3_only_sources", "boolean");
() -> PyStructUtils.getHasPy3OnlySources(info), "has_py3_only_sources", "boolean");
}

/** Checks values set by the builder. */
Expand All @@ -349,26 +351,26 @@ public void builder() throws Exception {
NestedSetBuilder.create(Order.COMPILE_ORDER, getSourceArtifact("dummy"));
NestedSet<String> imports = NestedSetBuilder.create(Order.COMPILE_ORDER, "abc");
StructImpl info =
PyProvider.builder()
PyStructUtils.builder()
.setTransitiveSources(sources)
.setUsesSharedLibraries(true)
.setImports(imports)
.setHasPy2OnlySources(true)
.setHasPy3OnlySources(true)
.build();
// Assert using struct operations, not PyProvider accessors, which aren't necessarily trusted to
// be correct.
// Assert using struct operations, not PyStructUtils accessors, which aren't necessarily trusted
// to be correct.
assertHasOrderAndContainsExactly(
((SkylarkNestedSet) info.getValue(PyProvider.TRANSITIVE_SOURCES)).getSet(Artifact.class),
((SkylarkNestedSet) info.getValue(PyStructUtils.TRANSITIVE_SOURCES)).getSet(Artifact.class),
Order.COMPILE_ORDER,
getSourceArtifact("dummy"));
assertThat((Boolean) info.getValue(PyProvider.USES_SHARED_LIBRARIES)).isTrue();
assertThat((Boolean) info.getValue(PyStructUtils.USES_SHARED_LIBRARIES)).isTrue();
assertHasOrderAndContainsExactly(
((SkylarkNestedSet) info.getValue(PyProvider.IMPORTS)).getSet(String.class),
((SkylarkNestedSet) info.getValue(PyStructUtils.IMPORTS)).getSet(String.class),
Order.COMPILE_ORDER,
"abc");
assertThat((Boolean) info.getValue(PyProvider.HAS_PY2_ONLY_SOURCES)).isTrue();
assertThat((Boolean) info.getValue(PyProvider.HAS_PY3_ONLY_SOURCES)).isTrue();
assertThat((Boolean) info.getValue(PyStructUtils.HAS_PY2_ONLY_SOURCES)).isTrue();
assertThat((Boolean) info.getValue(PyStructUtils.HAS_PY3_ONLY_SOURCES)).isTrue();
}

/** Checks the defaults set by the builder. */
Expand All @@ -377,14 +379,14 @@ public void builderDefaults() throws Exception {
// transitive_sources is mandatory, so create a dummy value but no need to assert on it.
NestedSet<Artifact> sources =
NestedSetBuilder.create(Order.COMPILE_ORDER, getSourceArtifact("dummy"));
StructImpl info = PyProvider.builder().setTransitiveSources(sources).build();
// Assert using struct operations, not PyProvider accessors, which aren't necessarily trusted to
// be correct.
assertThat((Boolean) info.getValue(PyProvider.USES_SHARED_LIBRARIES)).isFalse();
StructImpl info = PyStructUtils.builder().setTransitiveSources(sources).build();
// Assert using struct operations, not PyStructUtils accessors, which aren't necessarily trusted
// to be correct.
assertThat((Boolean) info.getValue(PyStructUtils.USES_SHARED_LIBRARIES)).isFalse();
assertHasOrderAndContainsExactly(
((SkylarkNestedSet) info.getValue(PyProvider.IMPORTS)).getSet(String.class),
((SkylarkNestedSet) info.getValue(PyStructUtils.IMPORTS)).getSet(String.class),
Order.COMPILE_ORDER);
assertThat((Boolean) info.getValue(PyProvider.HAS_PY2_ONLY_SOURCES)).isFalse();
assertThat((Boolean) info.getValue(PyProvider.HAS_PY3_ONLY_SOURCES)).isFalse();
assertThat((Boolean) info.getValue(PyStructUtils.HAS_PY2_ONLY_SOURCES)).isFalse();
assertThat((Boolean) info.getValue(PyStructUtils.HAS_PY3_ONLY_SOURCES)).isFalse();
}
}
Loading

0 comments on commit d75e7ea

Please sign in to comment.