Skip to content

Commit

Permalink
Rollback of commit abb0b63.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

breaks bazel linux sandbox

*** Original change description ***

Move -l/-L link opts to FeatureConfiguration

This cl moves -l/-L link opts generation from Java to
FeatureConfiguration making it possible to alter the flags in CROSSTOOL.

Change-Id: I1ea7501435ab7e62992e1e9b0cb7f5e22d52c521

--
MOS_MIGRATED_REVID=137184226
  • Loading branch information
@aragos @katre
aragos authored and katre committed Oct 25, 2016
1 parent 03d6302 commit 8734267
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 142 deletions.
82 changes: 7 additions & 75 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,6 @@ public class CppLinkActionBuilder {
*/
public static final String LIBOPTS_VARIABLE = "libopts";

/** A build variable containing all folders to be added to library search path. */
public static final String LIBRARY_SEARCH_DIRECTORIES_VARIABLE = "library_search_directories";

/** A build variable containing all libraries to be linked with current artifact. */
public static final String DYNAMIC_LIBRARIES_TO_LINK_VARIABLE = "dynamic_libraries_to_link";

/**
* A build variable containing all interface shared objects to be linked with current artifact.
*/
public static final String INTERFACE_LIBRARIES_TO_LINK_VARIABLE = "interface_libraries_to_link";

/**
* A build variable for flags providing files to link as inputs in the linker invocation that
* should not go in a -whole_archive block.
Expand Down Expand Up @@ -1165,9 +1154,6 @@ private static class LinkArgCollector {
String rpathRoot;
List<String> rpathEntries;
Set<String> libopts;
Set<String> librarySearchDirectories;
Set<String> dynamicLibrariesToLink;
Set<String> interfaceLibrariesToLink;
List<String> linkerInputParams;
List<String> wholeArchiveLinkerInputParams;
List<String> noWholeArchiveInputs;
Expand Down Expand Up @@ -1196,18 +1182,6 @@ public void setNoWholeArchiveInputs(List<String> noWholeArchiveInputs) {
this.noWholeArchiveInputs = noWholeArchiveInputs;
}

public void setLibrarySearchDirectories(Set<String> librarySearchDirectories) {
this.librarySearchDirectories = librarySearchDirectories;
}

public void setDynamicLibrariesToLink(Set<String> dynamicLibrariesToLink) {
this.dynamicLibrariesToLink = dynamicLibrariesToLink;
}

public void setInterfaceLibrariesToLink(Set<String> interfaceLibrariesToLink) {
this.interfaceLibrariesToLink = interfaceLibrariesToLink;
}

public String getRpathRoot() {
return rpathRoot;
}
Expand All @@ -1231,18 +1205,6 @@ public List<String> getWholeArchiveLinkerInputParams() {
public List<String> getNoWholeArchiveInputs() {
return noWholeArchiveInputs;
}

public Set<String> getLibrarySearchDirectories() {
return librarySearchDirectories;
}

public Set<String> getDynamicLibrariesToLink() {
return dynamicLibrariesToLink;
}

public Set<String> getInterfaceLibrariesToLink() {
return interfaceLibrariesToLink;
}
}

private class CppLinkVariablesExtension implements VariablesExtension {
Expand Down Expand Up @@ -1328,12 +1290,6 @@ public void addVariables(Variables.Builder buildVariables) {
}

buildVariables.addSequenceVariable(LIBOPTS_VARIABLE, linkArgCollector.getLibopts());
buildVariables.addSequenceVariable(
LIBRARY_SEARCH_DIRECTORIES_VARIABLE, linkArgCollector.getLibrarySearchDirectories());
buildVariables.addSequenceVariable(
DYNAMIC_LIBRARIES_TO_LINK_VARIABLE, linkArgCollector.getDynamicLibrariesToLink());
buildVariables.addSequenceVariable(
INTERFACE_LIBRARIES_TO_LINK_VARIABLE, linkArgCollector.getInterfaceLibrariesToLink());
buildVariables.addSequenceVariable(
LINKER_INPUT_PARAMS_VARIABLE, linkArgCollector.getLinkerInputParams());
buildVariables.addSequenceVariable(
Expand Down Expand Up @@ -1417,9 +1373,6 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) {

// Used to collect -L and -Wl,-rpath options, ensuring that each used only once.
Set<String> libOpts = new LinkedHashSet<>();
Set<String> librarySearchDirectories = new LinkedHashSet<>();
Set<String> dynamicLibrariesToLink = new LinkedHashSet<>();
Set<String> interfaceLibrariesToLink = new LinkedHashSet<>();

// List of command line parameters that need to be placed *outside* of
// --whole-archive ... --no-whole-archive.
Expand Down Expand Up @@ -1534,13 +1487,7 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) {
includeSolibDir = true;
}
addDynamicInputLinkOptions(
input,
libOpts,
librarySearchDirectories,
dynamicLibrariesToLink,
interfaceLibrariesToLink,
solibDir,
rpathRoot);
input, standardArchiveInputParams, libOpts, solibDir, rpathRoot);
} else {
addStaticInputLinkOptions(
input, wholeArchiveInputParams, standardArchiveInputParams, ltoMap);
Expand All @@ -1561,14 +1508,7 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) {
input.getArtifact(),
solibDir);
includeRuntimeSolibDir = true;
addDynamicInputLinkOptions(
input,
libOpts,
librarySearchDirectories,
dynamicLibrariesToLink,
interfaceLibrariesToLink,
solibDir,
rpathRoot);
addDynamicInputLinkOptions(input, optionsList, libOpts, solibDir, rpathRoot);
} else {
addStaticInputLinkOptions(input,
needWholeArchive ? noWholeArchiveInputs : wholeArchiveInputParams,
Expand All @@ -1589,9 +1529,6 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) {
}

linkArgCollector.setLibopts(libOpts);
linkArgCollector.setLibrarySearchDirectories(librarySearchDirectories);
linkArgCollector.setDynamicLibrariesToLink(dynamicLibrariesToLink);
linkArgCollector.setInterfaceLibrariesToLink(interfaceLibrariesToLink);

linkArgCollector.setLinkerInputParams(standardArchiveInputParams);
linkArgCollector.setWholeArchiveLinkerInputParams(wholeArchiveInputParams);
Expand All @@ -1605,10 +1542,8 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) {
/** Adds command-line options for a dynamic library input file into options and libOpts. */
private void addDynamicInputLinkOptions(
LinkerInput input,
List<String> options,
Set<String> libOpts,
Set<String> librarySearchDirectories,
Set<String> dynamicLibrariesToLink,
Set<String> interfaceLibrariesToLink,
PathFragment solibDir,
String rpathRoot) {
Preconditions.checkState(input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY);
Expand All @@ -1625,25 +1560,22 @@ private void addDynamicInputLinkOptions(
dotdots += "../";
commonParent = commonParent.getParentDirectory();
}

libOpts.add(rpathRoot + dotdots + libDir.relativeTo(commonParent).getPathString());
}

librarySearchDirectories.add(
inputArtifact.getExecPath().getParentDirectory().getPathString());
libOpts.add("-L" + inputArtifact.getExecPath().getParentDirectory().getPathString());

String name = inputArtifact.getFilename();
if (CppFileTypes.SHARED_LIBRARY.matches(name)) {
String libName = name.replaceAll("(^lib|\\.(so|dylib)$)", "");
dynamicLibrariesToLink.add(libName);
options.add("-l" + libName);
} else {
// Interface shared objects have a non-standard extension
// that the linker won't be able to find. So use the
// filename directly rather than a -l option. Since the
// library has an SONAME attribute, this will work fine.
// NOTE: we assume interface shared objects have no
// dependencies and can appear anywhere in the Cpp link
// command line.
interfaceLibrariesToLink.add(inputArtifact.getExecPathString());
options.add(inputArtifact.getExecPathString());
}
}

