Skip to content

Commit

Permalink
Automated rollback of commit 4594b7f.
Browse files Browse the repository at this point in the history
*** 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
  • Loading branch information
buchgr authored and aehlig committed Jul 17, 2017
1 parent 01ddfb7 commit 8851852
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1416,6 +1416,6 @@ public void testLoadableBundleBinaryAddsRpathLinkOptWithBundleLoader() throws Ex

@Test
public void testCustomModuleMap() throws Exception {
checkCustomModuleMap(RULE_TYPE, true);
checkCustomModuleMap(RULE_TYPE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'],",
Expand All @@ -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'],",
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ public void testFilesToCompileOutputGroup() throws Exception {

@Test
public void testCustomModuleMap() throws Exception {
checkCustomModuleMap(RULE_TYPE, false);
checkCustomModuleMap(RULE_TYPE);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,6 @@ public void testDefaultEnabledFeatureIsUsed() throws Exception {

@Test
public void testCustomModuleMap() throws Exception {
checkCustomModuleMap(RULE_TYPE, false);
checkCustomModuleMap(RULE_TYPE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -760,8 +760,7 @@ private static String sortedJoin(Iterable<String> 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",
Expand All @@ -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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
}
}

0 comments on commit 8851852

Please sign in to comment.