Python provider is now available in Skylark
Using mandatoryProvidersList to validate python rules' dependency.
Added a SkylarkProvider named 'py' which is a SkylarkClassObject in Java and a
struct in Skylark. Native python rule and Skylark python rule should have this provider
so that they can depend on each other.
RELNOTES[NEW]: Native python rule can depend on skylark rule as long as skylark
rule provides 'py' provider.
--
MOS_MIGRATED_REVID=116241504
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 9fd4fd4..8fa5cb5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -16,6 +16,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
@@ -1557,16 +1558,10 @@
}
}
- if (attribute.isStrictLabelCheckingEnabled()) {
- if (prerequisiteTarget instanceof Rule) {
- RuleClass ruleClass = ((Rule) prerequisiteTarget).getRuleClassObject();
- if (!attribute.getAllowedRuleClassesPredicate().apply(ruleClass)) {
- boolean allowedWithWarning = attribute.getAllowedRuleClassesWarningPredicate()
- .apply(ruleClass);
- reportBadPrerequisite(attribute, prerequisiteTarget.getTargetKind(), prerequisiteLabel,
- "expected " + attribute.getAllowedRuleClassesPredicate(), allowedWithWarning);
- }
- } else if (prerequisiteTarget instanceof FileTarget) {
+ if (prerequisiteTarget instanceof Rule) {
+ validateRuleDependency(prerequisite, attribute);
+ } else if (prerequisiteTarget instanceof FileTarget) {
+ if (attribute.isStrictLabelCheckingEnabled()) {
if (!attribute.getAllowedFileTypesPredicate()
.apply(((FileTarget) prerequisiteTarget).getFilename())) {
if (prerequisiteTarget instanceof InputFile
@@ -1660,10 +1655,10 @@
}
}
- private void validateMandatoryProviders(ConfiguredTarget prerequisite, Attribute attribute) {
+ private String getMissingMandatoryProviders(ConfiguredTarget prerequisite, Attribute attribute){
List<ImmutableSet<String>> mandatoryProvidersList = attribute.getMandatoryProvidersList();
if (mandatoryProvidersList.isEmpty()) {
- return;
+ return null;
}
List<List<String>> missingProvidersList = new ArrayList<>();
for (ImmutableSet<String> providers : mandatoryProvidersList) {
@@ -1674,7 +1669,7 @@
}
}
if (missing.isEmpty()) {
- return;
+ return null;
} else {
missingProvidersList.add(missing);
}
@@ -1691,16 +1686,55 @@
missingProviders.append("'")
.append((providers.size() > 1) ? "]" : "");
}
- attributeError(
- attribute.getName(),
- "'" + prerequisite.getLabel() + "' does not have mandatory provider "
- + missingProviders);
+ return missingProviders.toString();
+ }
+
+ /**
+ * Because some rules still have to use allowedRuleClasses to do rule dependency validation.
+ * We implemented the allowedRuleClasses OR mandatoryProvidersList mechanism. Either condition
+ * is satisfied, we consider the dependency valid.
+ */
+ private void validateRuleDependency(ConfiguredTarget prerequisite, Attribute attribute) {
+ Target prerequisiteTarget = prerequisite.getTarget();
+ Label prerequisiteLabel = prerequisiteTarget.getLabel();
+ RuleClass ruleClass = ((Rule) prerequisiteTarget).getRuleClassObject();
+ Boolean allowed = null;
+ Boolean allowedWithWarning = null;
+
+ if (attribute.getAllowedRuleClassesPredicate() != Predicates.<RuleClass>alwaysTrue()) {
+ allowed = attribute.getAllowedRuleClassesPredicate().apply(ruleClass);
+ if (allowed) {
+ return;
+ }
+ }
+
+ if (attribute.getAllowedRuleClassesWarningPredicate()
+ != Predicates.<RuleClass>alwaysTrue()) {
+ allowedWithWarning = attribute.getAllowedRuleClassesWarningPredicate().apply(ruleClass);
+ if (allowedWithWarning) {
+ reportBadPrerequisite(attribute, prerequisiteTarget.getTargetKind(), prerequisiteLabel,
+ "expected " + attribute.getAllowedRuleClassesPredicate(), true);
+ return;
+ }
+ }
+
+ if (!attribute.getMandatoryProvidersList().isEmpty()) {
+ String missingMandatoryProviders = getMissingMandatoryProviders(prerequisite, attribute);
+ if (missingMandatoryProviders != null) {
+ attributeError(
+ attribute.getName(),
+ "'" + prerequisite.getLabel() + "' does not have mandatory provider "
+ + missingMandatoryProviders);
+ }
+ } else if (Boolean.FALSE.equals(allowed)) {
+ reportBadPrerequisite(attribute, prerequisiteTarget.getTargetKind(), prerequisiteLabel,
+ "expected " + attribute.getAllowedRuleClassesPredicate(), false);
+ }
}
private void validateDirectPrerequisite(Attribute attribute, ConfiguredTarget prerequisite) {
validateDirectPrerequisiteType(prerequisite, attribute);
validateDirectPrerequisiteFileTypes(prerequisite, attribute);
- validateMandatoryProviders(prerequisite, attribute);
if (attribute.performPrereqValidatorCheck()) {
prerequisiteValidator.validate(this, prerequisite, attribute);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
index 53708cf..1f8f128 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
@@ -16,6 +16,8 @@
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.SkylarkType;
import com.google.devtools.build.lib.util.Preconditions;
import java.util.HashSet;
@@ -49,6 +51,18 @@
}
/**
+ * Returns a Skylark provider and try to cast it into the specified type
+ */
+ public <TYPE> TYPE getValue(String key, Class<TYPE> type) throws EvalException {
+ Object obj = skylarkProviders.get(key);
+ if (obj == null) {
+ return null;
+ }
+ SkylarkType.checkType(obj, type, key);
+ return type.cast(obj);
+ }
+
+ /**
* Merges skylark providers. The set of providers must be disjoint.
*
* @param providers providers to merge {@code this} with.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
index 7c0fc71..35b2349 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.packages.RuleClass;
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.PyRuleClasses;
import com.google.devtools.build.lib.rules.python.PythonVersion;
import com.google.devtools.build.lib.util.FileType;
@@ -40,11 +41,6 @@
public final class BazelPyRuleClasses {
public static final FileType PYTHON_SOURCE = FileType.of(".py");
- public static final String[] ALLOWED_RULES_IN_DEPS = new String[] {
- "py_binary",
- "py_library",
- };
-
/**
* Base class for Python rule definitions.
*/
@@ -63,7 +59,7 @@
<a href="c-cpp.html#cc_library"><code>cc_library</code></a> rules,
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.override(builder.copy("deps")
- .allowedRuleClasses(ALLOWED_RULES_IN_DEPS)
+ .mandatoryProviders(ImmutableList.of(PyCommon.PYTHON_SKYLARK_PROVIDER_NAME))
.allowedFileTypes())
/* <!-- #BLAZE_RULE($base_py).ATTRIBUTE(imports) -->
List of import directories to be added to the <code>PYTHONPATH</code>.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
index f8923e7..1db5c46 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -713,7 +713,9 @@
}
public Builder<TYPE> mandatoryProviders(Iterable<String> providers) {
- mandatoryProvidersList(ImmutableList.of(providers));
+ if (providers.iterator().hasNext()) {
+ mandatoryProvidersList(ImmutableList.of(providers));
+ }
return this;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java
index ae048a0..ab8d490 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java
@@ -76,7 +76,7 @@
Runfiles commonRunfiles = collectCommonRunfiles(ruleContext, common, semantics);
Runfiles.Builder defaultRunfilesBuilder = new Runfiles.Builder(ruleContext.getWorkspaceName())
- .merge(commonRunfiles);
+ .merge(commonRunfiles);
semantics.collectDefaultRunfilesForBinary(ruleContext, defaultRunfilesBuilder);
Runfiles defaultRunfiles = defaultRunfilesBuilder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
index 9b6813f..eff1239 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
@@ -15,6 +15,7 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionOwner;
@@ -30,6 +31,7 @@
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.SkylarkProviders;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.Util;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
@@ -44,6 +46,11 @@
import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector;
import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector.LocalMetadataCollector;
import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
+import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.EvalUtils;
+import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
+import com.google.devtools.build.lib.syntax.SkylarkType;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.Preconditions;
@@ -61,6 +68,10 @@
*/
public final class PyCommon {
+ public static final String PYTHON_SKYLARK_PROVIDER_NAME = "py";
+ public static final String TRANSITIVE_PYTHON_SRCS = "transitive_sources";
+ public static final String IS_USING_SHARED_LIBRARY = "uses_shared_libraries";
+
private static final LocalMetadataCollector METADATA_COLLECTOR = new LocalMetadataCollector() {
@Override
public void collectMetadataArtifacts(Iterable<Artifact> artifacts,
@@ -125,13 +136,15 @@
public void addCommonTransitiveInfoProviders(RuleConfiguredTargetBuilder builder,
PythonSemantics semantics, NestedSet<Artifact> filesToBuild) {
- PythonSourcesProvider sourcesProvider =
- new PythonSourcesProvider(transitivePythonSources, usesSharedLibraries());
+
+ SkylarkClassObject sourcesProvider =
+ new SkylarkClassObject(ImmutableMap.<String, Object>of(
+ TRANSITIVE_PYTHON_SRCS, SkylarkNestedSet.of(Artifact.class, transitivePythonSources),
+ IS_USING_SHARED_LIBRARY, usesSharedLibraries()), "No such attribute '%s'");
builder
.add(InstrumentedFilesProvider.class, InstrumentedFilesCollector.collect(ruleContext,
semantics.getCoverageInstrumentationSpec(), METADATA_COLLECTOR, filesToBuild))
- .add(PythonSourcesProvider.class, sourcesProvider)
- .addSkylarkTransitiveInfo(PythonSourcesProvider.SKYLARK_NAME, sourcesProvider)
+ .addSkylarkTransitiveInfo(PYTHON_SKYLARK_PROVIDER_NAME, sourcesProvider)
// Python targets are not really compilable. The best we can do is make sure that all
// generated source files are ready.
.addOutputGroup(OutputGroupProvider.FILES_TO_COMPILE, transitivePythonSources)
@@ -260,12 +273,44 @@
return ruleContext.getPrerequisites("deps", Mode.TARGET);
}
+ private NestedSet<Artifact> getTransitivePythonSourcesFromSkylarkProvider(
+ TransitiveInfoCollection dep) {
+ SkylarkClassObject pythonSkylarkProvider = null;
+ SkylarkProviders skylarkProviders = dep.getProvider(SkylarkProviders.class);
+ try {
+ if (skylarkProviders != null) {
+ pythonSkylarkProvider = skylarkProviders.getValue(PYTHON_SKYLARK_PROVIDER_NAME,
+ SkylarkClassObject.class);
+ }
+
+ if (pythonSkylarkProvider != null) {
+ Object sourceFiles = pythonSkylarkProvider.getValue(TRANSITIVE_PYTHON_SRCS);
+ String errorType;
+ if (sourceFiles == null) {
+ errorType = "null";
+ } else {
+ errorType = EvalUtils.getDataTypeNameFromClass(sourceFiles.getClass());
+ }
+ String errorMsg = "Illegal Argument: attribute '%s' in provider '%s' is "
+ + "of unexpected type. Should be a set, but got a '%s'";
+ NestedSet<Artifact> pythonSourceFiles = SkylarkType.cast(
+ sourceFiles, SkylarkNestedSet.class, Artifact.class, null,
+ errorMsg, TRANSITIVE_PYTHON_SRCS, PYTHON_SKYLARK_PROVIDER_NAME, errorType)
+ .getSet(Artifact.class);
+ return pythonSourceFiles;
+ }
+ } catch (EvalException e) {
+ ruleContext.ruleError(e.getMessage());
+ }
+ return null;
+ }
+
private void collectTransitivePythonSourcesFrom(
Iterable<? extends TransitiveInfoCollection> deps, NestedSetBuilder<Artifact> builder) {
for (TransitiveInfoCollection dep : deps) {
- if (dep.getProvider(PythonSourcesProvider.class) != null) {
- PythonSourcesProvider provider = dep.getProvider(PythonSourcesProvider.class);
- builder.addTransitive(provider.getTransitivePythonSources());
+ NestedSet<Artifact> pythonSourceFiles = getTransitivePythonSourcesFromSkylarkProvider(dep);
+ if (pythonSourceFiles != null) {
+ builder.addTransitive(pythonSourceFiles);
} else {
// TODO(bazel-team): We also collect .py source files from deps (e.g. for proto_library
// rules). Rules should implement PythonSourcesProvider instead.
@@ -381,9 +426,14 @@
}
public boolean usesSharedLibraries() {
- return checkForSharedLibraries(Iterables.concat(
- ruleContext.getPrerequisites("deps", Mode.TARGET),
- ruleContext.getPrerequisites("data", Mode.DATA)));
+ try {
+ return checkForSharedLibraries(Iterables.concat(
+ ruleContext.getPrerequisites("deps", Mode.TARGET),
+ ruleContext.getPrerequisites("data", Mode.DATA)));
+ } catch (EvalException e) {
+ ruleContext.ruleError(e.getMessage());
+ return false;
+ }
}
protected static final ResourceSet PY_COMPILE_RESOURCE_SET =
@@ -453,11 +503,18 @@
/**
* Returns true if this target has an .so file in its transitive dependency closure.
*/
- public static boolean checkForSharedLibraries(Iterable<TransitiveInfoCollection> deps) {
+ public static boolean checkForSharedLibraries(Iterable<TransitiveInfoCollection> deps)
+ throws EvalException{
for (TransitiveInfoCollection dep : deps) {
- PythonSourcesProvider provider = dep.getProvider(PythonSourcesProvider.class);
+ SkylarkProviders providers = dep.getProvider(SkylarkProviders.class);
+ SkylarkClassObject provider = null;
+ if (providers != null) {
+ provider = providers.getValue(PYTHON_SKYLARK_PROVIDER_NAME,
+ SkylarkClassObject.class);
+ }
if (provider != null) {
- if (provider.usesSharedLibraries()) {
+ Boolean isUsingSharedLibrary = provider.getValue(IS_USING_SHARED_LIBRARY, Boolean.class);
+ if (Boolean.TRUE.equals(isUsingSharedLibrary)) {
return true;
}
} else if (FileType.contains(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonSourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonSourcesProvider.java
deleted file mode 100644
index b818987..0000000
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonSourcesProvider.java
+++ /dev/null
@@ -1,57 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.rules.python;
-
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
-
-/**
- * A provider interface for configured targets that provide source files to
- * Python targets.
- */
-@Immutable
-@SkylarkModule(name = "PythonSourcesProvider", doc = "")
-public final class PythonSourcesProvider implements TransitiveInfoProvider {
- /** The name of the field in Skylark used to access this class. */
- public static final String SKYLARK_NAME = "py";
-
- private final NestedSet<Artifact> transitivePythonSources;
- private final boolean usesSharedLibraries;
-
- public PythonSourcesProvider(NestedSet<Artifact> transitivePythonSources,
- boolean usesSharedLibraries) {
- this.transitivePythonSources = transitivePythonSources;
- this.usesSharedLibraries = usesSharedLibraries;
- }
-
- /**
- * Returns the Python sources in the transitive closure of this target.
- */
- @SkylarkCallable(
- name = "transitive_sources", doc = "The transitive set of Python sources", structField = true)
- public NestedSet<Artifact> getTransitivePythonSources() {
- return transitivePythonSources;
- }
-
- /**
- * Returns true if this target transitively depends on any shared libraries.
- */
- public boolean usesSharedLibraries() {
- return usesSharedLibraries;
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ClassObject.java b/src/main/java/com/google/devtools/build/lib/syntax/ClassObject.java
index 69d8217..cb13c65 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ClassObject.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ClassObject.java
@@ -100,6 +100,18 @@
return values.get(name);
}
+ /**
+ * Returns a value and try to cast it into specified type
+ */
+ public <TYPE> TYPE getValue(String key, Class<TYPE> type) throws EvalException {
+ Object obj = values.get(key);
+ if (obj == null) {
+ return null;
+ }
+ SkylarkType.checkType(obj, type, key);
+ return type.cast(obj);
+ }
+
@Override
public ImmutableCollection<String> getKeys() {
return values.keySet();