Expand Down
27 changes: 0 additions & 27 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ public static String getCppLinkActionConfigs(
" implies: 'global_whole_archive_open'",
" implies: 'runtime_root_flags'",
" implies: 'input_param_flags'",
" implies: 'libraries_to_link_opts'",
" implies: 'global_whole_archive_close'",
" implies: 'force_pic_flags'",
"}",
Expand All @@ -84,7 +83,6 @@ public static String getCppLinkActionConfigs(
" implies: 'global_whole_archive_open'",
" implies: 'runtime_root_flags'",
" implies: 'input_param_flags'",
" implies: 'libraries_to_link_opts'",
" implies: 'global_whole_archive_close'",
"}",
"action_config {",
Expand All @@ -96,7 +94,6 @@ public static String getCppLinkActionConfigs(
" implies: 'global_whole_archive_open'",
" implies: 'runtime_root_flags'",
" implies: 'input_param_flags'",
" implies: 'libraries_to_link_opts'",
" implies: 'global_whole_archive_close'",
"}",
"action_config {",
Expand All @@ -108,7 +105,6 @@ public static String getCppLinkActionConfigs(
" implies: 'global_whole_archive_open'",
" implies: 'runtime_root_flags'",
" implies: 'input_param_flags'",
" implies: 'libraries_to_link_opts'",
" implies: 'global_whole_archive_close'",
"}",
"action_config {",
Expand All @@ -120,7 +116,6 @@ public static String getCppLinkActionConfigs(
" implies: 'global_whole_archive_open'",
" implies: 'runtime_root_flags'",
" implies: 'input_param_flags'",
" implies: 'libraries_to_link_opts'",
" implies: 'global_whole_archive_close'",
"}",
"action_config {",
Expand All @@ -132,7 +127,6 @@ public static String getCppLinkActionConfigs(
" implies: 'global_whole_archive_open'",
" implies: 'runtime_root_flags'",
" implies: 'input_param_flags'",
" implies: 'libraries_to_link_opts'",
" implies: 'global_whole_archive_close'",
"}",
"feature {",
Expand Down Expand Up @@ -266,27 +260,6 @@ public static String getCppLinkActionConfigs(
" }",
"}",
"feature {",
" name: 'libraries_to_link_opts'",
" flag_set {",
" expand_if_all_available: 'library_search_directories'",
" action: 'c++-link-executable'",
" action: 'c++-link-dynamic-library'",
" action: 'c++-link-static-library'",
" action: 'c++-link-alwayslink-static-library'",
" action: 'c++-link-pic-static-library'",
" action: 'c++-link-alwayslink-pic-static-library'",
" flag_group {",
" flag: '-L%{library_search_directories}'",
" }",
" flag_group {",
" flag: '%{interface_libraries_to_link}'",
" }",
" flag_group {",
" flag: '-l%{dynamic_libraries_to_link}'",
" }",
" }",
"}",
"feature {",
" name: 'input_param_flags'",
" flag_set {",
" expand_if_all_available: 'libopts'",
Expand Down
40 changes: 0 additions & 40 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,46 +163,6 @@ public void testGlobalWholeArchiveOrWholeArchiveBuildVariables() throws Exceptio
;
}

/**
* TODO(pcloudy): Add test for testing that necessary build variables are populated when
* alwayslink=1.
*/

/**
* Tests that librarySearchDirectories, dynamicLibrariesToLink, and interfaceLibrariesToLink are
* exposed.
*/
@Test
public void testLinkingLibrariesFlags() throws Exception {
scratch.file(
"app/BUILD",
"cc_binary(",
" name = 'foo',",
" srcs = ['foo.cc', 'libbar.so'],",
" deps = ['//baz:qux'],",
" linkstatic = 0,",
")");
scratch.file("baz/BUILD", "cc_library(", " name = 'qux',", " srcs = ['qux.cc'],", ")");
scratch.file("app/foo.cc");
scratch.file("app/libbar.so");
scratch.file("baz/qux.cc");

ConfiguredTarget target = getConfiguredTarget("//app:foo");
Variables variables = getLinkBuildVariables(target, LinkTargetType.EXECUTABLE);
List<String> librarySearchDirectories =
getVariableValue(variables, CppLinkActionBuilder.LIBRARY_SEARCH_DIRECTORIES_VARIABLE);
List<String> dynamicLibrariesToLink =
getVariableValue(variables, CppLinkActionBuilder.DYNAMIC_LIBRARIES_TO_LINK_VARIABLE);
List<String> interfaceLibrariesToLink =
getVariableValue(variables, CppLinkActionBuilder.INTERFACE_LIBRARIES_TO_LINK_VARIABLE);

assertThat(librarySearchDirectories).hasSize(2);
assertThat(librarySearchDirectories.get(0)).matches(".*app.*foo.*");
assertThat(dynamicLibrariesToLink).containsExactly("bar");
assertThat(Iterables.getOnlyElement(interfaceLibrariesToLink))
.matches(".*libbaz.*libqux.ifso");
}

@Test
public void testInterfaceLibraryBuildingVariablesWhenGenerationPossible() throws Exception {
// Make sure the interface shared object generation is enabled in the configuration
Expand Down

0 comments on commit 8734267

Please sign in to comment.