Clean up framework support
RELNOTES: Add --incompatible_objc_framework_cleanup to control whether to enable some objc framework cleanup that changes the API. Specifically, the cleanup changes the objc provider API pertaining to frameworks. This change is expected to be transparent to most users unless they write their own Starlark rules to handle frameworks. See https://github.com/bazelbuild/bazel/issues/7594 for details.
PiperOrigin-RevId: 243373818
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 08aad7f..5b08481 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -153,6 +153,7 @@
"--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(),
"--incompatible_no_target_output_group=" + rand.nextBoolean(),
"--incompatible_no_transitive_loads=" + rand.nextBoolean(),
+ "--incompatible_objc_framework_cleanup=" + rand.nextBoolean(),
"--incompatible_remap_main_repo=" + rand.nextBoolean(),
"--incompatible_remove_native_maven_jar=" + rand.nextBoolean(),
"--incompatible_static_name_resolution_in_build_files=" + rand.nextBoolean(),
@@ -199,6 +200,7 @@
.incompatibleNoSupportToolsInActionInputs(rand.nextBoolean())
.incompatibleNoTargetOutputGroup(rand.nextBoolean())
.incompatibleNoTransitiveLoads(rand.nextBoolean())
+ .incompatibleObjcFrameworkCleanup(rand.nextBoolean())
.incompatibleRemapMainRepo(rand.nextBoolean())
.incompatibleRemoveNativeMavenJar(rand.nextBoolean())
.incompatibleStaticNameResolutionInBuildFiles(rand.nextBoolean())
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 597579a..42a021e 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
@@ -967,13 +967,23 @@
}
@Test
- public void testFrameworkDepLinkFlags() throws Exception {
- checkFrameworkDepLinkFlags(getRuleType(), new ExtraLinkArgs());
+ public void testFrameworkDepLinkFlagsPreCleanup() throws Exception {
+ checkFrameworkDepLinkFlags(getRuleType(), new ExtraLinkArgs(), false);
}
@Test
- public void testDylibDependencies() throws Exception {
- checkDylibDependencies(getRuleType(), new ExtraLinkArgs());
+ public void testFrameworkDepLinkFlagsPostCleanup() throws Exception {
+ checkFrameworkDepLinkFlags(getRuleType(), new ExtraLinkArgs(), true);
+ }
+
+ @Test
+ public void testDylibDependenciesPreCleanup() throws Exception {
+ checkDylibDependencies(getRuleType(), new ExtraLinkArgs(), false);
+ }
+
+ @Test
+ public void testDylibDependenciesPostCleanup() throws Exception {
+ checkDylibDependencies(getRuleType(), new ExtraLinkArgs(), true);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java
index d215df7..3e61fc8 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java
@@ -136,13 +136,23 @@
}
@Test
- public void testFrameworkDepLinkFlags() throws Exception {
- checkFrameworkDepLinkFlags(RULE_TYPE, new ExtraLinkArgs("-dynamiclib"));
+ public void testFrameworkDepLinkFlagsPreCleanup() throws Exception {
+ checkFrameworkDepLinkFlags(RULE_TYPE, new ExtraLinkArgs("-dynamiclib"), false);
}
@Test
- public void testDylibDependencies() throws Exception {
- checkDylibDependencies(RULE_TYPE, new ExtraLinkArgs("-dynamiclib"));
+ public void testFrameworkDepLinkFlagsPostCleanup() throws Exception {
+ checkFrameworkDepLinkFlags(RULE_TYPE, new ExtraLinkArgs("-dynamiclib"), true);
+ }
+
+ @Test
+ public void testDylibDependenciesPreCleanup() throws Exception {
+ checkDylibDependencies(RULE_TYPE, new ExtraLinkArgs("-dynamiclib"), false);
+ }
+
+ @Test
+ public void testDylibDependenciesPostCleanup() throws Exception {
+ checkDylibDependencies(RULE_TYPE, new ExtraLinkArgs("-dynamiclib"), true);
}
@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 3675f2d..6810fd4 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
@@ -518,9 +518,11 @@
}
@Test
- public void testCompileWithFrameworkImportsIncludesFlagsAndInputArtifacts() throws Exception {
+ public void testCompileWithFrameworkImportsIncludesFlagsAndInputArtifactsPreCleanup()
+ throws Exception {
useConfiguration("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL);
- addBinWithTransitiveDepOnFrameworkImport();
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ addBinWithTransitiveDepOnFrameworkImport(false);
CommandAction compileAction = compileAction("//lib:lib", "a.o");
assertThat(compileAction.getArguments()).doesNotContain("-framework");
@@ -534,6 +536,17 @@
}
@Test
+ public void testCompileWithFrameworkImportsIncludesFlagsPostCleanup() throws Exception {
+ useConfiguration("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL);
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ addBinWithTransitiveDepOnFrameworkImport(true);
+ CommandAction compileAction = compileAction("//lib:lib", "a.o");
+
+ assertThat(compileAction.getArguments()).doesNotContain("-framework");
+ assertThat(Joiner.on("").join(compileAction.getArguments())).contains("-Ffx");
+ }
+
+ @Test
public void testPrecompiledHeaders() throws Exception {
useConfiguration("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL);
scratch.file("objc/a.m");
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 8b05356..f9b6ae2 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
@@ -470,15 +470,21 @@
assertThat(action.getExecutionInfo()).containsKey(ExecutionRequirements.REQUIRES_DARWIN);
}
- protected ConfiguredTarget addBinWithTransitiveDepOnFrameworkImport() throws Exception {
- ConfiguredTarget lib = addLibWithDepOnFrameworkImport();
+ protected ConfiguredTarget addBinWithTransitiveDepOnFrameworkImport(boolean cleanup)
+ throws Exception {
+ ConfiguredTarget lib;
+ if (!cleanup) {
+ lib = addLibWithDepOnFrameworkImportPreCleanup();
+ } else {
+ lib = addLibWithDepOnFrameworkImportPostCleanup();
+ }
return createBinaryTargetWriter("//bin:bin")
.setList("deps", lib.getLabel().toString())
.write();
}
- protected ConfiguredTarget addLibWithDepOnFrameworkImport() throws Exception {
+ private ConfiguredTarget addLibWithDepOnFrameworkImportPreCleanup() throws Exception {
scratch.file(
"fx/defs.bzl",
"def _custom_static_framework_import_impl(ctx):",
@@ -505,6 +511,29 @@
.write();
}
+ private ConfiguredTarget addLibWithDepOnFrameworkImportPostCleanup() throws Exception {
+ scratch.file(
+ "fx/defs.bzl",
+ "def _custom_static_framework_import_impl(ctx):",
+ " return [apple_common.new_objc_provider(",
+ " framework_search_paths=depset(ctx.attr.framework_search_paths))]",
+ "custom_static_framework_import = rule(",
+ " _custom_static_framework_import_impl,",
+ " attrs={'framework_search_paths': attr.string_list()},",
+ ")");
+ scratch.file(
+ "fx/BUILD",
+ "load(':defs.bzl', 'custom_static_framework_import')",
+ "custom_static_framework_import(",
+ " name = 'fx',",
+ " framework_search_paths = ['fx/fx1.framework', 'fx/fx2.framework'],",
+ ")");
+ return createLibraryTargetWriter("//lib:lib")
+ .setAndCreateFiles("srcs", "a.m", "b.m", "private.h")
+ .setList("deps", "//fx:fx")
+ .write();
+ }
+
protected CommandAction compileAction(String ownerLabel, String objFileName) throws Exception {
Action archiveAction = archiveAction(ownerLabel);
return (CommandAction)
@@ -836,8 +865,8 @@
assertThat(protoProvider).isNotNull();
}
- protected void checkFrameworkDepLinkFlags(RuleType ruleType,
- ExtraLinkArgs extraLinkArgs) throws Exception {
+ protected void checkFrameworkDepLinkFlags(
+ RuleType ruleType, ExtraLinkArgs extraLinkArgs, boolean cleanup) throws Exception {
scratch.file(
"libs/defs.bzl",
"def _custom_static_framework_import_impl(ctx):",
@@ -861,6 +890,12 @@
" link_inputs = glob(['buzzbuzz.framework/*']),",
")");
+ if (!cleanup) {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ } else {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ }
+
ruleType.scratchTarget(scratch, "deps", "['//libs:objc_lib']");
CommandAction linkAction = linkAction("//x:x");
@@ -1558,11 +1593,7 @@
ruleType.target(scratch, "x", "x", "minimum_os_version", "'4.3.1'"));
}
- protected void checkDylibDependencies(RuleType ruleType,
- ExtraLinkArgs extraLinkArgs) throws Exception {
- ruleType.scratchTarget(scratch,
- "dylibs", "['//fx:framework_import']");
-
+ private void checkDylibDependenciesSetupFrameworkPreCleanup() throws Exception {
scratch.file(
"fx/defs.bzl",
"def _custom_dynamic_framework_import_impl(ctx):",
@@ -1584,6 +1615,43 @@
" name = 'framework_import',",
" link_inputs = glob(['MyFramework.framework/*']),",
")");
+ }
+
+ private void checkDylibDependenciesSetupFrameworkPostCleanup() throws Exception {
+ scratch.file(
+ "fx/defs.bzl",
+ "def _custom_dynamic_framework_import_impl(ctx):",
+ " return [",
+ " apple_common.new_objc_provider(",
+ " dynamic_framework_file=depset(ctx.files.link_inputs)),",
+ " apple_common.new_dynamic_framework_provider(objc=apple_common.new_objc_provider()),",
+ " ]",
+ "custom_dynamic_framework_import = rule(",
+ " _custom_dynamic_framework_import_impl,",
+ " attrs={'link_inputs': attr.label_list(allow_files=True)},",
+ ")");
+ scratch.file("fx/MyFramework.framework/MyFramework");
+ scratch.file(
+ "fx/BUILD",
+ "load(':defs.bzl', 'custom_dynamic_framework_import')",
+ "custom_dynamic_framework_import(",
+ " name = 'framework_import',",
+ " link_inputs = ['MyFramework.framework/MyFramework'],",
+ ")");
+ }
+
+ protected void checkDylibDependencies(
+ RuleType ruleType, ExtraLinkArgs extraLinkArgs, boolean cleanup) throws Exception {
+ ruleType.scratchTarget(scratch, "dylibs", "['//fx:framework_import']");
+
+ if (!cleanup) {
+ checkDylibDependenciesSetupFrameworkPreCleanup();
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ } else {
+ checkDylibDependenciesSetupFrameworkPostCleanup();
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ }
+
useConfiguration("--ios_multi_cpus=i386,x86_64");
Action lipobinAction = lipoBinAction("//x:x");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcSkylarkTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcSkylarkTest.java
index 5b6cc48..67c168e 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcSkylarkTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcSkylarkTest.java
@@ -1590,4 +1590,168 @@
boolean runMemleaks = (boolean) skylarkTarget.get("run_memleaks");
assertThat(runMemleaks).isEqualTo(expectedValue);
}
+
+ private void addDummyObjcProviderRule(String name) throws Exception {
+ scratch.file(
+ "fx/defs.bzl",
+ "def _my_rule_impl(ctx):",
+ " objc = apple_common.new_objc_provider()",
+ String.format(" return struct(names=objc.%s)", name),
+ "my_rule = rule(implementation = _my_rule_impl,",
+ " attrs = {})");
+ scratch.file("fx/BUILD", "load(':defs.bzl', 'my_rule')", "my_rule(name = 'lib')");
+ }
+
+ private void testObjcProviderHas(String name) throws Exception {
+ addDummyObjcProviderRule(name);
+ assertThat(view.hasErrors(getConfiguredTarget("//fx:lib"))).isFalse();
+ }
+
+ private void testObjcProviderDoesNotHave(String name) throws Exception {
+ addDummyObjcProviderRule(name);
+ try {
+ getConfiguredTarget("//fx:lib");
+ fail("Should throw AssertionError");
+ } catch (AssertionError e) {
+ if (name.endsWith("()")) {
+ assertThat(e).hasMessageThat().contains("'ObjcProvider' has no method " + name);
+ } else {
+ assertThat(e).hasMessageThat().contains("'ObjcProvider' has no field '" + name + "'");
+ }
+ }
+ }
+
+ @Test
+ public void testObjcProviderDynamicFrameworkDirPreCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ testObjcProviderHas("dynamic_framework_dir");
+ }
+
+ @Test
+ public void testObjcProviderFrameworkDirPreCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ testObjcProviderHas("framework_dir");
+ }
+
+ @Test
+ public void testObjcProviderDynamicFrameworkNamesPreCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ testObjcProviderDoesNotHave("dynamic_framework_names");
+ }
+
+ @Test
+ public void testObjcProviderDynamicFrameworkPathsPreCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ testObjcProviderDoesNotHave("dynamic_framework_paths");
+ }
+
+ @Test
+ public void testObjcProviderStaticFrameworkNamesPreCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ testObjcProviderDoesNotHave("static_framework_names");
+ }
+
+ @Test
+ public void testObjcProviderStaticFrameworkPathsPreCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
+ testObjcProviderDoesNotHave("static_framework_paths");
+ }
+
+ @Test
+ public void testObjcProviderDynamicFrameworkDirPostCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ testObjcProviderDoesNotHave("dynamic_framework_dir");
+ }
+
+ @Test
+ public void testObjcProviderFrameworkDirPostCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ testObjcProviderDoesNotHave("framework_dir");
+ }
+
+ @Test
+ public void testObjcProviderDynamicFrameworkNamesPostCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ testObjcProviderHas("dynamic_framework_names");
+ }
+
+ @Test
+ public void testObjcProviderDynamicFrameworkPathsPostCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ testObjcProviderHas("dynamic_framework_paths");
+ }
+
+ @Test
+ public void testObjcProviderStaticFrameworkNamesPostCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ testObjcProviderHas("static_framework_names");
+ }
+
+ @Test
+ public void testObjcProviderStaticFrameworkPathsPostCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+ testObjcProviderHas("static_framework_paths");
+ }
+
+ @Test
+ public void testStaticFrameworkApiPostCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+
+ scratch.file(
+ "fx/defs.bzl",
+ "def _custom_static_framework_import_impl(ctx):",
+ " return [apple_common.new_objc_provider(",
+ " static_framework_file=depset(ctx.files.link_inputs))]",
+ "custom_static_framework_import = rule(",
+ " _custom_static_framework_import_impl,",
+ " attrs={'link_inputs': attr.label_list(allow_files=True)},",
+ ")");
+ scratch.file("fx/fx1.framework/fx1");
+ scratch.file("fx/fx2.framework/fx2");
+ scratch.file(
+ "fx/BUILD",
+ "load(':defs.bzl', 'custom_static_framework_import')",
+ "custom_static_framework_import(",
+ " name = 'framework',",
+ " link_inputs = ['fx1.framework/fx1', 'fx2.framework/fx2'],",
+ ")");
+
+ ConfiguredTarget framework = getConfiguredTarget("//fx:framework");
+ ObjcProvider objc = framework.get(ObjcProvider.SKYLARK_CONSTRUCTOR);
+ assertThat(Artifact.toRootRelativePaths(objc.staticFrameworkFile()))
+ .containsExactly("fx/fx1.framework/fx1", "fx/fx2.framework/fx2");
+ assertThat(objc.staticFrameworkNames()).containsExactly("fx1", "fx2");
+ assertThat(objc.staticFrameworkPaths()).containsExactly("fx");
+ }
+
+ @Test
+ public void testDynamicFrameworkApiPostCleanup() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=true");
+
+ scratch.file(
+ "fx/defs.bzl",
+ "def _custom_dynamic_framework_import_impl(ctx):",
+ " return [apple_common.new_objc_provider(",
+ " dynamic_framework_file=depset(ctx.files.link_inputs))]",
+ "custom_dynamic_framework_import = rule(",
+ " _custom_dynamic_framework_import_impl,",
+ " attrs={'link_inputs': attr.label_list(allow_files=True)},",
+ ")");
+ scratch.file("fx/fx1.framework/fx1");
+ scratch.file("fx/fx2.framework/fx2");
+ scratch.file(
+ "fx/BUILD",
+ "load(':defs.bzl', 'custom_dynamic_framework_import')",
+ "custom_dynamic_framework_import(",
+ " name = 'framework',",
+ " link_inputs = ['fx1.framework/fx1', 'fx2.framework/fx2'],",
+ ")");
+
+ ConfiguredTarget framework = getConfiguredTarget("//fx:framework");
+ ObjcProvider objc = framework.get(ObjcProvider.SKYLARK_CONSTRUCTOR);
+ assertThat(Artifact.toRootRelativePaths(objc.dynamicFrameworkFile()))
+ .containsExactly("fx/fx1.framework/fx1", "fx/fx2.framework/fx2");
+ assertThat(objc.dynamicFrameworkNames()).containsExactly("fx1", "fx2");
+ assertThat(objc.dynamicFrameworkPaths()).containsExactly("fx");
+ }
}
