diff options
Diffstat (limited to 'src')
56 files changed, 1280 insertions, 728 deletions
diff --git a/src/browse.cc b/src/browse.cc index 83bfe43..14900f8 100644 --- a/src/browse.cc +++ b/src/browse.cc @@ -17,11 +17,12 @@ #include <stdio.h> #include <stdlib.h> #include <unistd.h> +#include <vector> -#include "../build/browse_py.h" +#include "build/browse_py.h" void RunBrowsePython(State* state, const char* ninja_command, - const char* initial_target) { + const char* input_file, int argc, char* argv[]) { // Fork off a Python process and have it run our code via its stdin. // (Actually the Python process becomes the parent.) int pipefd[2]; @@ -44,11 +45,18 @@ void RunBrowsePython(State* state, const char* ninja_command, break; } - // exec Python, telling it to run the program from stdin. - const char* command[] = { - NINJA_PYTHON, "-", ninja_command, initial_target, NULL - }; - execvp(command[0], (char**)command); + std::vector<const char *> command; + command.push_back(NINJA_PYTHON); + command.push_back("-"); + command.push_back("--ninja-command"); + command.push_back(ninja_command); + command.push_back("-f"); + command.push_back(input_file); + for (int i = 0; i < argc; i++) { + command.push_back(argv[i]); + } + command.push_back(NULL); + execvp(command[0], (char**)&command[0]); perror("ninja: execvp"); } while (false); _exit(1); diff --git a/src/browse.h b/src/browse.h index 263641f..8d6d285 100644 --- a/src/browse.h +++ b/src/browse.h @@ -19,9 +19,10 @@ struct State; /// Run in "browse" mode, which execs a Python webserver. /// \a ninja_command is the command used to invoke ninja. -/// \a initial_target is the first target to load. +/// \a args are the number of arguments to be passed to the Python script. +/// \a argv are arguments to be passed to the Python script. /// This function does not return if it runs successfully. void RunBrowsePython(State* state, const char* ninja_command, - const char* initial_target); + const char* input_file, int argc, char* argv[]); #endif // NINJA_BROWSE_H_ diff --git a/src/browse.py b/src/browse.py index 9e59bd8..32792f3 100755 --- a/src/browse.py +++ b/src/browse.py @@ -26,12 +26,16 @@ try: import http.server as httpserver except ImportError: import BaseHTTPServer as httpserver +import argparse import os import socket import subprocess import sys import webbrowser -import urllib2 +try: + from urllib.request import unquote +except ImportError: + from urllib2 import unquote from collections import namedtuple Node = namedtuple('Node', ['inputs', 'rule', 'target', 'outputs']) @@ -146,19 +150,19 @@ def generate_html(node): return '\n'.join(document) def ninja_dump(target): - proc = subprocess.Popen([sys.argv[1], '-t', 'query', target], - stdout=subprocess.PIPE, stderr=subprocess.PIPE, + cmd = [args.ninja_command, '-f', args.f, '-t', 'query', target] + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) return proc.communicate() + (proc.returncode,) class RequestHandler(httpserver.BaseHTTPRequestHandler): def do_GET(self): assert self.path[0] == '/' - target = urllib2.unquote(self.path[1:]) + target = unquote(self.path[1:]) if target == '': self.send_response(302) - self.send_header('Location', '?' + sys.argv[2]) + self.send_header('Location', '?' + args.initial_target) self.end_headers() return @@ -182,13 +186,28 @@ class RequestHandler(httpserver.BaseHTTPRequestHandler): def log_message(self, format, *args): pass # Swallow console spam. -port = 8000 +parser = argparse.ArgumentParser(prog='ninja -t browse') +parser.add_argument('--port', '-p', default=8000, type=int, + help='Port number to use (default %(default)d)') +parser.add_argument('--no-browser', action='store_true', + help='Do not open a webbrowser on startup.') + +parser.add_argument('--ninja-command', default='ninja', + help='Path to ninja binary (default %(default)s)') +parser.add_argument('-f', default='build.ninja', + help='Path to build.ninja file (default %(default)s)') +parser.add_argument('initial_target', default='all', nargs='?', + help='Initial target to show (default %(default)s)') + +args = parser.parse_args() +port = args.port httpd = httpserver.HTTPServer(('',port), RequestHandler) try: hostname = socket.gethostname() print('Web server running on %s:%d, ctl-C to abort...' % (hostname,port) ) print('Web server pid %d' % os.getpid(), file=sys.stderr ) - webbrowser.open_new('http://%s:%s' % (hostname, port) ) + if not args.no_browser: + webbrowser.open_new('http://%s:%s' % (hostname, port) ) httpd.serve_forever() except KeyboardInterrupt: print() diff --git a/src/build.cc b/src/build.cc index cdb8e0a..3a17bdb 100644 --- a/src/build.cc +++ b/src/build.cc @@ -25,12 +25,12 @@ #endif #include "build_log.h" +#include "clparser.h" #include "debug_flags.h" #include "depfile_parser.h" #include "deps_log.h" #include "disk_interface.h" #include "graph.h" -#include "msvc_helper.h" #include "state.h" #include "subprocess.h" #include "util.h" @@ -96,7 +96,8 @@ void BuildStatus::BuildEdgeStarted(Edge* edge) { running_edges_.insert(make_pair(edge, start_time)); ++started_edges_; - PrintStatus(edge); + if (edge->use_console() || printer_.is_smart_terminal()) + PrintStatus(edge); if (edge->use_console()) printer_.SetConsoleLocked(true); @@ -121,12 +122,19 @@ void BuildStatus::BuildEdgeFinished(Edge* edge, if (config_.verbosity == BuildConfig::QUIET) return; - if (!edge->use_console() && printer_.is_smart_terminal()) + if (!edge->use_console()) PrintStatus(edge); // Print the command that is spewing before printing its output. - if (!success) - printer_.PrintOnNewLine("FAILED: " + edge->EvaluateCommand() + "\n"); + if (!success) { + string outputs; + for (vector<Node*>::const_iterator o = edge->outputs_.begin(); + o != edge->outputs_.end(); ++o) + outputs += (*o)->path() + " "; + + printer_.PrintOnNewLine("FAILED: " + outputs + "\n"); + printer_.PrintOnNewLine(edge->EvaluateCommand() + "\n"); + } if (!output.empty()) { // ninja sets stdout and stderr of subprocesses to a pipe, to be able to @@ -358,36 +366,44 @@ Edge* Plan::FindWork() { } void Plan::ScheduleWork(Edge* edge) { + set<Edge*>::iterator e = ready_.lower_bound(edge); + if (e != ready_.end() && !ready_.key_comp()(edge, *e)) { + // This edge has already been scheduled. We can get here again if an edge + // and one of its dependencies share an order-only input, or if a node + // duplicates an out edge (see https://github.com/ninja-build/ninja/pull/519). + // Avoid scheduling the work again. + return; + } + Pool* pool = edge->pool(); if (pool->ShouldDelayEdge()) { - // The graph is not completely clean. Some Nodes have duplicate Out edges. - // We need to explicitly ignore these here, otherwise their work will get - // scheduled twice (see https://github.com/martine/ninja/pull/519) - if (ready_.count(edge)) { - return; - } pool->DelayEdge(edge); pool->RetrieveReadyEdges(&ready_); } else { pool->EdgeScheduled(*edge); - ready_.insert(edge); + ready_.insert(e, edge); } } -void Plan::EdgeFinished(Edge* edge) { +void Plan::EdgeFinished(Edge* edge, EdgeResult result) { map<Edge*, bool>::iterator e = want_.find(edge); assert(e != want_.end()); bool directly_wanted = e->second; - if (directly_wanted) - --wanted_edges_; - want_.erase(e); - edge->outputs_ready_ = true; // See if this job frees up any delayed jobs. if (directly_wanted) edge->pool()->EdgeFinished(*edge); edge->pool()->RetrieveReadyEdges(&ready_); + // The rest of this function only applies to successful commands. + if (result != kEdgeSucceeded) + return; + + if (directly_wanted) + --wanted_edges_; + want_.erase(e); + edge->outputs_ready_ = true; + // Check off any nodes we were waiting for with this edge. for (vector<Node*>::iterator o = edge->outputs_.begin(); o != edge->outputs_.end(); ++o) { @@ -410,7 +426,7 @@ void Plan::NodeFinished(Node* node) { } else { // We do not need to build this edge, but we might need to build one of // its dependents. - EdgeFinished(*oe); + EdgeFinished(*oe, kEdgeSucceeded); } } } @@ -643,7 +659,7 @@ bool Builder::Build(string* err) { } if (edge->is_phony()) { - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); } else { ++pending_commands; } @@ -762,8 +778,10 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { &start_time, &end_time); // The rest of this function only applies to successful commands. - if (!result->success()) + if (!result->success()) { + plan_.EdgeFinished(edge, Plan::kEdgeFailed); return true; + } // Restat the edge outputs, if necessary. TimeStamp restat_mtime = 0; @@ -812,7 +830,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { } } - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); // Delete any left over response file. string rspfile = edge->GetUnescapedRspfile(); @@ -846,10 +864,12 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, const string& deps_prefix, vector<Node*>* deps_nodes, string* err) { -#ifdef _WIN32 if (deps_type == "msvc") { CLParser parser; - result->output = parser.Parse(result->output, deps_prefix); + string output; + if (!parser.Parse(result->output, deps_prefix, &output, err)) + return false; + result->output = output; for (set<string>::iterator i = parser.includes_.begin(); i != parser.includes_.end(); ++i) { // ~0 is assuming that with MSVC-parsed headers, it's ok to always make @@ -859,7 +879,6 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, deps_nodes->push_back(state_->GetNode(*i, ~0u)); } } else -#endif if (deps_type == "gcc") { string depfile = result->edge->GetUnescapedDepfile(); if (depfile.empty()) { @@ -867,9 +886,17 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, return false; } - string content = disk_interface_->ReadFile(depfile, err); - if (!err->empty()) + // Read depfile content. Treat a missing depfile as empty. + string content; + switch (disk_interface_->ReadFile(depfile, &content, err)) { + case DiskInterface::Okay: + break; + case DiskInterface::NotFound: + err->clear(); + break; + case DiskInterface::OtherError: return false; + } if (content.empty()) return true; @@ -888,9 +915,11 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, deps_nodes->push_back(state_->GetNode(*i, slash_bits)); } - if (disk_interface_->RemoveFile(depfile) < 0) { - *err = string("deleting depfile: ") + strerror(errno) + string("\n"); - return false; + if (!g_keep_depfile) { + if (disk_interface_->RemoveFile(depfile) < 0) { + *err = string("deleting depfile: ") + strerror(errno) + string("\n"); + return false; + } } } else { Fatal("unknown deps type '%s'", deps_type.c_str()); diff --git a/src/build.h b/src/build.h index 8106faa..51589ef 100644 --- a/src/build.h +++ b/src/build.h @@ -56,9 +56,13 @@ struct Plan { /// Dumps the current state of the plan. void Dump(); - /// Mark an edge as done building. Used internally and by - /// tests. - void EdgeFinished(Edge* edge); + enum EdgeResult { + kEdgeFailed, + kEdgeSucceeded + }; + + /// Mark an edge as done building (whether it succeeded or failed). + void EdgeFinished(Edge* edge, EdgeResult result); /// Clean the given node during the build. /// Return false on error. diff --git a/src/build_log.cc b/src/build_log.cc index 281b851..8a52514 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -12,6 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +// On AIX, inttypes.h gets indirectly included by build_log.h. +// It's easiest just to ask for the printf format macros right away. +#ifndef _WIN32 +#ifndef __STDC_FORMAT_MACROS +#define __STDC_FORMAT_MACROS +#endif +#endif + #include "build_log.h" #include <errno.h> @@ -19,7 +27,6 @@ #include <string.h> #ifndef _WIN32 -#define __STDC_FORMAT_MACROS #include <inttypes.h> #include <unistd.h> #endif diff --git a/src/build_log.h b/src/build_log.h index fe81a85..785961e 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -27,7 +27,7 @@ struct Edge; /// Can answer questions about the manifest for the BuildLog. struct BuildLogUser { - /// Return if a given output no longer part of the build manifest. + /// Return if a given output is no longer part of the build manifest. /// This is only called during recompaction and doesn't have to be fast. virtual bool IsPathDead(StringPiece s) const = 0; }; diff --git a/src/build_log_perftest.cc b/src/build_log_perftest.cc index 810c065..185c512 100644 --- a/src/build_log_perftest.cc +++ b/src/build_log_perftest.cc @@ -71,7 +71,7 @@ bool WriteTestData(string* err) { long_rule_command += "$in -o $out\n"; State state; - ManifestParser parser(&state, NULL); + ManifestParser parser(&state, NULL, kDupeEdgeActionWarn); if (!parser.ParseTest("rule cxx\n command = " + long_rule_command, err)) return false; diff --git a/src/build_log_test.cc b/src/build_log_test.cc index 2c41ba6..f4c9044 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -121,13 +121,15 @@ TEST_F(BuildLogTest, Truncate) { "build out: cat mid\n" "build mid: cat in\n"); - BuildLog log1; - string err; - EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); - ASSERT_EQ("", err); - log1.RecordCommand(state_.edges_[0], 15, 18); - log1.RecordCommand(state_.edges_[1], 20, 25); - log1.Close(); + { + BuildLog log1; + string err; + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); + ASSERT_EQ("", err); + log1.RecordCommand(state_.edges_[0], 15, 18); + log1.RecordCommand(state_.edges_[1], 20, 25); + log1.Close(); + } struct stat statbuf; ASSERT_EQ(0, stat(kTestFilename, &statbuf)); diff --git a/src/build_test.cc b/src/build_test.cc index 52a17c9..55d093e 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -68,14 +68,14 @@ TEST_F(PlanTest, Basic) { ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); ASSERT_EQ("mid", edge->inputs_[0]->path()); ASSERT_EQ("out", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); ASSERT_FALSE(plan_.more_to_do()); edge = plan_.FindWork(); @@ -99,11 +99,11 @@ TEST_F(PlanTest, DoubleOutputDirect) { Edge* edge; edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat in - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat mid1 mid2 - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_FALSE(edge); // done @@ -129,19 +129,19 @@ TEST_F(PlanTest, DoubleOutputIndirect) { Edge* edge; edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat in - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat a1 - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat a2 - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat b1 b2 - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_FALSE(edge); // done @@ -167,19 +167,19 @@ TEST_F(PlanTest, DoubleDependent) { Edge* edge; edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat in - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat mid - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat mid - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); // cat a1 a2 - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_FALSE(edge); // done @@ -257,7 +257,7 @@ void PlanTest::TestPoolWithDepthOne(const char* test_case) { // This will be false since poolcat is serialized ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -266,7 +266,7 @@ void PlanTest::TestPoolWithDepthOne(const char* test_case) { ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); ASSERT_FALSE(plan_.more_to_do()); edge = plan_.FindWork(); @@ -342,7 +342,7 @@ TEST_F(PlanTest, PoolsWithDepthTwo) { ASSERT_EQ("outb3", edge->outputs_[0]->path()); // finish out1 - plan_.EdgeFinished(edges.front()); + plan_.EdgeFinished(edges.front(), Plan::kEdgeSucceeded); edges.pop_front(); // out3 should be available @@ -353,19 +353,19 @@ TEST_F(PlanTest, PoolsWithDepthTwo) { ASSERT_FALSE(plan_.FindWork()); - plan_.EdgeFinished(out3); + plan_.EdgeFinished(out3, Plan::kEdgeSucceeded); ASSERT_FALSE(plan_.FindWork()); for (deque<Edge*>::iterator it = edges.begin(); it != edges.end(); ++it) { - plan_.EdgeFinished(*it); + plan_.EdgeFinished(*it, Plan::kEdgeSucceeded); } Edge* last = plan_.FindWork(); ASSERT_TRUE(last); ASSERT_EQ("allTheThings", last->outputs_[0]->path()); - plan_.EdgeFinished(last); + plan_.EdgeFinished(last, Plan::kEdgeSucceeded); ASSERT_FALSE(plan_.more_to_do()); ASSERT_FALSE(plan_.FindWork()); @@ -407,7 +407,7 @@ TEST_F(PlanTest, PoolWithRedundantEdges) { edge = initial_edges[1]; // Foo first ASSERT_EQ("foo.cpp", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -415,11 +415,11 @@ TEST_F(PlanTest, PoolWithRedundantEdges) { ASSERT_EQ("foo.cpp", edge->inputs_[0]->path()); ASSERT_EQ("foo.cpp", edge->inputs_[1]->path()); ASSERT_EQ("foo.cpp.obj", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = initial_edges[0]; // Now for bar ASSERT_EQ("bar.cpp", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -427,7 +427,7 @@ TEST_F(PlanTest, PoolWithRedundantEdges) { ASSERT_EQ("bar.cpp", edge->inputs_[0]->path()); ASSERT_EQ("bar.cpp", edge->inputs_[1]->path()); ASSERT_EQ("bar.cpp.obj", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); @@ -435,20 +435,62 @@ TEST_F(PlanTest, PoolWithRedundantEdges) { ASSERT_EQ("foo.cpp.obj", edge->inputs_[0]->path()); ASSERT_EQ("bar.cpp.obj", edge->inputs_[1]->path()); ASSERT_EQ("libfoo.a", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_TRUE(edge); ASSERT_FALSE(plan_.FindWork()); ASSERT_EQ("libfoo.a", edge->inputs_[0]->path()); ASSERT_EQ("all", edge->outputs_[0]->path()); - plan_.EdgeFinished(edge); + plan_.EdgeFinished(edge, Plan::kEdgeSucceeded); edge = plan_.FindWork(); ASSERT_FALSE(edge); ASSERT_FALSE(plan_.more_to_do()); } +TEST_F(PlanTest, PoolWithFailingEdge) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, + "pool foobar\n" + " depth = 1\n" + "rule poolcat\n" + " command = cat $in > $out\n" + " pool = foobar\n" + "build out1: poolcat in\n" + "build out2: poolcat in\n")); + GetNode("out1")->MarkDirty(); + GetNode("out2")->MarkDirty(); + string err; + EXPECT_TRUE(plan_.AddTarget(GetNode("out1"), &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(plan_.AddTarget(GetNode("out2"), &err)); + ASSERT_EQ("", err); + ASSERT_TRUE(plan_.more_to_do()); + + Edge* edge = plan_.FindWork(); + ASSERT_TRUE(edge); + ASSERT_EQ("in", edge->inputs_[0]->path()); + ASSERT_EQ("out1", edge->outputs_[0]->path()); + + // This will be false since poolcat is serialized + ASSERT_FALSE(plan_.FindWork()); + + plan_.EdgeFinished(edge, Plan::kEdgeFailed); + + edge = plan_.FindWork(); + ASSERT_TRUE(edge); + ASSERT_EQ("in", edge->inputs_[0]->path()); + ASSERT_EQ("out2", edge->outputs_[0]->path()); + + ASSERT_FALSE(plan_.FindWork()); + + plan_.EdgeFinished(edge, Plan::kEdgeFailed); + + ASSERT_TRUE(plan_.more_to_do()); // Jobs have failed + edge = plan_.FindWork(); + ASSERT_EQ(0, edge); +} + /// Fake implementation of CommandRunner, useful for tests. struct FakeCommandRunner : public CommandRunner { explicit FakeCommandRunner(VirtualFileSystem* fs) : @@ -717,8 +759,24 @@ TEST_F(BuildTest, TwoOutputs) { EXPECT_EQ("touch out1 out2", command_runner_.commands_ran_[0]); } +TEST_F(BuildTest, ImplicitOutput) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule touch\n" +" command = touch $out $out.imp\n" +"build out | out.imp: touch in.txt\n")); + fs_.Create("in.txt", ""); + + string err; + EXPECT_TRUE(builder_.AddTarget("out.imp", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_EQ("", err); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); + EXPECT_EQ("touch out out.imp", command_runner_.commands_ran_[0]); +} + // Test case from -// https://github.com/martine/ninja/issues/148 +// https://github.com/ninja-build/ninja/issues/148 TEST_F(BuildTest, MultiOutIn) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule touch\n" @@ -859,6 +917,29 @@ TEST_F(BuildTest, DepFileParseError) { EXPECT_EQ("foo.o.d: expected ':' in depfile", err); } +TEST_F(BuildTest, EncounterReadyTwice) { + string err; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule touch\n" +" command = touch $out\n" +"build c: touch\n" +"build b: touch || c\n" +"build a: touch | b || c\n")); + + vector<Edge*> c_out = GetNode("c")->out_edges(); + ASSERT_EQ(2u, c_out.size()); + EXPECT_EQ("b", c_out[0]->outputs_[0]->path()); + EXPECT_EQ("a", c_out[1]->outputs_[0]->path()); + + fs_.Create("b", ""); + EXPECT_TRUE(builder_.AddTarget("a", &err)); + ASSERT_EQ("", err); + + EXPECT_TRUE(builder_.Build(&err)); + ASSERT_EQ("", err); + ASSERT_EQ(2u, command_runner_.commands_ran_.size()); +} + TEST_F(BuildTest, OrderOnlyDeps) { string err; ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, @@ -1095,6 +1176,30 @@ TEST_F(BuildTest, SwallowFailuresLimit) { ASSERT_EQ("cannot make progress due to previous errors", err); } +TEST_F(BuildTest, SwallowFailuresPool) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"pool failpool\n" +" depth = 1\n" +"rule fail\n" +" command = fail\n" +" pool = failpool\n" +"build out1: fail\n" +"build out2: fail\n" +"build out3: fail\n" +"build final: cat out1 out2 out3\n")); + + // Swallow ten failures; we should stop before building final. + config_.failures_allowed = 11; + + string err; + EXPECT_TRUE(builder_.AddTarget("final", &err)); + ASSERT_EQ("", err); + + EXPECT_FALSE(builder_.Build(&err)); + ASSERT_EQ(3u, command_runner_.commands_ran_.size()); + ASSERT_EQ("cannot make progress due to previous errors", err); +} + TEST_F(BuildTest, PoolEdgesReadyButNotWanted) { fs_.Create("x", ""); @@ -1299,7 +1404,7 @@ TEST_F(BuildWithLogTest, RestatSingleDependentOutputDirty) { } // Test scenario, in which an input file is removed, but output isn't changed -// https://github.com/martine/ninja/issues/295 +// https://github.com/ninja-build/ninja/issues/295 TEST_F(BuildWithLogTest, RestatMissingInput) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule true\n" @@ -2047,7 +2152,7 @@ TEST_F(BuildWithDepsLogTest, DepFileDepsLogCanonicalize) { #endif /// Check that a restat rule doesn't clear an edge if the depfile is missing. -/// Follows from: https://github.com/martine/ninja/issues/603 +/// Follows from: https://github.com/ninja-build/ninja/issues/603 TEST_F(BuildTest, RestatMissingDepfile) { const char* manifest = "rule true\n" @@ -2071,7 +2176,7 @@ const char* manifest = } /// Check that a restat rule doesn't clear an edge if the deps are missing. -/// https://github.com/martine/ninja/issues/603 +/// https://github.com/ninja-build/ninja/issues/603 TEST_F(BuildWithDepsLogTest, RestatMissingDepfileDepslog) { string err; const char* manifest = diff --git a/src/clparser.cc b/src/clparser.cc new file mode 100644 index 0000000..f73a8c1 --- /dev/null +++ b/src/clparser.cc @@ -0,0 +1,116 @@ +// Copyright 2015 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "clparser.h" + +#include <algorithm> +#include <assert.h> +#include <string.h> + +#ifdef _WIN32 +#include "includes_normalize.h" +#else +#include "util.h" +#endif + +namespace { + +/// Return true if \a input ends with \a needle. +bool EndsWith(const string& input, const string& needle) { + return (input.size() >= needle.size() && + input.substr(input.size() - needle.size()) == needle); +} + +} // anonymous namespace + +// static +string CLParser::FilterShowIncludes(const string& line, + const string& deps_prefix) { + const string kDepsPrefixEnglish = "Note: including file: "; + const char* in = line.c_str(); + const char* end = in + line.size(); + const string& prefix = deps_prefix.empty() ? kDepsPrefixEnglish : deps_prefix; + if (end - in > (int)prefix.size() && + memcmp(in, prefix.c_str(), (int)prefix.size()) == 0) { + in += prefix.size(); + while (*in == ' ') + ++in; + return line.substr(in - line.c_str()); + } + return ""; +} + +// static +bool CLParser::IsSystemInclude(string path) { + transform(path.begin(), path.end(), path.begin(), ::tolower); + // TODO: this is a heuristic, perhaps there's a better way? + return (path.find("program files") != string::npos || + path.find("microsoft visual studio") != string::npos); +} + +// static +bool CLParser::FilterInputFilename(string line) { + transform(line.begin(), line.end(), line.begin(), ::tolower); + // TODO: other extensions, like .asm? + return EndsWith(line, ".c") || + EndsWith(line, ".cc") || + EndsWith(line, ".cxx") || + EndsWith(line, ".cpp"); +} + +// static +bool CLParser::Parse(const string& output, const string& deps_prefix, + string* filtered_output, string* err) { + // Loop over all lines in the output to process them. + assert(&output != filtered_output); + size_t start = 0; + while (start < output.size()) { + size_t end = output.find_first_of("\r\n", start); + if (end == string::npos) + end = output.size(); + string line = output.substr(start, end - start); + + string include = FilterShowIncludes(line, deps_prefix); + if (!include.empty()) { + string normalized; +#ifdef _WIN32 + if (!IncludesNormalize::Normalize(include, NULL, &normalized, err)) + return false; +#else + // TODO: should this make the path relative to cwd? + normalized = include; + unsigned int slash_bits; + if (!CanonicalizePath(&normalized, &slash_bits, err)) + return false; +#endif + if (!IsSystemInclude(normalized)) + includes_.insert(normalized); + } else if (FilterInputFilename(line)) { + // Drop it. + // TODO: if we support compiling multiple output files in a single + // cl.exe invocation, we should stash the filename. + } else { + filtered_output->append(line); + filtered_output->append("\n"); + } + + if (end < output.size() && output[end] == '\r') + ++end; + if (end < output.size() && output[end] == '\n') + ++end; + start = end; + } + + return true; +} diff --git a/src/clparser.h b/src/clparser.h new file mode 100644 index 0000000..e597e7e --- /dev/null +++ b/src/clparser.h @@ -0,0 +1,52 @@ +// Copyright 2015 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef NINJA_CLPARSER_H_ +#define NINJA_CLPARSER_H_ + +#include <set> +#include <string> +using namespace std; + +/// Visual Studio's cl.exe requires some massaging to work with Ninja; +/// for example, it emits include information on stderr in a funny +/// format when building with /showIncludes. This class parses this +/// output. +struct CLParser { + /// Parse a line of cl.exe output and extract /showIncludes info. + /// If a dependency is extracted, returns a nonempty string. + /// Exposed for testing. + static string FilterShowIncludes(const string& line, + const string& deps_prefix); + + /// Return true if a mentioned include file is a system path. + /// Filtering these out reduces dependency information considerably. + static bool IsSystemInclude(string path); + + /// Parse a line of cl.exe output and return true if it looks like + /// it's printing an input filename. This is a heuristic but it appears + /// to be the best we can do. + /// Exposed for testing. + static bool FilterInputFilename(string line); + + /// Parse the full output of cl, filling filtered_output with the text that + /// should be printed (if any). Returns true on success, or false with err + /// filled. output must not be the same object as filtered_object. + bool Parse(const string& output, const string& deps_prefix, + string* filtered_output, string* err); + + set<string> includes_; +}; + +#endif // NINJA_CLPARSER_H_ diff --git a/src/clparser_test.cc b/src/clparser_test.cc new file mode 100644 index 0000000..1549ab1 --- /dev/null +++ b/src/clparser_test.cc @@ -0,0 +1,117 @@ +// Copyright 2011 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "clparser.h" + +#include "test.h" +#include "util.h" + +TEST(CLParserTest, ShowIncludes) { + ASSERT_EQ("", CLParser::FilterShowIncludes("", "")); + + ASSERT_EQ("", CLParser::FilterShowIncludes("Sample compiler output", "")); + ASSERT_EQ("c:\\Some Files\\foobar.h", + CLParser::FilterShowIncludes("Note: including file: " + "c:\\Some Files\\foobar.h", "")); + ASSERT_EQ("c:\\initspaces.h", + CLParser::FilterShowIncludes("Note: including file: " + "c:\\initspaces.h", "")); + ASSERT_EQ("c:\\initspaces.h", + CLParser::FilterShowIncludes("Non-default prefix: inc file: " + "c:\\initspaces.h", + "Non-default prefix: inc file:")); +} + +TEST(CLParserTest, FilterInputFilename) { + ASSERT_TRUE(CLParser::FilterInputFilename("foobar.cc")); + ASSERT_TRUE(CLParser::FilterInputFilename("foo bar.cc")); + ASSERT_TRUE(CLParser::FilterInputFilename("baz.c")); + ASSERT_TRUE(CLParser::FilterInputFilename("FOOBAR.CC")); + + ASSERT_FALSE(CLParser::FilterInputFilename( + "src\\cl_helper.cc(166) : fatal error C1075: end " + "of file found ...")); +} + +TEST(CLParserTest, ParseSimple) { + CLParser parser; + string output, err; + ASSERT_TRUE(parser.Parse( + "foo\r\n" + "Note: inc file prefix: foo.h\r\n" + "bar\r\n", + "Note: inc file prefix:", &output, &err)); + + ASSERT_EQ("foo\nbar\n", output); + ASSERT_EQ(1u, parser.includes_.size()); + ASSERT_EQ("foo.h", *parser.includes_.begin()); +} + +TEST(CLParserTest, ParseFilenameFilter) { + CLParser parser; + string output, err; + ASSERT_TRUE(parser.Parse( + "foo.cc\r\n" + "cl: warning\r\n", + "", &output, &err)); + ASSERT_EQ("cl: warning\n", output); +} + +TEST(CLParserTest, ParseSystemInclude) { + CLParser parser; + string output, err; + ASSERT_TRUE(parser.Parse( + "Note: including file: c:\\Program Files\\foo.h\r\n" + "Note: including file: d:\\Microsoft Visual Studio\\bar.h\r\n" + "Note: including file: path.h\r\n", + "", &output, &err)); + // We should have dropped the first two includes because they look like + // system headers. + ASSERT_EQ("", output); + ASSERT_EQ(1u, parser.includes_.size()); + ASSERT_EQ("path.h", *parser.includes_.begin()); +} + +TEST(CLParserTest, DuplicatedHeader) { + CLParser parser; + string output, err; + ASSERT_TRUE(parser.Parse( + "Note: including file: foo.h\r\n" + "Note: including file: bar.h\r\n" + "Note: including file: foo.h\r\n", + "", &output, &err)); + // We should have dropped one copy of foo.h. + ASSERT_EQ("", output); + ASSERT_EQ(2u, parser.includes_.size()); +} + +TEST(CLParserTest, DuplicatedHeaderPathConverted) { + CLParser parser; + string output, err; + + // This isn't inline in the Parse() call below because the #ifdef in + // a macro expansion would confuse MSVC2013's preprocessor. + const char kInput[] = + "Note: including file: sub/./foo.h\r\n" + "Note: including file: bar.h\r\n" +#ifdef _WIN32 + "Note: including file: sub\\foo.h\r\n"; +#else + "Note: including file: sub/foo.h\r\n"; +#endif + ASSERT_TRUE(parser.Parse(kInput, "", &output, &err)); + // We should have dropped one copy of foo.h. + ASSERT_EQ("", output); + ASSERT_EQ(2u, parser.includes_.size()); +} diff --git a/src/debug_flags.cc b/src/debug_flags.cc index 8065001..44b14c4 100644 --- a/src/debug_flags.cc +++ b/src/debug_flags.cc @@ -14,6 +14,8 @@ bool g_explaining = false; +bool g_keep_depfile = false; + bool g_keep_rsp = false; bool g_experimental_statcache = true; diff --git a/src/debug_flags.h b/src/debug_flags.h index 7965585..e08a43b 100644 --- a/src/debug_flags.h +++ b/src/debug_flags.h @@ -24,6 +24,8 @@ extern bool g_explaining; +extern bool g_keep_depfile; + extern bool g_keep_rsp; extern bool g_experimental_statcache; diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc index 7268f31..7cee892 100644 --- a/src/depfile_parser.cc +++ b/src/depfile_parser.cc @@ -47,7 +47,7 @@ bool DepfileParser::Parse(string* content, string* err) { const char* start = in; { - char yych; + unsigned char yych; static const unsigned char yybm[] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -65,22 +65,22 @@ bool DepfileParser::Parse(string* content, string* err) { 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 0, 128, 128, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, + 128, 128, 128, 128, 128, 128, 128, 128, }; yych = *in; @@ -106,24 +106,28 @@ bool DepfileParser::Parse(string* content, string* err) { } } } else { - if (yych <= '^') { - if (yych <= 'Z') { + if (yych <= '_') { + if (yych <= '[') { if (yych <= '?') goto yy9; - goto yy5; + if (yych <= 'Z') goto yy5; + goto yy9; } else { - if (yych != '\\') goto yy9; + if (yych <= '\\') goto yy2; + if (yych <= '^') goto yy9; + goto yy5; } } else { - if (yych <= '{') { - if (yych == '`') goto yy9; - goto yy5; - } else { - if (yych <= '|') goto yy9; - if (yych <= '~') goto yy5; + if (yych <= '|') { + if (yych <= '`') goto yy9; + if (yych <= '{') goto yy5; goto yy9; + } else { + if (yych == 0x7F) goto yy9; + goto yy5; } } } +yy2: ++in; if ((yych = *in) <= '"') { if (yych <= '\f') { diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc index deaee5b..98c1621 100644 --- a/src/depfile_parser.in.cc +++ b/src/depfile_parser.in.cc @@ -45,7 +45,7 @@ bool DepfileParser::Parse(string* content, string* err) { // start: beginning of the current parsed span. const char* start = in; /*!re2c - re2c:define:YYCTYPE = "char"; + re2c:define:YYCTYPE = "unsigned char"; re2c:define:YYCURSOR = in; re2c:define:YYLIMIT = end; @@ -73,7 +73,7 @@ bool DepfileParser::Parse(string* content, string* err) { *out++ = yych; continue; } - [a-zA-Z0-9+,/_:.~()}{@=!-]+ { + [a-zA-Z0-9+,/_:.~()}{@=!\x80-\xFF-]+ { // Got a span of plain text. int len = (int)(in - start); // Need to shift it over if we're overwriting backslashes. diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc index 8b57a1e..ee798f8 100644 --- a/src/depfile_parser_test.cc +++ b/src/depfile_parser_test.cc @@ -121,26 +121,29 @@ TEST_F(DepfileParserTest, SpecialChars) { EXPECT_TRUE(Parse( "C:/Program\\ Files\\ (x86)/Microsoft\\ crtdefs.h: \n" " en@quot.header~ t+t-x!=1 \n" -" openldap/slapd.d/cn=config/cn=schema/cn={0}core.ldif", +" openldap/slapd.d/cn=config/cn=schema/cn={0}core.ldif\n" +" Fu\303\244ball", &err)); ASSERT_EQ("", err); EXPECT_EQ("C:/Program Files (x86)/Microsoft crtdefs.h", parser_.out_.AsString()); - ASSERT_EQ(3u, parser_.ins_.size()); + ASSERT_EQ(4u, parser_.ins_.size()); EXPECT_EQ("en@quot.header~", parser_.ins_[0].AsString()); EXPECT_EQ("t+t-x!=1", parser_.ins_[1].AsString()); EXPECT_EQ("openldap/slapd.d/cn=config/cn=schema/cn={0}core.ldif", parser_.ins_[2].AsString()); + EXPECT_EQ("Fu\303\244ball", + parser_.ins_[3].AsString()); } TEST_F(DepfileParserTest, UnifyMultipleOutputs) { // check that multiple duplicate targets are properly unified string err; EXPECT_TRUE(Parse("foo foo: x y z", &err)); - ASSERT_EQ(parser_.out_.AsString(), "foo"); - ASSERT_EQ(parser_.ins_.size(), 3u); + ASSERT_EQ("foo", parser_.out_.AsString()); + ASSERT_EQ(3u, parser_.ins_.size()); EXPECT_EQ("x", parser_.ins_[0].AsString()); EXPECT_EQ("y", parser_.ins_[1].AsString()); EXPECT_EQ("z", parser_.ins_[2].AsString()); diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index cab06fb..89f7be1 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -431,10 +431,12 @@ TEST_F(DepsLogTest, TruncatedRecovery) { } // Shorten the file, corrupting the last record. - struct stat st; - ASSERT_EQ(0, stat(kTestFilename, &st)); - string err; - ASSERT_TRUE(Truncate(kTestFilename, st.st_size - 2, &err)); + { + struct stat st; + ASSERT_EQ(0, stat(kTestFilename, &st)); + string err; + ASSERT_TRUE(Truncate(kTestFilename, st.st_size - 2, &err)); + } // Load the file again, add an entry. { diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 70282c0..451a9b4 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -229,14 +229,14 @@ bool RealDiskInterface::MakeDir(const string& path) { return true; } -string RealDiskInterface::ReadFile(const string& path, string* err) { - string contents; - int ret = ::ReadFile(path, &contents, err); - if (ret == -ENOENT) { - // Swallow ENOENT. - err->clear(); +FileReader::Status RealDiskInterface::ReadFile(const string& path, + string* contents, + string* err) { + switch (::ReadFile(path, contents, err)) { + case 0: return Okay; + case -ENOENT: return NotFound; + default: return OtherError; } - return contents; } int RealDiskInterface::RemoveFile(const string& path) { diff --git a/src/disk_interface.h b/src/disk_interface.h index b61d192..145e089 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -21,13 +21,29 @@ using namespace std; #include "timestamp.h" +/// Interface for reading files from disk. See DiskInterface for details. +/// This base offers the minimum interface needed just to read files. +struct FileReader { + virtual ~FileReader() {} + + /// Result of ReadFile. + enum Status { + Okay, + NotFound, + OtherError + }; + + /// Read and store in given string. On success, return Okay. + /// On error, return another Status and fill |err|. + virtual Status ReadFile(const string& path, string* contents, + string* err) = 0; +}; + /// Interface for accessing the disk. /// /// Abstract so it can be mocked out for tests. The real implementation /// is RealDiskInterface. -struct DiskInterface { - virtual ~DiskInterface() {} - +struct DiskInterface: public FileReader { /// stat() a file, returning the mtime, or 0 if missing and -1 on /// other errors. virtual TimeStamp Stat(const string& path, string* err) const = 0; @@ -39,9 +55,6 @@ struct DiskInterface { /// Returns true on success, false on failure virtual bool WriteFile(const string& path, const string& contents) = 0; - /// Read a file to a string. Fill in |err| on error. - virtual string ReadFile(const string& path, string* err) = 0; - /// Remove the file named @a path. It behaves like 'rm -f path' so no errors /// are reported if it does not exists. /// @returns 0 if the file has been removed, @@ -65,7 +78,7 @@ struct RealDiskInterface : public DiskInterface { virtual TimeStamp Stat(const string& path, string* err) const; virtual bool MakeDir(const string& path); virtual bool WriteFile(const string& path, const string& contents); - virtual string ReadFile(const string& path, string* err); + virtual Status ReadFile(const string& path, string* contents, string* err); virtual int RemoveFile(const string& path); /// Whether stat information can be cached. Only has an effect on Windows. diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index 9d210b4..7187bdf 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -157,8 +157,12 @@ TEST_F(DiskInterfaceTest, StatCache) { TEST_F(DiskInterfaceTest, ReadFile) { string err; - EXPECT_EQ("", disk_.ReadFile("foobar", &err)); - EXPECT_EQ("", err); + std::string content; + ASSERT_EQ(DiskInterface::NotFound, + disk_.ReadFile("foobar", &content, &err)); + EXPECT_EQ("", content); + EXPECT_NE("", err); // actual value is platform-specific + err.clear(); const char* kTestFile = "testfile"; FILE* f = fopen(kTestFile, "wb"); @@ -167,7 +171,9 @@ TEST_F(DiskInterfaceTest, ReadFile) { fprintf(f, "%s", kTestContent); ASSERT_EQ(0, fclose(f)); - EXPECT_EQ(kTestContent, disk_.ReadFile(kTestFile, &err)); + ASSERT_EQ(DiskInterface::Okay, + disk_.ReadFile(kTestFile, &content, &err)); + EXPECT_EQ(kTestContent, content); EXPECT_EQ("", err); } @@ -208,9 +214,9 @@ struct StatTest : public StateTestWithBuiltinRules, assert(false); return false; } - virtual string ReadFile(const string& path, string* err) { + virtual Status ReadFile(const string& path, string* contents, string* err) { assert(false); - return ""; + return NotFound; } virtual int RemoveFile(const string& path) { assert(false); diff --git a/src/edit_distance.cc b/src/edit_distance.cc index a6719d3..3bb62b8 100644 --- a/src/edit_distance.cc +++ b/src/edit_distance.cc @@ -28,40 +28,42 @@ int EditDistance(const StringPiece& s1, // http://en.wikipedia.org/wiki/Levenshtein_distance // // Although the algorithm is typically described using an m x n - // array, only two rows are used at a time, so this implementation - // just keeps two separate vectors for those two rows. + // array, only one row plus one element are used at a time, so this + // implementation just keeps one vector for the row. To update one entry, + // only the entries to the left, top, and top-left are needed. The left + // entry is in row[x-1], the top entry is what's in row[x] from the last + // iteration, and the top-left entry is stored in previous. int m = s1.len_; int n = s2.len_; - vector<int> previous(n + 1); - vector<int> current(n + 1); - - for (int i = 0; i <= n; ++i) - previous[i] = i; + vector<int> row(n + 1); + for (int i = 1; i <= n; ++i) + row[i] = i; for (int y = 1; y <= m; ++y) { - current[0] = y; - int best_this_row = current[0]; + row[0] = y; + int best_this_row = row[0]; + int previous = y - 1; for (int x = 1; x <= n; ++x) { + int old_row = row[x]; if (allow_replacements) { - current[x] = min(previous[x-1] + (s1.str_[y-1] == s2.str_[x-1] ? 0 : 1), - min(current[x-1], previous[x])+1); + row[x] = min(previous + (s1.str_[y - 1] == s2.str_[x - 1] ? 0 : 1), + min(row[x - 1], row[x]) + 1); } else { - if (s1.str_[y-1] == s2.str_[x-1]) - current[x] = previous[x-1]; + if (s1.str_[y - 1] == s2.str_[x - 1]) + row[x] = previous; else - current[x] = min(current[x-1], previous[x]) + 1; + row[x] = min(row[x - 1], row[x]) + 1; } - best_this_row = min(best_this_row, current[x]); + previous = old_row; + best_this_row = min(best_this_row, row[x]); } if (max_edit_distance && best_this_row > max_edit_distance) return max_edit_distance + 1; - - current.swap(previous); } - return previous[n]; + return row[n]; } diff --git a/src/eval_env.cc b/src/eval_env.cc index e991d21..8817a87 100644 --- a/src/eval_env.cc +++ b/src/eval_env.cc @@ -55,7 +55,7 @@ void Rule::AddBinding(const string& key, const EvalString& val) { } const EvalString* Rule::GetBinding(const string& key) const { - map<string, EvalString>::const_iterator i = bindings_.find(key); + Bindings::const_iterator i = bindings_.find(key); if (i == bindings_.end()) return NULL; return &i->second; @@ -71,7 +71,8 @@ bool Rule::IsReservedBinding(const string& var) { var == "pool" || var == "restat" || var == "rspfile" || - var == "rspfile_content"; + var == "rspfile_content" || + var == "msvc_deps_prefix"; } const map<string, const Rule*>& BindingEnv::GetRules() const { diff --git a/src/eval_env.h b/src/eval_env.h index 28c4d16..999ce42 100644 --- a/src/eval_env.h +++ b/src/eval_env.h @@ -57,7 +57,6 @@ struct Rule { const string& name() const { return name_; } - typedef map<string, EvalString> Bindings; void AddBinding(const string& key, const EvalString& val); static bool IsReservedBinding(const string& var); @@ -69,7 +68,8 @@ struct Rule { friend struct ManifestParser; string name_; - map<string, EvalString> bindings_; + typedef map<string, EvalString> Bindings; + Bindings bindings_; }; /// An Env which contains a mapping of variables to values diff --git a/src/getopt.c b/src/getopt.c index 3350fb9..0c2ef35 100644 --- a/src/getopt.c +++ b/src/getopt.c @@ -385,11 +385,13 @@ getopt_internal (int argc, char **argv, char *shortopts, return optopt; } +#ifndef _AIX int getopt (int argc, char **argv, char *optstring) { return getopt_internal (argc, argv, optstring, NULL, NULL, 0); } +#endif int getopt_long (int argc, char **argv, const char *shortopts, diff --git a/src/getopt.h b/src/getopt.h index b4247fb..965dc29 100644 --- a/src/getopt.h +++ b/src/getopt.h @@ -39,7 +39,9 @@ extern "C" extern int optopt; /* function prototypes */ +#ifndef _AIX int getopt (int argc, char **argv, char *optstring); +#endif int getopt_long (int argc, char **argv, const char *shortopts, const GETOPT_LONG_OPTION_T * longopts, int *longind); int getopt_long_only (int argc, char **argv, const char *shortopts, diff --git a/src/graph.cc b/src/graph.cc index 355285c..f1d9ca2 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -37,6 +37,7 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { edge->outputs_ready_ = true; edge->deps_missing_ = false; + // Load output mtimes so we can compare them to the most recent input below. // RecomputeDirty() recursively walks the graph following the input nodes // of |edge| and the in_edges of these nodes. It uses the stat state of each // node to mark nodes as visited and doesn't traverse across nodes that have @@ -126,8 +127,6 @@ bool DependencyScan::RecomputeOutputsDirty(Edge* edge, Node* most_recent_input, string command = edge->EvaluateCommand(/*incl_rsp_file=*/true); for (vector<Node*>::iterator o = edge->outputs_.begin(); o != edge->outputs_.end(); ++o) { - if (!(*o)->StatIfNecessary(disk_interface_, err)) - return false; if (RecomputeOutputDirty(edge, most_recent_input, command, *o)) { *outputs_dirty = true; return true; @@ -241,8 +240,9 @@ string EdgeEnv::LookupVariable(const string& var) { edge_->inputs_.begin() + explicit_deps_count, var == "in" ? ' ' : '\n'); } else if (var == "out") { + int explicit_outs_count = edge_->outputs_.size() - edge_->implicit_outs_; return MakePathList(edge_->outputs_.begin(), - edge_->outputs_.end(), + edge_->outputs_.begin() + explicit_outs_count, ' '); } @@ -347,12 +347,13 @@ bool Edge::use_console() const { return pool() == &State::kConsolePool; } -string Node::PathDecanonicalized() const { - string result = path_; +// static +string Node::PathDecanonicalized(const string& path, unsigned int slash_bits) { + string result = path; #ifdef _WIN32 unsigned int mask = 1; for (char* c = &result[0]; (c = strchr(c, '/')) != NULL;) { - if (slash_bits_ & mask) + if (slash_bits & mask) *c = '\\'; c++; mask <<= 1; @@ -394,8 +395,15 @@ bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) { bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, string* err) { METRIC_RECORD("depfile load"); - string content = disk_interface_->ReadFile(path, err); - if (!err->empty()) { + // Read depfile content. Treat a missing depfile as empty. + string content; + switch (disk_interface_->ReadFile(path, &content, err)) { + case DiskInterface::Okay: + break; + case DiskInterface::NotFound: + err->clear(); + break; + case DiskInterface::OtherError: *err = "loading '" + path + "': " + *err; return false; } diff --git a/src/graph.h b/src/graph.h index 5f8d41a..add8d3d 100644 --- a/src/graph.h +++ b/src/graph.h @@ -72,8 +72,13 @@ struct Node { const string& path() const { return path_; } /// Get |path()| but use slash_bits to convert back to original slash styles. - string PathDecanonicalized() const; + string PathDecanonicalized() const { + return PathDecanonicalized(path_, slash_bits_); + } + static string PathDecanonicalized(const string& path, + unsigned int slash_bits); unsigned int slash_bits() const { return slash_bits_; } + TimeStamp mtime() const { return mtime_; } bool dirty() const { return dirty_; } @@ -124,7 +129,7 @@ private: struct Edge { Edge() : rule_(NULL), pool_(NULL), env_(NULL), outputs_ready_(false), deps_missing_(false), - implicit_deps_(0), order_only_deps_(0) {} + implicit_deps_(0), order_only_deps_(0), implicit_outs_(0) {} /// Return true if all inputs' in-edges are ready. bool AllInputsReady() const; @@ -176,6 +181,16 @@ struct Edge { return index >= inputs_.size() - order_only_deps_; } + // There are two types of outputs. + // 1) explicit outs, which show up as $out on the command line; + // 2) implicit outs, which the target generates but are not part of $out. + // These are stored in outputs_ in that order, and we keep a count of + // #2 to use when we need to access the various subsets. + int implicit_outs_; + bool is_implicit_out(size_t index) const { + return index >= outputs_.size() - implicit_outs_; + } + bool is_phony() const; bool use_console() const; }; diff --git a/src/graph_test.cc b/src/graph_test.cc index e41f5cc..723e8ea 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -105,6 +105,50 @@ TEST_F(GraphTest, ExplicitImplicit) { EXPECT_TRUE(GetNode("out.o")->dirty()); } +TEST_F(GraphTest, ImplicitOutputParse) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build out | out.imp: cat in\n")); + + Edge* edge = GetNode("out")->in_edge(); + EXPECT_EQ(2, edge->outputs_.size()); + EXPECT_EQ("out", edge->outputs_[0]->path()); + EXPECT_EQ("out.imp", edge->outputs_[1]->path()); + EXPECT_EQ(1, edge->implicit_outs_); + EXPECT_EQ(edge, GetNode("out.imp")->in_edge()); +} + +TEST_F(GraphTest, ImplicitOutputMissing) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build out | out.imp: cat in\n")); + fs_.Create("in", ""); + fs_.Create("out", ""); + + Edge* edge = GetNode("out")->in_edge(); + string err; + EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + ASSERT_EQ("", err); + + EXPECT_TRUE(GetNode("out")->dirty()); + EXPECT_TRUE(GetNode("out.imp")->dirty()); +} + +TEST_F(GraphTest, ImplicitOutputOutOfDate) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build out | out.imp: cat in\n")); + fs_.Create("out.imp", ""); + fs_.Tick(); + fs_.Create("in", ""); + fs_.Create("out", ""); + + Edge* edge = GetNode("out")->in_edge(); + string err; + EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + ASSERT_EQ("", err); + + EXPECT_TRUE(GetNode("out")->dirty()); + EXPECT_TRUE(GetNode("out.imp")->dirty()); +} + TEST_F(GraphTest, PathWithCurrentDirectory) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule catdep\n" @@ -153,7 +197,7 @@ TEST_F(GraphTest, VarInOutPathEscaping) { #endif } -// Regression test for https://github.com/martine/ninja/issues/380 +// Regression test for https://github.com/ninja-build/ninja/issues/380 TEST_F(GraphTest, DepfileWithCanonicalizablePath) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule catdep\n" @@ -172,7 +216,7 @@ TEST_F(GraphTest, DepfileWithCanonicalizablePath) { EXPECT_FALSE(GetNode("out.o")->dirty()); } -// Regression test for https://github.com/martine/ninja/issues/404 +// Regression test for https://github.com/ninja-build/ninja/issues/404 TEST_F(GraphTest, DepfileRemoved) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule catdep\n" diff --git a/src/graphviz.h b/src/graphviz.h index 1e2a29d..408496d 100644 --- a/src/graphviz.h +++ b/src/graphviz.h @@ -16,7 +16,6 @@ #define NINJA_GRAPHVIZ_H_ #include <set> -using namespace std; struct Node; struct Edge; @@ -27,8 +26,8 @@ struct GraphViz { void AddTarget(Node* node); void Finish(); - set<Node*> visited_nodes_; - set<Edge*> visited_edges_; + std::set<Node*> visited_nodes_; + std::set<Edge*> visited_edges_; }; #endif // NINJA_GRAPHVIZ_H_ diff --git a/src/hash_collision_bench.cc b/src/hash_collision_bench.cc index 5be0531..ff947dc 100644 --- a/src/hash_collision_bench.cc +++ b/src/hash_collision_bench.cc @@ -17,6 +17,7 @@ #include <algorithm> using namespace std; +#include <stdlib.h> #include <time.h> int random(int low, int high) { diff --git a/src/hash_map.h b/src/hash_map.h index abdba92..a91aeb9 100644 --- a/src/hash_map.h +++ b/src/hash_map.h @@ -76,7 +76,7 @@ struct StringPieceCmp : public hash_compare<StringPiece> { return MurmurHash2(key.str_, key.len_); } bool operator()(const StringPiece& a, const StringPiece& b) const { - int cmp = strncmp(a.str_, b.str_, min(a.len_, b.len_)); + int cmp = memcmp(a.str_, b.str_, min(a.len_, b.len_)); if (cmp < 0) { return true; } else if (cmp > 0) { diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index 1e88a0a..ca35012 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -94,15 +94,18 @@ string IncludesNormalize::Relativize(StringPiece path, const string& start) { return Join(rel_list, '/'); } -string IncludesNormalize::Normalize(const string& input, - const char* relative_to) { - char copy[_MAX_PATH]; +bool IncludesNormalize::Normalize(const string& input, const char* relative_to, + string* result, string* err) { + char copy[_MAX_PATH + 1]; size_t len = input.size(); + if (len > _MAX_PATH) { + *err = "path too long"; + return false; + } strncpy(copy, input.c_str(), input.size() + 1); - string err; unsigned int slash_bits; - if (!CanonicalizePath(copy, &len, &slash_bits, &err)) - Warning("couldn't canonicalize '%s': %s\n", input.c_str(), err.c_str()); + if (!CanonicalizePath(copy, &len, &slash_bits, err)) + return false; StringPiece partially_fixed(copy, len); string curdir; @@ -110,7 +113,10 @@ string IncludesNormalize::Normalize(const string& input, curdir = AbsPath("."); relative_to = curdir.c_str(); } - if (!SameDrive(partially_fixed, relative_to)) - return partially_fixed.AsString(); - return Relativize(partially_fixed, relative_to); + if (!SameDrive(partially_fixed, relative_to)) { + *result = partially_fixed.AsString(); + return true; + } + *result = Relativize(partially_fixed, relative_to); + return true; } diff --git a/src/includes_normalize.h b/src/includes_normalize.h index 634fef3..98e912f 100644 --- a/src/includes_normalize.h +++ b/src/includes_normalize.h @@ -30,5 +30,6 @@ struct IncludesNormalize { /// Normalize by fixing slashes style, fixing redundant .. and . and makes the /// path relative to |relative_to|. - static string Normalize(const string& input, const char* relative_to); + static bool Normalize(const string& input, const char* relative_to, + string* result, string* err); }; diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index c4c2476..f18795c 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -14,18 +14,13 @@ #include "includes_normalize.h" +#include <algorithm> + #include <direct.h> #include "test.h" #include "util.h" -TEST(IncludesNormalize, Simple) { - EXPECT_EQ("b", IncludesNormalize::Normalize("a\\..\\b", NULL)); - EXPECT_EQ("b", IncludesNormalize::Normalize("a\\../b", NULL)); - EXPECT_EQ("a/b", IncludesNormalize::Normalize("a\\.\\b", NULL)); - EXPECT_EQ("a/b", IncludesNormalize::Normalize("a\\./b", NULL)); -} - namespace { string GetCurDir() { @@ -35,28 +30,50 @@ string GetCurDir() { return parts[parts.size() - 1]; } +string NormalizeAndCheckNoError(const string& input) { + string result, err; + EXPECT_TRUE(IncludesNormalize::Normalize(input.c_str(), NULL, &result, &err)); + EXPECT_EQ("", err); + return result; +} + +string NormalizeRelativeAndCheckNoError(const string& input, + const string& relative_to) { + string result, err; + EXPECT_TRUE(IncludesNormalize::Normalize(input.c_str(), relative_to.c_str(), + &result, &err)); + EXPECT_EQ("", err); + return result; +} + } // namespace +TEST(IncludesNormalize, Simple) { + EXPECT_EQ("b", NormalizeAndCheckNoError("a\\..\\b")); + EXPECT_EQ("b", NormalizeAndCheckNoError("a\\../b")); + EXPECT_EQ("a/b", NormalizeAndCheckNoError("a\\.\\b")); + EXPECT_EQ("a/b", NormalizeAndCheckNoError("a\\./b")); +} + TEST(IncludesNormalize, WithRelative) { string currentdir = GetCurDir(); - EXPECT_EQ("c", IncludesNormalize::Normalize("a/b/c", "a/b")); - EXPECT_EQ("a", IncludesNormalize::Normalize(IncludesNormalize::AbsPath("a"), - NULL)); + EXPECT_EQ("c", NormalizeRelativeAndCheckNoError("a/b/c", "a/b")); + EXPECT_EQ("a", NormalizeAndCheckNoError(IncludesNormalize::AbsPath("a"))); EXPECT_EQ(string("../") + currentdir + string("/a"), - IncludesNormalize::Normalize("a", "../b")); + NormalizeRelativeAndCheckNoError("a", "../b")); EXPECT_EQ(string("../") + currentdir + string("/a/b"), - IncludesNormalize::Normalize("a/b", "../c")); - EXPECT_EQ("../../a", IncludesNormalize::Normalize("a", "b/c")); - EXPECT_EQ(".", IncludesNormalize::Normalize("a", "a")); + NormalizeRelativeAndCheckNoError("a/b", "../c")); + EXPECT_EQ("../../a", NormalizeRelativeAndCheckNoError("a", "b/c")); + EXPECT_EQ(".", NormalizeRelativeAndCheckNoError("a", "a")); } TEST(IncludesNormalize, Case) { - EXPECT_EQ("b", IncludesNormalize::Normalize("Abc\\..\\b", NULL)); - EXPECT_EQ("BdEf", IncludesNormalize::Normalize("Abc\\..\\BdEf", NULL)); - EXPECT_EQ("A/b", IncludesNormalize::Normalize("A\\.\\b", NULL)); - EXPECT_EQ("a/b", IncludesNormalize::Normalize("a\\./b", NULL)); - EXPECT_EQ("A/B", IncludesNormalize::Normalize("A\\.\\B", NULL)); - EXPECT_EQ("A/B", IncludesNormalize::Normalize("A\\./B", NULL)); + EXPECT_EQ("b", NormalizeAndCheckNoError("Abc\\..\\b")); + EXPECT_EQ("BdEf", NormalizeAndCheckNoError("Abc\\..\\BdEf")); + EXPECT_EQ("A/b", NormalizeAndCheckNoError("A\\.\\b")); + EXPECT_EQ("a/b", NormalizeAndCheckNoError("a\\./b")); + EXPECT_EQ("A/B", NormalizeAndCheckNoError("A\\.\\B")); + EXPECT_EQ("A/B", NormalizeAndCheckNoError("A\\./B")); } TEST(IncludesNormalize, Join) { @@ -89,16 +106,63 @@ TEST(IncludesNormalize, ToLower) { TEST(IncludesNormalize, DifferentDrive) { EXPECT_EQ("stuff.h", - IncludesNormalize::Normalize("p:\\vs08\\stuff.h", "p:\\vs08")); + NormalizeRelativeAndCheckNoError("p:\\vs08\\stuff.h", "p:\\vs08")); EXPECT_EQ("stuff.h", - IncludesNormalize::Normalize("P:\\Vs08\\stuff.h", "p:\\vs08")); + NormalizeRelativeAndCheckNoError("P:\\Vs08\\stuff.h", "p:\\vs08")); EXPECT_EQ("p:/vs08/stuff.h", - IncludesNormalize::Normalize("p:\\vs08\\stuff.h", "c:\\vs08")); - EXPECT_EQ("P:/vs08/stufF.h", - IncludesNormalize::Normalize("P:\\vs08\\stufF.h", "D:\\stuff/things")); - EXPECT_EQ("P:/vs08/stuff.h", - IncludesNormalize::Normalize("P:/vs08\\stuff.h", "D:\\stuff/things")); + NormalizeRelativeAndCheckNoError("p:\\vs08\\stuff.h", "c:\\vs08")); + EXPECT_EQ("P:/vs08/stufF.h", NormalizeRelativeAndCheckNoError( + "P:\\vs08\\stufF.h", "D:\\stuff/things")); + EXPECT_EQ("P:/vs08/stuff.h", NormalizeRelativeAndCheckNoError( + "P:/vs08\\stuff.h", "D:\\stuff/things")); EXPECT_EQ("P:/wee/stuff.h", - IncludesNormalize::Normalize("P:/vs08\\../wee\\stuff.h", - "D:\\stuff/things")); + NormalizeRelativeAndCheckNoError("P:/vs08\\../wee\\stuff.h", + "D:\\stuff/things")); +} + +TEST(IncludesNormalize, LongInvalidPath) { + const char kLongInputString[] = + "C:\\Program Files (x86)\\Microsoft Visual Studio " + "12.0\\VC\\INCLUDEwarning #31001: The dll for reading and writing the " + "pdb (for example, mspdb110.dll) could not be found on your path. This " + "is usually a configuration error. Compilation will continue using /Z7 " + "instead of /Zi, but expect a similar error when you link your program."; + // Too long, won't be canonicalized. Ensure doesn't crash. + string result, err; + EXPECT_FALSE( + IncludesNormalize::Normalize(kLongInputString, NULL, &result, &err)); + EXPECT_EQ("path too long", err); + + const char kExactlyMaxPath[] = + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "012345678\\" + "0123456789"; + string forward_slashes(kExactlyMaxPath); + replace(forward_slashes.begin(), forward_slashes.end(), '\\', '/'); + // Make sure a path that's exactly _MAX_PATH long is canonicalized. + EXPECT_EQ(forward_slashes, + NormalizeAndCheckNoError(kExactlyMaxPath)); } diff --git a/src/inline.sh b/src/inline.sh index 5acc17b..fa282fa 100755 --- a/src/inline.sh +++ b/src/inline.sh @@ -20,6 +20,6 @@ varname="$1" echo "const char $varname[] =" -od -t x1 -A n -v | sed -e 's| ||g; s|..|\\x&|g; s|^|"|; s|$|"|' +od -t x1 -A n -v | sed -e 's|[ \t]||g; s|..|\\x&|g; s|^|"|; s|$|"|' echo ";" diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index e8c0436..a4f489e 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -18,6 +18,7 @@ #include <stdlib.h> #include <vector> +#include "disk_interface.h" #include "graph.h" #include "metrics.h" #include "state.h" @@ -25,9 +26,9 @@ #include "version.h" ManifestParser::ManifestParser(State* state, FileReader* file_reader, - bool dupe_edge_should_err) + DupeEdgeAction dupe_edge_action) : state_(state), file_reader_(file_reader), - dupe_edge_should_err_(dupe_edge_should_err), quiet_(false) { + dupe_edge_action_(dupe_edge_action), quiet_(false) { env_ = &state->bindings_; } @@ -35,7 +36,7 @@ bool ManifestParser::Load(const string& filename, string* err, Lexer* parent) { METRIC_RECORD(".ninja parse"); string contents; string read_err; - if (!file_reader_->ReadFile(filename, &contents, &read_err)) { + if (file_reader_->ReadFile(filename, &contents, &read_err) != FileReader::Okay) { *err = "loading '" + filename + "': " + read_err; if (parent) parent->Error(string(*err), err); @@ -247,6 +248,20 @@ bool ManifestParser::ParseEdge(string* err) { } while (!out.empty()); } + // Add all implicit outs, counting how many as we go. + int implicit_outs = 0; + if (lexer_.PeekToken(Lexer::PIPE)) { + for (;;) { + EvalString out; + if (!lexer_.ReadPath(&out, err)) + return err; + if (out.empty()) + break; + outs.push_back(out); + ++implicit_outs; + } + } + if (!ExpectToken(Lexer::COLON, err)) return false; @@ -324,22 +339,26 @@ bool ManifestParser::ParseEdge(string* err) { } edge->outputs_.reserve(outs.size()); - for (vector<EvalString>::iterator i = outs.begin(); i != outs.end(); ++i) { - string path = i->Evaluate(env); + for (size_t i = 0, e = outs.size(); i != e; ++i) { + string path = outs[i].Evaluate(env); string path_err; unsigned int slash_bits; if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); if (!state_->AddOut(edge, path, slash_bits)) { - if (dupe_edge_should_err_) { + if (dupe_edge_action_ == kDupeEdgeActionError) { lexer_.Error("multiple rules generate " + path + " [-w dupbuild=err]", err); return false; - } else if (!quiet_) { - Warning("multiple rules generate %s. " - "builds involving this target will not be correct; " - "continuing anyway [-w dupbuild=warn]", - path.c_str()); + } else { + if (!quiet_) { + Warning("multiple rules generate %s. " + "builds involving this target will not be correct; " + "continuing anyway [-w dupbuild=warn]", + path.c_str()); + } + if (e - i <= static_cast<size_t>(implicit_outs)) + --implicit_outs; } } } @@ -350,6 +369,7 @@ bool ManifestParser::ParseEdge(string* err) { delete edge; return true; } + edge->implicit_outs_ = implicit_outs; edge->inputs_.reserve(ins.size()); for (vector<EvalString>::iterator i = ins.begin(); i != ins.end(); ++i) { @@ -380,7 +400,7 @@ bool ManifestParser::ParseFileInclude(bool new_scope, string* err) { return false; string path = eval.Evaluate(env_); - ManifestParser subparser(state_, file_reader_); + ManifestParser subparser(state_, file_reader_, dupe_edge_action_); if (new_scope) { subparser.env_ = new BindingEnv(env_); } else { diff --git a/src/manifest_parser.h b/src/manifest_parser.h index f72cd6f..043e4b2 100644 --- a/src/manifest_parser.h +++ b/src/manifest_parser.h @@ -23,17 +23,18 @@ using namespace std; struct BindingEnv; struct EvalString; +struct FileReader; struct State; +enum DupeEdgeAction { + kDupeEdgeActionWarn, + kDupeEdgeActionError, +}; + /// Parses .ninja files. struct ManifestParser { - struct FileReader { - virtual ~FileReader() {} - virtual bool ReadFile(const string& path, string* content, string* err) = 0; - }; - ManifestParser(State* state, FileReader* file_reader, - bool dupe_edge_should_err = false); + DupeEdgeAction dupe_edge_action); /// Load and parse a file. bool Load(const string& filename, string* err, Lexer* parent = NULL); @@ -66,7 +67,7 @@ private: BindingEnv* env_; FileReader* file_reader_; Lexer lexer_; - bool dupe_edge_should_err_; + DupeEdgeAction dupe_edge_action_; bool quiet_; }; diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc index 6b56ab0..60c2054 100644 --- a/src/manifest_parser_perftest.cc +++ b/src/manifest_parser_perftest.cc @@ -19,6 +19,7 @@ #include <errno.h> #include <stdio.h> +#include <stdlib.h> #include <string.h> #ifdef _WIN32 @@ -36,12 +37,6 @@ #include "state.h" #include "util.h" -struct RealFileReader : public ManifestParser::FileReader { - virtual bool ReadFile(const string& path, string* content, string* err) { - return ::ReadFile(path, content, err) == 0; - } -}; - bool WriteFakeManifests(const string& dir, string* err) { RealDiskInterface disk_interface; TimeStamp mtime = disk_interface.Stat(dir + "/build.ninja", err); @@ -59,9 +54,9 @@ bool WriteFakeManifests(const string& dir, string* err) { int LoadManifests(bool measure_command_evaluation) { string err; - RealFileReader file_reader; + RealDiskInterface disk_interface; State state; - ManifestParser parser(&state, &file_reader); + ManifestParser parser(&state, &disk_interface, kDupeEdgeActionWarn); if (!parser.Load("build.ninja", &err)) { fprintf(stderr, "Failed to read test data: %s\n", err.c_str()); exit(1); diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 8f7b575..1312d26 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -21,30 +21,17 @@ #include "state.h" #include "test.h" -struct ParserTest : public testing::Test, - public ManifestParser::FileReader { +struct ParserTest : public testing::Test { void AssertParse(const char* input) { - ManifestParser parser(&state, this); + ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); string err; EXPECT_TRUE(parser.ParseTest(input, &err)); ASSERT_EQ("", err); VerifyGraph(state); } - virtual bool ReadFile(const string& path, string* content, string* err) { - files_read_.push_back(path); - map<string, string>::iterator i = files_.find(path); - if (i == files_.end()) { - *err = "No such file or directory"; // Match strerror() for ENOENT. - return false; - } - *content = i->second; - return true; - } - State state; - map<string, string> files_; - vector<string> files_read_; + VirtualFileSystem fs_; }; TEST_F(ParserTest, Empty) { @@ -371,12 +358,28 @@ TEST_F(ParserTest, DuplicateEdgeWithMultipleOutputsError) { "build out1 out2: cat in1\n" "build out1: cat in2\n" "build final: cat out1\n"; - ManifestParser parser(&state, this, /*dupe_edges_should_err=*/true); + ManifestParser parser(&state, &fs_, kDupeEdgeActionError); string err; EXPECT_FALSE(parser.ParseTest(kInput, &err)); EXPECT_EQ("input:5: multiple rules generate out1 [-w dupbuild=err]\n", err); } +TEST_F(ParserTest, DuplicateEdgeInIncludedFile) { + fs_.Create("sub.ninja", + "rule cat\n" + " command = cat $in > $out\n" + "build out1 out2: cat in1\n" + "build out1: cat in2\n" + "build final: cat out1\n"); + const char kInput[] = + "subninja sub.ninja\n"; + ManifestParser parser(&state, &fs_, kDupeEdgeActionError); + string err; + EXPECT_FALSE(parser.ParseTest(kInput, &err)); + EXPECT_EQ("sub.ninja:5: multiple rules generate out1 [-w dupbuild=err]\n", + err); +} + TEST_F(ParserTest, ReservedWords) { ASSERT_NO_FATAL_FAILURE(AssertParse( "rule build\n" @@ -387,8 +390,8 @@ TEST_F(ParserTest, ReservedWords) { TEST_F(ParserTest, Errors) { { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest(string("subn", 4), &err)); EXPECT_EQ("input:1: expected '=', got eof\n" @@ -398,8 +401,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("foobar", &err)); EXPECT_EQ("input:1: expected '=', got eof\n" @@ -409,8 +412,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("x 3", &err)); EXPECT_EQ("input:1: expected '=', got identifier\n" @@ -420,8 +423,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("x = 3", &err)); EXPECT_EQ("input:1: unexpected EOF\n" @@ -431,8 +434,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("x = 3\ny 2", &err)); EXPECT_EQ("input:2: expected '=', got identifier\n" @@ -442,8 +445,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("x = $", &err)); EXPECT_EQ("input:1: bad $-escape (literal $ must be written as $$)\n" @@ -453,8 +456,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("x = $\n $[\n", &err)); EXPECT_EQ("input:2: bad $-escape (literal $ must be written as $$)\n" @@ -464,8 +467,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("x = a$\n b$\n $\n", &err)); EXPECT_EQ("input:4: unexpected EOF\n" @@ -473,8 +476,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("build\n", &err)); EXPECT_EQ("input:1: expected path\n" @@ -484,8 +487,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("build x: y z\n", &err)); EXPECT_EQ("input:1: unknown build rule 'y'\n" @@ -495,8 +498,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("build x:: y z\n", &err)); EXPECT_EQ("input:1: expected build command name\n" @@ -506,8 +509,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n command = cat ok\n" "build x: cat $\n :\n", @@ -519,8 +522,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n", &err)); @@ -528,8 +531,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = echo\n" @@ -542,8 +545,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = echo\n" @@ -554,8 +557,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = ${fafsd\n" @@ -569,8 +572,8 @@ TEST_F(ParserTest, Errors) { { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = cat\n" @@ -584,8 +587,8 @@ TEST_F(ParserTest, Errors) { { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = cat\n" @@ -598,8 +601,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule %foo\n", &err)); @@ -607,8 +610,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n" " command = foo\n" @@ -621,8 +624,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n" "build $.: cc bar.cc\n", @@ -634,8 +637,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n && bar", &err)); @@ -643,8 +646,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n" "build $: cc bar.cc\n", @@ -656,8 +659,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("default\n", &err)); @@ -668,8 +671,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("default nonexistent\n", &err)); @@ -680,8 +683,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule r\n command = r\n" "build b: r\n" @@ -694,8 +697,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("default $a\n", &err)); EXPECT_EQ("input:1: empty path\n" @@ -705,8 +708,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule r\n" " command = r\n" @@ -717,8 +720,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; // the indented blank line must terminate the rule // this also verifies that "unexpected (token)" errors are correct @@ -730,24 +733,24 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("pool\n", &err)); EXPECT_EQ("input:1: expected pool name\n", err); } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("pool foo\n", &err)); EXPECT_EQ("input:2: expected 'depth =' line\n", err); } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("pool foo\n" " depth = 4\n" @@ -759,8 +762,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("pool foo\n" " depth = -1\n", &err)); @@ -771,8 +774,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("pool foo\n" " bar = 1\n", &err)); @@ -783,8 +786,8 @@ TEST_F(ParserTest, Errors) { } { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; // Pool names are dereferenced at edge parsing time. EXPECT_FALSE(parser.ParseTest("rule run\n" @@ -796,16 +799,16 @@ TEST_F(ParserTest, Errors) { } TEST_F(ParserTest, MissingInput) { - State state; - ManifestParser parser(&state, this); + State local_state; + ManifestParser parser(&local_state, &fs_, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.Load("build.ninja", &err)); EXPECT_EQ("loading 'build.ninja': No such file or directory", err); } TEST_F(ParserTest, MultipleOutputs) { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_TRUE(parser.ParseTest("rule cc\n command = foo\n depfile = bar\n" "build a.o b.o: cc c.cc\n", @@ -814,8 +817,8 @@ TEST_F(ParserTest, MultipleOutputs) { } TEST_F(ParserTest, MultipleOutputsWithDeps) { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n deps = gcc\n" "build a.o b.o: cc c.cc\n", @@ -825,9 +828,9 @@ TEST_F(ParserTest, MultipleOutputsWithDeps) { } TEST_F(ParserTest, SubNinja) { - files_["test.ninja"] = + fs_.Create("test.ninja", "var = inner\n" - "build $builddir/inner: varref\n"; + "build $builddir/inner: varref\n"); ASSERT_NO_FATAL_FAILURE(AssertParse( "builddir = some_dir/\n" "rule varref\n" @@ -836,9 +839,9 @@ TEST_F(ParserTest, SubNinja) { "build $builddir/outer: varref\n" "subninja test.ninja\n" "build $builddir/outer2: varref\n")); - ASSERT_EQ(1u, files_read_.size()); + ASSERT_EQ(1u, fs_.files_read_.size()); - EXPECT_EQ("test.ninja", files_read_[0]); + EXPECT_EQ("test.ninja", fs_.files_read_[0]); EXPECT_TRUE(state.LookupNode("some_dir/outer")); // Verify our builddir setting is inherited. EXPECT_TRUE(state.LookupNode("some_dir/inner")); @@ -850,7 +853,7 @@ TEST_F(ParserTest, SubNinja) { } TEST_F(ParserTest, MissingSubNinja) { - ManifestParser parser(&state, this); + ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("subninja foo.ninja\n", &err)); EXPECT_EQ("input:1: loading 'foo.ninja': No such file or directory\n" @@ -861,9 +864,9 @@ TEST_F(ParserTest, MissingSubNinja) { TEST_F(ParserTest, DuplicateRuleInDifferentSubninjas) { // Test that rules are scoped to subninjas. - files_["test.ninja"] = "rule cat\n" - " command = cat\n"; - ManifestParser parser(&state, this); + fs_.Create("test.ninja", "rule cat\n" + " command = cat\n"); + ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); string err; EXPECT_TRUE(parser.ParseTest("rule cat\n" " command = cat\n" @@ -872,11 +875,11 @@ TEST_F(ParserTest, DuplicateRuleInDifferentSubninjas) { TEST_F(ParserTest, DuplicateRuleInDifferentSubninjasWithInclude) { // Test that rules are scoped to subninjas even with includes. - files_["rules.ninja"] = "rule cat\n" - " command = cat\n"; - files_["test.ninja"] = "include rules.ninja\n" - "build x : cat\n"; - ManifestParser parser(&state, this); + fs_.Create("rules.ninja", "rule cat\n" + " command = cat\n"); + fs_.Create("test.ninja", "include rules.ninja\n" + "build x : cat\n"); + ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); string err; EXPECT_TRUE(parser.ParseTest("include rules.ninja\n" "subninja test.ninja\n" @@ -884,19 +887,19 @@ TEST_F(ParserTest, DuplicateRuleInDifferentSubninjasWithInclude) { } TEST_F(ParserTest, Include) { - files_["include.ninja"] = "var = inner\n"; + fs_.Create("include.ninja", "var = inner\n"); ASSERT_NO_FATAL_FAILURE(AssertParse( "var = outer\n" "include include.ninja\n")); - ASSERT_EQ(1u, files_read_.size()); - EXPECT_EQ("include.ninja", files_read_[0]); + ASSERT_EQ(1u, fs_.files_read_.size()); + EXPECT_EQ("include.ninja", fs_.files_read_[0]); EXPECT_EQ("inner", state.bindings_.LookupVariable("var")); } TEST_F(ParserTest, BrokenInclude) { - files_["include.ninja"] = "build\n"; - ManifestParser parser(&state, this); + fs_.Create("include.ninja", "build\n"); + ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); string err; EXPECT_FALSE(parser.ParseTest("include include.ninja\n", &err)); EXPECT_EQ("include.ninja:1: expected path\n" @@ -924,6 +927,64 @@ TEST_F(ParserTest, OrderOnly) { ASSERT_TRUE(edge->is_order_only(1)); } +TEST_F(ParserTest, ImplicitOutput) { + ASSERT_NO_FATAL_FAILURE(AssertParse( +"rule cat\n" +" command = cat $in > $out\n" +"build foo | imp: cat bar\n")); + + Edge* edge = state.LookupNode("imp")->in_edge(); + ASSERT_EQ(edge->outputs_.size(), 2); + EXPECT_TRUE(edge->is_implicit_out(1)); +} + +TEST_F(ParserTest, ImplicitOutputEmpty) { + ASSERT_NO_FATAL_FAILURE(AssertParse( +"rule cat\n" +" command = cat $in > $out\n" +"build foo | : cat bar\n")); + + Edge* edge = state.LookupNode("foo")->in_edge(); + ASSERT_EQ(edge->outputs_.size(), 1); + EXPECT_FALSE(edge->is_implicit_out(0)); +} + +TEST_F(ParserTest, ImplicitOutputDupe) { + ASSERT_NO_FATAL_FAILURE(AssertParse( +"rule cat\n" +" command = cat $in > $out\n" +"build foo baz | foo baq foo: cat bar\n")); + + Edge* edge = state.LookupNode("foo")->in_edge(); + ASSERT_EQ(edge->outputs_.size(), 3); + EXPECT_FALSE(edge->is_implicit_out(0)); + EXPECT_FALSE(edge->is_implicit_out(1)); + EXPECT_TRUE(edge->is_implicit_out(2)); +} + +TEST_F(ParserTest, ImplicitOutputDupes) { + ASSERT_NO_FATAL_FAILURE(AssertParse( +"rule cat\n" +" command = cat $in > $out\n" +"build foo foo foo | foo foo foo foo: cat bar\n")); + + Edge* edge = state.LookupNode("foo")->in_edge(); + ASSERT_EQ(edge->outputs_.size(), 1); + EXPECT_FALSE(edge->is_implicit_out(0)); +} + +TEST_F(ParserTest, NoExplicitOutput) { + ManifestParser parser(&state, NULL, kDupeEdgeActionWarn); + string err; + EXPECT_FALSE(parser.ParseTest( +"rule cat\n" +" command = cat $in > $out\n" +"build | imp : cat bar\n", &err)); + ASSERT_EQ("input:3: expected path\n" + "build | imp : cat bar\n" + " ^ near here", err); +} + TEST_F(ParserTest, DefaultDefault) { ASSERT_NO_FATAL_FAILURE(AssertParse( "rule cat\n command = cat $in > $out\n" @@ -975,8 +1036,8 @@ TEST_F(ParserTest, UTF8) { } TEST_F(ParserTest, CRLF) { - State state; - ManifestParser parser(&state, NULL); + State local_state; + ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); string err; EXPECT_TRUE(parser.ParseTest("# comment with crlf\r\n", &err)); diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index e465279..e37a26e 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -14,22 +14,12 @@ #include "msvc_helper.h" -#include <algorithm> -#include <stdio.h> -#include <string.h> #include <windows.h> -#include "includes_normalize.h" #include "util.h" namespace { -/// Return true if \a input ends with \a needle. -bool EndsWith(const string& input, const string& needle) { - return (input.size() >= needle.size() && - input.substr(input.size() - needle.size()) == needle); -} - string Replace(const string& input, const string& find, const string& replace) { string result = input; size_t start_pos = 0; @@ -47,76 +37,6 @@ string EscapeForDepfile(const string& path) { return Replace(path, " ", "\\ "); } -// static -string CLParser::FilterShowIncludes(const string& line, - const string& deps_prefix) { - const string kDepsPrefixEnglish = "Note: including file: "; - const char* in = line.c_str(); - const char* end = in + line.size(); - const string& prefix = deps_prefix.empty() ? kDepsPrefixEnglish : deps_prefix; - if (end - in > (int)prefix.size() && - memcmp(in, prefix.c_str(), (int)prefix.size()) == 0) { - in += prefix.size(); - while (*in == ' ') - ++in; - return line.substr(in - line.c_str()); - } - return ""; -} - -// static -bool CLParser::IsSystemInclude(string path) { - transform(path.begin(), path.end(), path.begin(), ::tolower); - // TODO: this is a heuristic, perhaps there's a better way? - return (path.find("program files") != string::npos || - path.find("microsoft visual studio") != string::npos); -} - -// static -bool CLParser::FilterInputFilename(string line) { - transform(line.begin(), line.end(), line.begin(), ::tolower); - // TODO: other extensions, like .asm? - return EndsWith(line, ".c") || - EndsWith(line, ".cc") || - EndsWith(line, ".cxx") || - EndsWith(line, ".cpp"); -} - -string CLParser::Parse(const string& output, const string& deps_prefix) { - string filtered_output; - - // Loop over all lines in the output to process them. - size_t start = 0; - while (start < output.size()) { - size_t end = output.find_first_of("\r\n", start); - if (end == string::npos) - end = output.size(); - string line = output.substr(start, end - start); - - string include = FilterShowIncludes(line, deps_prefix); - if (!include.empty()) { - include = IncludesNormalize::Normalize(include, NULL); - if (!IsSystemInclude(include)) - includes_.insert(include); - } else if (FilterInputFilename(line)) { - // Drop it. - // TODO: if we support compiling multiple output files in a single - // cl.exe invocation, we should stash the filename. - } else { - filtered_output.append(line); - filtered_output.append("\n"); - } - - if (end < output.size() && output[end] == '\r') - ++end; - if (end < output.size() && output[end] == '\n') - ++end; - start = end; - } - - return filtered_output; -} - int CLWrapper::Run(const string& command, string* output) { SECURITY_ATTRIBUTES security_attributes = {}; security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); diff --git a/src/msvc_helper.h b/src/msvc_helper.h index 5d7dcb0..70d1fff 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -13,40 +13,10 @@ // limitations under the License. #include <string> -#include <set> -#include <vector> using namespace std; string EscapeForDepfile(const string& path); -/// Visual Studio's cl.exe requires some massaging to work with Ninja; -/// for example, it emits include information on stderr in a funny -/// format when building with /showIncludes. This class parses this -/// output. -struct CLParser { - /// Parse a line of cl.exe output and extract /showIncludes info. - /// If a dependency is extracted, returns a nonempty string. - /// Exposed for testing. - static string FilterShowIncludes(const string& line, - const string& deps_prefix); - - /// Return true if a mentioned include file is a system path. - /// Filtering these out reduces dependency information considerably. - static bool IsSystemInclude(string path); - - /// Parse a line of cl.exe output and return true if it looks like - /// it's printing an input filename. This is a heuristic but it appears - /// to be the best we can do. - /// Exposed for testing. - static bool FilterInputFilename(string line); - - /// Parse the full output of cl, returning the output (if any) that - /// should printed. - string Parse(const string& output, const string& deps_prefix); - - set<string> includes_; -}; - /// Wraps a synchronous execution of a CL subprocess. struct CLWrapper { CLWrapper() : env_block_(NULL) {} diff --git a/src/msvc_helper_main-win32.cc b/src/msvc_helper_main-win32.cc index 58bc797..e419cd7 100644 --- a/src/msvc_helper_main-win32.cc +++ b/src/msvc_helper_main-win32.cc @@ -19,6 +19,7 @@ #include <stdio.h> #include <windows.h> +#include "clparser.h" #include "util.h" #include "getopt.h" @@ -127,7 +128,9 @@ int MSVCHelperMain(int argc, char** argv) { if (output_filename) { CLParser parser; - output = parser.Parse(output, deps_prefix); + string err; + if (!parser.Parse(output, deps_prefix, &output, &err)) + Fatal("%s\n", err.c_str()); WriteDepFileOrDie(output_filename, parser); } diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 29db650..eaae51f 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -17,94 +17,7 @@ #include "test.h" #include "util.h" -TEST(CLParserTest, ShowIncludes) { - ASSERT_EQ("", CLParser::FilterShowIncludes("", "")); - - ASSERT_EQ("", CLParser::FilterShowIncludes("Sample compiler output", "")); - ASSERT_EQ("c:\\Some Files\\foobar.h", - CLParser::FilterShowIncludes("Note: including file: " - "c:\\Some Files\\foobar.h", "")); - ASSERT_EQ("c:\\initspaces.h", - CLParser::FilterShowIncludes("Note: including file: " - "c:\\initspaces.h", "")); - ASSERT_EQ("c:\\initspaces.h", - CLParser::FilterShowIncludes("Non-default prefix: inc file: " - "c:\\initspaces.h", - "Non-default prefix: inc file:")); -} - -TEST(CLParserTest, FilterInputFilename) { - ASSERT_TRUE(CLParser::FilterInputFilename("foobar.cc")); - ASSERT_TRUE(CLParser::FilterInputFilename("foo bar.cc")); - ASSERT_TRUE(CLParser::FilterInputFilename("baz.c")); - ASSERT_TRUE(CLParser::FilterInputFilename("FOOBAR.CC")); - - ASSERT_FALSE(CLParser::FilterInputFilename( - "src\\cl_helper.cc(166) : fatal error C1075: end " - "of file found ...")); -} - -TEST(CLParserTest, ParseSimple) { - CLParser parser; - string output = parser.Parse( - "foo\r\n" - "Note: inc file prefix: foo.h\r\n" - "bar\r\n", - "Note: inc file prefix:"); - - ASSERT_EQ("foo\nbar\n", output); - ASSERT_EQ(1u, parser.includes_.size()); - ASSERT_EQ("foo.h", *parser.includes_.begin()); -} - -TEST(CLParserTest, ParseFilenameFilter) { - CLParser parser; - string output = parser.Parse( - "foo.cc\r\n" - "cl: warning\r\n", - ""); - ASSERT_EQ("cl: warning\n", output); -} - -TEST(CLParserTest, ParseSystemInclude) { - CLParser parser; - string output = parser.Parse( - "Note: including file: c:\\Program Files\\foo.h\r\n" - "Note: including file: d:\\Microsoft Visual Studio\\bar.h\r\n" - "Note: including file: path.h\r\n", - ""); - // We should have dropped the first two includes because they look like - // system headers. - ASSERT_EQ("", output); - ASSERT_EQ(1u, parser.includes_.size()); - ASSERT_EQ("path.h", *parser.includes_.begin()); -} - -TEST(CLParserTest, DuplicatedHeader) { - CLParser parser; - string output = parser.Parse( - "Note: including file: foo.h\r\n" - "Note: including file: bar.h\r\n" - "Note: including file: foo.h\r\n", - ""); - // We should have dropped one copy of foo.h. - ASSERT_EQ("", output); - ASSERT_EQ(2u, parser.includes_.size()); -} - -TEST(CLParserTest, DuplicatedHeaderPathConverted) { - CLParser parser; - string output = parser.Parse( - "Note: including file: sub/foo.h\r\n" - "Note: including file: bar.h\r\n" - "Note: including file: sub\\foo.h\r\n", - ""); - // We should have dropped one copy of foo.h. - ASSERT_EQ("", output); - ASSERT_EQ(2u, parser.includes_.size()); -} - -TEST(CLParserTest, SpacesInFilename) { +TEST(EscapeForDepfileTest, SpacesInFilename) { ASSERT_EQ("sub\\some\\ sdk\\foo.h", EscapeForDepfile("sub\\some sdk\\foo.h")); } diff --git a/src/ninja.cc b/src/ninja.cc index e5bf98d..25eafe8 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -22,6 +22,9 @@ #include "getopt.h" #include <direct.h> #include <windows.h> +#elif defined(_AIX) +#include "getopt.h" +#include <unistd.h> #else #include <getopt.h> #include <unistd.h> @@ -94,7 +97,7 @@ struct NinjaMain : public BuildLogUser { DepsLog deps_log_; /// The type of functions that are the entry points to tools (subcommands). - typedef int (NinjaMain::*ToolFunc)(int, char**); + typedef int (NinjaMain::*ToolFunc)(const Options*, int, char**); /// Get the Node for a given command-line path, handling features like /// spell correction. @@ -105,17 +108,17 @@ struct NinjaMain : public BuildLogUser { vector<Node*>* targets, string* err); // The various subcommands, run via "-t XXX". - int ToolGraph(int argc, char* argv[]); - int ToolQuery(int argc, char* argv[]); - int ToolDeps(int argc, char* argv[]); - int ToolBrowse(int argc, char* argv[]); - int ToolMSVC(int argc, char* argv[]); - int ToolTargets(int argc, char* argv[]); - int ToolCommands(int argc, char* argv[]); - int ToolClean(int argc, char* argv[]); - int ToolCompilationDatabase(int argc, char* argv[]); - int ToolRecompact(int argc, char* argv[]); - int ToolUrtle(int argc, char** argv); + int ToolGraph(const Options* options, int argc, char* argv[]); + int ToolQuery(const Options* options, int argc, char* argv[]); + int ToolDeps(const Options* options, int argc, char* argv[]); + int ToolBrowse(const Options* options, int argc, char* argv[]); + int ToolMSVC(const Options* options, int argc, char* argv[]); + int ToolTargets(const Options* options, int argc, char* argv[]); + int ToolCommands(const Options* options, int argc, char* argv[]); + int ToolClean(const Options* options, int argc, char* argv[]); + int ToolCompilationDatabase(const Options* options, int argc, char* argv[]); + int ToolRecompact(const Options* options, int argc, char* argv[]); + int ToolUrtle(const Options* options, int argc, char** argv); /// Open the build log. /// @return false on error. @@ -226,14 +229,6 @@ int GuessParallelism() { } } -/// An implementation of ManifestParser::FileReader that actually reads -/// the file. -struct RealFileReader : public ManifestParser::FileReader { - virtual bool ReadFile(const string& path, string* content, string* err) { - return ::ReadFile(path, content, err) == 0; - } -}; - /// Rebuild the build manifest, if necessary. /// Returns true if the manifest was rebuilt. bool NinjaMain::RebuildManifest(const char* input_file, string* err) { @@ -254,13 +249,13 @@ bool NinjaMain::RebuildManifest(const char* input_file, string* err) { // Even if the manifest was cleaned by a restat rule, claim that it was // rebuilt. Not doing so can lead to crashes, see - // https://github.com/martine/ninja/issues/874 + // https://github.com/ninja-build/ninja/issues/874 return builder.Build(err); } Node* NinjaMain::CollectTarget(const char* cpath, string* err) { string path = cpath; - unsigned int slash_bits; // Unused because this path is only used for lookup. + unsigned int slash_bits; if (!CanonicalizePath(&path, &slash_bits, err)) return NULL; @@ -287,8 +282,8 @@ Node* NinjaMain::CollectTarget(const char* cpath, string* err) { } return node; } else { - *err = "unknown target '" + path + "'"; - + *err = + "unknown target '" + Node::PathDecanonicalized(path, slash_bits) + "'"; if (path == "clean") { *err += ", did you mean 'ninja -t clean'?"; } else if (path == "help") { @@ -319,7 +314,7 @@ bool NinjaMain::CollectTargetsFromArgs(int argc, char* argv[], return true; } -int NinjaMain::ToolGraph(int argc, char* argv[]) { +int NinjaMain::ToolGraph(const Options* options, int argc, char* argv[]) { vector<Node*> nodes; string err; if (!CollectTargetsFromArgs(argc, argv, &nodes, &err)) { @@ -336,7 +331,7 @@ int NinjaMain::ToolGraph(int argc, char* argv[]) { return 0; } -int NinjaMain::ToolQuery(int argc, char* argv[]) { +int NinjaMain::ToolQuery(const Options* options, int argc, char* argv[]) { if (argc == 0) { Error("expected a target to query"); return 1; @@ -375,19 +370,15 @@ int NinjaMain::ToolQuery(int argc, char* argv[]) { } #if defined(NINJA_HAVE_BROWSE) -int NinjaMain::ToolBrowse(int argc, char* argv[]) { - if (argc < 1) { - Error("expected a target to browse"); - return 1; - } - RunBrowsePython(&state_, ninja_command_, argv[0]); +int NinjaMain::ToolBrowse(const Options* options, int argc, char* argv[]) { + RunBrowsePython(&state_, ninja_command_, options->input_file, argc, argv); // If we get here, the browse failed. return 1; } #endif // _WIN32 #if defined(_MSC_VER) -int NinjaMain::ToolMSVC(int argc, char* argv[]) { +int NinjaMain::ToolMSVC(const Options* options, int argc, char* argv[]) { // Reset getopt: push one argument onto the front of argv, reset optind. argc++; argv--; @@ -462,7 +453,7 @@ int ToolTargetsList(State* state) { return 0; } -int NinjaMain::ToolDeps(int argc, char** argv) { +int NinjaMain::ToolDeps(const Options* options, int argc, char** argv) { vector<Node*> nodes; if (argc == 0) { for (vector<Node*>::const_iterator ni = deps_log_.nodes().begin(); @@ -502,7 +493,7 @@ int NinjaMain::ToolDeps(int argc, char** argv) { return 0; } -int NinjaMain::ToolTargets(int argc, char* argv[]) { +int NinjaMain::ToolTargets(const Options* options, int argc, char* argv[]) { int depth = 1; if (argc >= 1) { string mode = argv[0]; @@ -556,7 +547,7 @@ void PrintCommands(Edge* edge, set<Edge*>* seen) { puts(edge->EvaluateCommand().c_str()); } -int NinjaMain::ToolCommands(int argc, char* argv[]) { +int NinjaMain::ToolCommands(const Options* options, int argc, char* argv[]) { vector<Node*> nodes; string err; if (!CollectTargetsFromArgs(argc, argv, &nodes, &err)) { @@ -571,7 +562,7 @@ int NinjaMain::ToolCommands(int argc, char* argv[]) { return 0; } -int NinjaMain::ToolClean(int argc, char* argv[]) { +int NinjaMain::ToolClean(const Options* options, int argc, char* argv[]) { // The clean tool uses getopt, and expects argv[0] to contain the name of // the tool, i.e. "clean". argc++; @@ -629,7 +620,7 @@ void EncodeJSONString(const char *str) { } } -int NinjaMain::ToolCompilationDatabase(int argc, char* argv[]) { +int NinjaMain::ToolCompilationDatabase(const Options* options, int argc, char* argv[]) { bool first = true; vector<char> cwd; @@ -669,7 +660,7 @@ int NinjaMain::ToolCompilationDatabase(int argc, char* argv[]) { return 0; } -int NinjaMain::ToolRecompact(int argc, char* argv[]) { +int NinjaMain::ToolRecompact(const Options* options, int argc, char* argv[]) { if (!EnsureBuildDirExists()) return 1; @@ -680,7 +671,7 @@ int NinjaMain::ToolRecompact(int argc, char* argv[]) { return 0; } -int NinjaMain::ToolUrtle(int argc, char** argv) { +int NinjaMain::ToolUrtle(const Options* options, int argc, char** argv) { // RLE encoded. const char* urtle = " 13 ,3;2!2;\n8 ,;<11!;\n5 `'<10!(2`'2!\n11 ,6;, `\\. `\\9 .,c13$ec,.\n6 " @@ -698,7 +689,7 @@ int NinjaMain::ToolUrtle(int argc, char** argv) { if ('0' <= *p && *p <= '9') { count = count*10 + *p - '0'; } else { - for (int i = 0; i < std::max(count, 1); ++i) + for (int i = 0; i < max(count, 1); ++i) printf("%c", *p); count = 0; } @@ -771,9 +762,10 @@ const Tool* ChooseTool(const string& tool_name) { bool DebugEnable(const string& name) { if (name == "list") { printf("debugging modes:\n" -" stats print operation counts/timing info\n" -" explain explain what caused a command to execute\n" -" keeprsp don't delete @response files on success\n" +" stats print operation counts/timing info\n" +" explain explain what caused a command to execute\n" +" keepdepfile don't delete depfiles after they're read by ninja\n" +" keeprsp don't delete @response files on success\n" #ifdef _WIN32 " nostatcache don't batch stat() calls per directory and cache them\n" #endif @@ -785,6 +777,9 @@ bool DebugEnable(const string& name) { } else if (name == "explain") { g_explaining = true; return true; + } else if (name == "keepdepfile") { + g_keep_depfile = true; + return true; } else if (name == "keeprsp") { g_keep_rsp = true; return true; @@ -793,8 +788,9 @@ bool DebugEnable(const string& name) { return true; } else { const char* suggestion = - SpellcheckString(name.c_str(), "stats", "explain", "keeprsp", - "nostatcache", NULL); + SpellcheckString(name.c_str(), + "stats", "explain", "keepdepfile", "keeprsp", + "nostatcache", NULL); if (suggestion) { Error("unknown debug setting '%s', did you mean '%s'?", name.c_str(), suggestion); @@ -1101,7 +1097,7 @@ int real_main(int argc, char** argv) { // None of the RUN_AFTER_FLAGS actually use a NinjaMain, but it's needed // by other tools. NinjaMain ninja(ninja_command, config); - return (ninja.*options.tool->func)(argc, argv); + return (ninja.*options.tool->func)(&options, argc, argv); } // Limit number of rebuilds, to prevent infinite loops. @@ -1109,9 +1105,10 @@ int real_main(int argc, char** argv) { for (int cycle = 1; cycle <= kCycleLimit; ++cycle) { NinjaMain ninja(ninja_command, config); - RealFileReader file_reader; - ManifestParser parser(&ninja.state_, &file_reader, - options.dupe_edges_should_err); + ManifestParser parser(&ninja.state_, &ninja.disk_interface_, + options.dupe_edges_should_err + ? kDupeEdgeActionError + : kDupeEdgeActionWarn); string err; if (!parser.Load(options.input_file, &err)) { Error("%s", err.c_str()); @@ -1119,7 +1116,7 @@ int real_main(int argc, char** argv) { } if (options.tool && options.tool->when == Tool::RUN_AFTER_LOAD) - return (ninja.*options.tool->func)(argc, argv); + return (ninja.*options.tool->func)(&options, argc, argv); if (!ninja.EnsureBuildDirExists()) return 1; @@ -1128,10 +1125,14 @@ int real_main(int argc, char** argv) { return 1; if (options.tool && options.tool->when == Tool::RUN_AFTER_LOGS) - return (ninja.*options.tool->func)(argc, argv); + return (ninja.*options.tool->func)(&options, argc, argv); // Attempt to rebuild the manifest before building anything else if (ninja.RebuildManifest(options.input_file, &err)) { + // In dry_run mode the regeneration will succeed without changing the + // manifest forever. Better to return immediately. + if (config.dry_run) + return 0; // Start the build over with the new manifest. continue; } else if (!err.empty()) { @@ -1156,7 +1157,7 @@ int main(int argc, char** argv) { #if defined(_MSC_VER) // Set a handler to catch crashes not caught by the __try..__except // block (e.g. an exception in a stack-unwind-block). - set_terminate(TerminateHandler); + std::set_terminate(TerminateHandler); __try { // Running inside __try ... __except suppresses any Windows error // dialogs for errors such as bad_alloc. diff --git a/src/ninja_test.cc b/src/ninja_test.cc index 54d8784..d642c5c 100644 --- a/src/ninja_test.cc +++ b/src/ninja_test.cc @@ -14,9 +14,13 @@ #include <stdarg.h> #include <stdio.h> +#include <stdlib.h> #ifdef _WIN32 #include "getopt.h" +#elif defined(_AIX) +#include "getopt.h" +#include <unistd.h> #else #include <getopt.h> #endif diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index f3baec2..5ffe85b 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -22,6 +22,9 @@ #include <stdio.h> #include <string.h> #include <sys/wait.h> +#include <spawn.h> + +extern char** environ; #include "util.h" @@ -50,63 +53,60 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { #endif // !USE_PPOLL SetCloseOnExec(fd_); - pid_ = fork(); - if (pid_ < 0) - Fatal("fork: %s", strerror(errno)); - - if (pid_ == 0) { - close(output_pipe[0]); - - // Track which fd we use to report errors on. - int error_pipe = output_pipe[1]; - do { - if (sigaction(SIGINT, &set->old_int_act_, 0) < 0) - break; - if (sigaction(SIGTERM, &set->old_term_act_, 0) < 0) - break; - if (sigprocmask(SIG_SETMASK, &set->old_mask_, 0) < 0) - break; - - if (!use_console_) { - // Put the child in its own session and process group. It will be - // detached from the current terminal and ctrl-c won't reach it. - // Since this process was just forked, it is not a process group leader - // and setsid() will succeed. - if (setsid() < 0) - break; - - // Open /dev/null over stdin. - int devnull = open("/dev/null", O_RDONLY); - if (devnull < 0) - break; - if (dup2(devnull, 0) < 0) - break; - close(devnull); - - if (dup2(output_pipe[1], 1) < 0 || - dup2(output_pipe[1], 2) < 0) - break; - - // Now can use stderr for errors. - error_pipe = 2; - close(output_pipe[1]); - } - // In the console case, output_pipe is still inherited by the child and - // closed when the subprocess finishes, which then notifies ninja. - - execl("/bin/sh", "/bin/sh", "-c", command.c_str(), (char *) NULL); - } while (false); - - // If we get here, something went wrong; the execl should have - // replaced us. - char* err = strerror(errno); - if (write(error_pipe, err, strlen(err)) < 0) { - // If the write fails, there's nothing we can do. - // But this block seems necessary to silence the warning. + posix_spawn_file_actions_t action; + if (posix_spawn_file_actions_init(&action) != 0) + Fatal("posix_spawn_file_actions_init: %s", strerror(errno)); + + if (posix_spawn_file_actions_addclose(&action, output_pipe[0]) != 0) + Fatal("posix_spawn_file_actions_addclose: %s", strerror(errno)); + + posix_spawnattr_t attr; + if (posix_spawnattr_init(&attr) != 0) + Fatal("posix_spawnattr_init: %s", strerror(errno)); + + short flags = 0; + + flags |= POSIX_SPAWN_SETSIGMASK; + if (posix_spawnattr_setsigmask(&attr, &set->old_mask_) != 0) + Fatal("posix_spawnattr_setsigmask: %s", strerror(errno)); + // Signals which are set to be caught in the calling process image are set to + // default action in the new process image, so no explicit + // POSIX_SPAWN_SETSIGDEF parameter is needed. + + // TODO: Consider using POSIX_SPAWN_USEVFORK on Linux with glibc? + + if (!use_console_) { + // Put the child in its own process group, so ctrl-c won't reach it. + flags |= POSIX_SPAWN_SETPGROUP; + // No need to posix_spawnattr_setpgroup(&attr, 0), it's the default. + + // Open /dev/null over stdin. + if (posix_spawn_file_actions_addopen(&action, 0, "/dev/null", O_RDONLY, + 0) != 0) { + Fatal("posix_spawn_file_actions_addopen: %s", strerror(errno)); } - _exit(1); + + if (posix_spawn_file_actions_adddup2(&action, output_pipe[1], 1) != 0) + Fatal("posix_spawn_file_actions_adddup2: %s", strerror(errno)); + if (posix_spawn_file_actions_adddup2(&action, output_pipe[1], 2) != 0) + Fatal("posix_spawn_file_actions_adddup2: %s", strerror(errno)); + // In the console case, output_pipe is still inherited by the child and + // closed when the subprocess finishes, which then notifies ninja. } + if (posix_spawnattr_setflags(&attr, flags) != 0) + Fatal("posix_spawnattr_setflags: %s", strerror(errno)); + + const char* spawned_args[] = { "/bin/sh", "-c", command.c_str(), NULL }; + if (posix_spawn(&pid_, "/bin/sh", &action, &attr, + const_cast<char**>(spawned_args), environ) != 0) + Fatal("posix_spawn: %s", strerror(errno)); + + if (posix_spawnattr_destroy(&attr) != 0) + Fatal("posix_spawnattr_destroy: %s", strerror(errno)); + if (posix_spawn_file_actions_destroy(&action) != 0) + Fatal("posix_spawn_file_actions_destroy: %s", strerror(errno)); + close(output_pipe[1]); return true; } @@ -136,7 +136,8 @@ ExitStatus Subprocess::Finish() { if (exit == 0) return ExitSuccess; } else if (WIFSIGNALED(status)) { - if (WTERMSIG(status) == SIGINT || WTERMSIG(status) == SIGTERM) + if (WTERMSIG(status) == SIGINT || WTERMSIG(status) == SIGTERM + || WTERMSIG(status) == SIGHUP) return ExitInterrupted; } return ExitFailure; @@ -167,6 +168,8 @@ void SubprocessSet::HandlePendingInterruption() { interrupted_ = SIGINT; else if (sigismember(&pending, SIGTERM)) interrupted_ = SIGTERM; + else if (sigismember(&pending, SIGHUP)) + interrupted_ = SIGHUP; } SubprocessSet::SubprocessSet() { @@ -174,6 +177,7 @@ SubprocessSet::SubprocessSet() { sigemptyset(&set); sigaddset(&set, SIGINT); sigaddset(&set, SIGTERM); + sigaddset(&set, SIGHUP); if (sigprocmask(SIG_BLOCK, &set, &old_mask_) < 0) Fatal("sigprocmask: %s", strerror(errno)); @@ -184,6 +188,8 @@ SubprocessSet::SubprocessSet() { Fatal("sigaction: %s", strerror(errno)); if (sigaction(SIGTERM, &act, &old_term_act_) < 0) Fatal("sigaction: %s", strerror(errno)); + if (sigaction(SIGHUP, &act, &old_hup_act_) < 0) + Fatal("sigaction: %s", strerror(errno)); } SubprocessSet::~SubprocessSet() { @@ -193,6 +199,8 @@ SubprocessSet::~SubprocessSet() { Fatal("sigaction: %s", strerror(errno)); if (sigaction(SIGTERM, &old_term_act_, 0) < 0) Fatal("sigaction: %s", strerror(errno)); + if (sigaction(SIGHUP, &old_hup_act_, 0) < 0) + Fatal("sigaction: %s", strerror(errno)); if (sigprocmask(SIG_SETMASK, &old_mask_, 0) < 0) Fatal("sigprocmask: %s", strerror(errno)); } diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index fad66e8..4bab719 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -255,7 +255,7 @@ bool SubprocessSet::DoWork() { if (subproc->Done()) { vector<Subprocess*>::iterator end = - std::remove(running_.begin(), running_.end(), subproc); + remove(running_.begin(), running_.end(), subproc); if (running_.end() != end) { finished_.push(subproc); running_.resize(end - running_.begin()); diff --git a/src/subprocess.h b/src/subprocess.h index a001fc9..51f40b2 100644 --- a/src/subprocess.h +++ b/src/subprocess.h @@ -98,6 +98,7 @@ struct SubprocessSet { struct sigaction old_int_act_; struct sigaction old_term_act_; + struct sigaction old_hup_act_; sigset_t old_mask_; #endif }; diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index 07cc52f..ee16190 100644 --- a/src/subprocess_test.cc +++ b/src/subprocess_test.cc @@ -16,8 +16,6 @@ #include "test.h" -#include <string> - #ifndef _WIN32 // SetWithLots need setrlimit. #include <stdio.h> @@ -122,22 +120,35 @@ TEST_F(SubprocessTest, InterruptParentWithSigTerm) { ASSERT_FALSE("We should have been interrupted"); } -// A shell command to check if the current process is connected to a terminal. -// This is different from having stdin/stdout/stderr be a terminal. (For -// instance consider the command "yes < /dev/null > /dev/null 2>&1". -// As "ps" will confirm, "yes" could still be connected to a terminal, despite -// not having any of the standard file descriptors be a terminal. -static const char kIsConnectedToTerminal[] = "tty < /dev/tty > /dev/null"; +TEST_F(SubprocessTest, InterruptChildWithSigHup) { + Subprocess* subproc = subprocs_.Add("kill -HUP $$"); + ASSERT_NE((Subprocess *) 0, subproc); + + while (!subproc->Done()) { + subprocs_.DoWork(); + } + + EXPECT_EQ(ExitInterrupted, subproc->Finish()); +} + +TEST_F(SubprocessTest, InterruptParentWithSigHup) { + Subprocess* subproc = subprocs_.Add("kill -HUP $PPID ; sleep 1"); + ASSERT_NE((Subprocess *) 0, subproc); + + while (!subproc->Done()) { + bool interrupted = subprocs_.DoWork(); + if (interrupted) + return; + } + + ASSERT_FALSE("We should have been interrupted"); +} TEST_F(SubprocessTest, Console) { // Skip test if we don't have the console ourselves. if (isatty(0) && isatty(1) && isatty(2)) { - // Test that stdin, stdout and stderr are a terminal. - // Also check that the current process is connected to a terminal. Subprocess* subproc = - subprocs_.Add(std::string("test -t 0 -a -t 1 -a -t 2 && ") + - std::string(kIsConnectedToTerminal), - /*use_console=*/true); + subprocs_.Add("test -t 0 -a -t 1 -a -t 2", /*use_console=*/true); ASSERT_NE((Subprocess*)0, subproc); while (!subproc->Done()) { @@ -148,18 +159,6 @@ TEST_F(SubprocessTest, Console) { } } -TEST_F(SubprocessTest, NoConsole) { - Subprocess* subproc = - subprocs_.Add(kIsConnectedToTerminal, /*use_console=*/false); - ASSERT_NE((Subprocess*)0, subproc); - - while (!subproc->Done()) { - subprocs_.DoWork(); - } - - EXPECT_NE(ExitSuccess, subproc->Finish()); -} - #endif TEST_F(SubprocessTest, SetWithSingle) { @@ -227,7 +226,8 @@ TEST_F(SubprocessTest, SetWithLots) { rlimit rlim; ASSERT_EQ(0, getrlimit(RLIMIT_NOFILE, &rlim)); if (rlim.rlim_cur < kNumProcs) { - printf("Raise [ulimit -n] well above %u (currently %lu) to make this test go\n", kNumProcs, rlim.rlim_cur); + printf("Raise [ulimit -n] above %u (currently %lu) to make this test go\n", + kNumProcs, rlim.rlim_cur); return; } diff --git a/src/test.cc b/src/test.cc index aed8db7..51882f0 100644 --- a/src/test.cc +++ b/src/test.cc @@ -21,6 +21,7 @@ #include <algorithm> #include <errno.h> +#include <stdlib.h> #ifdef _WIN32 #include <windows.h> #else @@ -95,7 +96,7 @@ Node* StateTestWithBuiltinRules::GetNode(const string& path) { } void AssertParse(State* state, const char* input) { - ManifestParser parser(state, NULL); + ManifestParser parser(state, NULL, kDupeEdgeActionWarn); string err; EXPECT_TRUE(parser.ParseTest(input, &err)); ASSERT_EQ("", err); @@ -115,7 +116,7 @@ void VerifyGraph(const State& state) { for (vector<Node*>::const_iterator in_node = (*e)->inputs_.begin(); in_node != (*e)->inputs_.end(); ++in_node) { const vector<Edge*>& out_edges = (*in_node)->out_edges(); - EXPECT_NE(std::find(out_edges.begin(), out_edges.end(), *e), + EXPECT_NE(find(out_edges.begin(), out_edges.end(), *e), out_edges.end()); } // Check that the edge's outputs have the edge as in-edge. @@ -164,12 +165,17 @@ bool VirtualFileSystem::MakeDir(const string& path) { return true; // success } -string VirtualFileSystem::ReadFile(const string& path, string* err) { +FileReader::Status VirtualFileSystem::ReadFile(const string& path, + string* contents, + string* err) { files_read_.push_back(path); FileMap::iterator i = files_.find(path); - if (i != files_.end()) - return i->second.contents; - return ""; + if (i != files_.end()) { + *contents = i->second.contents; + return Okay; + } + *err = strerror(ENOENT); + return NotFound; } int VirtualFileSystem::RemoveFile(const string& path) { @@ -93,14 +93,14 @@ extern testing::Test* g_current_test; if (!EXPECT_TRUE(a)) { g_current_test->AddAssertionFailure(); return; } #define ASSERT_FALSE(a) \ if (!EXPECT_FALSE(a)) { g_current_test->AddAssertionFailure(); return; } -#define ASSERT_NO_FATAL_FAILURE(a) \ - { \ - int f = g_current_test->AssertionFailures(); \ - a; \ - if (f != g_current_test->AssertionFailures()) { \ - g_current_test->AddAssertionFailure(); \ - return; \ - } \ +#define ASSERT_NO_FATAL_FAILURE(a) \ + { \ + int fail_count = g_current_test->AssertionFailures(); \ + a; \ + if (fail_count != g_current_test->AssertionFailures()) { \ + g_current_test->AddAssertionFailure(); \ + return; \ + } \ } // Support utilites for tests. @@ -145,7 +145,7 @@ struct VirtualFileSystem : public DiskInterface { virtual TimeStamp Stat(const string& path, string* err) const; virtual bool WriteFile(const string& path, const string& contents); virtual bool MakeDir(const string& path); - virtual string ReadFile(const string& path, string* err); + virtual Status ReadFile(const string& path, string* contents, string* err); virtual int RemoveFile(const string& path); /// An entry for a single in-memory file. diff --git a/src/util.cc b/src/util.cc index aa47f2f..e31fd1f 100644 --- a/src/util.cc +++ b/src/util.cc @@ -45,6 +45,8 @@ #elif defined(__SVR4) && defined(__sun) #include <unistd.h> #include <sys/loadavg.h> +#elif defined(_AIX) +#include <libperfstat.h> #elif defined(linux) || defined(__GLIBC__) #include <sys/sysinfo.h> #endif @@ -224,8 +226,8 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, } if (dst == start) { - *err = "path canonicalizes to the empty path"; - return false; + *dst++ = '.'; + *dst++ = '\0'; } *len = dst - start - 1; @@ -573,6 +575,16 @@ double GetLoadAverage() { return posix_compatible_load; } +#elif defined(_AIX) +double GetLoadAverage() { + perfstat_cpu_total_t cpu_stats; + if (perfstat_cpu_total(NULL, &cpu_stats, sizeof(cpu_stats), 1) < 0) { + return -0.0f; + } + + // Calculation taken from comment in libperfstats.h + return double(cpu_stats.loadavg[0]) / double(1 << SBITS); +} #else double GetLoadAverage() { double loadavg[3] = { 0.0f, 0.0f, 0.0f }; diff --git a/src/util_test.cc b/src/util_test.cc index 8ca7f56..33a4107 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -91,6 +91,22 @@ TEST(CanonicalizePath, PathSamples) { path = "/"; EXPECT_TRUE(CanonicalizePath(&path, &err)); EXPECT_EQ("", path); + + path = "/foo/.."; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("", path); + + path = "."; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ(".", path); + + path = "./."; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ(".", path); + + path = "foo/.."; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ(".", path); } #ifdef _WIN32 @@ -288,22 +304,6 @@ TEST(CanonicalizePath, TooManyComponents) { } #endif -TEST(CanonicalizePath, EmptyResult) { - string path; - string err; - - EXPECT_FALSE(CanonicalizePath(&path, &err)); - EXPECT_EQ("empty path", err); - - path = "."; - EXPECT_FALSE(CanonicalizePath(&path, &err)); - EXPECT_EQ("path canonicalizes to the empty path", err); - - path = "./."; - EXPECT_FALSE(CanonicalizePath(&path, &err)); - EXPECT_EQ("path canonicalizes to the empty path", err); -} - TEST(CanonicalizePath, UpDir) { string path, err; path = "../../foo/bar.h"; diff --git a/src/version.cc b/src/version.cc index 14d43f0..0a01238 100644 --- a/src/version.cc +++ b/src/version.cc @@ -18,7 +18,7 @@ #include "util.h" -const char* kNinjaVersion = "1.6.0"; +const char* kNinjaVersion = "1.7.0"; void ParseVersion(const string& version, int* major, int* minor) { size_t end = version.find('.'); |