diff options
author | 2017-05-02 20:53:52 +0200 | |
---|---|---|
committer | 2017-05-03 10:57:17 +0200 | |
commit | 6bde6fb6b4e6a77ceef66a3782d68961a68c53d1 (patch) | |
tree | 2ed41fc5ebd08b98b3af62ca7b5ad8e56945954d /src/main/java/com/google/devtools/build/lib/rules/objc | |
parent | da21ba7a48ea4f3a1b67dbecfc3d30c93b42beac (diff) |
Add "uses_protobuf" attribute to objc_proto_library rule.
PiperOrigin-RevId: 154860105
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules/objc')
5 files changed, 177 insertions, 43 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java index befd3d4953..07f432dbf7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java @@ -14,12 +14,17 @@ package com.google.devtools.build.lib.rules.objc; +import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; +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.AspectDefinition; import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.BuildType; @@ -55,16 +60,13 @@ public class ObjcProtoAspect extends NativeAspectClass implements ConfiguredAspe aspectObjcProtoProvider.addTransitive(depObjcProtoProviders); } - // If the rule has the portable_proto_filters, it must be an objc_proto_library configured - // to use the third party protobuf library, in contrast with the PB2 internal library. Only - // the third party library is enabled to propagate the protos with this aspect. + ProtoAttributes attributes = new ProtoAttributes(ruleContext); + + // If the rule has the portable_proto_filters or uses_protobuf, it must be an objc_proto_library + // configured to use the third party protobuf library, in contrast with the PB2 internal + // library. Only the third party library is enabled to propagate the protos with this aspect. // Validation for the correct target attributes is done in ProtoSupport.java. - if (ruleContext - .attributes() - .isAttributeValueExplicitlySpecified(ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR)) { - aspectObjcProtoProvider.addPortableProtoFilters( - PrerequisiteArtifacts.nestedSet( - ruleContext, ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR, Mode.HOST)); + if (attributes.requiresProtobuf()) { // Gather up all the dependency protos depended by this target. Iterable<ProtoSourcesProvider> protoProviders = @@ -74,6 +76,23 @@ public class ObjcProtoAspect extends NativeAspectClass implements ConfiguredAspe aspectObjcProtoProvider.addProtoGroup(protoProvider.getTransitiveProtoSources()); } + NestedSet<Artifact> portableProtoFilters = + PrerequisiteArtifacts.nestedSet( + ruleContext, ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR, Mode.HOST); + + // If this target does not provide filters but specifies direct proto_library dependencies, + // generate a filter file only for those proto files. + if (Iterables.isEmpty(portableProtoFilters) && !Iterables.isEmpty(protoProviders)) { + Artifact generatedFilter = ProtobufSupport.getGeneratedPortableFilter(ruleContext); + ProtobufSupport.registerPortableFilterGenerationAction( + ruleContext, + generatedFilter, + protoProviders); + portableProtoFilters = NestedSetBuilder.create(Order.STABLE_ORDER, generatedFilter); + } + + aspectObjcProtoProvider.addPortableProtoFilters(portableProtoFilters); + // Propagate protobuf's headers and search paths so the BinaryLinkingTargetFactory subclasses // (i.e. objc_binary) don't have to depend on it. ObjcProvider protobufObjcProvider = diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java index 55fbba932f..f2d460e5ec 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.objc; import com.google.common.base.Optional; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; @@ -37,7 +38,7 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory { ProtoAttributes attributes = new ProtoAttributes(ruleContext); attributes.validate(); - if (attributes.hasPortableProtoFilters()) { + if (attributes.requiresProtobuf()) { return createProtobufTarget(ruleContext); } else { ruleContext.ruleWarning("The usage of objc_proto_library without the portable_proto_filters " @@ -63,7 +64,7 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory { ruleContext.getConfiguration(), protoProviders, objcProtoProviders, - getPortableProtoFilters(ruleContext, objcProtoProviders)) + getPortableProtoFilters(ruleContext, objcProtoProviders, protoProviders)) .registerGenerationActions() .addFilesToBuild(filesToBuild); @@ -80,13 +81,27 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory { } private static NestedSet<Artifact> getPortableProtoFilters( - RuleContext ruleContext, Iterable<ObjcProtoProvider> objcProtoProviders) { + RuleContext ruleContext, + Iterable<ObjcProtoProvider> objcProtoProviders, + Iterable<ProtoSourcesProvider> protoProviders) { ProtoAttributes attributes = new ProtoAttributes(ruleContext); NestedSetBuilder<Artifact> portableProtoFilters = NestedSetBuilder.stableOrder(); portableProtoFilters.addTransitive( ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders)); - portableProtoFilters.addAll(attributes.getPortableProtoFilters()); + + // If this target specifies filters, use those. If not, generate a filter only if there are + // direct proto_library targets, and generate a filter only for those files. + if (attributes.hasPortableProtoFilters()) { + portableProtoFilters.addAll(attributes.getPortableProtoFilters()); + } else if (!Iterables.isEmpty(protoProviders)) { + Artifact generatedFilter = ProtobufSupport.getGeneratedPortableFilter(ruleContext); + ProtobufSupport.registerPortableFilterGenerationAction( + ruleContext, + generatedFilter, + protoProviders); + portableProtoFilters.add(generatedFilter); + } return portableProtoFilters.build(); } @@ -110,5 +125,4 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory { .addProvider(XcodeProvider.class, xcodeProvider) .build(); } - } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryRule.java index 09c2e11ff0..d5fbcaf712 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryRule.java @@ -47,6 +47,7 @@ public class ObjcProtoLibraryRule implements RuleDefinition { static final String USE_OBJC_HEADER_NAMES_ATTR = "use_objc_header_names"; static final String PER_PROTO_INCLUDES_ATTR = "per_proto_includes"; static final String PORTABLE_PROTO_FILTERS_ATTR = "portable_proto_filters"; + static final String USES_PROTOBUF_ATTR = "uses_protobuf"; static final String XCODE_GEN_ATTR = "$xcodegen"; @@ -90,6 +91,12 @@ public class ObjcProtoLibraryRule implements RuleDefinition { If true, always add all directories to objc_library includes. <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ .add(attr(PER_PROTO_INCLUDES_ATTR, BOOLEAN).value(false)) + /* <!-- #BLAZE_RULE(objc_proto_library).ATTRIBUTE(uses_protobuf) --> + Whether to use the new protobuf compiler and runtime for this target. If you enable this + option without passing portable_proto_filters, it will generate a filter file for all the + protos that passed through proto_library targets as deps. + <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ + .add(attr(USES_PROTOBUF_ATTR, BOOLEAN).value(false)) /* <!-- #BLAZE_RULE(objc_proto_library).ATTRIBUTE(portable_proto_filters) --> List of portable proto filters to be passed on to the protobuf compiler. This attribute cannot be used together with the options_file, output_cpp, per_proto_includes and @@ -106,10 +113,10 @@ public class ObjcProtoLibraryRule implements RuleDefinition { .cfg(HOST) .singleArtifact() .value( - new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR) { + new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR, USES_PROTOBUF_ATTR) { @Override public Object getDefault(AttributeMap rule) { - return rule.isAttributeValueExplicitlySpecified(PORTABLE_PROTO_FILTERS_ATTR) + return usesProtobuf(rule) ? env.getToolsLabel("//tools/objc:protobuf_compiler_wrapper") : env.getToolsLabel("//tools/objc:compile_protos"); } @@ -119,10 +126,10 @@ public class ObjcProtoLibraryRule implements RuleDefinition { .legacyAllowAnyFileType() .cfg(HOST) .value( - new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR) { + new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR, USES_PROTOBUF_ATTR) { @Override public Object getDefault(AttributeMap rule) { - return rule.isAttributeValueExplicitlySpecified(PORTABLE_PROTO_FILTERS_ATTR) + return usesProtobuf(rule) ? env.getToolsLabel("//tools/objc:protobuf_compiler_support") : env.getToolsLabel("//tools/objc:proto_support"); } @@ -131,10 +138,10 @@ public class ObjcProtoLibraryRule implements RuleDefinition { attr(PROTO_LIB_ATTR, LABEL) .allowedRuleClasses("objc_library") .value( - new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR) { + new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR, USES_PROTOBUF_ATTR) { @Override public Object getDefault(AttributeMap rule) { - if (rule.isAttributeValueExplicitlySpecified(PORTABLE_PROTO_FILTERS_ATTR)) { + if (usesProtobuf(rule)) { return env.getLabel("//external:objc_protobuf_lib"); } else { return env.getLabel("//external:objc_proto_lib"); @@ -162,6 +169,12 @@ public class ObjcProtoLibraryRule implements RuleDefinition { ObjcRuleClasses.XcrunRule.class, ObjcRuleClasses.CrosstoolRule.class) .build(); } + + static boolean usesProtobuf(AttributeMap attributes) { + return attributes.isAttributeValueExplicitlySpecified( + ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR) + || attributes.get(ObjcProtoLibraryRule.USES_PROTOBUF_ATTR, BOOLEAN); + } } /*<!-- #BLAZE_RULE (NAME = objc_proto_library, TYPE = LIBRARY, FAMILY = Objective-C) --> diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtoAttributes.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtoAttributes.java index e620f3294e..1dff8a4058 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtoAttributes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtoAttributes.java @@ -18,6 +18,7 @@ import static com.google.common.base.CaseFormat.LOWER_UNDERSCORE; import static com.google.common.base.CaseFormat.UPPER_CAMEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; +import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.CharMatcher; @@ -63,9 +64,9 @@ final class ProtoAttributes { "no protos to compile - a non-empty deps attribute is required"; @VisibleForTesting - static final String PORTABLE_PROTO_FILTERS_NOT_EXCLUSIVE_ERROR = - "The portable_proto_filters attribute is incompatible with the options_file, output_cpp, " - + "per_proto_includes and use_objc_header_names attributes."; + static final String PROTOBUF_ATTRIBUTES_NOT_EXCLUSIVE_ERROR = + "The portable_proto_filters and uses_protobuf attributes are incompatible with the " + + "options_file, output_cpp, per_proto_includes and use_objc_header_names attributes."; @VisibleForTesting static final String PORTABLE_PROTO_FILTERS_EMPTY_ERROR = @@ -83,6 +84,11 @@ final class ProtoAttributes { + "targets. Please migrate your Protocol Buffers 2 objc_proto_library targets to use the " + "portable_proto_filters attribute."; + @VisibleForTesting + static final String USES_PROTOBUF_CANT_BE_SPECIFIED_AS_FALSE = + "objc_proto_library targets can't specify the uses_protobuf attribute to false when also " + + "specifying portable_proto_filters."; + private final RuleContext ruleContext; /** @@ -108,40 +114,46 @@ final class ProtoAttributes { public void validate() throws RuleErrorException { PrerequisiteArtifacts prerequisiteArtifacts = ruleContext.getPrerequisiteArtifacts("deps", Mode.TARGET); - ImmutableList<Artifact> protos = prerequisiteArtifacts.filter(FileType.of(".proto")).list(); + ImmutableList<Artifact> protoFiles = prerequisiteArtifacts.filter(FileType.of(".proto")).list(); + + if (hasPortableProtoFilters() + && ruleContext + .attributes() + .isAttributeValueExplicitlySpecified(ObjcProtoLibraryRule.USES_PROTOBUF_ATTR) + && !ruleContext.attributes().get(ObjcProtoLibraryRule.USES_PROTOBUF_ATTR, BOOLEAN)) { + ruleContext.throwWithAttributeError( + ObjcProtoLibraryRule.USES_PROTOBUF_ATTR, USES_PROTOBUF_CANT_BE_SPECIFIED_AS_FALSE); + } - if (ruleContext - .attributes() - .isAttributeValueExplicitlySpecified(ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR)) { - if (!protos.isEmpty()) { + if (requiresProtobuf()) { + if (!protoFiles.isEmpty()) { ruleContext.throwWithAttributeError("deps", FILES_NOT_ALLOWED_ERROR); } if (getProtoFiles().isEmpty() && !hasObjcProtoLibraryDependencies()) { - ruleContext.throwWithRuleError(NO_PROTOS_ERROR); + ruleContext.throwWithAttributeError("deps", NO_PROTOS_ERROR); } - - if (getPortableProtoFilters().isEmpty()) { - ruleContext.throwWithRuleError(PORTABLE_PROTO_FILTERS_EMPTY_ERROR); + if (getPortableProtoFilters().isEmpty() && !usesProtobuf()) { + ruleContext.throwWithAttributeError( + ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR, PORTABLE_PROTO_FILTERS_EMPTY_ERROR); } - if (usesObjcHeaderNames() || needsPerProtoIncludes() || getOptionsFile().isPresent()) { - ruleContext.throwWithRuleError(PORTABLE_PROTO_FILTERS_NOT_EXCLUSIVE_ERROR); + ruleContext.throwWithRuleError(PROTOBUF_ATTRIBUTES_NOT_EXCLUSIVE_ERROR); } if (hasPB2Dependencies()) { - ruleContext.throwWithRuleError(PROTOCOL_BUFFERS2_IN_PROTOBUF_DEPS_ERROR); + ruleContext.throwWithAttributeError("deps", PROTOCOL_BUFFERS2_IN_PROTOBUF_DEPS_ERROR); } - } else { - if (!protos.isEmpty()) { + if (!protoFiles.isEmpty()) { ruleContext.attributeWarning("deps", FILES_DEPRECATED_WARNING); } if (getProtoFiles().isEmpty()) { - ruleContext.throwWithRuleError(NO_PROTOS_ERROR); + ruleContext.throwWithAttributeError("deps", NO_PROTOS_ERROR); } if (hasObjcProtoLibraryDependencies()) { - ruleContext.throwWithRuleError(OBJC_PROTO_LIB_DEP_IN_PROTOCOL_BUFFERS2_DEPS_ERROR); + ruleContext.throwWithAttributeError( + "deps", OBJC_PROTO_LIB_DEP_IN_PROTOCOL_BUFFERS2_DEPS_ERROR); } } } @@ -168,6 +180,14 @@ final class ProtoAttributes { return false; } + /** Returns whether this target configures the uses_protobuf attribute. */ + boolean usesProtobuf() { + if (ruleContext.attributes().has(ObjcProtoLibraryRule.USES_PROTOBUF_ATTR, Type.BOOLEAN)) { + return ruleContext.attributes().get(ObjcProtoLibraryRule.USES_PROTOBUF_ATTR, Type.BOOLEAN); + } + return false; + } + /** Returns whether to use the protobuf library instead of the PB2 library. */ boolean hasPortableProtoFilters() { return ruleContext @@ -175,6 +195,15 @@ final class ProtoAttributes { .isAttributeValueExplicitlySpecified(ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR); } + /** + * Returns whether the target requires the use of protobuf or not. This is a convenience method to + * check that either the {@code portable_proto_filters} or the {@code uses_protobuf} attributes + * were passed. + */ + boolean requiresProtobuf() { + return hasPortableProtoFilters() || usesProtobuf(); + } + /** Returns the list of portable proto filters. */ ImmutableList<Artifact> getPortableProtoFilters() { if (ruleContext diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java index b1e1caecea..3a9af858c9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java @@ -16,6 +16,8 @@ package com.google.devtools.build.lib.rules.objc; import static com.google.devtools.build.lib.rules.objc.XcodeProductType.LIBRARY_STATIC; +import com.google.common.base.Function; +import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; @@ -584,12 +586,15 @@ final class ProtobufSupport { } private boolean isLinkingTarget() { - return !ruleContext - .attributes() - .isAttributeValueExplicitlySpecified(ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR); + // Since this is the ProtobufSupport helper class, check whether the current target has + // configured the protobuf attributes. If not, it's not an objc_proto_library rule, so it must + // be a linking rule (e.g. apple_binary). + return !attributes.requiresProtobuf(); } - // Returns the transitive portable proto filter files from a list of ObjcProtoProviders. + /** + * Returns the transitive portable proto filter files from a list of ObjcProtoProviders. + */ public static NestedSet<Artifact> getTransitivePortableProtoFilters( Iterable<ObjcProtoProvider> objcProtoProviders) { NestedSetBuilder<Artifact> portableProtoFilters = NestedSetBuilder.stableOrder(); @@ -598,4 +603,58 @@ final class ProtobufSupport { } return portableProtoFilters.build(); } + + /** + * Returns a target specific generated artifact that represents a portable filter file. + */ + public static Artifact getGeneratedPortableFilter(RuleContext ruleContext) { + return ruleContext.getUniqueDirectoryArtifact( + "_proto_filters", + "generated_filter_file.pbascii", + ruleContext.getConfiguration().getGenfilesDirectory()); + } + + /** + * Registers a FileWriteAction what writes a filter file into the given artifact. The contents + * of this file is a portable filter that allows all the transitive proto files contained in the + * given {@link ProtoSourcesProvider} providers. + */ + public static void registerPortableFilterGenerationAction( + RuleContext ruleContext, + Artifact generatedPortableFilter, + Iterable<ProtoSourcesProvider> protoProviders) { + ruleContext.registerAction( + FileWriteAction.create( + ruleContext, + generatedPortableFilter, + getGeneratedPortableFilterContents(ruleContext, protoProviders), + false)); + } + + private static String getGeneratedPortableFilterContents( + RuleContext ruleContext, Iterable<ProtoSourcesProvider> protoProviders) { + NestedSetBuilder<Artifact> protoFilesBuilder = NestedSetBuilder.stableOrder(); + for (ProtoSourcesProvider protoProvider : protoProviders) { + protoFilesBuilder.addTransitive(protoProvider.getTransitiveProtoSources()); + } + + Iterable<String> protoFilePaths = + Artifact.toRootRelativePaths( + Ordering.natural().immutableSortedCopy(protoFilesBuilder.build())); + + Iterable<String> filterLines = + Iterables.transform( + protoFilePaths, + new Function<String, String>() { + @Override + public String apply(String protoFilePath) { + return String.format("allowed_file: \"%s\"", protoFilePath); + } + }); + + return String.format( + "# Generated portable filter for %s\n\n", ruleContext.getLabel().getCanonicalForm()) + + Joiner.on("\n").join(filterLines); + } + } |