Skip to content

Commit

Permalink
Add more phony rules to GN build.
Browse files Browse the repository at this point in the history
For a target //foo/bar:bar this adds the additional phony rules: "foo/bar:bar" and "foo/bar" to build.ninja. This is on top of the phony rules "bar" which will exist for all targets with a unique name "bar". This allows one to disambiguate targets with different paths but the same short name.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/247663006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266680 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
brettw@chromium.org committed Apr 28, 2014
1 parent 3698670 commit a3372c7
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 15 deletions.
3 changes: 3 additions & 0 deletions tools/gn/escape.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ void EscapeStringToString(const base::StringPiece& str,
} else if (str[i] == '\\' && (options.mode & ESCAPE_JSON)) {
dest->push_back('\\');
dest->push_back('\\');
} else if (str[i] == ':' && (options.mode & ESCAPE_NINJA)) {
dest->push_back('$');
dest->push_back(':');
} else {
dest->push_back(str[i]);
}
Expand Down
19 changes: 19 additions & 0 deletions tools/gn/escape_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,25 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "tools/gn/escape.h"

TEST(Escape, Ninja) {
EscapeOptions opts;
opts.mode = ESCAPE_NINJA;
std::string result = EscapeString("asdf: \"$\\bar", opts, NULL);
EXPECT_EQ("asdf$:$ \"$$\\bar", result);
}

TEST(Escape, Shell) {
EscapeOptions opts;
opts.mode = ESCAPE_SHELL;
std::string result = EscapeString("asdf: \"$\\bar", opts, NULL);
#if defined(OS_WIN)
// Windows shell doesn't escape backslashes.
EXPECT_EQ("\"asdf: \"$\\bar\"", result);
#else
EXPECT_EQ("\"asdf: \\\"$\\\\bar\"", result);
#endif
}

