diff options
-rw-r--r-- | HACKING.md | 4 | ||||
-rwxr-xr-x | configure.py | 12 | ||||
-rw-r--r-- | doc/manual.asciidoc | 23 | ||||
-rw-r--r-- | misc/ninja_syntax.py | 7 | ||||
-rwxr-xr-x | misc/ninja_syntax_test.py | 7 | ||||
-rw-r--r-- | src/browse.cc | 18 | ||||
-rw-r--r-- | src/browse.h | 5 | ||||
-rwxr-xr-x | src/browse.py | 29 | ||||
-rw-r--r-- | src/build.cc | 4 | ||||
-rw-r--r-- | src/clparser.cc | 116 | ||||
-rw-r--r-- | src/clparser.h | 52 | ||||
-rw-r--r-- | src/clparser_test.cc | 117 | ||||
-rw-r--r-- | src/graph.cc | 3 | ||||
-rw-r--r-- | src/msvc_helper-win32.cc | 83 | ||||
-rw-r--r-- | src/msvc_helper.h | 32 | ||||
-rw-r--r-- | src/msvc_helper_main-win32.cc | 1 | ||||
-rw-r--r-- | src/msvc_helper_test.cc | 94 | ||||
-rw-r--r-- | src/ninja.cc | 8 | ||||
-rw-r--r-- | src/subprocess-posix.cc | 110 | ||||
-rw-r--r-- | src/subprocess_test.cc | 30 |
20 files changed, 420 insertions, 335 deletions
@@ -56,8 +56,8 @@ particular, new build file syntax or command-line flags) or increase the maintenance burden of Ninja. Ninja is already successfully used by hundreds of developers for large projects and it already achieves (most of) the goals I set out for it to do. It's probably best to -discuss new feature ideas on the mailing list before I shoot down your -patch. +discuss new feature ideas on the [mailing list](https://groups.google.com/forum/#!forum/ninja-build) +before I shoot down your patch. ## Testing diff --git a/configure.py b/configure.py index 1c97db7..84218b9 100755 --- a/configure.py +++ b/configure.py @@ -133,7 +133,9 @@ class Bootstrap: return self.writer.newline() def variable(self, key, val): - self.vars[key] = self._expand(val) + # In bootstrap mode, we have no ninja process to catch /showIncludes + # output. + self.vars[key] = self._expand(val).replace('/showIncludes', '') return self.writer.variable(key, val) def rule(self, name, **kwargs): @@ -302,6 +304,8 @@ if platform.is_msvc(): '/WX', # Warnings as errors. '/wd4530', '/wd4100', '/wd4706', '/wd4512', '/wd4800', '/wd4702', '/wd4819', + # Disable warnings about constant conditional expressions. + '/wd4127', # Disable warnings about passing "this" during initialization. '/wd4355', # Disable warnings about ignored typedef in DbgHelp.h @@ -313,10 +317,6 @@ if platform.is_msvc(): '/DNOMINMAX', '/D_CRT_SECURE_NO_WARNINGS', '/D_HAS_EXCEPTIONS=0', '/DNINJA_PYTHON="%s"' % options.with_python] - if options.bootstrap: - # In bootstrap mode, we have no ninja process to catch /showIncludes - # output. - cflags.remove('/showIncludes') if platform.msvc_needs_fs(): cflags.append('/FS') ldflags = ['/DEBUG', '/libpath:$builddir'] @@ -475,6 +475,7 @@ n.comment('Core source files all build into ninja library.') for name in ['build', 'build_log', 'clean', + 'clparser', 'debug_flags', 'depfile_parser', 'deps_log', @@ -540,6 +541,7 @@ objs = [] for name in ['build_log_test', 'build_test', 'clean_test', + 'clparser_test', 'depfile_parser_test', 'deps_log_test', 'disk_interface_test', diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 4e73df3..9fc5fb9 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -158,17 +158,13 @@ http://code.google.com/p/gyp/[gyp]:: The meta-build system used to generate build files for Google Chrome and related projects (v8, node.js). gyp can generate Ninja files for all platforms supported by Chrome. See the -http://code.google.com/p/chromium/wiki/NinjaBuild[Chromium Ninja -documentation for more details]. +https://chromium.googlesource.com/chromium/src/+/master/docs/ninja_build.md[Chromium Ninja documentation for more details]. -http://www.cmake.org/[CMake]:: A widely used meta-build system that -can generate Ninja files on Linux as of CMake version 2.8.8. (There -is some Mac and Windows support -- http://www.reactos.org[ReactOS] -uses Ninja on Windows for their buildbots, but those platforms are not -yet officially supported by CMake as the full test suite doesn't -pass.) +https://cmake.org/[CMake]:: A widely used meta-build system that +can generate Ninja files on Linux as of CMake version 2.8.8. Newer versions +of CMake support generating Ninja files on Windows and Mac OS X too. -others:: Ninja ought to fit perfectly into other meta-build software +https://github.com/ninja-build/ninja/wiki/List-of-generators-producing-ninja-build-files[others]:: Ninja ought to fit perfectly into other meta-build software like http://industriousone.com/premake[premake]. If you do this work, please let us know! @@ -227,8 +223,13 @@ found useful during Ninja's development. The current tools are: `browse`:: browse the dependency graph in a web browser. Clicking a file focuses the view on that file, showing inputs and outputs. This -feature requires a Python installation. - +feature requires a Python installation. By default port 8000 is used +and a web browser will be opened. This can be changed as follows: ++ +---- +ninja -t browse --port=8000 --no-browser mytarget +---- ++ `graph`:: output a file in the syntax used by `graphviz`, a automatic graph layout tool. Use it like: + diff --git a/misc/ninja_syntax.py b/misc/ninja_syntax.py index 73d2209..5c52ea2 100644 --- a/misc/ninja_syntax.py +++ b/misc/ninja_syntax.py @@ -60,7 +60,7 @@ class Writer(object): self.variable('deps', deps, indent=1) def build(self, outputs, rule, inputs=None, implicit=None, order_only=None, - variables=None): + variables=None, implicit_outputs=None): outputs = as_list(outputs) out_outputs = [escape_path(x) for x in outputs] all_inputs = [escape_path(x) for x in as_list(inputs)] @@ -73,6 +73,11 @@ class Writer(object): order_only = [escape_path(x) for x in as_list(order_only)] all_inputs.append('||') all_inputs.extend(order_only) + if implicit_outputs: + implicit_outputs = [escape_path(x) + for x in as_list(implicit_outputs)] + out_outputs.append('|') + out_outputs.extend(implicit_outputs) self._line('build %s: %s' % (' '.join(out_outputs), ' '.join([rule] + all_inputs))) diff --git a/misc/ninja_syntax_test.py b/misc/ninja_syntax_test.py index c9755b8..07e3ed3 100755 --- a/misc/ninja_syntax_test.py +++ b/misc/ninja_syntax_test.py @@ -154,6 +154,13 @@ build out: cc in ''', self.out.getvalue()) + def test_implicit_outputs(self): + self.n.build('o', 'cc', 'i', implicit_outputs='io') + self.assertEqual('''\ +build o | io: cc i +''', + self.out.getvalue()) + class TestExpand(unittest.TestCase): def test_basic(self): vars = {'x': 'X'} diff --git a/src/browse.cc b/src/browse.cc index 8673919..46434d7 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" void RunBrowsePython(State* state, const char* ninja_command, - const char* initial_target) { + 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,16 @@ 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); + 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..842c6ff 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); + int argc, char* argv[]); #endif // NINJA_BROWSE_H_ diff --git a/src/browse.py b/src/browse.py index 9e59bd8..63c60aa 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,7 +150,7 @@ def generate_html(node): return '\n'.join(document) def ninja_dump(target): - proc = subprocess.Popen([sys.argv[1], '-t', 'query', target], + proc = subprocess.Popen([args.ninja_command, '-t', 'query', target], stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) return proc.communicate() + (proc.returncode,) @@ -154,11 +158,11 @@ def ninja_dump(target): 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,26 @@ 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('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 0f3c8ef..f6f022e 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" @@ -857,7 +857,6 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, const string& deps_prefix, vector<Node*>* deps_nodes, string* err) { -#ifdef _WIN32 if (deps_type == "msvc") { CLParser parser; string output; @@ -873,7 +872,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()) { 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/graph.cc b/src/graph.cc index 98a2c18..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; diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index d516240..e37a26e 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -14,23 +14,12 @@ #include "msvc_helper.h" -#include <algorithm> -#include <assert.h> -#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; @@ -48,78 +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"); -} - -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; - if (!IncludesNormalize::Normalize(include, NULL, &normalized, err)) - return false; - 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; -} - 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 30f87f3..70d1fff 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -13,42 +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, 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_; -}; - /// 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 680aaad..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" diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 49d27c1..eaae51f 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -17,99 +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, 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; - ASSERT_TRUE(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", - "", &output, &err)); - // 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 a3f1be0..b3b9bed 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -371,11 +371,7 @@ 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]); + RunBrowsePython(&state_, ninja_command_, argc, argv); // If we get here, the browse failed. return 1; } @@ -1161,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/subprocess-posix.cc b/src/subprocess-posix.cc index 2ddc709..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,65 +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 (sigaction(SIGHUP, &set->old_hup_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; } diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index 2fe4bce..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> @@ -146,22 +144,11 @@ TEST_F(SubprocessTest, InterruptParentWithSigHup) { 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, 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(string("test -t 0 -a -t 1 -a -t 2 && ") + - 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()) { @@ -172,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) { @@ -251,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; } |