diff options
11 files changed, 105 insertions, 986 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 6a13ac18cc..5d74c1bf94 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 @@ -62,11 +62,10 @@ public class ObjcProtoAspect extends NativeAspectClass implements ConfiguredAspe 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 (attributes.requiresProtobuf()) { + // If this current target is an objc_proto_library, only then read the protos and portable + // filters it has configured in the attributes and add them to the ObjcProtoProvider.Builder + // instance. Validation for the correct target attributes is already done in ProtoSupport.java. + if (attributes.isObjcProtoLibrary()) { // Gather up all the dependency protos depended by this target. Iterable<ProtoSourcesProvider> protoProviders = 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 412968ece2..5a3f00553a 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 @@ -31,18 +31,8 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory { @Override public ConfiguredTarget create(final RuleContext ruleContext) throws InterruptedException, RuleErrorException { - - ProtoAttributes attributes = new ProtoAttributes(ruleContext); - attributes.validate(); - - if (attributes.requiresProtobuf()) { - return createProtobufTarget(ruleContext); - } else { - ruleContext.ruleWarning("The usage of objc_proto_library without the portable_proto_filters " - + "attribute has been deprecated with a deadline to migrate set to June 30th. Please " - + "refer to b/37274743 for more information."); - return createProtocolBuffers2Target(ruleContext); - } + new ProtoAttributes(ruleContext).validate(); + return createProtobufTarget(ruleContext); } private ConfiguredTarget createProtobufTarget(RuleContext ruleContext) @@ -96,19 +86,4 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory { return portableProtoFilters.build(); } - - private ConfiguredTarget createProtocolBuffers2Target(RuleContext ruleContext) - throws InterruptedException, RuleErrorException { - NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.stableOrder(); - - ProtocolBuffers2Support protoSupport = - new ProtocolBuffers2Support(ruleContext) - .registerGenerationActions() - .registerCompilationActions() - .addFilesToBuild(filesToBuild); - - return ObjcRuleClasses.ruleConfiguredTarget(ruleContext, filesToBuild.build()) - .addNativeDeclaredProvider(protoSupport.getObjcProvider()) - .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 bddb0a9a85..5a209c878e 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 @@ -28,8 +28,6 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; -import com.google.devtools.build.lib.packages.Attribute.ComputedDefault; -import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.rules.apple.AppleConfiguration; @@ -40,12 +38,9 @@ import com.google.devtools.build.lib.util.FileType; /** * Rule definition for objc_proto_library. * - * This is a temporary rule until it is better known how to support proto_library rules. + * <p>This is a temporary rule until it is better known how to support proto_library rules. */ public class ObjcProtoLibraryRule implements RuleDefinition { - static final String OPTIONS_FILE_ATTR = "options_file"; - 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"; @@ -54,9 +49,8 @@ public class ObjcProtoLibraryRule implements RuleDefinition { /** * Returns a newly built rule definition for objc_proto_library. * - * @param objcProtoAspect Aspect that traverses the dependency graph through the deps attribute - * to gather all proto files and portable filters depended by - * objc_proto_library targets using the new protobuf compiler/library. + * @param objcProtoAspect Aspect that traverses the dependency graph through the deps attribute to + * gather all proto files and portable filters depended by objc_proto_library targets. */ public ObjcProtoLibraryRule(ObjcProtoAspect objcProtoAspect) { this.objcProtoAspect = objcProtoAspect; @@ -68,33 +62,19 @@ public class ObjcProtoLibraryRule implements RuleDefinition { .requiresConfigurationFragments( CppConfiguration.class, ObjcConfiguration.class, AppleConfiguration.class) /* <!-- #BLAZE_RULE(objc_proto_library).ATTRIBUTE(deps) --> - The directly depended upon proto_library rules. + The directly depended upon proto_library and objc_proto_library rules. <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ .override( attr("deps", LABEL_LIST) - // Support for files in deps is for backwards compatibility. - .allowedRuleClasses("proto_library", "filegroup", "objc_proto_library") + .allowedRuleClasses("proto_library", "objc_proto_library") .aspect(objcProtoAspect) .legacyAllowAnyFileType()) - /* <!-- #BLAZE_RULE(objc_proto_library).ATTRIBUTE(options_file) --> - Optional options file to apply to protos which affects compilation (e.g. class - whitelist/blacklist settings). - <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ - .add(attr(OPTIONS_FILE_ATTR, LABEL).legacyAllowAnyFileType().singleArtifact().cfg(HOST)) - /* <!-- #BLAZE_RULE(objc_proto_library).ATTRIBUTE(use_objc_header_names) --> - If true, output headers with .pbobjc.h, rather than .pb.h. - <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ - .add(attr(USE_OBJC_HEADER_NAMES_ATTR, BOOLEAN).value(false)) - /* <!-- #BLAZE_RULE(objc_proto_library).ATTRIBUTE(per_proto_includes) --> - 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)) + .add(attr(USES_PROTOBUF_ATTR, BOOLEAN).value(true)) /* <!-- #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 @@ -110,42 +90,16 @@ public class ObjcProtoLibraryRule implements RuleDefinition { .allowedFileTypes(FileType.of(".py"), FileType.of(".sh")) .cfg(HOST) .singleArtifact() - .value( - new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR, USES_PROTOBUF_ATTR) { - @Override - public Object getDefault(AttributeMap rule) { - return usesProtobuf(rule) - ? env.getToolsLabel("//tools/objc:protobuf_compiler_wrapper") - : env.getToolsLabel("//tools/objc:compile_protos"); - } - })) + .value(env.getToolsLabel("//tools/objc:protobuf_compiler_wrapper"))) .add( attr(PROTO_COMPILER_SUPPORT_ATTR, LABEL) .legacyAllowAnyFileType() .cfg(HOST) - .value( - new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR, USES_PROTOBUF_ATTR) { - @Override - public Object getDefault(AttributeMap rule) { - return usesProtobuf(rule) - ? env.getToolsLabel("//tools/objc:protobuf_compiler_support") - : env.getToolsLabel("//tools/objc:proto_support"); - } - })) + .value(env.getToolsLabel("//tools/objc:protobuf_compiler_support"))) .add( attr(PROTO_LIB_ATTR, LABEL) .allowedRuleClasses("objc_library") - .value( - new ComputedDefault(PORTABLE_PROTO_FILTERS_ATTR, USES_PROTOBUF_ATTR) { - @Override - public Object getDefault(AttributeMap rule) { - if (usesProtobuf(rule)) { - return env.getLabel("//external:objc_protobuf_lib"); - } else { - return env.getLabel("//external:objc_proto_lib"); - } - } - })) + .value(env.getToolsLabel("//tools/objc:protobuf_lib"))) .add( ProtoSourceFileBlacklist.blacklistFilegroupAttribute( PROTOBUF_WELL_KNOWN_TYPES, @@ -159,21 +113,18 @@ public class ObjcProtoLibraryRule implements RuleDefinition { return RuleDefinition.Metadata.builder() .name("objc_proto_library") .factoryClass(ObjcProtoLibrary.class) - .ancestors(BaseRuleClasses.RuleBase.class, ObjcRuleClasses.LibtoolRule.class, - ObjcRuleClasses.XcrunRule.class, ObjcRuleClasses.CrosstoolRule.class) + .ancestors( + BaseRuleClasses.RuleBase.class, + ObjcRuleClasses.LibtoolRule.class, + 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) --> -<p>This rule produces a static library from the given proto_library dependencies, after applying an -options file.</p> +<p>This rule generates ObjC headers for the proto files given through the proto_library and +objc_proto_library dependencies.</p> <!-- #END_BLAZE_RULE -->*/ 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 890a37cb01..d525817d69 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 @@ -16,17 +16,13 @@ package com.google.devtools.build.lib.rules.objc; 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; -import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.configuredtargets.AbstractConfiguredTarget; @@ -36,8 +32,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.proto.ProtoSourceFileBlacklist; import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider; -import com.google.devtools.build.lib.syntax.Type; -import com.google.devtools.build.lib.util.FileType; import java.util.ArrayList; /** Common rule attributes used by an objc_proto_library. */ @@ -51,43 +45,12 @@ final class ProtoAttributes { ImmutableSet.of("url", "http", "https"); @VisibleForTesting - static final String FILES_NOT_ALLOWED_ERROR = - "Using files and filegroups in objc_proto_library with portable_proto_filters is not " - + "allowed. Please wrap the files inside a proto_library target."; - - @VisibleForTesting - static final String FILES_DEPRECATED_WARNING = - "Using files and filegroups in objc_proto_library is deprecated"; - - @VisibleForTesting - static final String NO_PROTOS_ERROR = - "no protos to compile - a non-empty deps attribute is required"; - - @VisibleForTesting - 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 = "The portable_proto_filters attribute can't be empty"; @VisibleForTesting - static final String OBJC_PROTO_LIB_DEP_IN_PROTOCOL_BUFFERS2_DEPS_ERROR = - "Protocol Buffers 2 objc_proto_library targets can't depend on other objc_proto_library " - + "targets. Please migrate your Protocol Buffers 2 objc_proto_library targets to use the " - + "portable_proto_filters attribute."; - - @VisibleForTesting - static final String PROTOCOL_BUFFERS2_IN_PROTOBUF_DEPS_ERROR = - "Protobuf objc_proto_library targets can't depend on Protocol Buffers 2 objc_proto_library " - + "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."; + static final String NO_PROTOS_ERROR = + "no protos to compile - a non-empty deps attribute is required"; private final RuleContext ruleContext; @@ -112,80 +75,21 @@ final class ProtoAttributes { * </ul> */ public void validate() throws RuleErrorException { - PrerequisiteArtifacts prerequisiteArtifacts = - ruleContext.getPrerequisiteArtifacts("deps", Mode.TARGET); - 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 (requiresProtobuf()) { - if (!protoFiles.isEmpty()) { - ruleContext.throwWithAttributeError("deps", FILES_NOT_ALLOWED_ERROR); - } - if (getProtoFiles().isEmpty() && !hasObjcProtoLibraryDependencies()) { - ruleContext.throwWithAttributeError("deps", NO_PROTOS_ERROR); - } - if (getPortableProtoFilters().isEmpty() && !usesProtobuf()) { - ruleContext.throwWithAttributeError( - ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR, PORTABLE_PROTO_FILTERS_EMPTY_ERROR); - } - if (usesObjcHeaderNames() - || needsPerProtoIncludes() - || getOptionsFile().isPresent()) { - ruleContext.throwWithRuleError(PROTOBUF_ATTRIBUTES_NOT_EXCLUSIVE_ERROR); - } - if (hasPB2Dependencies()) { - ruleContext.throwWithAttributeError("deps", PROTOCOL_BUFFERS2_IN_PROTOBUF_DEPS_ERROR); - } - } else { - if (!protoFiles.isEmpty()) { - ruleContext.attributeWarning("deps", FILES_DEPRECATED_WARNING); - } - if (getProtoFiles().isEmpty()) { - ruleContext.throwWithAttributeError("deps", NO_PROTOS_ERROR); - } - if (hasObjcProtoLibraryDependencies()) { - ruleContext.throwWithAttributeError( - "deps", OBJC_PROTO_LIB_DEP_IN_PROTOCOL_BUFFERS2_DEPS_ERROR); - } - } - } - - /** Returns whether the generated header files should have be of type pb.h or pbobjc.h. */ - boolean usesObjcHeaderNames() { - if (ruleContext - .attributes() - .has(ObjcProtoLibraryRule.USE_OBJC_HEADER_NAMES_ATTR, Type.BOOLEAN)) { - return ruleContext - .attributes() - .get(ObjcProtoLibraryRule.USE_OBJC_HEADER_NAMES_ATTR, Type.BOOLEAN); + if (getProtoFiles().isEmpty() && !hasObjcProtoLibraryDependencies()) { + ruleContext.throwWithAttributeError("deps", NO_PROTOS_ERROR); } - return false; - } - - /** Returns whether the includes should include each of the proto generated headers. */ - boolean needsPerProtoIncludes() { - if (ruleContext.attributes().has(ObjcProtoLibraryRule.PER_PROTO_INCLUDES_ATTR, Type.BOOLEAN)) { - return ruleContext - .attributes() - .get(ObjcProtoLibraryRule.PER_PROTO_INCLUDES_ATTR, Type.BOOLEAN); + if (hasPortableProtoFilters() && getPortableProtoFilters().isEmpty()) { + ruleContext.throwWithAttributeError( + ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR, PORTABLE_PROTO_FILTERS_EMPTY_ERROR); } - 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 the target is an objc_proto_library. It does so by making sure that the + * portable_proto_filters attribute exists in this target's attributes (even if it's empty). + */ + boolean isObjcProtoLibrary() { + return ruleContext.attributes().has(ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR); } /** Returns whether to use the protobuf library instead of the PB2 library. */ @@ -195,15 +99,6 @@ 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 @@ -223,19 +118,9 @@ final class ProtoAttributes { .list(); } - /** Returns the options file, or {@link Optional#absent} if it was not specified. */ - Optional<Artifact> getOptionsFile() { - if (ruleContext.attributes().has(ObjcProtoLibraryRule.OPTIONS_FILE_ATTR, LABEL)) { - return Optional.fromNullable( - ruleContext.getPrerequisiteArtifact(ObjcProtoLibraryRule.OPTIONS_FILE_ATTR, Mode.HOST)); - } - return Optional.absent(); - } - /** Returns the list of proto files to compile. */ NestedSet<Artifact> getProtoFiles() { return NestedSetBuilder.<Artifact>stableOrder() - .addAll(getProtoDepsFiles()) .addTransitive(getProtoDepsSources()) .build(); } @@ -353,25 +238,6 @@ final class ProtoAttributes { return artifacts.build(); } - /** - * Returns the list of proto files that were added directly into the deps attributes. This way of - * specifying the protos is deprecated, and displays a warning when the target does so. - */ - private ImmutableList<Artifact> getProtoDepsFiles() { - PrerequisiteArtifacts prerequisiteArtifacts = - ruleContext.getPrerequisiteArtifacts("deps", Mode.TARGET); - return prerequisiteArtifacts.filter(FileType.of(".proto")).list(); - } - - private boolean hasPB2Dependencies() { - for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) { - if (isObjcProtoLibrary(dep) && !hasProtobufProvider(dep)) { - return true; - } - } - return false; - } - private boolean hasObjcProtoLibraryDependencies() { for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) { if (isObjcProtoLibrary(dep)) { @@ -390,8 +256,4 @@ final class ProtoAttributes { return false; } } - - private boolean hasProtobufProvider(TransitiveInfoCollection dependency) { - return dependency.getProvider(ObjcProtoProvider.class) != null; - } } 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 348890eda1..4c7bb87400 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 @@ -528,10 +528,9 @@ final class ProtobufSupport { } private boolean isLinkingTarget() { - // 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(); + // Since this is the ProtobufSupport helper class, check whether the current target is + // an objc_proto_library. If not, it must be a linking rule (e.g. apple_binary). + return !attributes.isObjcProtoLibrary(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java deleted file mode 100644 index 1a4b2fbdb6..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java +++ /dev/null @@ -1,228 +0,0 @@ -// Copyright 2016 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.objc; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; -import com.google.common.collect.Ordering; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; -import com.google.devtools.build.lib.analysis.actions.FileWriteAction; -import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.vfs.FileSystemUtils; -import com.google.devtools.build.lib.vfs.PathFragment; - -/** - * Support for generating Objective C proto static libraries that registers actions which generate - * and compile the Objective C protos by using the deprecated ProtocolBuffers2 library and compiler. - * - * <p>Methods on this class can be called in any order without impacting the result. - */ -final class ProtocolBuffers2Support { - - private static final String UNIQUE_DIRECTORY_NAME = "_generated_protos"; - - private final RuleContext ruleContext; - private final ProtoAttributes attributes; - - /** - * Creates a new proto support for the ProtocolBuffers2 library. - * - * @param ruleContext context this proto library is constructed in - */ - public ProtocolBuffers2Support(RuleContext ruleContext) { - this.ruleContext = ruleContext; - this.attributes = new ProtoAttributes(ruleContext); - } - - /** - * Register the proto generation actions. These actions generate the ObjC/CPP code to be compiled - * by this rule. - */ - public ProtocolBuffers2Support registerGenerationActions() throws InterruptedException { - ruleContext.registerAction( - FileWriteAction.create( - ruleContext, - getProtoInputsFile(), - getProtoInputsFileContents( - attributes.filterWellKnownProtos(attributes.getProtoFiles())), - false)); - - ruleContext.registerAction( - ObjcRuleClasses.spawnOnDarwinActionBuilder() - .setMnemonic("GenObjcPB2Protos") - .addInput(attributes.getProtoCompiler()) - .addInputs(attributes.getProtoCompilerSupport()) - .addInput(getProtoInputsFile()) - .addTransitiveInputs(attributes.getProtoFiles()) - .addInputs(attributes.getOptionsFile().asSet()) - .addOutputs(getGeneratedProtoOutputs(getHeaderExtension())) - .addOutputs(getGeneratedProtoOutputs(getSourceExtension())) - .setExecutable(PathFragment.create("/usr/bin/python")) - .addCommandLine(getGenerationCommandLine()) - .build(ruleContext)); - return this; - } - - /** - * Registers the actions that will compile the generated code. - */ - public ProtocolBuffers2Support registerCompilationActions() - throws RuleErrorException, InterruptedException { - CompilationSupport compilationSupport = - new CompilationSupport.Builder() - .setRuleContext(ruleContext) - .doNotUseDeps() - .doNotUsePch() - .build(); - - compilationSupport.registerCompileAndArchiveActions(getCommon()); - return this; - } - - /** Adds the generated files to the set of files to be output when this rule is built. */ - public ProtocolBuffers2Support addFilesToBuild(NestedSetBuilder<Artifact> filesToBuild) - throws InterruptedException { - filesToBuild - .addAll(getGeneratedProtoOutputs(getHeaderExtension())) - .addAll(getGeneratedProtoOutputs(getSourceExtension())) - .addAll(getCompilationArtifacts().getArchive().asSet()); - return this; - } - - /** Returns the ObjcProvider for this target. */ - public ObjcProvider getObjcProvider() { - return getCommon().getObjcProvider(); - } - - private String getHeaderExtension() { - return ".pb" + (attributes.usesObjcHeaderNames() ? "objc.h" : ".h"); - } - - private String getSourceExtension() { - return ".pb.m"; - } - - private ObjcCommon getCommon() { - return new ObjcCommon.Builder(ruleContext) - .setIntermediateArtifacts(new IntermediateArtifacts(ruleContext, "")) - .setHasModuleMap() - .setCompilationArtifacts(getCompilationArtifacts()) - .addIncludes(getIncludes()) - .addDepObjcProviders( - ruleContext.getPrerequisites( - ObjcRuleClasses.PROTO_LIB_ATTR, Mode.TARGET, ObjcProvider.SKYLARK_CONSTRUCTOR)) - .build(); - } - - private CompilationArtifacts getCompilationArtifacts() { - Iterable<Artifact> generatedSources = getGeneratedProtoOutputs(getSourceExtension()); - return new CompilationArtifacts.Builder() - .setIntermediateArtifacts(new IntermediateArtifacts(ruleContext, "")) - .addAdditionalHdrs(getGeneratedProtoOutputs(getHeaderExtension())) - .addAdditionalHdrs(generatedSources) - .addNonArcSrcs(generatedSources) - .build(); - } - - private Artifact getProtoInputsFile() { - return ruleContext.getUniqueDirectoryArtifact( - "_protos", "_proto_input_files", ruleContext.getConfiguration().getGenfilesDirectory()); - } - - private String getProtoInputsFileContents(Iterable<Artifact> outputProtos) { - // Sort the file names to make the remote action key independent of the precise deps structure. - // compile_protos.py will sort the input list anyway. - Iterable<Artifact> sorted = Ordering.natural().immutableSortedCopy(outputProtos); - return Artifact.joinExecPaths("\n", sorted); - } - - private CustomCommandLine getGenerationCommandLine() { - CustomCommandLine.Builder commandLineBuilder = - new CustomCommandLine.Builder() - .addExecPath(attributes.getProtoCompiler()) - .add("--input-file-list") - .addExecPath(getProtoInputsFile()) - .add("--output-dir") - .addDynamicString(getWorkspaceRelativeOutputDir().getSafePathString()) - .add("--working-dir") - .add("."); - - if (attributes.getOptionsFile().isPresent()) { - commandLineBuilder - .add("--compiler-options-path") - .addExecPath(attributes.getOptionsFile().get()); - } - - if (attributes.usesObjcHeaderNames()) { - commandLineBuilder.add("--use-objc-header-names"); - } - return commandLineBuilder.build(); - } - - public ImmutableSet<PathFragment> getIncludes() { - ImmutableSet.Builder<PathFragment> searchPathEntriesBuilder = - new ImmutableSet.Builder<PathFragment>().add(getWorkspaceRelativeOutputDir()); - - if (attributes.needsPerProtoIncludes()) { - PathFragment generatedProtoDir = PathFragment.create( - getWorkspaceRelativeOutputDir(), ruleContext.getLabel().getPackageFragment()); - - searchPathEntriesBuilder - .add(generatedProtoDir) - .addAll( - Iterables.transform( - getGeneratedProtoOutputs(getHeaderExtension()), - input -> input.getExecPath().getParentDirectory())); - } - - return searchPathEntriesBuilder.build(); - } - - private PathFragment getWorkspaceRelativeOutputDir() { - // Generate sources in a package-and-rule-scoped directory; adds both the - // package-and-rule-scoped directory and the header-containing-directory to the include path - // of dependers. - PathFragment rootRelativeOutputDir = ruleContext.getUniqueDirectory(UNIQUE_DIRECTORY_NAME); - - return PathFragment.create( - ruleContext.getBinOrGenfilesDirectory().getExecPath(), rootRelativeOutputDir); - } - - private Iterable<Artifact> getGeneratedProtoOutputs(String extension) { - ImmutableList.Builder<Artifact> builder = new ImmutableList.Builder<>(); - for (Artifact protoFile : attributes.filterWellKnownProtos(attributes.getProtoFiles())) { - String protoFileName = FileSystemUtils.removeExtension(protoFile.getFilename()); - String generatedOutputName = attributes.getGeneratedProtoFilename(protoFileName, false); - - PathFragment generatedFilePath = PathFragment.create( - protoFile.getRootRelativePath().getParentDirectory(), - PathFragment.create(generatedOutputName)); - - PathFragment outputFile = FileSystemUtils.appendExtension(generatedFilePath, extension); - - if (outputFile != null) { - builder.add( - ruleContext.getUniqueDirectoryArtifact( - UNIQUE_DIRECTORY_NAME, outputFile, ruleContext.getBinOrGenfilesDirectory())); - } - } - return builder.build(); - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index 1cf1cb8c89..bde6e37421 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -57,14 +57,6 @@ public final class BazelAnalysisMock extends AnalysisMock { ImmutableList.of( "local_repository(name = 'bazel_tools', path = '" + bazelToolWorkspace + "')", "local_repository(name = 'local_config_xcode', path = '/local_config_xcode')", - "bind(", - " name = 'objc_proto_lib',", - " actual = '@bazel_tools//objcproto:ProtocolBuffers_lib',", - ")", - "bind(", - " name = 'objc_protobuf_lib',", - " actual = '@bazel_tools//objcproto:protobuf_lib',", - ")", "local_repository(name = 'com_google_protobuf', path = '/protobuf')", "local_repository(name = 'com_google_protobuf_java', path = '/protobuf')", "bind(name = 'android/sdk', actual='@bazel_tools//tools/android:sdk')", @@ -168,10 +160,6 @@ public final class BazelAnalysisMock extends AnalysisMock { "/bazel_tools_workspace/objcproto/BUILD", "package(default_visibility=['//visibility:public'])", "objc_library(", - " name = 'ProtocolBuffers_lib',", - " srcs = ['empty.m'],", - ")", - "objc_library(", " name = 'protobuf_lib',", " srcs = ['empty.m'],", " hdrs = ['include/header.h'],", diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java index bc99b16aa2..43176008a5 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockObjcSupport.java @@ -145,7 +145,8 @@ public final class MockObjcSupport { " name = 'version5_8',", " version = '5.8',", ")", - "objc_library(name = 'dummy_lib', srcs = ['objc_dummy.mm'])"); + "objc_library(name = 'dummy_lib', srcs = ['objc_dummy.mm'])", + "alias(name = 'protobuf_lib', actual = '//objcproto:protobuf_lib')"); // If the bazel tools repository is not in the workspace, also create a workspace tools/objc // package with a few lingering dependencies. // TODO(b/64537078): Move these dependencies underneath the tools workspace. @@ -220,10 +221,6 @@ public final class MockObjcSupport { TestConstants.TOOLS_REPOSITORY_SCRATCH + "objcproto/BUILD", "package(default_visibility=['//visibility:public'])", "objc_library(", - " name = 'ProtocolBuffers_lib',", - " srcs = ['empty.m'],", - ")", - "objc_library(", " name = 'protobuf_lib',", " srcs = ['empty.m'],", " hdrs = ['include/header.h'],", diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/LegacyObjcProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/LegacyObjcProtoLibraryTest.java deleted file mode 100644 index c4ef886914..0000000000 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/LegacyObjcProtoLibraryTest.java +++ /dev/null @@ -1,32 +0,0 @@ -// 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.rules.objc; - -import com.google.devtools.build.lib.rules.objc.ObjcCommandLineOptions.ObjcCrosstoolMode; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** - * Legacy test: These tests test --experimental_objc_crosstool=off. See README. - */ -@RunWith(JUnit4.class) -@LegacyTest -public class LegacyObjcProtoLibraryTest extends ObjcProtoLibraryTest { - @Override - protected ObjcCrosstoolMode getObjcCrosstoolMode() { - return ObjcCrosstoolMode.OFF; - } - -} diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspectTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspectTest.java index ae80e06379..c19475834c 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspectTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspectTest.java @@ -155,7 +155,6 @@ public final class ObjcProtoAspectTest extends ObjcRuleTestCase { "objc_proto_library(", " name = 'objc_proto',", " deps = [':protos'],", - " uses_protobuf = 1,", ")"); ConfiguredTarget topTarget = getObjcProtoAspectConfiguredTarget("//x:x"); ObjcProtoProvider objcProtoProvider = topTarget.getProvider(ObjcProtoProvider.class); @@ -187,7 +186,6 @@ public final class ObjcProtoAspectTest extends ObjcRuleTestCase { "objc_proto_library(", " name = 'objc_proto_all',", " deps = [':objc_proto_1', ':objc_proto_2'],", - " uses_protobuf = 1,", ")", "objc_proto_library(", " name = 'objc_proto_1',", @@ -197,7 +195,6 @@ public final class ObjcProtoAspectTest extends ObjcRuleTestCase { "objc_proto_library(", " name = 'objc_proto_2',", " deps = [':protos'],", - " uses_protobuf = 1,", ")"); ConfiguredTarget topTarget = getObjcProtoAspectConfiguredTarget("//x:x"); ObjcProtoProvider objcProtoProvider = topTarget.getProvider(ObjcProtoProvider.class); diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryTest.java index dc4873cd04..c2a6633ec8 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryTest.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandAction; @@ -26,6 +27,7 @@ import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.actions.SymlinkAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.packages.util.MockObjcSupport; @@ -56,7 +58,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { "apple_binary(", " name = 'opl_binary',", " deps = [':opl_protobuf'],", - " platform_type = 'tvos'", + " platform_type = 'ios'", ")", "objc_library(", " name = 'non_strict_lib',", @@ -69,11 +71,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { ")", "", "objc_proto_library(", - " name = 'opl',", - " deps = [':protolib'],", - ")", - "", - "objc_proto_library(", " name = 'nested_opl',", " deps = [':opl_protobuf'],", " portable_proto_filters = ['nested_filter.txt'],", @@ -95,10 +92,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { " 'proto_filter.txt',", " ],", ")", - "objc_proto_library(", - " name = 'opl_well_known_types',", - " deps = [':protolib_well_known_types'],", - ")", "", "filegroup(", " name = 'portable_proto_filters',", @@ -158,7 +151,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { " srcs = ['file_a_genfile.proto'],", " deps = ['//dep:dep_lib'],", ")", - "", "objc_proto_library(", " name = 'gen_opl',", " deps = [':gen_protolib'],", @@ -178,19 +170,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { @Test public void testOutputs() throws Exception { - NestedSet<Artifact> filesToBuild = getFilesToBuild(getConfiguredTarget("//package:opl")); - assertThat(Artifact.toRootRelativePaths(filesToBuild)) - .containsAllOf( - "package/libopl.a", - "package/_generated_protos/opl/package/FileA.pb.h", - "package/_generated_protos/opl/package/FileA.pb.m", - "package/_generated_protos/opl/package/dir/FileB.pb.h", - "package/_generated_protos/opl/package/dir/FileB.pb.m", - "package/_generated_protos/opl/dep/File.pb.h"); - } - - @Test - public void testOutputsProtobuf() throws Exception { NestedSet<Artifact> filesToBuild = getFilesToBuild(getConfiguredTarget("//package:opl_protobuf")); assertThat(Artifact.toRootRelativePaths(filesToBuild)) @@ -203,20 +182,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testOutputsWithAutoUnionExperiment() throws Exception { - NestedSet<Artifact> filesToBuild = getFilesToBuild(getConfiguredTarget("//package:opl")); - assertThat(Artifact.toRootRelativePaths(filesToBuild)) - .containsAllOf( - "package/libopl.a", - "package/_generated_protos/opl/package/FileA.pb.h", - "package/_generated_protos/opl/package/FileA.pb.m", - "package/_generated_protos/opl/package/dir/FileB.pb.h", - "package/_generated_protos/opl/package/dir/FileB.pb.m", - "package/_generated_protos/opl/dep/File.pb.h"); - } - - @Test - public void testDependingOnProtobufObjcProtoLibrary() throws Exception { + public void testDependingObjcProtoLibrary() throws Exception { NestedSet<Artifact> filesToBuild = getFilesToBuild(getConfiguredTarget("//package:nested_opl")); assertThat(Artifact.toRootRelativePaths(filesToBuild)) .containsAllOf( @@ -227,7 +193,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testOutputsProtobufWithAutoUnionExperiment() throws Exception { + public void testOutputsWithAutoUnion() throws Exception { NestedSet<Artifact> filesToBuild = getFilesToBuild(getConfiguredTarget("//package:opl_protobuf")); assertThat(Artifact.toRootRelativePaths(filesToBuild)) @@ -242,31 +208,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testPB2GeneratedFileNames() throws Exception { - NestedSet<Artifact> filesToBuild = - getFilesToBuild(getConfiguredTarget("//package:opl_pb2_special_names")); - assertThat(Artifact.toRootRelativePaths(filesToBuild)) - .containsAllOf( - "package/_generated_protos/opl_pb2_special_names/package/J2ObjcDescriptor.pb.h", - "package/_generated_protos/opl_pb2_special_names/package/J2ObjcDescriptor.pb.m", - "package/_generated_protos/opl_pb2_special_names/package/Http.pb.h", - "package/_generated_protos/opl_pb2_special_names/package/Http.pb.m", - "package/_generated_protos/opl_pb2_special_names/package/Https.pb.h", - "package/_generated_protos/opl_pb2_special_names/package/Https.pb.m", - "package/_generated_protos/opl_pb2_special_names/package/SomeUrlBlah.pb.h", - "package/_generated_protos/opl_pb2_special_names/package/SomeUrlBlah.pb.m", - "package/_generated_protos/opl_pb2_special_names/package/ThumbnailUrl.pb.h", - "package/_generated_protos/opl_pb2_special_names/package/ThumbnailUrl.pb.m", - "package/_generated_protos/opl_pb2_special_names/package/Url.pb.h", - "package/_generated_protos/opl_pb2_special_names/package/Url.pb.m", - "package/_generated_protos/opl_pb2_special_names/package/Url2Https.pb.h", - "package/_generated_protos/opl_pb2_special_names/package/Url2Https.pb.m", - "package/_generated_protos/opl_pb2_special_names/package/Urlbar.pb.h", - "package/_generated_protos/opl_pb2_special_names/package/Urlbar.pb.m"); - } - - @Test - public void testProtobufGeneratedFileNames() throws Exception { + public void testGeneratedFileNames() throws Exception { NestedSet<Artifact> filesToBuild = getFilesToBuild(getConfiguredTarget("//package:opl_protobuf_special_names")); String outputPath = "package/_generated_protos/opl_protobuf_special_names/package/"; @@ -291,21 +233,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testOutputsPB2WithWellKnownTypes() throws Exception { - NestedSet<Artifact> filesToBuild = - getFilesToBuild(getConfiguredTarget("//package:opl_well_known_types")); - assertThat(Artifact.toRootRelativePaths(filesToBuild)) - .containsAllOf( - "package/_generated_protos/opl_well_known_types/package/FileA.pb.h", - "package/_generated_protos/opl_well_known_types/package/FileA.pb.m"); - assertThat(Artifact.toRootRelativePaths(filesToBuild)) - .containsNoneOf( - "package/_generated_protos/opl_well_known_types/objcproto/WellKnownType.pb.h", - "package/_generated_protos/opl_well_known_types/objcproto/WellKnownType.pb.m"); - } - - @Test - public void testOutputsProtobufWithWellKnownTypes() throws Exception { + public void testOutputsWithWellKnownTypes() throws Exception { NestedSet<Artifact> filesToBuild = getFilesToBuild(getConfiguredTarget("//package:opl_protobuf_well_known_types")); assertThat(Artifact.toRootRelativePaths(filesToBuild)) @@ -325,50 +253,14 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { NestedSet<Artifact> filesToBuild = getFilesToBuild(getConfiguredTarget("//package:gen_opl")); assertThat(Artifact.toRootRelativePaths(filesToBuild)) .containsAllOf( - "package/libgen_opl.a", - "package/_generated_protos/gen_opl/package/FileAGenfile.pb.h", - "package/_generated_protos/gen_opl/package/FileAGenfile.pb.m"); + "package/_generated_protos/gen_opl/package/FileAGenfile.pbobjc.h", + "package/_generated_protos/gen_opl/package/FileAGenfile.pbobjc.m"); } @Test public void testSourceGenerationAction() throws Exception { Artifact sourceFile = ActionsTestUtil.getFirstArtifactEndingWith( - getFilesToBuild(getConfiguredTarget("//package:opl")), "/FileA.pb.m"); - SpawnAction action = (SpawnAction) getGeneratingAction(sourceFile); - - Artifact inputFileList = - ActionsTestUtil.getFirstArtifactEndingWith(action.getInputs(), "/_proto_input_files"); - - ImmutableList<String> protoInputs = - ImmutableList.of("dep/file.proto", "package/file_a.proto", "package/dir/file_b.proto"); - - assertThat(action.getArguments()) - .containsExactly( - "/usr/bin/python", - "tools/objc/compile_protos.py", - "--input-file-list", - inputFileList.getExecPathString(), - "--output-dir", - // 2x parent directory because the package has one element ("package") - sourceFile.getExecPath().getParentDirectory().getParentDirectory().toString(), - "--working-dir", ".") - .inOrder(); - assertRequiresDarwin(action); - assertThat(Artifact.toRootRelativePaths(action.getInputs())).containsAllOf( - "tools/objc/compile_protos.py", - "tools/objc/proto_support"); - assertThat(Artifact.toRootRelativePaths(action.getInputs())).containsAllIn(protoInputs); - assertThat(action.getInputs()).contains(inputFileList); - - FileWriteAction inputListAction = (FileWriteAction) getGeneratingAction(inputFileList); - assertThat(inputListAction.getFileContents()).isEqualTo(sortedJoin(protoInputs)); - } - - @Test - public void testProtobufSourceGenerationAction() throws Exception { - Artifact sourceFile = - ActionsTestUtil.getFirstArtifactEndingWith( getFilesToBuild(getConfiguredTarget("//package:opl_protobuf")), "/FileA.pbobjc.m"); SpawnAction action = (SpawnAction) getGeneratingAction(sourceFile); @@ -413,7 +305,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testProtobufWithWellKnownTypesProtoListInput() throws Exception { + public void testWellKnownTypesProtoListInput() throws Exception { Artifact sourceFile = ActionsTestUtil.getFirstArtifactEndingWith( getFilesToBuild(getConfiguredTarget("//package:opl_protobuf_well_known_types")), @@ -437,46 +329,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testUseObjcHeaders() throws Exception { - scratch.file("objcheaderpackage/BUILD", - "objc_proto_library(", - " name = 'opl',", - " deps = ['//package:protolib'],", - " use_objc_header_names = 1,", - ")"); - - Artifact sourceFile = - ActionsTestUtil.getFirstArtifactEndingWith( - getFilesToBuild(getConfiguredTarget("//objcheaderpackage:opl")), "/FileA.pb.m"); - SpawnAction action = (SpawnAction) getGeneratingAction(sourceFile); - - assertThat(action.getArguments()).contains("--use-objc-header-names"); - - NestedSet<Artifact> filesToBuild = - getFilesToBuild(getConfiguredTarget("//objcheaderpackage:opl")); - assertThat(Artifact.toRootRelativePaths(filesToBuild)).containsAllOf( - "objcheaderpackage/_generated_protos/opl/package/FileA.pbobjc.h", - "objcheaderpackage/_generated_protos/opl/package/FileA.pb.m", - "objcheaderpackage/_generated_protos/opl/package/dir/FileB.pbobjc.h", - "objcheaderpackage/_generated_protos/opl/package/dir/FileB.pb.m" - ); - } - - @Test - public void testProtobufCompilationAction() throws Exception { - useConfiguration("--ios_cpu=i386"); - - ConfiguredTarget target = getConfiguredTarget("//package:opl_protobuf"); - Artifact sourceFile = - ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), "/FileA.pbobjc.m"); - SpawnAction generateAction = (SpawnAction) getGeneratingAction(sourceFile); - - assertThat(generateAction).isNotNull(); - } - - - @Test - public void testProtobufObjcProviderWithAutoUnionExperiment() throws Exception { + public void testObjcProviderWithAutoUnion() throws Exception { ConfiguredTarget target = getConfiguredTarget("//package:opl_protobuf"); Artifact headerFile = ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), "/FileA.pbobjc.h"); @@ -492,19 +345,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testPerProtoIncludes() throws Exception { - ConfiguredTarget target = getConfiguredTarget("//package:opl"); - Artifact headerFile = ActionsTestUtil.getFirstArtifactEndingWith( - getFilesToBuild(target), "/FileA.pb.h"); - - ObjcProvider provider = providerForTarget("//package:opl"); - assertThat(provider.get(ObjcProvider.INCLUDE).toSet()).containsExactly( - // 2x parent directory because the package has one element ("package") - headerFile.getExecPath().getParentDirectory().getParentDirectory() - ); - } - - @Test public void testErrorForNoDepsAttribute() throws Exception { checkError( "x", "x", ProtoAttributes.NO_PROTOS_ERROR, "objc_proto_library(", " name = 'x',", ")"); @@ -522,67 +362,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { ")"); } - // This is a test for deprecated functionality. - @Test - public void testErrorForDepWithFilegroupWithoutProtoFiles() throws Exception { - checkError( - "x", - "x", - ProtoAttributes.NO_PROTOS_ERROR, - "objc_proto_library(", - " name = 'x',", - " deps = [':fg'],", - ")", - "filegroup(", - " name = 'fg',", - " srcs = ['file.dat'],", - ")"); - } - - @Test - public void testWarningForProtoSourceDeps() throws Exception { - checkWarning( - "x", - "x", - ProtoAttributes.FILES_DEPRECATED_WARNING, - "objc_proto_library(", - " name = 'x',", - " deps = ['foo.proto'],", - ")"); - } - - @Test - public void testWarningForFilegroupDeps() throws Exception { - checkWarning( - "x", - "x", - ProtoAttributes.FILES_DEPRECATED_WARNING, - "filegroup(", - " name = 'protos',", - " srcs = ['foo.proto'],", - ")", - "objc_proto_library(", - " name = 'x',", - " deps = [':protos'],", - ")"); - } - - @Test - public void testObjcCopts() throws Exception { - useConfiguration("--objccopt=-foo"); - - List<String> args = compileAction("//package:opl", "FileA.pb.o").getArguments(); - assertThat(args).contains("-foo"); - } - - @Test - public void testObjcCopts_argumentOrdering() throws Exception { - useConfiguration("--objccopt=-foo"); - - List<String> args = compileAction("//package:opl", "FileA.pb.o").getArguments(); - assertThat(args).containsAllOf("-fno-objc-arc", "-foo").inOrder(); - } - @Test public void testErrorForPortableProtoFiltersEmpty() throws Exception { checkError( @@ -601,85 +380,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testErrorWhenDependingOnPB2FromProtobufTarget() throws Exception { - checkError( - "x", - "x", - ProtoAttributes.PROTOCOL_BUFFERS2_IN_PROTOBUF_DEPS_ERROR, - "objc_proto_library(", - " name = 'x',", - " portable_proto_filters = ['filter.pbascii'],", - " deps = [':pb2', ':protos_b'],", - ")", - "objc_proto_library(", - " name = 'pb2',", - " deps = [':protos_a'],", - ")", - "proto_library(", - " name = 'protos_a',", - " srcs = ['a.proto'],", - ")", - "proto_library(", - " name = 'protos_b',", - " srcs = ['b.proto'],", - ")"); - } - - @Test - public void testErrorWhenDependingOnPB2FromPB2Target() throws Exception { - checkError( - "x", - "x", - ProtoAttributes.OBJC_PROTO_LIB_DEP_IN_PROTOCOL_BUFFERS2_DEPS_ERROR, - "objc_proto_library(", - " name = 'x',", - " deps = [':pb2', ':protos_b'],", - ")", - "objc_proto_library(", - " name = 'pb2',", - " deps = [':protos_a'],", - ")", - "proto_library(", - " name = 'protos_a',", - " srcs = ['a.proto'],", - ")", - "proto_library(", - " name = 'protos_b',", - " srcs = ['b.proto'],", - ")"); - } - - @Test - public void testErrorForPortableProtoFiltersWithUseObjcHeaderNames() throws Exception { - checkErrorForPortableProtoFilterWithPb2Option("use_objc_header_names = 1"); - } - - @Test - public void testErrorForPortableProtoFiltersWithPerProtoIncludes() throws Exception { - checkErrorForPortableProtoFilterWithPb2Option("per_proto_includes = 1"); - } - - @Test - public void testErrorForPortableProtoFiltersWithOptionsFile() throws Exception { - checkErrorForPortableProtoFilterWithPb2Option("options_file = 'options_file.txt'"); - } - - @Test - public void testErrorForUsesProtobufWithUseObjcHeaderNames() throws Exception { - checkErrorForUsesProtobufWithPb2Option("use_objc_header_names = 1"); - } - - @Test - public void testErrorForUsesProtobufWithPerProtoIncludes() throws Exception { - checkErrorForUsesProtobufWithPb2Option("per_proto_includes = 1"); - } - - @Test - public void testErrorForUsesProtobufWithOptionsFile() throws Exception { - checkErrorForUsesProtobufWithPb2Option("options_file = 'options_file.txt'"); - } - - @Test public void testModulemapCreatedForNonLinkingTargets() throws Exception { checkOnlyLibModuleMapsArePresentForTarget("//package:opl_protobuf"); } @@ -689,75 +389,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { checkOnlyLibModuleMapsArePresentForTarget("//package:opl_binary"); } - @Test - public void testErrorForPortableProtoFilterFilesInDeps() throws Exception { - checkError( - "x", - "x", - ProtoAttributes.FILES_NOT_ALLOWED_ERROR, - "objc_proto_library(", - " name = 'x',", - " portable_proto_filters = ['proto_filter.txt'],", - " deps = [':protos'],", - ")", - "filegroup(", - " name = 'protos',", - " srcs = ['file.proto'],", - ")"); - } - - @Test - public void testErrorForUsesProtobufAsFalseWithFilters() throws Exception { - checkError( - "x", - "x", - ProtoAttributes.USES_PROTOBUF_CANT_BE_SPECIFIED_AS_FALSE, - "objc_proto_library(", - " name = 'x',", - " uses_protobuf = 0,", - " portable_proto_filters = ['myfilter.pbascii'],", - " deps = [':protos'],", - ")", - "proto_library(", - " name = 'protos',", - " srcs = ['file.proto'],", - ")"); - } - - private void checkErrorForPortableProtoFilterWithPb2Option(String pb2Option) throws Exception { - checkError( - "x", - "x", - ProtoAttributes.PROTOBUF_ATTRIBUTES_NOT_EXCLUSIVE_ERROR, - "objc_proto_library(", - " name = 'x',", - " portable_proto_filters = ['proto_filter.txt'],", - " " + pb2Option + ",", - " deps = [':protos'],", - ")", - "proto_library(", - " name = 'protos',", - " srcs = ['file.proto'],", - ")"); - } - - private void checkErrorForUsesProtobufWithPb2Option(String pb2Option) throws Exception { - checkError( - "x", - "x", - ProtoAttributes.PROTOBUF_ATTRIBUTES_NOT_EXCLUSIVE_ERROR, - "objc_proto_library(", - " name = 'x',", - " uses_protobuf = 1,", - " " + pb2Option + ",", - " deps = [':protos'],", - ")", - "proto_library(", - " name = 'protos',", - " srcs = ['file.proto'],", - ")"); - } - private static String sortedJoin(Iterable<String> elements) { return Joiner.on('\n').join(Ordering.natural().immutableSortedCopy(elements)); } @@ -780,30 +411,6 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { @Test public void testObjcProvider() throws Exception { - ConfiguredTarget target = getConfiguredTarget("//package:opl"); - Artifact headerFile = - ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), "/FileA.pb.h"); - Artifact sourceFile = - ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), "/FileA.pb.m"); - - ObjcProvider provider = providerForTarget("//package:opl"); - assertThat(provider.get(ObjcProvider.INCLUDE).toSet()) - .contains(headerFile.getExecPath().getParentDirectory().getParentDirectory()); - - ConfiguredTarget libProtoBufTarget = getConfiguredTarget("//objcproto:ProtocolBuffers_lib"); - assertThat(provider.get(ObjcProvider.LIBRARY).toSet()) - .containsExactly( - getBinArtifact("libopl.a", target), - getBinArtifact("libProtocolBuffers_lib.a", libProtoBufTarget)); - - assertThat(provider.get(ObjcProvider.HEADER).toSet()).containsAllOf(headerFile, sourceFile); - - assertThat(provider.get(ObjcProvider.INCLUDE).toSet()) - .contains(headerFile.getExecPath().getParentDirectory().getParentDirectory()); - } - - @Test - public void testProtobufObjcProvider() throws Exception { ConfiguredTarget target = getConfiguredTarget("//package:opl_protobuf"); Artifact headerFile = ActionsTestUtil.getFirstArtifactEndingWith(getFilesToBuild(target), "/FileA.pbobjc.h"); @@ -822,36 +429,18 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { } @Test - public void testCompilationActionInCoverageMode() throws Exception { - useConfiguration("--collect_code_coverage"); - - ConfiguredTarget target = getConfiguredTarget("//package:opl"); - CommandAction linkAction = - (CommandAction) getGeneratingAction(getBinArtifact("libopl.a", target)); - - CommandAction compileAction = - (CommandAction) - getGeneratingAction( - ActionsTestUtil.getFirstArtifactEndingWith(linkAction.getInputs(), "/FileA.pb.o")); - - assertThat(Artifact.toRootRelativePaths(compileAction.getOutputs())) - .containsAllOf( - "package/_objs/opl/package/_generated_protos/opl/package/FileA.pb.o", - "package/_objs/opl/package/_generated_protos/opl/package/FileA.pb.gcno"); - } - - @Test public void testModuleMapActionFiltersHeaders() throws Exception { - ConfiguredTarget configuredTarget = getConfiguredTarget("//package:opl"); - Artifact moduleMap = getGenfilesArtifact("opl.modulemaps/module.modulemap", configuredTarget); + ConfiguredTarget configuredTarget = getConfiguredTarget("//package:opl_protobuf"); + Artifact moduleMap = + getGenfilesArtifact("opl_protobuf.modulemaps/module.modulemap", configuredTarget); CppModuleMapAction genMap = (CppModuleMapAction) getGeneratingAction(moduleMap); assertThat(Artifact.toRootRelativePaths(genMap.getPrivateHeaders())).isEmpty(); assertThat(Artifact.toRootRelativePaths(genMap.getPublicHeaders())) .containsExactly( - "package/_generated_protos/opl/package/FileA.pb.h", - "package/_generated_protos/opl/package/dir/FileB.pb.h", - "package/_generated_protos/opl/dep/File.pb.h"); + "package/_generated_protos/opl_protobuf/package/FileA.pbobjc.h", + "package/_generated_protos/opl_protobuf/package/dir/FileB.pbobjc.h", + "package/_generated_protos/opl_protobuf/dep/File.pbobjc.h"); } @Test @@ -859,25 +448,34 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { useConfiguration("--cpu=ios_i386"); ApplePlatform platform = ApplePlatform.IOS_SIMULATOR; - ConfiguredTarget target = getConfiguredTarget("//package:opl"); - CommandAction linkAction = - (CommandAction) getGeneratingAction(getBinArtifact("libopl.a", target)); + // Because protos are linked/compiled within the apple_binary context, we need to traverse the + // action graph to find the linked protos (.a) and compiled protos (.o). + ConfiguredTarget binaryTarget = getConfiguredTarget("//package:opl_binary"); + SymlinkAction symlinkAction = + (SymlinkAction) getGeneratingAction(getBinArtifact("opl_binary_lipobin", binaryTarget)); - CommandAction compileAction = - (CommandAction) - getGeneratingAction( - ActionsTestUtil.getFirstArtifactEndingWith(linkAction.getInputs(), "/FileA.pb.o")); + Artifact binaryInput = Iterables.getOnlyElement(symlinkAction.getInputs()); - Artifact sourceFile = + CommandAction linkAction = (CommandAction) getGeneratingAction(binaryInput); + + Artifact linkedProtos = ActionsTestUtil.getFirstArtifactEndingWith( - getFilesToBuild(getConfiguredTarget("//package:opl")), "/FileA.pb.m"); + linkAction.getInputs(), "libopl_binary_BundledProtos_0.a"); + CommandAction linkedProtosAction = (CommandAction) getGeneratingAction(linkedProtos); Artifact objectFile = - ActionsTestUtil.getFirstArtifactEndingWith(compileAction.getOutputs(), ".o"); + ActionsTestUtil.getFirstArtifactEndingWith( + linkedProtosAction.getInputs(), "FileA.pbobjc.o"); + CommandAction compiledProtoAction = (CommandAction) getGeneratingAction(objectFile); + + Artifact sourceFile = + ActionsTestUtil.getFirstArtifactEndingWith( + compiledProtoAction.getInputs(), "/FileA.pbobjc.m"); Artifact dotdFile = - ActionsTestUtil.getFirstArtifactEndingWith(compileAction.getOutputs(), ".d"); + ActionsTestUtil.getFirstArtifactEndingWith(compiledProtoAction.getOutputs(), ".d"); + // We remove spaces since the crosstool rules do not use spaces in command line args. - String compileArgs = Joiner.on("").join(compileAction.getArguments()).replace(" ", ""); + String compileArgs = Joiner.on("").join(compiledProtoAction.getArguments()).replace(" ", ""); List<String> expectedArgs = new ImmutableList.Builder<String>() @@ -895,7 +493,7 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { .addAll(FASTBUILD_COPTS) .addAll( ObjcLibraryTest.iquoteArgs( - target.get(ObjcProvider.SKYLARK_CONSTRUCTOR), getTargetConfiguration())) + binaryTarget.get(ObjcProvider.SKYLARK_CONSTRUCTOR), getTargetConfiguration())) .add("-I") .add(sourceFile.getExecPath().getParentDirectory().getParentDirectory().toString()) .add("-fno-objc-arc") @@ -911,34 +509,47 @@ public class ObjcProtoLibraryTest extends ObjcRuleTestCase { assertThat(compileArgs).contains(expectedArg); } - assertRequiresDarwin(compileAction); - assertThat(Artifact.toRootRelativePaths(compileAction.getInputs())) + assertRequiresDarwin(compiledProtoAction); + assertThat(Artifact.toRootRelativePaths(compiledProtoAction.getInputs())) .containsAllOf( - "package/_generated_protos/opl/package/FileA.pb.m", - "package/_generated_protos/opl/package/FileA.pb.h", - "package/_generated_protos/opl/package/dir/FileB.pb.h", - "package/_generated_protos/opl/dep/File.pb.h"); + "package/_generated_protos/opl_binary/package/FileA.pbobjc.m", + "package/_generated_protos/opl_binary/package/FileA.pbobjc.h", + "package/_generated_protos/opl_binary/package/dir/FileB.pbobjc.h", + "package/_generated_protos/opl_binary/dep/File.pbobjc.h"); } @Test public void testLibraryLinkAction() throws Exception { useConfiguration("--cpu=ios_armv7"); - Artifact libFile = + + // Because protos are linked within the apple_binary context, we need to traverse the action + // graph to find the linked protos (.a). + ConfiguredTarget binaryTarget = getConfiguredTarget("//package:opl_binary"); + SymlinkAction symlinkAction = + (SymlinkAction) getGeneratingAction(getBinArtifact("opl_binary_lipobin", binaryTarget)); + + Artifact binaryInput = Iterables.getOnlyElement(symlinkAction.getInputs()); + + CommandAction linkAction = (CommandAction) getGeneratingAction(binaryInput); + + Artifact linkedProtos = ActionsTestUtil.getFirstArtifactEndingWith( - getFilesToBuild(getConfiguredTarget("//package:opl")), "/libopl.a"); - CommandAction action = (CommandAction) getGeneratingAction(libFile); - assertThat(action.getArguments()) + linkAction.getInputs(), "libopl_binary_BundledProtos_0.a"); + CommandAction linkedProtosAction = (CommandAction) getGeneratingAction(linkedProtos); + Artifact objListFile = + ActionsTestUtil.getFirstArtifactEndingWith(linkedProtosAction.getInputs(), ".objlist"); + assertThat(linkedProtosAction.getArguments()) .containsAllIn( ImmutableList.of( "-static", "-filelist", - getBinArtifact("opl-archive.objlist", "//package:opl").getExecPathString(), + objListFile.getExecPathString(), "-arch_only", "armv7", "-syslibroot", AppleToolchain.sdkDir(), "-o", - libFile.getExecPathString())); - assertRequiresDarwin(action); + linkedProtos.getExecPathString())); + assertRequiresDarwin(linkedProtosAction); } } |