TEST(Escape, UsedQuotes) {
EscapeOptions shell_options;
shell_options.mode = ESCAPE_SHELL;
Expand Down
17 changes: 17 additions & 0 deletions tools/gn/filesystem_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,23 @@ base::StringPiece FindDir(const std::string* path) {
return base::StringPiece(path->data(), filename_offset);
}

base::StringPiece FindLastDirComponent(const SourceDir& dir) {
const std::string& dir_string = dir.value();

if (dir_string.empty())
return base::StringPiece();
int cur = static_cast<int>(dir_string.size()) - 1;
DCHECK(dir_string[cur] == '/');
int end = cur;
cur--; // Skip before the last slash.

for (; cur >= 0; cur--) {
if (dir_string[cur] == '/')
return base::StringPiece(&dir_string[cur + 1], end - cur - 1);
}
return base::StringPiece(&dir_string[0], end);
}

bool EnsureStringIsInOutputDir(const SourceDir& dir,
const std::string& str,
const Value& originating,
Expand Down
4 changes: 4 additions & 0 deletions tools/gn/filesystem_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ bool EndsWithSlash(const std::string& s);
// input pointer must outlive the output.
base::StringPiece FindDir(const std::string* path);

// Returns the substring identifying the last component of the dir, or the
// empty substring if none. For example "//foo/bar/" -> "bar".
base::StringPiece FindLastDirComponent(const SourceDir& dir);

// Verifies that the given string references a file inside of the given
// directory. This is pretty stupid and doesn't handle "." and "..", etc.,
// it is designed for a sanity check to keep people from writing output files
Expand Down
17 changes: 17 additions & 0 deletions tools/gn/filesystem_utils_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,23 @@ TEST(FilesystemUtils, FindDir) {
EXPECT_EQ("foo/bar/", FindDir(&input));
}

TEST(FilesystemUtils, FindLastDirComponent) {
SourceDir empty;
EXPECT_EQ("", FindLastDirComponent(empty));

SourceDir root("/");
EXPECT_EQ("", FindLastDirComponent(root));

SourceDir srcroot("//");
EXPECT_EQ("", FindLastDirComponent(srcroot));

SourceDir regular1("//foo/");
EXPECT_EQ("foo", FindLastDirComponent(regular1));

SourceDir regular2("//foo/bar/");
EXPECT_EQ("bar", FindLastDirComponent(regular2));
}

TEST(FilesystemUtils, IsPathAbsolute) {
EXPECT_TRUE(IsPathAbsolute("/foo/bar"));
EXPECT_TRUE(IsPathAbsolute("/"));
Expand Down
12 changes: 6 additions & 6 deletions tools/gn/ninja_action_target_writer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,11 @@ TEST(NinjaActionTargetWriter, ActionWithSources) {
// depending if we're on actual Windows or Linux pretending to be Windows.
const char expected_win[] =
"rule __foo_bar___rule\n"
" command = C:/python/python.exe gyp-win-tool action-wrapper environment.x86 __foo_bar___rule.$unique_name.rsp\n"
" command = C$:/python/python.exe gyp-win-tool action-wrapper environment.x86 __foo_bar___rule.$unique_name.rsp\n"
" description = ACTION //foo:bar()\n"
" restat = 1\n"
" rspfile = __foo_bar___rule.$unique_name.rsp\n"
" rspfile_content = C:/python/python.exe ../../foo/script.py\n"
" rspfile_content = C$:/python/python.exe ../../foo/script.py\n"
"\n"
"build foo.out: __foo_bar___rule | ../../foo/included.txt ../../foo/source.txt\n"
"\n"
Expand Down Expand Up @@ -237,12 +237,12 @@ TEST(NinjaActionTargetWriter, ForEach) {
// depending if we're on actual Windows or Linux pretending to be Windows.
const char expected_win[] =
"rule __foo_bar___rule\n"
" command = C:/python/python.exe gyp-win-tool action-wrapper "
" command = C$:/python/python.exe gyp-win-tool action-wrapper "
"environment.x86 __foo_bar___rule.$unique_name.rsp\n"
" description = ACTION //foo:bar()\n"
" restat = 1\n"
" rspfile = __foo_bar___rule.$unique_name.rsp\n"
" rspfile_content = C:/python/python.exe ../../foo/script.py -i "
" rspfile_content = C$:/python/python.exe ../../foo/script.py -i "
"${source} \"--out=foo$ bar${source_name_part}.o\"\n"
"\n"
"build input1.out: __foo_bar___rule ../../foo/input1.txt | "
Expand Down Expand Up @@ -341,12 +341,12 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
// depending if we're on actual Windows or Linux pretending to be Windows.
const char expected_win[] =
"rule __foo_bar___rule\n"
" command = C:/python/python.exe gyp-win-tool action-wrapper "
" command = C$:/python/python.exe gyp-win-tool action-wrapper "
"environment.x86 __foo_bar___rule.$unique_name.rsp\n"
" description = ACTION //foo:bar()\n"
" restat = 1\n"
" rspfile = __foo_bar___rule.$unique_name.rsp\n"
" rspfile_content = C:/python/python.exe ../../foo/script.py -i "
" rspfile_content = C$:/python/python.exe ../../foo/script.py -i "
"${source} \"--out=foo$ bar${source_name_part}.o\"\n"
"\n"
"build gen/input1.d input1.out: __foo_bar___rule ../../foo/input1.txt"
Expand Down
76 changes: 67 additions & 9 deletions tools/gn/ninja_build_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/file_util.h"
#include "base/path_service.h"
#include "base/process/process_handle.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "tools/gn/build_settings.h"
Expand Down Expand Up @@ -179,30 +180,87 @@ void NinjaBuildWriter::WritePhonyAndAllRules() {

// Write phony rules for all uniquely-named targets in the default toolchain.
// Don't do other toolchains or we'll get naming conflicts, and if the name
// isn't unique, also skip it.
// isn't unique, also skip it. The exception is for the toplevel targets
// which we also find.
std::map<std::string, int> small_name_count;
for (size_t i = 0; i < default_toolchain_targets_.size(); i++)
small_name_count[default_toolchain_targets_[i]->label().name()]++;

std::vector<const Target*> toplevel_targets;
for (size_t i = 0; i < default_toolchain_targets_.size(); i++) {
const Target* target = default_toolchain_targets_[i];
const Label& label = target->label();
small_name_count[label.name()]++;

// Look for targets with a name of the form
// dir = "//foo/", name = "foo"
// i.e. where the target name matches the top level directory. We will
// always write phony rules for these even if there is another target with
// the same short name.
const std::string& dir_string = label.dir().value();
if (dir_string.size() == label.name().size() + 3 && // Size matches.
dir_string[0] == '/' && dir_string[1] == '/' && // "//" at beginning.
dir_string[dir_string.size() - 1] == '/' && // "/" at end.
dir_string.compare(2, label.name().size(), label.name()) == 0)
toplevel_targets.push_back(target);
}

for (size_t i = 0; i < default_toolchain_targets_.size(); i++) {
const Target* target = default_toolchain_targets_[i];
const Label& label = target->label();
OutputFile target_file = helper_.GetTargetOutputFile(target);
if (target_file.value() != target->label().name() &&
small_name_count[default_toolchain_targets_[i]->label().name()] == 1) {
out_ << "build " << target->label().name() << ": phony ";
path_output_.WriteFile(out_, target_file);
out_ << std::endl;

// Write the long name "foo/bar:baz" for the target "//foo/bar:baz".
std::string long_name = label.GetUserVisibleName(false);
base::TrimString(long_name, "/", &long_name);
WritePhonyRule(target, target_file, long_name);

// Write the directory name with no target name if they match
// (e.g. "//foo/bar:bar" -> "foo/bar").
if (FindLastDirComponent(label.dir()) == label.name()) {
std::string medium_name = DirectoryWithNoLastSlash(label.dir());
base::TrimString(medium_name, "/", &medium_name);
// That may have generated a name the same as the short name of the
// target which we already wrote.
if (medium_name != label.name())
WritePhonyRule(target, target_file, medium_name);
}

// Write short names for ones which are unique.
if (small_name_count[label.name()] == 1)
WritePhonyRule(target, target_file, label.name());

if (!all_rules.empty())
all_rules.append(" $\n ");
all_rules.append(target_file.value());
}

// Pick up phony rules for the toplevel targets with non-unique names (which
// would have been skipped in the above loop).
for (size_t i = 0; i < toplevel_targets.size(); i++) {
if (small_name_count[toplevel_targets[i]->label().name()] > 1) {
const Target* target = toplevel_targets[i];
WritePhonyRule(target, helper_.GetTargetOutputFile(target),
target->label().name());
}
}

if (!all_rules.empty()) {
out_ << "\nbuild all: phony " << all_rules << std::endl;
out_ << "default all" << std::endl;
}
}

void NinjaBuildWriter::WritePhonyRule(const Target* target,
const OutputFile& target_file,
const std::string& phony_name) {
if (target_file.value() == phony_name)
return; // No need for a phony rule.

EscapeOptions ninja_escape;
ninja_escape.mode = ESCAPE_NINJA;

// Escape for special chars Ninja will handle.
std::string escaped = EscapeString(phony_name, ninja_escape, NULL);

out_ << "build " << escaped << ": phony ";
path_output_.WriteFile(out_, target_file);
out_ << std::endl;
}
3 changes: 3 additions & 0 deletions tools/gn/ninja_build_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class NinjaBuildWriter {
void WriteSubninjas();
void WritePhonyAndAllRules();

void WritePhonyRule(const Target* target, const OutputFile& target_file,
const std::string& phony_name);

const BuildSettings* build_settings_;
std::vector<const Settings*> all_settings_;
std::vector<const Target*> default_toolchain_targets_;
Expand Down

0 comments on commit a3372c7

Please sign in to comment.