diff options
8 files changed, 115 insertions, 45 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index 6f3c3d812c..9778608e09 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java @@ -57,6 +57,7 @@ import com.google.devtools.build.lib.packages.RuleTransitionFactory; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; +import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.Extension; @@ -811,9 +812,9 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { ImmutableMap.Builder<String, Class<?>> mapBuilder = ImmutableMap.builder(); for (ConfigurationFragmentFactory fragmentFactory : configurationFragmentFactories) { Class<? extends Fragment> fragmentClass = fragmentFactory.creates(); - String fragmentName = SkylarkModule.Resolver.resolveName(fragmentClass); - if (fragmentName != null) { - mapBuilder.put(fragmentName, fragmentClass); + SkylarkModule fragmentModule = SkylarkInterfaceUtils.getSkylarkModule((fragmentClass)); + if (fragmentModule != null) { + mapBuilder.put(fragmentModule.name(), fragmentClass); } } return mapBuilder.build(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 1dc1440224..afd1735f62 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -52,6 +52,7 @@ import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TestTimeout; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; +import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.util.OS; @@ -1395,15 +1396,13 @@ public class BuildConfiguration { return options; } - - private ImmutableMap<String, Class<? extends Fragment>> buildIndexOfSkylarkVisibleFragments() { ImmutableMap.Builder<String, Class<? extends Fragment>> builder = ImmutableMap.builder(); for (Class<? extends Fragment> fragmentClass : fragments.keySet()) { - String name = SkylarkModule.Resolver.resolveName(fragmentClass); - if (name != null) { - builder.put(name, fragmentClass); + SkylarkModule module = SkylarkInterfaceUtils.getSkylarkModule(fragmentClass); + if (module != null) { + builder.put(module.name(), fragmentClass); } } return builder.build(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkLateBoundDefault.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkLateBoundDefault.java index bf91cafbab..139f8656cb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkLateBoundDefault.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkLateBoundDefault.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skylarkbuildapi.LateBoundDefaultApi; +import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; import java.lang.reflect.InvocationTargetException; @@ -192,27 +193,31 @@ public class SkylarkLateBoundDefault<FragmentT> extends AbstractLabelLateBoundDe ImmutableMap.Builder<String, SkylarkLateBoundDefault<?>> lateBoundDefaultMap = new ImmutableMap.Builder<>(); Class<?> fragmentClass = key.fragmentClass; - String fragmentName = SkylarkModule.Resolver.resolveName(fragmentClass); - for (Method method : fragmentClass.getMethods()) { - if (method.isAnnotationPresent(SkylarkConfigurationField.class)) { - // TODO(b/68817606): Use annotation processors to verify these constraints. - Preconditions.checkArgument( - method.getReturnType() == Label.class, - String.format("Method %s must have return type 'Label'", method)); - Preconditions.checkArgument( - method.getParameterTypes().length == 0, - String.format("Method %s must not accept arguments", method)); + SkylarkModule fragmentModule = + SkylarkInterfaceUtils.getSkylarkModule(fragmentClass); - SkylarkConfigurationField configField = - method.getAnnotation(SkylarkConfigurationField.class); - lateBoundDefaultMap.put( - configField.name(), - new SkylarkLateBoundDefault<>( - configField, - fragmentClass, - fragmentName, - method, - key.toolsRepository)); + if (fragmentModule != null) { + for (Method method : fragmentClass.getMethods()) { + if (method.isAnnotationPresent(SkylarkConfigurationField.class)) { + // TODO(b/68817606): Use annotation processors to verify these constraints. + Preconditions.checkArgument( + method.getReturnType() == Label.class, + String.format("Method %s must have return type 'Label'", method)); + Preconditions.checkArgument( + method.getParameterTypes().length == 0, + String.format("Method %s must not accept arguments", method)); + + SkylarkConfigurationField configField = + method.getAnnotation(SkylarkConfigurationField.class); + lateBoundDefaultMap.put( + configField.name(), + new SkylarkLateBoundDefault<>( + configField, + fragmentClass, + fragmentModule.name(), + method, + key.toolsRepository)); + } } } return lateBoundDefaultMap.build(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessor.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessor.java index fc4ff11b02..8f7b998269 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessor.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.analysis.skylark.annotations.processor; import com.google.devtools.build.lib.analysis.skylark.annotations.SkylarkConfigurationField; -import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import java.util.Set; import javax.annotation.processing.AbstractProcessor; import javax.annotation.processing.Messager; @@ -38,7 +37,7 @@ import javax.tools.Diagnostic; * * <p>Checks the following invariants about {@link SkylarkConfigurationField}-annotated methods: * <ul> - * <li>The annotated method must be on a configuration fragment exposed to skylark.</li> + * <li>The annotated method must be on a configuration fragment.</li> * <li>The method must have return type Label.</li> * <li>The method must be public.</li> * <li>The method must have zero arguments.</li> @@ -52,6 +51,7 @@ public final class SkylarkConfigurationFieldProcessor extends AbstractProcessor private Messager messager; private Types typeUtils; + private Elements elementUtils; private TypeMirror labelType; private TypeMirror configurationFragmentType; @@ -65,7 +65,7 @@ public final class SkylarkConfigurationFieldProcessor extends AbstractProcessor super.init(processingEnv); messager = processingEnv.getMessager(); typeUtils = processingEnv.getTypeUtils(); - Elements elementUtils = processingEnv.getElementUtils(); + elementUtils = processingEnv.getElementUtils(); labelType = elementUtils.getTypeElement("com.google.devtools.build.lib.cmdline.Label").asType(); configurationFragmentType = @@ -82,7 +82,7 @@ public final class SkylarkConfigurationFieldProcessor extends AbstractProcessor if (!isMethodOfSkylarkExposedConfigurationFragment(methodElement)) { error(methodElement, "@SkylarkConfigurationField annotated methods must be methods " - + "of configuration fragments with the @SkylarkModule annotation."); + + "of configuration fragments."); } if (!typeUtils.isSameType(methodElement.getReturnType(), labelType)) { error(methodElement, "@SkylarkConfigurationField annotated methods must return Label."); @@ -104,6 +104,7 @@ public final class SkylarkConfigurationFieldProcessor extends AbstractProcessor private boolean isMethodOfSkylarkExposedConfigurationFragment( ExecutableElement methodElement) { + if (methodElement.getEnclosingElement().getKind() != ElementKind.CLASS) { return false; } @@ -111,9 +112,7 @@ public final class SkylarkConfigurationFieldProcessor extends AbstractProcessor if (!typeUtils.isAssignable(classElement.asType(), configurationFragmentType)) { return false; } - if (classElement.getAnnotation(SkylarkModule.class) == null) { - return false; - } + return true; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java index 59b534ea1a..773210ac9d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java +++ b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java @@ -21,6 +21,7 @@ import com.google.common.collect.SetMultimap; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import java.util.Collection; import java.util.Set; @@ -236,8 +237,11 @@ public final class ConfigurationFragmentPolicy { */ private boolean hasLegalFragmentName( Class<?> configurationFragment, ConfigurationTransition transition) { - return requiredConfigurationFragmentNames - .containsEntry(transition, SkylarkModule.Resolver.resolveName(configurationFragment)); + SkylarkModule fragmentModule = SkylarkInterfaceUtils.getSkylarkModule(configurationFragment); + + return fragmentModule != null + ? requiredConfigurationFragmentNames.containsEntry(transition, fragmentModule.name()) + : false; } /** @@ -245,8 +249,11 @@ public final class ConfigurationFragmentPolicy { * configuration. */ private boolean hasLegalFragmentName(Class<?> configurationFragment) { - return requiredConfigurationFragmentNames.containsValue( - SkylarkModule.Resolver.resolveName(configurationFragment)); + SkylarkModule fragmentModule = SkylarkInterfaceUtils.getSkylarkModule(configurationFragment); + + return fragmentModule != null + ? requiredConfigurationFragmentNames.containsValue(fragmentModule.name()) + : false; } /** diff --git a/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessorTest.java b/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessorTest.java index 4f6bc4dfb6..d17b226ccd 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessorTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessorTest.java @@ -50,6 +50,18 @@ public final class SkylarkConfigurationFieldProcessorTest { } @Test + public void testGoldenConfigurationFieldThroughApi() throws Exception { + // TODO(b/71644521): Compile-testing is not fully functional on Windows; test sources are + // unable to resolve cross-package dependencies. + assumeTrue(OS.getCurrent() != OS.WINDOWS); + + assertAbout(javaSource()) + .that(getFile("GoldenConfigurationFieldThroughApi.java")) + .processedWith(new SkylarkConfigurationFieldProcessor()) + .compilesWithoutError(); + } + + @Test public void testHasMethodParameters() throws Exception { // TODO(b/71644521): Compile-testing is not fully functional on Windows; test sources are // unable to resolve cross-package dependencies. @@ -99,8 +111,8 @@ public final class SkylarkConfigurationFieldProcessorTest { .that(getFile("NonConfigurationFragment.java")) .processedWith(new SkylarkConfigurationFieldProcessor()) .failsToCompile() - .withErrorContaining("@SkylarkConfigurationField annotated methods must be methods of " - + "configuration fragments with the @SkylarkModule annotation."); + .withErrorContaining("@SkylarkConfigurationField annotated methods must be methods " + + "of configuration fragments."); } @Test @@ -112,9 +124,7 @@ public final class SkylarkConfigurationFieldProcessorTest { assertAbout(javaSource()) .that(getFile("NonExposedConfigurationFragment.java")) .processedWith(new SkylarkConfigurationFieldProcessor()) - .failsToCompile() - .withErrorContaining("@SkylarkConfigurationField annotated methods must be methods of " - + "configuration fragments with the @SkylarkModule annotation."); + .compilesWithoutError(); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/optiontestsources/GoldenConfigurationFieldThroughApi.java b/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/optiontestsources/GoldenConfigurationFieldThroughApi.java new file mode 100644 index 0000000000..29bca7fe24 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/optiontestsources/GoldenConfigurationFieldThroughApi.java @@ -0,0 +1,49 @@ +// Copyright 2017 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.analysis.skylark.annotations.processor.optiontestsources; + +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.skylark.annotations.SkylarkConfigurationField; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; +import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; + +/** + * A test case of SkylarkConfigurationFieldProcessorTest. + */ +public class GoldenConfigurationFieldThroughApi + extends BuildConfiguration.Fragment implements ApiInterface { + + /** + * Returns the label of the xcode_config rule to use for resolving the host system xcode version. + */ + @SkylarkConfigurationField( + name = "some_field", + doc = "Documentation ", + defaultLabel = "defaultLabel", + defaultInToolRepository = true + ) + public Label getXcodeConfigLabel() { + return null; + } +} + +@SkylarkModule( + name = "module_name", + doc = "A fake configuration fragment for a test.", + category = SkylarkModuleCategory.CONFIGURATION_FRAGMENT +) +interface ApiInterface { +} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/optiontestsources/NonConfigurationFragment.java b/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/optiontestsources/NonConfigurationFragment.java index 221493775f..f21175f2fb 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/optiontestsources/NonConfigurationFragment.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/optiontestsources/NonConfigurationFragment.java @@ -25,7 +25,7 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; @SkylarkModule( name = "module_name", doc = "A fake configuration fragment for a test.", - category = SkylarkModuleCategory.CONFIGURATION_FRAGMENT + category = SkylarkModuleCategory.BUILTIN // Not a configuration fragment! ) public class NonConfigurationFragment { |