diff options
author | 2016-03-03 13:14:38 +0000 | |
---|---|---|
committer | 2016-03-03 15:28:01 +0000 | |
commit | efd7ca1b420e00cf32839c07733e0e4a4d8b1bbb (patch) | |
tree | 04eee06d2b6e277f3d0eaa7f326f1ae843b56746 /src/main | |
parent | f745e99db7632cfb2145b6926f961e85f9084bc5 (diff) |
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
Diffstat (limited to 'src/main')
8 files changed, 154 insertions, 96 deletions
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 9fd4fd4666..8fa5cb55a2 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 @@ package com.google.devtools.build.lib.analysis; 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 @@ public final class RuleContext extends TargetContext } } - 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 @@ public final class RuleContext extends TargetContext } } - 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 @@ public final class RuleContext extends TargetContext } } if (missing.isEmpty()) { - return; + return null; } else { missingProvidersList.add(missing); } @@ -1691,16 +1686,55 @@ public final class RuleContext extends TargetContext 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 53708cf243..1f8f1280a0 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 @@ package com.google.devtools.build.lib.analysis; 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 @@ public final class SkylarkProviders implements TransitiveInfoProvider { } /** + * 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 7c0fc717b4..35b23497d0 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.bazel.rules.cpp.BazelCppRuleClasses; 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 @@ import com.google.devtools.build.lib.util.FileType; 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 @@ public final class BazelPyRuleClasses { <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 f8923e7565..1db5c46136 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 final class Attribute implements Comparable<Attribute> { } 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 ae048a0a03..ab8d490e4b 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 @@ public abstract class PyBinary implements RuleConfiguredTargetFactory { 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 9b6813fad5..eff1239649 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 @@ package com.google.devtools.build.lib.rules.python; 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.PseudoAction; 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.cpp.CppFileTypes; 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 @@ import java.util.UUID; */ 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 final class PyCommon { 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 @@ public final class PyCommon { 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 final class PyCommon { } 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 @@ public final class PyCommon { /** * 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 b8189875b5..0000000000 --- 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 69d821755c..cb13c65a30 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 @@ public interface ClassObject { 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(); |