Skip to content

Commit

Permalink
Add missing tests for object files support and a separate param for P…
Browse files Browse the repository at this point in the history 
…IC static library in cc_import

A follow-up to #11696

Closes #11735.

PiperOrigin-RevId: 321131430
  • Loading branch information
@agluszak @copybara-github
agluszak authored and copybara-github committed Jul 14, 2020
1 parent 930424b commit a91aad8
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 60 deletions.
131 changes: 102 additions & 29 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));

Expand Down Expand Up @@ -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')",
Expand Down Expand Up @@ -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')",
Expand Down Expand Up @@ -1447,7 +1447,7 @@ private void doTestCcLinkingContext(
if (experimentalCcSharedLibrary) {
setUpCcLinkingContextTestForExperimentalCcSharedLibrary();
} else {
setUpCcLinkingContextTest();
setUpCcLinkingContextTest(false);
}
ConfiguredTarget a = getConfiguredTarget("//a:a");

Expand Down Expand Up @@ -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')",
Expand Down Expand Up @@ -1564,18 +1567,21 @@ 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], ",
" static_library=static_library, pic_static_library=pic_static_library,",
" 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(",
Expand All @@ -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,",
Expand All @@ -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(",
Expand All @@ -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(),",
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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')",
Expand Down Expand Up @@ -5644,7 +5654,6 @@ public void testWrongPublicHdrExtensionGivesError() throws Exception {
doTestWrongExtensionOfSrcsAndHdrs("public_hdrs");
}


@Test
public void testWrongSrcExtensionGivesError() throws Exception {
createFiles(scratch, "tools/build_defs/foo");
Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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<LibraryToLink> 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'");
}
}
36 changes: 9 additions & 27 deletions tools/build_defs/cc/cc_import.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand All @@ -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"],
Expand Down
Loading

0 comments on commit a91aad8

Please sign in to comment.