From 88518522a18df5788736be6151fc67992efe2aad Mon Sep 17 00:00:00 2001 From: buchgr Date: Mon, 17 Jul 2017 13:34:06 +0200 Subject: [PATCH] Automated rollback of commit 4594b7fd6c2516341cdd6e57f9eaaf22cf691f95. *** Reason for rollback *** It breaks examples/tutorial on ci.bazel.build [1] with error: object of type 'Target' has no field 'objc'. [1] http://ci.bazel.io/view/Dashboard/job/Tutorial/BAZEL_VERSION=HEAD,PLATFORM_NAME=darwin-x86_64/1023/console *** Original change description *** AppleBinary and AppleStaticLibrary no longer propagate unwrapped ObjcProvider. This will prevent dependencies on apple_binary and apple_static_library from objc_library and other lower-level rules. RELNOTES: None. PiperOrigin-RevId: 162195726 --- .../build/lib/rules/objc/AppleBinary.java | 2 ++ .../lib/rules/objc/AppleStaticLibrary.java | 2 ++ .../build/lib/rules/objc/AppleBinaryTest.java | 2 +- .../rules/objc/AppleStaticLibraryTest.java | 4 +--- .../build/lib/rules/objc/IosTestTest.java | 21 +------------------ .../build/lib/rules/objc/ObjcBinaryTest.java | 2 +- .../build/lib/rules/objc/ObjcLibraryTest.java | 2 +- .../lib/rules/objc/ObjcProtoLibraryTest.java | 13 ++++-------- .../lib/rules/objc/ObjcRuleTestCase.java | 9 ++------ 9 files changed, 15 insertions(+), 42 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java index c7fe14fa3bf55e..d296905dfc3731 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java @@ -164,6 +164,8 @@ public final ConfiguredTarget create(RuleContext ruleContext) objcProviderBuilder.add(MULTI_ARCH_LINKED_BINARIES, outputArtifact); ObjcProvider objcProvider = objcProviderBuilder.build(); + // TODO(cparsons): Stop propagating ObjcProvider directly from this rule. + targetBuilder.addProvider(ObjcProvider.class, objcProvider); switch (getBinaryType(ruleContext)) { case EXECUTABLE: diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java index 9aeb22456bde9a..49575aa0de132b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java @@ -180,6 +180,8 @@ public final ConfiguredTarget create(RuleContext ruleContext) ObjcProvider objcProvider = objcProviderBuilder.build(); targetBuilder + // TODO(cparsons): Remove ObjcProvider as a direct provider. + .addProvider(ObjcProvider.class, objcProvider) .addNativeDeclaredProvider( new AppleStaticLibraryProvider( ruleIntermediateArtifacts.combinedArchitectureArchive(), diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java index 897ffafb27a2bc..65cb196269a63a 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java @@ -1416,6 +1416,6 @@ public void testLoadableBundleBinaryAddsRpathLinkOptWithBundleLoader() throws Ex @Test public void testCustomModuleMap() throws Exception { - checkCustomModuleMap(RULE_TYPE, true); + checkCustomModuleMap(RULE_TYPE); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java index 90744a517ba57b..01f8b08d995aa0 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java @@ -134,9 +134,7 @@ public void testAvoidDepsProviders() throws Exception { " resources = [':avoid.png']", ")"); - ObjcProvider provider = getConfiguredTarget("//package:test") - .get(AppleStaticLibraryProvider.SKYLARK_CONSTRUCTOR) - .getDepsObjcProvider(); + ObjcProvider provider = providerForTarget("//package:test"); // Do not remove SDK_FRAMEWORK or GENERAL_RESOURCE_FILE values in avoid_deps. assertThat(provider.get(ObjcProvider.SDK_FRAMEWORK)) .containsAllOf(new SdkFramework("AvoidSDK"), new SdkFramework("BaseSDK")); diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/IosTestTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/IosTestTest.java index 8744460e4fc486..8427e72fd8d512 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/IosTestTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/IosTestTest.java @@ -1167,22 +1167,7 @@ public void testMultiArchUserHeaderSearchPathsUsed() throws Exception { // Usually, an ios_test would depend on apple_binary through a skylark_ios_application in its // 'binary' attribute. Since we don't have skylark_ios_application here, we use the 'deps' // attribute instead. - scratch.file("skylarkstub/BUILD"); - scratch.file("skylarkstub/skylark_stub.bzl", - "def skylark_ios_application_stub_impl(ctx):", - " bin_provider = ctx.attr.binary[apple_common.AppleExecutableBinary]", - " return struct(objc=bin_provider.objc)", - "skylark_ios_application_stub = rule(", - " skylark_ios_application_stub_impl,", - // Both 'binary' and 'deps' are needed because ObjcProtoAspect is applied transitively - // along attribute 'deps' only. - " attrs = {'binary': attr.label(mandatory=True,", - " providers=[apple_common.AppleExecutableBinary])},", - " fragments = ['apple', 'objc'],", - ")"); - scratch.file("x/BUILD", - "load('//skylarkstub:skylark_stub.bzl', 'skylark_ios_application_stub')", "genrule(", " name = 'gen_hdrs',", " outs = ['generated.h'],", @@ -1194,10 +1179,6 @@ public void testMultiArchUserHeaderSearchPathsUsed() throws Exception { " platform_type = 'ios',", " hdrs = ['generated.h'],", ")", - "skylark_ios_application_stub(", - " name = 'stub_application',", - " binary = ':apple_bin',", - ")", "objc_binary(", " name = 'bin',", " srcs = ['bin.m'],", @@ -1211,7 +1192,7 @@ public void testMultiArchUserHeaderSearchPathsUsed() throws Exception { " srcs = ['test.m'],", " xctest = 1,", " xctest_app = ':testApp',", - " deps = [':stub_application']", + " deps = [':apple_bin']", ")"); CommandAction compileAction = compileAction("//x:test", "test.o"); // The genfiles root for child configurations must be present in the compile action so that diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBinaryTest.java index b645e72ebb800a..acc33621bd1376 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBinaryTest.java @@ -928,7 +928,7 @@ public void testFilesToCompileOutputGroup() throws Exception { @Test public void testCustomModuleMap() throws Exception { - checkCustomModuleMap(RULE_TYPE, false); + checkCustomModuleMap(RULE_TYPE); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java index e7efb1de02c099..6633fcc99aaa44 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java @@ -1599,6 +1599,6 @@ public void testDefaultEnabledFeatureIsUsed() throws Exception { @Test public void testCustomModuleMap() throws Exception { - checkCustomModuleMap(RULE_TYPE, false); + checkCustomModuleMap(RULE_TYPE); } } 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 4a870493502dc1..a2689cd8fc6ea9 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 @@ -679,12 +679,12 @@ public void testErrorForUsesProtobufWithOptionsFile() throws Exception { @Test public void testModulemapCreatedForNonLinkingTargets() throws Exception { - checkOnlyLibModuleMapsArePresentForTarget("//package:opl_protobuf", false); + checkOnlyLibModuleMapsArePresentForTarget("//package:opl_protobuf"); } @Test public void testModulemapNotCreatedForLinkingTargets() throws Exception { - checkOnlyLibModuleMapsArePresentForTarget("//package:opl_binary", true); + checkOnlyLibModuleMapsArePresentForTarget("//package:opl_binary"); } @Test @@ -760,8 +760,7 @@ private static String sortedJoin(Iterable elements) { return Joiner.on('\n').join(Ordering.natural().immutableSortedCopy(elements)); } - private void checkOnlyLibModuleMapsArePresentForTarget(String target, - boolean fromBinary) throws Exception { + private void checkOnlyLibModuleMapsArePresentForTarget(String target) throws Exception { Artifact libModuleMap = getGenfilesArtifact( "opl_protobuf.modulemaps/module.modulemap", @@ -771,11 +770,7 @@ private void checkOnlyLibModuleMapsArePresentForTarget(String target, "protobuf_lib.modulemaps/module.modulemap", getConfiguredTarget("//objcproto:protobuf_lib")); - ObjcProvider provider = fromBinary - ? getConfiguredTarget(target) - .get(AppleExecutableBinaryProvider.SKYLARK_CONSTRUCTOR) - .getDepsObjcProvider() - : providerForTarget(target); + ObjcProvider provider = providerForTarget(target); assertThat(Artifact.toRootRelativePaths(provider.get(ObjcProvider.MODULE_MAP).toSet())) .containsExactlyElementsIn( Artifact.toRootRelativePaths(ImmutableSet.of(libModuleMap, protolibModuleMap))); diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java index 1ac7d8ab2dcc3a..875258689d1501 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java @@ -4894,7 +4894,7 @@ public void checkFilesToCompileOutputGroup(RuleType ruleType) throws Exception { .isEqualTo("a.o"); } - protected void checkCustomModuleMap(RuleType ruleType, boolean fromBinary) throws Exception { + protected void checkCustomModuleMap(RuleType ruleType) throws Exception { useConfiguration("--experimental_objc_enable_module_maps"); ruleType.scratchTarget(scratch, "srcs", "['a.m']", "deps", "['//z:testModuleMap']"); scratch.file("x/a.m"); @@ -4919,15 +4919,10 @@ protected void checkCustomModuleMap(RuleType ruleType, boolean fromBinary) throw assertThat(compileActionA.getArguments()).doesNotContain("-fmodule-name"); ObjcProvider provider = providerForTarget("//z:testModuleMap"); - assertThat(Artifact.toExecPaths(provider.get(MODULE_MAP))) .containsExactly("y/module.modulemap"); - provider = fromBinary - ? getConfiguredTarget("//x:x") - .get(AppleExecutableBinaryProvider.SKYLARK_CONSTRUCTOR) - .getDepsObjcProvider() - : providerForTarget("//x:x"); + provider = providerForTarget("//x:x"); assertThat(Artifact.toExecPaths(provider.get(MODULE_MAP))).contains("y/module.modulemap"); } }