Skip to content

Commit

Permalink
Flip --incompatible_disable_objc_library_resources to true.
Browse files Browse the repository at this point in the history 
    RELNOTES[INC]: objc_library resource attributes are now disabled by default. Please migrate them to data instead. See bazelbuild/bazel#7594 for more info.

    PiperOrigin-RevId: 241782281
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent fb83100 commit e1a8fe9
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 266 deletions.
4 changes: 2 additions & 2 deletions ....bazel/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

package com.google.devtools.build.lib.rules.objc;

import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.LabelConverter;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.rules.apple.DottedVersion;
Expand Down Expand Up @@ -319,7 +319,7 @@ public class ObjcCommandLineOptions extends FragmentOptions {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "Unused. Will be removed in future versions of Bazel.")
help = "If enabled, objc_library resource attributes are disallowed.")
public boolean disableObjcLibraryResources;

@Override
Expand Down
50 changes: 43 additions & 7 deletions ....bazel/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,38 @@ public void testSymlinkInsteadOfLipoSingleArch() throws Exception {
.containsExactly(Iterables.getOnlyElement(linkAction.getOutputs()));
}

@Test
public void testAvoidDepsProviders() throws Exception {
useConfiguration("--incompatible_disable_objc_library_resources=false");
scratch.file(
"package/BUILD",
"apple_static_library(",
" name = 'test',",
" deps = [':objcLib'],",
" platform_type = 'ios',",
" avoid_deps = [':avoidLib'],",
")",
"objc_library(name = 'objcLib', srcs = [ 'b.m' ], deps = [':avoidLib', ':baseLib'])",
"objc_library(",
" name = 'baseLib',",
" srcs = [ 'base.m' ],",
" sdk_frameworks = ['BaseSDK'],",
" resources = [':base.png']",
")",
"objc_library(",
" name = 'avoidLib',",
" srcs = [ 'c.m' ],",
" sdk_frameworks = ['AvoidSDK'],",
" resources = [':avoid.png']",
")");

ObjcProvider provider = getConfiguredTarget("//package:test")
.get(AppleStaticLibraryInfo.SKYLARK_CONSTRUCTOR).getDepsObjcProvider();
// Do not remove SDK_FRAMEWORK values in avoid_deps.
assertThat(provider.get(ObjcProvider.SDK_FRAMEWORK))
.containsAllOf(new SdkFramework("AvoidSDK"), new SdkFramework("BaseSDK"));
}

