diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java index 743e0a43fd6f88..ab6fa7a6f4e699 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java @@ -1327,7 +1327,7 @@ public void testIncompatibleDepsetForLibrariesToLinkGetter() throws Exception { CppRuleClasses.SUPPORTS_PIC, CppRuleClasses.SUPPORTS_DYNAMIC_LINKER)); this.setStarlarkSemanticsOptions("--incompatible_depset_for_libraries_to_link_getter"); - setUpCcLinkingContextTest(); + setUpCcLinkingContextTest(false); ConfiguredTarget a = getConfiguredTarget("//a:a"); StructImpl info = ((StructImpl) getMyInfoFromTarget(a).getValue("info")); @@ -1358,7 +1358,7 @@ private static String getSolibRelativePath(Artifact library, CcToolchainProvider @Test public void testSolibLinkDefault() throws Exception { - setUpCcLinkingContextTest(); + setUpCcLinkingContextTest(false); scratch.file( "foo/BUILD", "load('//tools/build_defs/cc:rule.bzl', 'crule')", @@ -1392,7 +1392,7 @@ public void testSolibLinkDefault() throws Exception { @Test public void testSolibLinkCustom() throws Exception { - setUpCcLinkingContextTest(); + setUpCcLinkingContextTest(false); scratch.file( "foo/BUILD", "load('//tools/build_defs/cc:rule.bzl', 'crule')", @@ -1447,7 +1447,7 @@ private void doTestCcLinkingContext( if (experimentalCcSharedLibrary) { setUpCcLinkingContextTestForExperimentalCcSharedLibrary(); } else { - setUpCcLinkingContextTest(); + setUpCcLinkingContextTest(false); } ConfiguredTarget a = getConfiguredTarget("//a:a"); @@ -1504,7 +1504,10 @@ private void doTestCcLinkingContext( assertThat(bin).isNotNull(); } - private void setUpCcLinkingContextTest() throws Exception { + private void setUpCcLinkingContextTest(boolean enableExperimentalCcImport) throws Exception { + if (enableExperimentalCcImport) { + useConfiguration("--experimental_starlark_cc_import"); + } scratch.file( "a/BUILD", "load('//tools/build_defs/cc:rule.bzl', 'crule')", @@ -1564,10 +1567,10 @@ private void setUpCcLinkingContextTest() throws Exception { "load('//myinfo:myinfo.bzl', 'MyInfo')", "top_linking_context_smoke = cc_common.create_linking_context(libraries_to_link=[],", " user_link_flags=['-first_flag', '-second_flag'])", - "def _create(ctx, feature_configuration, static_library, pic_static_library," - + " dynamic_library,", - " interface_library, dynamic_library_symlink_path, interface_library_symlink_path," - + " alwayslink):", + "def _create(ctx, feature_configuration, static_library, pic_static_library,", + " dynamic_library,", + " interface_library, dynamic_library_symlink_path, interface_library_symlink_path,", + " alwayslink, objects, pic_objects):", " return cc_common.create_library_to_link(", " actions=ctx.actions, feature_configuration=feature_configuration, ", " cc_toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo], ", @@ -1575,7 +1578,10 @@ private void setUpCcLinkingContextTest() throws Exception { " dynamic_library=dynamic_library, interface_library=interface_library,", " dynamic_library_symlink_path=dynamic_library_symlink_path,", " interface_library_symlink_path=interface_library_symlink_path,", - " alwayslink=alwayslink)", + " alwayslink=alwayslink, ", + enableExperimentalCcImport ? " objects=objects, " : "", + enableExperimentalCcImport ? " pic_objects=pic_objects" : "", + " )", "def _impl(ctx):", " toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]", " feature_configuration = cc_common.configure_features(", @@ -1586,7 +1592,7 @@ private void setUpCcLinkingContextTest() throws Exception { " ctx.file.pic_static_library, ctx.file.dynamic_library, ctx.file.interface_library,", " ctx.attr.dynamic_library_symlink_path,", " ctx.attr.interface_library_symlink_path,", - " ctx.attr.alwayslink)", + " ctx.attr.alwayslink, ctx.files.objects, ctx.files.pic_objects)", " linking_context = cc_common.create_linking_context(", " libraries_to_link=[library_to_link],", " user_link_flags=ctx.attr.user_link_flags,", @@ -1606,7 +1612,9 @@ private void setUpCcLinkingContextTest() throws Exception { " pic_static_library = library_to_link.pic_static_library,", " dynamic_library = library_to_link.dynamic_library,", " interface_library = library_to_link.interface_library,", - " alwayslink = library_to_link.alwayslink),", + " alwayslink = library_to_link.alwayslink,", + " objects = library_to_link.objects,", + " pic_objects = library_to_link.pic_objects),", " ),", " merged_cc_info]", "crule = rule(", @@ -1620,6 +1628,8 @@ private void setUpCcLinkingContextTest() throws Exception { " 'dynamic_library_symlink_path': attr.string(),", " 'interface_library': attr.label(allow_single_file=True),", " 'interface_library_symlink_path': attr.string(),", + " 'objects': attr.label_list(allow_files=True),", + " 'pic_objects': attr.label_list(allow_files=True),", " 'alwayslink': attr.bool(),", " '_cc_toolchain': attr.label(default=Label('//a:alias')),", " 'deps': attr.label_list(),", @@ -2273,7 +2283,7 @@ public void testCustomMakeVariable_none_none() throws Exception { public void testCustomMakeVariable_string_none() throws Exception { createCustomMakeVariableRule("two", /* name= */ "'abc'", /* value= */ "None"); - ConfiguredTarget t = getConfiguredTarget("//two:a"); + ConfiguredTarget t = getConfiguredTarget("//two:a"); StarlarkInfo makeVariableProvider = (StarlarkInfo) getMyInfoFromTarget(t).getValue("variable"); EvalException e = assertThrows( @@ -2287,7 +2297,7 @@ public void testCustomMakeVariable_string_none() throws Exception { public void testCustomMakeVariable_list_none() throws Exception { createCustomMakeVariableRule("three", /* name= */ "[]", /* value= */ "None"); - ConfiguredTarget t = getConfiguredTarget("//three:a"); + ConfiguredTarget t = getConfiguredTarget("//three:a"); StarlarkInfo makeVariableProvider = (StarlarkInfo) getMyInfoFromTarget(t).getValue("variable"); EvalException e = assertThrows( @@ -2299,7 +2309,7 @@ public void testCustomMakeVariable_list_none() throws Exception { public void testCustomMakeVariable_string_boolean() throws Exception { createCustomMakeVariableRule("four", /* name= */ "'abc'", /* value= */ "True"); - ConfiguredTarget t = getConfiguredTarget("//four:a"); + ConfiguredTarget t = getConfiguredTarget("//four:a"); StarlarkInfo makeVariableProvider = (StarlarkInfo) getMyInfoFromTarget(t).getValue("variable"); EvalException e = assertThrows( @@ -2391,9 +2401,9 @@ private void createWithFeatureSetRule(String pkg, String features, String notFea public void testCustomWithFeatureSet_struct_none() throws Exception { createCustomWithFeatureSetRule("one", /* features= */ "struct()", /* notFeatures= */ "None"); - ConfiguredTarget t = getConfiguredTarget("//one:a"); + ConfiguredTarget t = getConfiguredTarget("//one:a"); StarlarkInfo withFeatureSetProvider = (StarlarkInfo) getMyInfoFromTarget(t).getValue("wfs"); - assertThat(withFeatureSetProvider).isNotNull(); + assertThat(withFeatureSetProvider).isNotNull(); EvalException e = assertThrows( EvalException.class, () -> CcModule.withFeatureSetFromStarlark(withFeatureSetProvider)); @@ -2404,9 +2414,9 @@ public void testCustomWithFeatureSet_struct_none() throws Exception { public void testCustomWithFeatureSet_listOfString_struct() throws Exception { createCustomWithFeatureSetRule("two", /* features= */ "['abc']", /* notFeatures= */ "struct()"); - ConfiguredTarget t = getConfiguredTarget("//two:a"); + ConfiguredTarget t = getConfiguredTarget("//two:a"); StarlarkInfo withFeatureSetProvider = (StarlarkInfo) getMyInfoFromTarget(t).getValue("wfs"); - assertThat(withFeatureSetProvider).isNotNull(); + assertThat(withFeatureSetProvider).isNotNull(); EvalException e = assertThrows( EvalException.class, () -> CcModule.withFeatureSetFromStarlark(withFeatureSetProvider)); @@ -2417,9 +2427,9 @@ public void testCustomWithFeatureSet_listOfString_struct() throws Exception { public void testCustomWithFeatureSet_listOfStruct_emptyList() throws Exception { createCustomWithFeatureSetRule("three", /* features= */ "[struct()]", /* notFeatures= */ "[]"); - ConfiguredTarget t = getConfiguredTarget("//three:a"); + ConfiguredTarget t = getConfiguredTarget("//three:a"); StarlarkInfo withFeatureSetProvider = (StarlarkInfo) getMyInfoFromTarget(t).getValue("wfs"); - assertThat(withFeatureSetProvider).isNotNull(); + assertThat(withFeatureSetProvider).isNotNull(); EvalException e = assertThrows( EvalException.class, () -> CcModule.withFeatureSetFromStarlark(withFeatureSetProvider)); @@ -2432,9 +2442,9 @@ public void testCustomWithFeatureSet_listOfStruct_emptyList() throws Exception { public void testCustomWithFeatureSet_emptyList_listOfStruct() throws Exception { createCustomWithFeatureSetRule("four", /* features= */ "[]", /* notFeatures= */ "[struct()]"); - ConfiguredTarget t = getConfiguredTarget("//four:a"); + ConfiguredTarget t = getConfiguredTarget("//four:a"); StarlarkInfo withFeatureSetProvider = (StarlarkInfo) getMyInfoFromTarget(t).getValue("wfs"); - assertThat(withFeatureSetProvider).isNotNull(); + assertThat(withFeatureSetProvider).isNotNull(); EvalException e = assertThrows( EvalException.class, () -> CcModule.withFeatureSetFromStarlark(withFeatureSetProvider)); @@ -3253,9 +3263,9 @@ public void testCustomTool_withFeatures_mustBeList() throws Exception { createCustomToolRule( "three", /* path= */ "'a'", /* withFeatures= */ "struct()", /* requirements= */ "[]"); - ConfiguredTarget t = getConfiguredTarget("//three:a"); + ConfiguredTarget t = getConfiguredTarget("//three:a"); StarlarkInfo toolStruct = (StarlarkInfo) getMyInfoFromTarget(t).getValue("tool"); - assertThat(toolStruct).isNotNull(); + assertThat(toolStruct).isNotNull(); EvalException e = assertThrows(EvalException.class, () -> CcModule.toolFromStarlark(toolStruct)); assertThat(e).hasMessageThat().contains("for with_features, got struct, want sequence"); @@ -5256,7 +5266,7 @@ public void testIsToolchainResolutionEnabled_enabled() throws Exception { @Test public void testWrongExtensionThrowsError() throws Exception { - setUpCcLinkingContextTest(); + setUpCcLinkingContextTest(false); scratch.file( "foo/BUILD", "load('//tools/build_defs/cc:rule.bzl', 'crule')", @@ -5644,7 +5654,6 @@ public void testWrongPublicHdrExtensionGivesError() throws Exception { doTestWrongExtensionOfSrcsAndHdrs("public_hdrs"); } - @Test public void testWrongSrcExtensionGivesError() throws Exception { createFiles(scratch, "tools/build_defs/foo"); @@ -5965,7 +5974,7 @@ public void testLtoBitcodeFilesApiNeverReturningNones() throws Exception { @Test public void testIncompatibleRequireLinkerInputCcApi() throws Exception { setStarlarkSemanticsOptions("--incompatible_require_linker_input_cc_api"); - setUpCcLinkingContextTest(); + setUpCcLinkingContextTest(false); checkError("//a:a", "It may be temporarily re-enabled by setting"); } @@ -6421,7 +6430,7 @@ public void testCcLibraryPropagatesCcInfoWithDirectTextualHeaders() throws Excep /** Fixes #10580 */ @Test public void testMixedLinkerInputsWithOwnerAndWithout() throws Exception { - setUpCcLinkingContextTest(); + setUpCcLinkingContextTest(false); scratch.file("foo/BUILD", "load(':rule.bzl', 'crule')", "crule(name='a')"); scratch.file( "foo/rule.bzl", @@ -6548,4 +6557,68 @@ public void testObjectFilesInCreateLibraryToLinkApiGuardedByFlag() throws Except ")"); checkError("//a:r", "Cannot use objects/pic_objects without --experimental_starlark_cc_import"); } + + @Test + public void testObjectFilesInCreateLibrary() throws Exception { + setUpCcLinkingContextTest(true); + scratch.file( + "b/BUILD", + "load('//tools/build_defs/cc:rule.bzl', 'crule')", + "crule(name='import_objects',", + " static_library = 'lib.a',", + " pic_static_library = 'lib.pic.a',", + " objects = ['object.o'],", + " pic_objects = ['object.pic.o'],", + ")"); + + assertNoEvents(); + ConfiguredTarget lib = getConfiguredTarget("//b:import_objects"); + CcLinkingContext ccLinkingContext = lib.get(CcInfo.PROVIDER).getCcLinkingContext(); + ImmutableList libraries = + ccLinkingContext.getLinkerInputs().toList().stream() + .flatMap(i -> i.getLibraries().stream()) + .collect(ImmutableList.toImmutableList()); + + assertThat( + baseArtifactNames( + libraries.stream() + .flatMap(l -> l.getObjectFiles().stream()) + .collect(ImmutableList.toImmutableList()))) + .containsExactly("object.o"); + assertThat( + baseArtifactNames( + libraries.stream() + .flatMap(l -> l.getPicObjectFiles().stream()) + .collect(ImmutableList.toImmutableList()))) + .containsExactly("object.pic.o"); + } + + @Test + public void testObjectFilesInCreateLibraryWithoutStaticLibrary() throws Exception { + setUpCcLinkingContextTest(true); + scratch.file( + "b/BUILD", + "load('//tools/build_defs/cc:rule.bzl', 'crule')", + "crule(name='import_objects_no_lib',", + " objects = ['object.o'],", + ")"); + + checkError( + "//b:import_objects_no_lib", "If you pass 'objects' you must also pass a 'static_library'"); + } + + @Test + public void testObjectFilesInCreateLibraryWithoutPicStaticLibrary() throws Exception { + setUpCcLinkingContextTest(true); + scratch.file( + "b/BUILD", + "load('//tools/build_defs/cc:rule.bzl', 'crule')", + "crule(name='import_objects_no_pic_lib',", + " pic_objects = ['object.pic.o'],", + ")"); + + checkError( + "//b:import_objects_no_pic_lib", + "If you pass 'pic_objects' you must also pass a 'pic_static_library'"); + } } diff --git a/tools/build_defs/cc/cc_import.bzl b/tools/build_defs/cc/cc_import.bzl index dfc324b5ffa60c..92875339dafc86 100644 --- a/tools/build_defs/cc/cc_import.bzl +++ b/tools/build_defs/cc/cc_import.bzl @@ -121,15 +121,6 @@ def _create_archive_action( mnemonic = "CppArchive", ) -def _get_no_pic_and_pic_static_library(static_library): - if static_library == None: - return (None, None) - - if static_library.extension == ".pic.a": - return (None, static_library) - else: - return (static_library, None) - def _cc_import_impl(ctx): cc_toolchain = find_cpp_toolchain(ctx) cc_common.check_experimental_starlark_cc_import( @@ -149,32 +140,25 @@ def _cc_import_impl(ctx): ctx.file.interface_library, ) - (no_pic_static_library, pic_static_library) = _get_no_pic_and_pic_static_library( - ctx.file.static_library, - ) - - output_file = None - object_files = None + pic_static_library = ctx.file.pic_static_library or None + static_library = ctx.file.static_library or None if ctx.files.pic_objects and not pic_static_library: lib_name = "lib" + ctx.label.name + ".pic.a" pic_static_library = ctx.actions.declare_file(lib_name) - output_file = pic_static_library - object_files = ctx.files.pic_objects + _create_archive_action(ctx, feature_configuration, cc_toolchain, pic_static_library, ctx.files.pic_objects) - if ctx.files.objects and not no_pic_static_library: + if ctx.files.objects and not static_library: lib_name = "lib" + ctx.label.name + ".a" - no_pic_static_library = ctx.actions.declare_file(lib_name) - output_file = no_pic_static_library - object_files = ctx.files.objects + static_library = ctx.actions.declare_file(lib_name) + _create_archive_action(ctx, feature_configuration, cc_toolchain, static_library, ctx.files.objects) library_to_link = cc_common.create_library_to_link( actions = ctx.actions, feature_configuration = feature_configuration, cc_toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo], - static_library = no_pic_static_library, + static_library = static_library, pic_static_library = pic_static_library, - dynamic_library = ctx.file.shared_library, interface_library = ctx.file.interface_library, pic_objects = ctx.files.pic_objects, objects = ctx.files.objects, @@ -200,16 +184,14 @@ def _cc_import_impl(ctx): name = ctx.label.name, ) - if output_file: - _create_archive_action(ctx, feature_configuration, cc_toolchain, output_file, object_files) - return [CcInfo(compilation_context = compilation_context, linking_context = linking_context)] cc_import = rule( implementation = _cc_import_impl, attrs = { "hdrs": attr.label_list(allow_files = [".h"]), - "static_library": attr.label(allow_single_file = [".a", ".lib", ".pic.a"]), + "static_library": attr.label(allow_single_file = [".a", ".lib"]), + "pic_static_library": attr.label(allow_single_file = [".pic.a", ".pic.lib"]), "shared_library": attr.label(allow_single_file = True), "interface_library": attr.label( allow_single_file = [".ifso", ".tbd", ".lib", ".so", ".dylib"], diff --git a/tools/build_defs/cc/tests/cc_import_test.bzl b/tools/build_defs/cc/tests/cc_import_test.bzl index 674077f0702bb1..c194c65f8d0154 100644 --- a/tools/build_defs/cc/tests/cc_import_test.bzl +++ b/tools/build_defs/cc/tests/cc_import_test.bzl @@ -104,10 +104,39 @@ def _cc_import_objects_archive_action_test_impl(ctx): asserts.true(env, found, "Archive creation action should be present if object files were given") + linker_inputs = target_under_test[CcInfo].linking_context.linker_inputs.to_list() + asserts.equals(env, 1, len(linker_inputs)) + + libraries = linker_inputs[0].libraries + asserts.equals(env, 1, len(libraries)) + + pic_static_library = libraries[0].pic_static_library + asserts.true(env, pic_static_library, "Pic static library should be defined") + + expected_name = "libcc_import_objects_archive_action_test_import.pic.a" + asserts.equals(env, expected_name, pic_static_library.basename) + return analysistest.end(env) cc_import_objects_archive_action_test = analysistest.make(_cc_import_objects_archive_action_test_impl) +def _cc_import_objects_two_archive_actions_test_impl(ctx): + env = analysistest.begin(ctx) + + target_under_test = analysistest.target_under_test(env) + actions = analysistest.target_actions(env) + + found = 0 + for action in actions: + if action.mnemonic == "CppArchive": + found += 1 + + asserts.equals(env, 2, found) + + return analysistest.end(env) + +cc_import_objects_two_archive_actions_test = analysistest.make(_cc_import_objects_two_archive_actions_test_impl) + def _cc_import_no_objects_no_archive_action_test_impl(ctx): env = analysistest.begin(ctx) @@ -130,12 +159,26 @@ def _cc_import_objects_present_in_linking_context_test_impl(ctx): target_under_test = analysistest.target_under_test(env) linker_inputs = target_under_test[CcInfo].linking_context.linker_inputs.to_list() - asserts.true(env, len(linker_inputs) == 1, "There should be 1 linker input") + asserts.equals(env, 1, len(linker_inputs)) + libraries = linker_inputs[0].libraries - asserts.true(env, len(libraries) == 1, "There should be 1 library to link") + asserts.equals(env, 1, len(libraries)) + objects = libraries[0].objects - asserts.true(env, len(objects) == 1, "There should be 1 object file") - asserts.true(env, objects[0].basename == "object.o", "Object's name should be 'object.o'") + asserts.equals(env, 1, len(objects)) + asserts.equals(env, "object.o", objects[0].basename) + + pic_objects = libraries[0].pic_objects + asserts.equals(env, 1, len(pic_objects)) + asserts.equals(env, "object.pic.o", pic_objects[0].basename) + + static_library = libraries[0].static_library + asserts.true(env, static_library) + asserts.equals(env, "lib.a", static_library.basename) + + pic_static_library = libraries[0].pic_static_library + asserts.true(env, pic_static_library) + asserts.equals(env, "lib.pic.a", pic_static_library.basename) return analysistest.end(env) @@ -165,6 +208,14 @@ def cc_import_test_suite(name): pic_objects = ["mylib.pic.o"], target_is_binary = False, ) + _generic_cc_import_test_setup( + name = "objects_two_archive_actions", + test = cc_import_objects_two_archive_actions_test, + tests_list = _tests, + pic_objects = ["mylib.pic.o"], + objects = ["mylib.o"], + target_is_binary = False, + ) _generic_cc_import_test_setup( name = "no_objects_no_archive_action", test = cc_import_no_objects_no_archive_action_test, @@ -178,6 +229,9 @@ def cc_import_test_suite(name): test = cc_import_objects_present_in_linking_context_test, tests_list = _tests, objects = ["object.o"], + pic_objects = ["object.pic.o"], + static_library = "lib.a", + pic_static_library = "lib.pic.a", target_is_binary = False, )