From 20b5c6fc2e9c6cedabc6cce40a922215f3c3887c Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 12 Sep 2022 08:19:29 -0700 Subject: [PATCH] Automated rollback of commit 2636bf6ca72e213b16f1233bee19d535bf00e8a9. PiperOrigin-RevId: 473752843 Change-Id: I10d24b1f1b693de17ee1dc71477f94a7a22dc7e3 --- .../build/lib/rules/cpp/CcModule.java | 13 +++- .../common/objc/compilation_support.bzl | 63 +++++++++++++--- .../objc/AppleBinaryStarlarkApiTest.java | 5 ++ .../rules/objc/AppleDynamicLibraryTest.java | 6 ++ .../rules/objc/AppleStaticLibraryTest.java | 26 +++++++ .../rules/objc/BazelJ2ObjcLibraryTest.java | 9 +-- .../build/lib/rules/objc/ObjcLibraryTest.java | 74 +++++++++++++------ .../lib/rules/objc/ObjcRuleTestCase.java | 18 +++++ 8 files changed, 172 insertions(+), 42 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java index 3eab6fc816f605..7259df11d0ea53 100755 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java @@ -1825,11 +1825,16 @@ public Tuple createLinkingContextFromCompilationOutputs( Label label = getCallerLabel(actions, name); FdoContext fdoContext = ccToolchainProvider.getFdoContext(); LinkTargetType staticLinkTargetType = null; - if (alwayslink && !actions.getRuleContext().getRule().getRuleClass().equals("swift_library")) { - // TODO(b/202252560): Fix for swift_library's implicit output. - staticLinkTargetType = LinkTargetType.ALWAYS_LINK_STATIC_LIBRARY; + if (language == Language.CPP) { + if (alwayslink + && !actions.getRuleContext().getRule().getRuleClass().equals("swift_library")) { + // TODO(b/202252560): Fix for swift_library's implicit output. + staticLinkTargetType = LinkTargetType.ALWAYS_LINK_STATIC_LIBRARY; + } else { + staticLinkTargetType = LinkTargetType.STATIC_LIBRARY; + } } else { - staticLinkTargetType = LinkTargetType.STATIC_LIBRARY; + staticLinkTargetType = LinkTargetType.OBJC_ARCHIVE; } Artifact winDefFile = convertFromNoneable(winDefFileObject, /* defaultValue= */ null); List ccLinkingContexts = diff --git a/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl b/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl index 83287fddab8e17..526fe9c41cf2d1 100644 --- a/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/compilation_support.bzl @@ -34,6 +34,9 @@ def _build_variable_extensions( if "MODULE_MAP_VARIABLES" in variable_categories: extensions["modules_cache_path"] = ctx.genfiles_dir.path + "/" + "_objc_module_cache" + if "ARCHIVE_VARIABLES" in variable_categories: + extensions["obj_list_path"] = intermediate_artifacts.archive_obj_list.path + if "FULLY_LINK_VARIABLES" in variable_categories: extensions["fully_linked_archive_path"] = fully_link_archive.path cc_libs = {} @@ -234,6 +237,12 @@ def _get_compile_rule_copts(common_variables): return copts +def _register_obj_file_list_action(common_variables, obj_files, obj_list): + args = common_variables.ctx.actions.args() + args.set_param_file_format("multiline") + args.add_all(obj_files) + common_variables.ctx.actions.write(obj_list, args) + def _paths_to_include_args(paths): new_paths = [] for path in paths: @@ -278,13 +287,38 @@ def _register_compile_and_archive_actions( extra_compile_args = [], priority_headers = [], generate_module_map_for_swift = False): - return _cc_compile_and_link( - common_variables, - extra_compile_args, - priority_headers, - variable_categories = ["MODULE_MAP_VARIABLES"], - generate_module_map_for_swift = generate_module_map_for_swift, - ) + compilation_result = None + + if common_variables.compilation_artifacts.archive != None: + obj_list = common_variables.intermediate_artifacts.archive_obj_list + + compilation_result = _cc_compile_and_link( + common_variables, + extra_compile_args, + priority_headers, + "OBJC_ARCHIVE", + obj_list, + ["ARCHIVE_VARIABLES", "MODULE_MAP_VARIABLES"], + generate_module_map_for_swift, + ) + + _register_obj_file_list_action( + common_variables, + compilation_result[1].objects, + obj_list, + ) + else: + compilation_result = _cc_compile_and_link( + common_variables, + extra_compile_args, + priority_headers, + link_type = None, + link_action_input = None, + variable_categories = ["MODULE_MAP_VARIABLES"], + generate_module_map_for_swift = generate_module_map_for_swift, + ) + + return compilation_result def _get_grep_includes(ctx): if hasattr(ctx.executable, "_grep_includes"): @@ -300,6 +334,8 @@ def _cc_compile_and_link( common_variables, extra_compile_args, priority_headers, + link_type, + link_action_input, variable_categories, generate_module_map_for_swift): compilation_artifacts = common_variables.compilation_artifacts @@ -393,6 +429,15 @@ def _cc_compile_and_link( ), ) + if link_type == "OBJC_ARCHIVE": + language = "objc" + else: + language = "c++" + + additional_inputs = [] + if link_action_input != None: + additional_inputs.append(link_action_input) + cc_compilation_context = cc_common.merge_compilation_contexts( compilation_contexts = [arc_compilation_context, non_arc_compilation_context], ) @@ -422,9 +467,9 @@ def _cc_compile_and_link( compilation_outputs = compilation_outputs, linking_contexts = linking_contexts, name = common_variables.ctx.label.name + intermediate_artifacts.archive_file_name_suffix, - language = "c++", + language = language, disallow_dynamic_library = True, - additional_inputs = [], + additional_inputs = additional_inputs, grep_includes = _get_grep_includes(ctx), variables_extension = non_arc_extensions, ) diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java index ff40ff58cff39d..3845ceeea10a7d 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java @@ -589,6 +589,11 @@ public void testLinkActionHasCorrectIosMinVersion() throws Exception { checkLinkMinimumOSVersion("-miphoneos-version-min=8.0"); } + @Test + public void testWatchSimulatorDepCompile() throws Exception { + checkWatchSimulatorDepCompile(getRuleType()); + } + @Test public void testDylibBinaryType() throws Exception { getRuleType().scratchTarget(scratch, "binary_type", "'dylib'"); 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 5d74e4c3738b24..a30e40b0036d1f 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 @@ -124,6 +124,12 @@ public void testLipoBinaryAction() throws Exception { checkLipoBinaryAction(RULE_TYPE); } + @Override + @Test + public void testWatchSimulatorDepCompile() throws Exception { + checkWatchSimulatorDepCompile(RULE_TYPE); + } + @Test public void testMultiarchCcDep() throws Exception { checkMultiarchCcDep(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 ffb67e59957d62..f154250808ab25 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 @@ -237,6 +237,32 @@ public void testLipoAction() throws Exception { assertRequiresDarwin(action); } + @Test + public void testWatchSimulatorDepCompile() throws Exception { + scratch.file( + "package/BUILD", + "load('//test_starlark:apple_static_library.bzl', 'apple_static_library')", + "apple_static_library(", + " name = 'test',", + " deps = [':objcLib'],", + " platform_type = 'watchos',", + " minimum_os_version = '2.0',", + ")", + "objc_library(name = 'objcLib', srcs = [ 'a.m' ])"); + + Action lipoAction = lipoLibAction("//package:test"); + + String i386Bin = "i386-apple-watchos-fl.a"; + Artifact libArtifact = getFirstArtifactEndingWith(lipoAction.getInputs(), i386Bin); + CommandAction linkAction = (CommandAction) getGeneratingAction(libArtifact); + CommandAction objcLibCompileAction = + (CommandAction) + getGeneratingAction(getFirstArtifactEndingWith(linkAction.getInputs(), "libobjcLib.a")); + + assertAppleSdkPlatformEnv(objcLibCompileAction, "WatchSimulator"); + assertThat(objcLibCompileAction.getArguments()).containsAtLeast("-arch_only", "i386").inOrder(); + } + @Test public void testMultiarchCcDep() throws Exception { scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java index 0db8aca4cd2dd2..0bbf975990bf21 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java @@ -51,7 +51,6 @@ import com.google.devtools.build.lib.rules.cpp.CcInfo; import com.google.devtools.build.lib.rules.cpp.CppCompileAction; import com.google.devtools.build.lib.rules.cpp.CppCompileActionTemplate; -import com.google.devtools.build.lib.rules.cpp.CppLinkAction; import com.google.devtools.build.lib.rules.cpp.CppModuleMapAction; import com.google.devtools.build.lib.rules.cpp.CppRuleClasses; import com.google.devtools.build.lib.rules.cpp.UmbrellaHeaderAction; @@ -755,15 +754,15 @@ public void testArchiveLinkActionWithTreeArtifactFromGenJar() throws Exception { useConfiguration("--ios_minimum_os=1.0"); addSimpleJ2ObjcLibraryWithJavaPlugin(); Artifact archive = j2objcArchive("//java/com/google/app/test:transpile", "test"); - CppLinkAction archiveAction = (CppLinkAction) getGeneratingAction(archive); + CommandAction archiveAction = (CommandAction) getGeneratingAction(archive); Artifact objectFilesFromGenJar = getFirstArtifactEndingWith(archiveAction.getInputs(), "source_files"); Artifact normalObjectFile = getFirstArtifactEndingWith(archiveAction.getInputs(), "test.o"); - // Test that the archive commandline contains the individual object files inside + // Test that the archive obj list param file contains the individual object files inside // the object file tree artifact. - assertThat(archiveAction.getLinkCommandLine().arguments(DUMMY_ARTIFACT_EXPANDER)) - .containsAtLeast( + assertThat(paramFileCommandLineForAction(archiveAction).arguments(DUMMY_ARTIFACT_EXPANDER)) + .containsExactly( objectFilesFromGenJar.getExecPathString() + "/children1", objectFilesFromGenJar.getExecPathString() + "/children2", normalObjectFile.getExecPathString()); 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 cd121ae578b979..1d187a89a5115c 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 @@ -899,15 +899,19 @@ public void testArchiveAction_simulator() throws Exception { assertThat(archiveAction.getArguments()) .isEqualTo( ImmutableList.of( - "tools/osx/crosstool/iossim/ar_wrapper", - "rcs", - Iterables.getOnlyElement(archiveAction.getOutputs()).getExecPathString(), - getBinArtifact("_objs/lib/arc/a.o", getConfiguredTarget("//objc:lib")) + "tools/osx/crosstool/iossim/libtool", + "-static", + "-filelist", + getBinArtifact("lib-archive.objlist", getConfiguredTarget("//objc:lib")) .getExecPathString(), - getBinArtifact("_objs/lib/arc/b.o", getConfiguredTarget("//objc:lib")) - .getExecPathString())); + "-arch_only", + "i386", + "-syslibroot", + AppleToolchain.sdkDir(), + "-o", + Iterables.getOnlyElement(archiveAction.getOutputs()).getExecPathString())); assertThat(baseArtifactNames(archiveAction.getInputs())) - .containsAtLeast("a.o", "b.o", "ar", "libempty.a", "libtool"); + .containsAtLeast("a.o", "b.o", "lib-archive.objlist", "ar", "libempty.a", "libtool"); assertThat(baseArtifactNames(archiveAction.getOutputs())).containsExactly("liblib.a"); assertRequiresDarwin(archiveAction); } @@ -924,14 +928,19 @@ public void testArchiveAction_device() throws Exception { assertThat(archiveAction.getArguments()) .isEqualTo( ImmutableList.of( - "tools/osx/crosstool/ios/ar_wrapper", - "rcs", - Iterables.getOnlyElement(archiveAction.getOutputs()).getExecPathString(), - getBinArtifact("_objs/lib/arc/a.o", getConfiguredTarget("//objc:lib")) + "tools/osx/crosstool/ios/libtool", + "-static", + "-filelist", + getBinArtifact("lib-archive.objlist", getConfiguredTarget("//objc:lib")) .getExecPathString(), - getBinArtifact("_objs/lib/arc/b.o", getConfiguredTarget("//objc:lib")) - .getExecPathString())); - assertThat(baseArtifactNames(archiveAction.getInputs())).containsAtLeast("a.o", "b.o"); + "-arch_only", + "armv7", + "-syslibroot", + AppleToolchain.sdkDir(), + "-o", + Iterables.getOnlyElement(archiveAction.getOutputs()).getExecPathString())); + assertThat(baseArtifactNames(archiveAction.getInputs())) + .containsAtLeast("a.o", "b.o", "lib-archive.objlist"); assertThat(baseArtifactNames(archiveAction.getOutputs())).containsExactly("liblib.a"); assertRequiresDarwin(archiveAction); } @@ -1157,6 +1166,19 @@ public void testPopulatesCompilationArtifacts() throws Exception { checkPopulatesCompilationArtifacts(RULE_TYPE); } + @Test + public void testObjcListFileInArchiveGeneration() throws Exception { + scratch.file("lib/a.m"); + scratch.file("lib/b.m"); + scratch.file("lib/BUILD", "objc_library(name = 'lib1', srcs = ['a.m', 'b.m'])"); + ConfiguredTarget target = getConfiguredTarget("//lib:lib1"); + Artifact lib = getBinArtifact("liblib1.a", target); + Action action = getGeneratingAction(lib); + assertThat(paramFileArgsForAction(action)) + .containsExactlyElementsIn( + Artifact.toExecPaths(inputsEndingWith(archiveAction("//lib:lib1"), ".o"))); + } + @Test public void testErrorsWrongFileTypeForSrcsWhenCompiling() throws Exception { checkErrorsWrongFileTypeForSrcsWhenCompiling(RULE_TYPE); @@ -1460,15 +1482,19 @@ public void testCollectsSdkFrameworksTransitively() throws Exception { // for creating binaries but is ignored for libraries. CommandAction archiveAction = archiveAction("//depender_lib:lib"); assertThat(archiveAction.getArguments()) - .isEqualTo( - ImmutableList.of( - "tools/osx/crosstool/mac/ar_wrapper", - "rcs", - Iterables.getOnlyElement(archiveAction.getOutputs()).getExecPathString(), - getBinArtifact("_objs/lib/arc/a.o", getConfiguredTarget("//depender_lib:lib")) - .getExecPathString(), - getBinArtifact("_objs/lib/arc/b.o", getConfiguredTarget("//depender_lib:lib")) - .getExecPathString())); + .containsAtLeastElementsIn( + new ImmutableList.Builder() + .add("-static") + .add("-filelist") + .add( + getBinArtifact("lib-archive.objlist", getConfiguredTarget("//depender_lib:lib")) + .getExecPathString()) + .add("-arch_only", "x86_64") + .add("-syslibroot") + .add(AppleToolchain.sdkDir()) + .add("-o") + .addAll(Artifact.toExecPaths(archiveAction.getOutputs())) + .build()); } @Test @@ -2191,7 +2217,7 @@ public void testCompileLanguageApi() throws Exception { " compilation_outputs=compilation_outputs,", " name = ctx.label.name,", " cc_toolchain=toolchain,", - " language='c++'", + " language='objc'", " )", " files_to_build = []", " files_to_build.extend(compilation_outputs.pic_objects)", 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 0814a46d7b6238..05d9091f09142f 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 @@ -1452,6 +1452,24 @@ protected void checkLinkMinimumOSVersion(String minOSVersionOption) throws Excep assertThat(Joiner.on(" ").join(linkAction.getArguments())).contains(minOSVersionOption); } + protected void checkWatchSimulatorDepCompile(RuleType ruleType) throws Exception { + ruleType.scratchTarget(scratch, "deps", "['//package:objcLib']", "platform_type", "'watchos'"); + scratch.file("package/BUILD", "objc_library(name = 'objcLib', srcs = [ 'b.m' ])"); + + Action lipoAction = actionProducingArtifact("//x:x", "_lipobin"); + + String i386Bin = + configurationBin("i386", ConfigurationDistinguisher.APPLEBIN_WATCHOS) + "x/x_bin"; + Artifact binArtifact = getFirstArtifactEndingWith(lipoAction.getInputs(), i386Bin); + CommandAction linkAction = (CommandAction) getGeneratingAction(binArtifact); + CommandAction objcLibCompileAction = + (CommandAction) + getGeneratingAction(getFirstArtifactEndingWith(linkAction.getInputs(), "libobjcLib.a")); + + assertAppleSdkPlatformEnv(objcLibCompileAction, "WatchSimulator"); + assertThat(objcLibCompileAction.getArguments()).containsAtLeast("-arch_only", "i386").inOrder(); + } + protected void checkWatchSimulatorLinkAction(RuleType ruleType) throws Exception { ruleType.scratchTarget(scratch, "deps", "['//package:objcLib']", "platform_type", "'watchos'"); scratch.file("package/BUILD", "objc_library(name = 'objcLib', srcs = [ 'b.m' ])");