@Test
public void testNoSrcs() throws Exception {
scratch.file("package/BUILD",
Expand Down Expand Up @@ -167,7 +199,7 @@ public void testWatchSimulatorDepCompile() throws Exception {
getGeneratingAction(getFirstArtifactEndingWith(linkAction.getInputs(), "libobjcLib.a"));

assertAppleSdkPlatformEnv(objcLibCompileAction, "WatchSimulator");
assertThat(objcLibCompileAction.getArguments()).containsAtLeast("-arch_only", "i386").inOrder();
assertThat(objcLibCompileAction.getArguments()).containsAllOf("-arch_only", "i386").inOrder();
}

@Test
Expand Down Expand Up @@ -222,7 +254,7 @@ public void testWatchSimulatorLipoAction() throws Exception {

assertContainsSublist(action.getArguments(), ImmutableList.of(
MOCK_XCRUNWRAPPER_EXECUTABLE_PATH, LIPO, "-create"));
assertThat(action.getArguments()).containsAtLeast(armv7kBin, i386Bin);
assertThat(action.getArguments()).containsAllOf(armv7kBin, i386Bin);
assertContainsSublist(action.getArguments(), ImmutableList.of(
"-o", execPathEndingWith(action.getOutputs(), "x_lipo.a")));

Expand Down Expand Up @@ -447,7 +479,7 @@ public void testWatchSimulatorLinkAction() throws Exception {

assertAppleSdkPlatformEnv(linkAction, "WatchSimulator");
assertThat(normalizeBashArgs(linkAction.getArguments()))
.containsAtLeast("-arch_only", "i386")
.containsAllOf("-arch_only", "i386")
.inOrder();
}

Expand Down Expand Up @@ -484,8 +516,8 @@ public void testAvoidDepsObjects() throws Exception {
"objc_library(name = 'avoidLib', srcs = [ 'c.m' ])");

CommandAction action = linkLibAction("//package:test");
assertThat(Artifact.toRootRelativePaths(action.getInputs()))
.containsAtLeast("package/libobjcLib.a", "package/libbaseLib.a");
assertThat(Artifact.toRootRelativePaths(action.getInputs())).containsAllOf(
"package/libobjcLib.a", "package/libbaseLib.a");
assertThat(Artifact.toRootRelativePaths(action.getInputs())).doesNotContain(
"package/libavoidLib.a");
}
Expand Down Expand Up @@ -641,7 +673,9 @@ public void testAppleStaticLibraryProvider() throws Exception {
" dep_provider = ctx.attr.proxy[apple_common.AppleStaticLibrary]",
" my_provider = apple_common.AppleStaticLibrary(archive = dep_provider.archive,",
" objc = dep_provider.objc)",
" return [my_provider]",
" return struct(",
" providers = [my_provider]",
" )",
"",
"skylark_static_lib = rule(",
" implementation = skylark_static_lib_impl,",
Expand Down Expand Up @@ -679,7 +713,9 @@ public void testAppleStaticLibraryProvider_invalidArgs() throws Exception {
" dep_provider = ctx.attr.proxy[apple_common.AppleStaticLibrary]",
" my_provider = apple_common.AppleStaticLibrary(archive = dep_provider.archive,",
" objc = dep_provider.objc, foo = 'bar')",
" return [my_provider]",
" return struct(",
" providers = [my_provider]",
" )",
"",
"skylark_static_lib = rule(",
" implementation = skylark_static_lib_impl,",
Expand Down
17 changes: 2 additions & 15 deletions ...elbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,9 @@ public void testArchivesPrecompiledObjectFiles() throws Exception {
}

@Test
public void testCompileWithFrameworkImportsIncludesFlagsAndInputArtifactsPreCleanup()
throws Exception {
public void testCompileWithFrameworkImportsIncludesFlagsAndInputArtifacts() throws Exception {
useConfiguration("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL);
setSkylarkSemanticsOptions("--incompatible_objc_framework_cleanup=false");
addBinWithTransitiveDepOnFrameworkImport(false);
addBinWithTransitiveDepOnFrameworkImport();
CommandAction compileAction = compileAction("//lib:lib", "a.o");

assertThat(compileAction.getArguments()).doesNotContain("-framework");
Expand All @@ -535,17 +533,6 @@ public void testCompileWithFrameworkImportsIncludesFlagsAndInputArtifactsPreClea
getSourceArtifact("fx/fx2.framework/d"));
}

@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);
Expand Down
88 changes: 10 additions & 78 deletions ...lbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,21 +470,15 @@ protected void assertRequiresDarwin(ExecutionInfoSpecifier action) {
assertThat(action.getExecutionInfo()).containsKey(ExecutionRequirements.REQUIRES_DARWIN);
}

protected ConfiguredTarget addBinWithTransitiveDepOnFrameworkImport(boolean cleanup)
throws Exception {
ConfiguredTarget lib;
if (!cleanup) {
lib = addLibWithDepOnFrameworkImportPreCleanup();
} else {
lib = addLibWithDepOnFrameworkImportPostCleanup();
}
protected ConfiguredTarget addBinWithTransitiveDepOnFrameworkImport() throws Exception {
ConfiguredTarget lib = addLibWithDepOnFrameworkImport();
return createBinaryTargetWriter("//bin:bin")
.setList("deps", lib.getLabel().toString())
.write();

}

private ConfiguredTarget addLibWithDepOnFrameworkImportPreCleanup() throws Exception {
protected ConfiguredTarget addLibWithDepOnFrameworkImport() throws Exception {
scratch.file(
"fx/defs.bzl",
"def _custom_static_framework_import_impl(ctx):",
Expand All @@ -511,29 +505,6 @@ private ConfiguredTarget addLibWithDepOnFrameworkImportPreCleanup() throws Excep
.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)
Expand Down Expand Up @@ -865,8 +836,8 @@ protected void checkProtoBundlingWithTargetsWithNoDeps(RuleType ruleType) throws
assertThat(protoProvider).isNotNull();
}

protected void checkFrameworkDepLinkFlags(
RuleType ruleType, ExtraLinkArgs extraLinkArgs, boolean cleanup) throws Exception {
protected void checkFrameworkDepLinkFlags(RuleType ruleType,
ExtraLinkArgs extraLinkArgs) throws Exception {
scratch.file(
"libs/defs.bzl",
"def _custom_static_framework_import_impl(ctx):",
Expand All @@ -890,12 +861,6 @@ protected void checkFrameworkDepLinkFlags(
" 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");
Expand Down Expand Up @@ -1593,7 +1558,11 @@ protected void checkMinimumOs_invalid_tooManyComponents(RuleType ruleType) throw
ruleType.target(scratch, "x", "x", "minimum_os_version", "'4.3.1'"));
}

private void checkDylibDependenciesSetupFrameworkPreCleanup() throws Exception {
protected void checkDylibDependencies(RuleType ruleType,
ExtraLinkArgs extraLinkArgs) throws Exception {
ruleType.scratchTarget(scratch,
"dylibs", "['//fx:framework_import']");

scratch.file(
"fx/defs.bzl",
"def _custom_dynamic_framework_import_impl(ctx):",
Expand All @@ -1615,43 +1584,6 @@ private void checkDylibDependenciesSetupFrameworkPreCleanup() throws Exception {
" 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");
Expand Down
Loading

0 comments on commit e1a8fe9

Please sign in to comment.