Skip to content

Commit

Permalink
C++: Adds tests for transitive link and fixes bugs
Browse files Browse the repository at this point in the history
Bugs fixed:
 - Creating static libraries during transitive link
 - Could not return None from linking_outputs' executable or library_to_link
 - Null pointer exception when not passing compilation_outputs to link()

#4570

RELNOTES:none
PiperOrigin-RevId: 245956660
  • Loading branch information
oquenchil authored and copybara-github committed Apr 30, 2019
1 parent da2d8d9 commit a2e6863
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public CcLinkingOutputs link(
SkylarkActionFactory actions,
FeatureConfigurationForStarlark skylarkFeatureConfiguration,
CcToolchainProvider skylarkCcToolchainProvider,
CcCompilationOutputs compilationOutputs,
Object compilationOutputs,
SkylarkList<String> userLinkFlags,
SkylarkList<CcLinkingContext> linkingContexts,
String name,
Expand All @@ -126,7 +126,7 @@ public CcLinkingOutputs link(
actions,
skylarkFeatureConfiguration,
skylarkCcToolchainProvider,
compilationOutputs,
convertFromNoneable(compilationOutputs, /* defaultValue= */ null),
userLinkFlags,
linkingContexts,
name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1595,7 +1595,6 @@ protected CcLinkingOutputs link(
throw new EvalException(
location, "Language '" + language + "' does not support " + outputType);
}

CcLinkingHelper helper =
new CcLinkingHelper(
actions.getActionConstructionContext().getRuleErrorConsumer(),
Expand All @@ -1617,13 +1616,11 @@ protected CcLinkingOutputs link(
.addNonCodeLinkerInputs(additionalInputs)
.setDynamicLinkType(dynamicLinkTargetType)
.addCcLinkingContexts(linkingContexts)
.setShouldCreateStaticLibraries(false)
.addLinkopts(userLinkFlags);
try {
CcLinkingOutputs ccLinkingOutputs = CcLinkingOutputs.EMPTY;
if (!compilationOutputs.isEmpty()) {
ccLinkingOutputs = helper.link(compilationOutputs);
}
return ccLinkingOutputs;
return helper.link(
compilationOutputs != null ? compilationOutputs : CcCompilationOutputs.EMPTY);
} catch (RuleErrorException e) {
throw new EvalException(location, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,10 @@ Tuple<Object> compile(
named = true,
defaultValue = "None",
noneable = true,
type = CcCompilationOutputsApi.class),
allowedTypes = {
@ParamType(type = CcCompilationOutputsApi.class),
@ParamType(type = NoneType.class)
}),
@Param(
name = "user_link_flags",
doc = "Additional list of linker options.",
Expand Down Expand Up @@ -303,7 +306,7 @@ LinkingOutputsT link(
SkylarkActionFactoryT skylarkActionFactoryApi,
FeatureConfigurationT skylarkFeatureConfiguration,
CcToolchainProviderT skylarkCcToolchainProvider,
CompilationOutputsT compilationOutputs,
Object compilationOutputs,
SkylarkList<String> userLinkFlags,
SkylarkList<LinkingContextT> linkingContexts,
String name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,17 @@
documented = false,
doc = "Helper class containing CC compilation outputs.")
public interface CcLinkingOutputsApi<FileT extends FileApi> {
@SkylarkCallable(name = "library_to_link", structField = true, documented = false)
@SkylarkCallable(
name = "library_to_link",
structField = true,
allowReturnNones = true,
documented = false)
LibraryToLinkApi<FileT> getLibraryToLink();

@SkylarkCallable(name = "executable", structField = true, documented = false)
@SkylarkCallable(
name = "executable",
structField = true,
allowReturnNones = true,
documented = false)
FileT getExecutable();
}
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public CcLinkingOutputsApi<FileApi> link(
SkylarkActionFactoryApi skylarkActionFactoryApi,
FeatureConfigurationApi skylarkFeatureConfiguration,
CcToolchainProviderApi<FeatureConfigurationApi> skylarkCcToolchainProvider,
CcCompilationOutputsApi<FileApi> compilationOutputs,
Object compilationOutputs,
SkylarkList<String> userLinkFlags,
SkylarkList<CcLinkingContextApi<FileApi>> linkingContexts,
String name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.WithFeatureSet;
import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.StringValueParser;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.SkylarkDict;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
Expand Down Expand Up @@ -5211,4 +5212,113 @@ private void setupCcOutputsTest() throws Exception {
" },",
")");
}

@Test
public void testTransitiveLinkWithDeps() throws Exception {
setupTestTransitiveLink(scratch, " linking_contexts = dep_linking_contexts");
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
assertThat(target).isNotNull();
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
assertThat(artifactsToStrings(getGeneratingAction(executable).getInputs()))
.containsAllOf("bin foo/libdep1.a", "bin foo/libdep2.a");
}

@Test
public void testTransitiveLinkForDynamicLibrary() throws Exception {
setupTestTransitiveLink(scratch, "output_type = 'dynamic_library'");
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
assertThat(target).isNotNull();
LibraryToLink library = (LibraryToLink) getMyInfoFromTarget(target).getValue("library");
assertThat(library).isNotNull();
Object executable = getMyInfoFromTarget(target).getValue("executable");
assertThat(EvalUtils.isNullOrNone(executable)).isTrue();
}

@Test
public void testTransitiveLinkForExecutable() throws Exception {
setupTestTransitiveLink(scratch, "output_type = 'executable'");
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
assertThat(target).isNotNull();
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
assertThat(executable).isNotNull();
Object library = getMyInfoFromTarget(target).getValue("library");
assertThat(EvalUtils.isNullOrNone(library)).isTrue();
}

@Test
public void testTransitiveLinkWithCompilationOutputs() throws Exception {
setupTestTransitiveLink(scratch, "compilation_outputs=objects");
ConfiguredTarget target = getConfiguredTarget("//foo:bin");
assertThat(target).isNotNull();
Artifact executable = (Artifact) getMyInfoFromTarget(target).getValue("executable");
assertThat(artifactsToStrings(getGeneratingAction(executable).getInputs()))
.contains("src foo/file.o");
}

private static void setupTestTransitiveLink(Scratch scratch, String... additionalLines)
throws Exception {
String fragments = " fragments = ['google_cpp', 'cpp'],";
if (AnalysisMock.get().isThisBazel()) {
fragments = " fragments = ['cpp'],";
}
scratch.overwriteFile("tools/build_defs/BUILD");
scratch.file(
"tools/build_defs/extension.bzl",
"load('//myinfo:myinfo.bzl', 'MyInfo')",
"def _cc_bin_impl(ctx):",
" toolchain = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]",
" feature_configuration = cc_common.configure_features(cc_toolchain = toolchain)",
" dep_linking_contexts = []",
" for dep in ctx.attr.deps:",
" dep_linking_contexts.append(dep[CcInfo].linking_context)",
" objects = cc_common.create_compilation_outputs(objects=depset(ctx.files.objects),",
" pic_objects=depset(ctx.files.pic_objects))",
" linking_outputs = cc_common.link(",
" actions=ctx.actions,",
" feature_configuration=feature_configuration,",
" name = ctx.label.name,",
" cc_toolchain=toolchain,",
" " + Joiner.on("").join(additionalLines),
" )",
" return [",
" MyInfo(",
" library=linking_outputs.library_to_link,",
" executable=linking_outputs.executable",
" )",
" ]",
"cc_bin = rule(",
" implementation = _cc_bin_impl,",
" attrs = {",
" 'objects': attr.label_list(allow_files=True),",
" 'pic_objects': attr.label_list(allow_files=True),",
" 'deps': attr.label_list(),",
" '_cc_toolchain': attr.label(default =",
" configuration_field(fragment = 'cpp', name = 'cc_toolchain'))",
" },",
fragments,
")");
scratch.file(
"foo/BUILD",
"load('//tools/build_defs:extension.bzl', 'cc_bin')",
"cc_library(",
" name = 'dep1',",
" srcs = ['dep1.cc'],",
" hdrs = ['dep1.h'],",
" includes = ['dep1/baz'],",
" defines = ['DEP1'],",
")",
"cc_library(",
" name = 'dep2',",
" srcs = ['dep2.cc'],",
" hdrs = ['dep2.h'],",
" includes = ['dep2/qux'],",
" defines = ['DEP2'],",
")",
"cc_bin(",
" name = 'bin',",
" objects = ['file.o'],",
" pic_objects = ['file.pic.o'],",
" deps = [':dep1', ':dep2'],",
")");
}
}

0 comments on commit a2e6863

Please sign in to comment.