Allow GN libs to be specified by path instead of by name
Android needs to link against libgcc.a, and it makes much more sense to
do so via direct path rather than using name + search-path to do so.
This change allows paths (any string that contains a '/') to appear in
a libs list. Paths are not prefixed by -l when passed to the linker,
and follow the normal path semantics.
BUG=570406
R=brettw@chromium.org, tfarina@chromium.org
Review URL: https://codereview.chromium.org/1530183005
Cr-Original-Commit-Position: refs/heads/master@{#366530}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: f865ab8f007295f7efc655a3685b7c4cab140635
diff --git a/tools/gn/BUILD.gn b/tools/gn/BUILD.gn
index cc7f7e4..198bf8d 100644
--- a/tools/gn/BUILD.gn
+++ b/tools/gn/BUILD.gn
@@ -96,6 +96,8 @@
"label_pattern.cc",
"label_pattern.h",
"label_ptr.h",
+ "lib_file.cc",
+ "lib_file.h",
"loader.cc",
"loader.h",
"location.cc",
diff --git a/tools/gn/command_desc.cc b/tools/gn/command_desc.cc
index 3f2bbaf..d920f2c 100644
--- a/tools/gn/command_desc.cc
+++ b/tools/gn/command_desc.cc
@@ -169,7 +169,7 @@
}
void PrintLibs(const Target* target, bool display_header) {
- const OrderedSet<std::string>& libs = target->all_libs();
+ const OrderedSet<LibFile>& libs = target->all_libs();
if (libs.empty())
return;
@@ -177,7 +177,7 @@
OutputString("\nlibs\n");
for (size_t i = 0; i < libs.size(); i++)
- OutputString(" " + libs[i] + "\n");
+ OutputString(" " + libs[i].value() + "\n");
}
void PrintPublic(const Target* target, bool display_header) {
diff --git a/tools/gn/config_values.h b/tools/gn/config_values.h
index b6592cf..823f1df 100644
--- a/tools/gn/config_values.h
+++ b/tools/gn/config_values.h
@@ -9,6 +9,7 @@
#include <vector>
#include "base/macros.h"
+#include "tools/gn/lib_file.h"
#include "tools/gn/source_dir.h"
#include "tools/gn/source_file.h"
@@ -39,12 +40,14 @@
DIR_VALUES_ACCESSOR (include_dirs)
STRING_VALUES_ACCESSOR(ldflags)
DIR_VALUES_ACCESSOR (lib_dirs)
- STRING_VALUES_ACCESSOR(libs)
// If you add a new one, be sure to update AppendValues().
#undef STRING_VALUES_ACCESSOR
#undef DIR_VALUES_ACCESSOR
+ const std::vector<LibFile>& libs() const { return libs_; }
+ std::vector<LibFile>& libs() { return libs_; }
+
bool has_precompiled_headers() const {
return !precompiled_header_.empty() || !precompiled_source_.is_null();
}
@@ -72,7 +75,7 @@
std::vector<SourceDir> include_dirs_;
std::vector<std::string> ldflags_;
std::vector<SourceDir> lib_dirs_;
- std::vector<std::string> libs_;
+ std::vector<LibFile> libs_;
// If you add a new one, be sure to update AppendValues().
std::string precompiled_header_;
diff --git a/tools/gn/config_values_generator.cc b/tools/gn/config_values_generator.cc
index ca87d63..3cc8235 100644
--- a/tools/gn/config_values_generator.cc
+++ b/tools/gn/config_values_generator.cc
@@ -24,9 +24,7 @@
if (!value)
return; // No value, empty input and succeed.
- std::vector<std::string> result;
- ExtractListOfStringValues(*value, &result, err);
- (config_values->*accessor)().swap(result);
+ ExtractListOfStringValues(*value, &(config_values->*accessor)(), err);
}
void GetDirList(
@@ -79,11 +77,17 @@
FILL_DIR_CONFIG_VALUE( include_dirs)
FILL_STRING_CONFIG_VALUE(ldflags)
FILL_DIR_CONFIG_VALUE( lib_dirs)
- FILL_STRING_CONFIG_VALUE(libs)
#undef FILL_STRING_CONFIG_VALUE
#undef FILL_DIR_CONFIG_VALUE
+ // Libs
+ const Value* libs_value = scope_->GetValue("libs", true);
+ if (libs_value) {
+ ExtractListOfLibs(scope_->settings()->build_settings(), *libs_value,
+ input_dir_, &config_values_->libs(), err_);
+ }
+
// Precompiled headers.
const Value* precompiled_header_value =
scope_->GetValue(variables::kPrecompiledHeader, true);
diff --git a/tools/gn/gn.gyp b/tools/gn/gn.gyp
index fbc2c37..524e1cd 100644
--- a/tools/gn/gn.gyp
+++ b/tools/gn/gn.gyp
@@ -96,6 +96,8 @@
'label_pattern.cc',
'label_pattern.h',
'label_ptr.h',
+ 'lib_file.cc',
+ 'lib_file.h',
'loader.cc',
'loader.h',
'location.cc',
diff --git a/tools/gn/lib_file.cc b/tools/gn/lib_file.cc
new file mode 100644
index 0000000..86eacb2
--- /dev/null
+++ b/tools/gn/lib_file.cc
@@ -0,0 +1,30 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tools/gn/lib_file.h"
+
+#include "base/logging.h"
+
+LibFile::LibFile() {}
+
+LibFile::LibFile(const SourceFile& source_file) : source_file_(source_file) {}
+
+LibFile::LibFile(const base::StringPiece& lib_name)
+ : name_(lib_name.data(), lib_name.size()) {
+ DCHECK(!lib_name.empty());
+}
+
+void LibFile::Swap(LibFile* other) {
+ name_.swap(other->name_);
+ source_file_.swap(other->source_file_);
+}
+
+const std::string& LibFile::value() const {
+ return is_source_file() ? source_file_.value() : name_;
+}
+
+const SourceFile& LibFile::source_file() const {
+ DCHECK(is_source_file());
+ return source_file_;
+}
diff --git a/tools/gn/lib_file.h b/tools/gn/lib_file.h
new file mode 100644
index 0000000..8e471f2
--- /dev/null
+++ b/tools/gn/lib_file.h
@@ -0,0 +1,56 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef TOOLS_GN_LIB_FILE_H_
+#define TOOLS_GN_LIB_FILE_H_
+
+#include <algorithm>
+#include <string>
+
+#include "base/strings/string_piece.h"
+#include "tools/gn/source_file.h"
+
+// Represents an entry in "libs" list. Can be either a path (a SourceFile) or
+// a library name (a string).
+class LibFile {
+ public:
+ LibFile();
+ explicit LibFile(const base::StringPiece& lib_name);
+ explicit LibFile(const SourceFile& source_file);
+
+ void Swap(LibFile* other);
+ bool is_source_file() const { return name_.empty(); }
+
+ // Returns name, or source_file().value() (whichever is set).
+ const std::string& value() const;
+ const SourceFile& source_file() const;
+
+ bool operator==(const LibFile& other) const {
+ return value() == other.value();
+ }
+ bool operator!=(const LibFile& other) const { return !operator==(other); }
+ bool operator<(const LibFile& other) const { return value() < other.value(); }
+
+ private:
+ std::string name_;
+ SourceFile source_file_;
+};
+
+namespace BASE_HASH_NAMESPACE {
+
+template <>
+struct hash<LibFile> {
+ std::size_t operator()(const LibFile& v) const {
+ hash<std::string> h;
+ return h(v.value());
+ }
+};
+
+} // namespace BASE_HASH_NAMESPACE
+
+inline void swap(LibFile& lhs, LibFile& rhs) {
+ lhs.Swap(&rhs);
+}
+
+#endif // TOOLS_GN_LIB_FILE_H_
diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc
index 98c6f1d..70ba039 100644
--- a/tools/gn/ninja_binary_target_writer.cc
+++ b/tools/gn/ninja_binary_target_writer.cc
@@ -786,6 +786,15 @@
}
}
+ // Libraries specified by paths.
+ const OrderedSet<LibFile>& libs = target_->all_libs();
+ for (size_t i = 0; i < libs.size(); i++) {
+ if (libs[i].is_source_file()) {
+ implicit_deps.push_back(
+ OutputFile(settings_->build_settings(), libs[i].source_file()));
+ }
+ }
+
// Append implicit dependencies collected above.
if (!implicit_deps.empty()) {
out_ << " |";
@@ -857,20 +866,25 @@
// Libraries that have been recursively pushed through the dependency tree.
EscapeOptions lib_escape_opts;
lib_escape_opts.mode = ESCAPE_NINJA_COMMAND;
- const OrderedSet<std::string> all_libs = target_->all_libs();
+ const OrderedSet<LibFile> all_libs = target_->all_libs();
const std::string framework_ending(".framework");
for (size_t i = 0; i < all_libs.size(); i++) {
- if (base::EndsWith(all_libs[i], framework_ending,
- base::CompareCase::INSENSITIVE_ASCII)) {
+ const LibFile& lib_file = all_libs[i];
+ const std::string& lib_value = lib_file.value();
+ if (lib_file.is_source_file()) {
+ out_ << " ";
+ path_output_.WriteFile(out_, lib_file.source_file());
+ } else if (base::EndsWith(lib_value, framework_ending,
+ base::CompareCase::INSENSITIVE_ASCII)) {
// Special-case libraries ending in ".framework" to support Mac: Add the
// -framework switch and don't add the extension to the output.
out_ << " -framework ";
- EscapeStringToStream(out_,
- all_libs[i].substr(0, all_libs[i].size() - framework_ending.size()),
+ EscapeStringToStream(
+ out_, lib_value.substr(0, lib_value.size() - framework_ending.size()),
lib_escape_opts);
} else {
out_ << " " << tool_->lib_switch();
- EscapeStringToStream(out_, all_libs[i], lib_escape_opts);
+ EscapeStringToStream(out_, lib_value, lib_escape_opts);
}
}
out_ << std::endl;
diff --git a/tools/gn/ninja_binary_target_writer_unittest.cc b/tools/gn/ninja_binary_target_writer_unittest.cc
index 0765ba6..b9c50b7 100644
--- a/tools/gn/ninja_binary_target_writer_unittest.cc
+++ b/tools/gn/ninja_binary_target_writer_unittest.cc
@@ -201,6 +201,43 @@
EXPECT_EQ(expected, out_str);
}
+// Tests libs are applied.
+TEST(NinjaBinaryTargetWriter, LibsAndLibDirs) {
+ TestWithScope setup;
+ Err err;
+
+ setup.build_settings()->SetBuildDir(SourceDir("//out/Debug/"));
+
+ // A shared library w/ libs and lib_dirs.
+ Target target(setup.settings(), Label(SourceDir("//foo/"), "shlib"));
+ target.set_output_type(Target::SHARED_LIBRARY);
+ target.config_values().libs().push_back(LibFile(SourceFile("//foo/lib1.a")));
+ target.config_values().libs().push_back(LibFile("foo"));
+ target.config_values().lib_dirs().push_back(SourceDir("//foo/bar/"));
+ target.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(target.OnResolved(&err));
+
+ std::ostringstream out;
+ NinjaBinaryTargetWriter writer(&target, out);
+ writer.Run();
+
+ const char expected[] =
+ "defines =\n"
+ "include_dirs =\n"
+ "root_out_dir = .\n"
+ "target_out_dir = obj/foo\n"
+ "target_output_name = libshlib\n"
+ "\n"
+ "\n"
+ "build ./libshlib.so: solink | ../../foo/lib1.a\n"
+ " ldflags = -L../../foo/bar\n"
+ " libs = ../../foo/lib1.a -lfoo\n"
+ " output_extension = .so\n";
+
+ std::string out_str = out.str();
+ EXPECT_EQ(expected, out_str);
+}
+
TEST(NinjaBinaryTargetWriter, EmptyProductExtension) {
TestWithScope setup;
Err err;
diff --git a/tools/gn/target.cc b/tools/gn/target.cc
index 16a0ba4..0d275ac 100644
--- a/tools/gn/target.cc
+++ b/tools/gn/target.cc
@@ -552,6 +552,10 @@
CheckSourceGenerated(file);
for (const SourceFile& file : inputs_)
CheckSourceGenerated(file);
+ for (size_t i = 0; i < all_libs_.size(); i++) {
+ if (all_libs_[i].is_source_file())
+ CheckSourceGenerated(all_libs_[i].source_file());
+ }
}
void Target::CheckSourceGenerated(const SourceFile& source) const {
diff --git a/tools/gn/target.h b/tools/gn/target.h
index eff85e8..eff4d15 100644
--- a/tools/gn/target.h
+++ b/tools/gn/target.h
@@ -17,6 +17,7 @@
#include "tools/gn/inherited_libraries.h"
#include "tools/gn/item.h"
#include "tools/gn/label_ptr.h"
+#include "tools/gn/lib_file.h"
#include "tools/gn/ordered_set.h"
#include "tools/gn/output_file.h"
#include "tools/gn/source_file.h"
@@ -197,7 +198,7 @@
const ActionValues& action_values() const { return action_values_; }
const OrderedSet<SourceDir>& all_lib_dirs() const { return all_lib_dirs_; }
- const OrderedSet<std::string>& all_libs() const { return all_libs_; }
+ const OrderedSet<LibFile>& all_libs() const { return all_libs_; }
const std::set<const Target*>& recursive_hard_deps() const {
return recursive_hard_deps_;
@@ -300,7 +301,7 @@
// These libs and dirs are inherited from statically linked deps and all
// configs applying to this target.
OrderedSet<SourceDir> all_lib_dirs_;
- OrderedSet<std::string> all_libs_;
+ OrderedSet<LibFile> all_libs_;
// All hard deps from this target and all dependencies. Filled in when this
// target is marked resolved. This will not include the current target.
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc
index e7c4c31..986fa65 100644
--- a/tools/gn/target_unittest.cc
+++ b/tools/gn/target_unittest.cc
@@ -35,7 +35,7 @@
TestWithScope setup;
Err err;
- const std::string lib("foo");
+ const LibFile lib("foo");
const SourceDir libdir("/foo_dir/");
// Leaf target with ldflags set.
@@ -52,7 +52,7 @@
// Shared library target should inherit the libs from the static library
// and its own. Its own flag should be before the inherited one.
- const std::string second_lib("bar");
+ const LibFile second_lib("bar");
const SourceDir second_libdir("/bar_dir/");
TestTarget shared(setup, "//foo:shared", Target::SHARED_LIBRARY);
shared.config_values().libs().push_back(second_lib);
@@ -386,7 +386,7 @@
Label pub_config_label(SourceDir("//a/"), "pubconfig");
Config pub_config(setup.settings(), pub_config_label);
- std::string lib_name("testlib");
+ LibFile lib_name("testlib");
pub_config.own_values().libs().push_back(lib_name);
ASSERT_TRUE(pub_config.OnResolved(&err));
diff --git a/tools/gn/test_with_scope.cc b/tools/gn/test_with_scope.cc
index 896b579..a6953ae 100644
--- a/tools/gn/test_with_scope.cc
+++ b/tools/gn/test_with_scope.cc
@@ -84,6 +84,8 @@
// ALINK
scoped_ptr<Tool> alink_tool(new Tool);
SetCommandForTool("ar {{output}} {{source}}", alink_tool.get());
+ alink_tool->set_lib_switch("-l");
+ alink_tool->set_lib_dir_switch("-L");
alink_tool->set_output_prefix("lib");
alink_tool->set_outputs(SubstitutionList::MakeForTest(
"{{target_out_dir}}/{{target_output_name}}.a"));
@@ -93,6 +95,8 @@
scoped_ptr<Tool> solink_tool(new Tool);
SetCommandForTool("ld -shared -o {{target_output_name}}.so {{inputs}} "
"{{ldflags}} {{libs}}", solink_tool.get());
+ solink_tool->set_lib_switch("-l");
+ solink_tool->set_lib_dir_switch("-L");
solink_tool->set_output_prefix("lib");
solink_tool->set_default_output_extension(".so");
solink_tool->set_outputs(SubstitutionList::MakeForTest(
@@ -103,6 +107,8 @@
scoped_ptr<Tool> solink_module_tool(new Tool);
SetCommandForTool("ld -bundle -o {{target_output_name}}.so {{inputs}} "
"{{ldflags}} {{libs}}", solink_module_tool.get());
+ solink_module_tool->set_lib_switch("-l");
+ solink_module_tool->set_lib_dir_switch("-L");
solink_module_tool->set_output_prefix("lib");
solink_module_tool->set_default_output_extension(".so");
solink_module_tool->set_outputs(SubstitutionList::MakeForTest(
@@ -113,6 +119,8 @@
scoped_ptr<Tool> link_tool(new Tool);
SetCommandForTool("ld -o {{target_output_name}} {{source}} "
"{{ldflags}} {{libs}}", link_tool.get());
+ link_tool->set_lib_switch("-l");
+ link_tool->set_lib_dir_switch("-L");
link_tool->set_outputs(SubstitutionList::MakeForTest(
"{{root_out_dir}}/{{target_output_name}}"));
toolchain->SetTool(Toolchain::TYPE_LINK, link_tool.Pass());
diff --git a/tools/gn/value_extractors.cc b/tools/gn/value_extractors.cc
index 365c534..ec42e66 100644
--- a/tools/gn/value_extractors.cc
+++ b/tools/gn/value_extractors.cc
@@ -73,6 +73,27 @@
const SourceDir& current_dir;
};
+struct LibFileConverter {
+ LibFileConverter(const BuildSettings* build_settings_in,
+ const SourceDir& current_dir_in)
+ : build_settings(build_settings_in),
+ current_dir(current_dir_in) {
+ }
+ bool operator()(const Value& v, LibFile* out, Err* err) const {
+ if (!v.VerifyTypeIs(Value::STRING, err))
+ return false;
+ if (v.string_value().find('/') == std::string::npos) {
+ *out = LibFile(v.string_value());
+ } else {
+ *out = LibFile(current_dir.ResolveRelativeFile(
+ v, err, build_settings->root_path_utf8()));
+ }
+ return !err->has_error();
+ }
+ const BuildSettings* build_settings;
+ const SourceDir& current_dir;
+};
+
struct RelativeDirConverter {
RelativeDirConverter(const BuildSettings* build_settings_in,
const SourceDir& current_dir_in)
@@ -147,6 +168,15 @@
RelativeFileConverter(build_settings, current_dir));
}
+bool ExtractListOfLibs(const BuildSettings* build_settings,
+ const Value& value,
+ const SourceDir& current_dir,
+ std::vector<LibFile>* libs,
+ Err* err) {
+ return ListValueExtractor(value, libs, err,
+ LibFileConverter(build_settings, current_dir));
+}
+
bool ExtractListOfRelativeDirs(const BuildSettings* build_settings,
const Value& value,
const SourceDir& current_dir,
diff --git a/tools/gn/value_extractors.h b/tools/gn/value_extractors.h
index 2482ba2..06d64ce 100644
--- a/tools/gn/value_extractors.h
+++ b/tools/gn/value_extractors.h
@@ -9,6 +9,7 @@
#include <vector>
#include "tools/gn/label_ptr.h"
+#include "tools/gn/lib_file.h"
#include "tools/gn/unique_vector.h"
class BuildSettings;
@@ -30,6 +31,14 @@
std::vector<SourceFile>* files,
Err* err);
+// Extracts a list of libraries. When they contain a "/" they are treated as
+// source paths and are otherwise treated as plain strings.
+bool ExtractListOfLibs(const BuildSettings* build_settings,
+ const Value& value,
+ const SourceDir& current_dir,
+ std::vector<LibFile>* libs,
+ Err* err);
+
// Looks for a list of source directories relative to a given current dir.
bool ExtractListOfRelativeDirs(const BuildSettings* build_settings,
const Value& value,
diff --git a/tools/gn/variables.cc b/tools/gn/variables.cc
index d79c3aa..a835b22 100644
--- a/tools/gn/variables.cc
+++ b/tools/gn/variables.cc
@@ -892,20 +892,20 @@
const char kLibs_Help[] =
"libs: Additional libraries to link.\n"
"\n"
- " A list of strings.\n"
+ " A list of library names or library paths.\n"
"\n"
- " These files will be passed to the linker, which will generally search\n"
- " the library include path. Unlike a normal list of files, they will be\n"
- " passed to the linker unmodified rather than being treated as file\n"
- " names relative to the current build file. Generally you would set\n"
- " the \"lib_dirs\" so your library is found. If you need to specify\n"
- " a path, you can use \"rebase_path\" to convert a path to be relative\n"
- " to the build directory.\n"
+ "Values containing '/' will be treated as references to files in the\n"
+ "build. They will be rebased to be relative to the build directory and\n"
+ "specified in the \"libs\" for linker tools. This facility should be used\n"
+ "for libraries that are checked in to the build. For libraries that are\n"
+ "generated by the build, use normal GN deps to link them.\n"
"\n"
- " When constructing the linker command, the \"lib_prefix\" attribute of\n"
- " the linker tool in the current toolchain will be prepended to each\n"
- " library. So your BUILD file should not specify the switch prefix\n"
- " (like \"-l\").\n"
+ "Values not containing '/' will be treated as system library names. These\n"
+ "will be passed unmodified to the linker and prefixed with the\n"
+ "\"lib_prefix\" attribute of the linker tool. Generally you would set the\n"
+ "\"lib_dirs\" so the given library is found. Your BUILD.gn file should\n"
+ "not specify the switch prefix (like \"-l\"): this will be encoded in\n"
+ "the \"lib_prefix\" of the tool.\n"
"\n"
" Libraries ending in \".framework\" will be special-cased: the switch\n"
" \"-framework\" will be prepended instead of the lib_prefix, and the\n"
