From 86c4b0618bf714367096c8b7661bde068b02e8fa Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Sat, 3 May 2014 16:07:09 -0700 Subject: [PATCH 01/84] Process the -C option before running RUN_AFTER_FLAGS tools. This brings the flag more inline with its description: "change to DIR before doing anything else". The use case is to make it possible to use -C together with -t msvc. When debugging Windows builds, it's handy to be able to copy-paste the commands from "ninja -v" and make them run in the correct directory by adding this flag. --- src/ninja.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/ninja.cc b/src/ninja.cc index 03ca83b544..25eb3a50ac 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -161,7 +161,8 @@ struct Tool { /// When to run the tool. enum { - /// Run after parsing the command-line flags (as early as possible). + /// Run after parsing the command-line flags and potentially changing + /// the current working directory (as early as possible). RUN_AFTER_FLAGS, /// Run after loading build.ninja. @@ -1026,13 +1027,6 @@ int real_main(int argc, char** argv) { if (exit_code >= 0) return exit_code; - if (options.tool && options.tool->when == Tool::RUN_AFTER_FLAGS) { - // 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); - } - if (options.working_dir) { // The formatting of this string, complete with funny quotes, is // so Emacs can properly identify that the cwd has changed for @@ -1046,6 +1040,13 @@ int real_main(int argc, char** argv) { } } + if (options.tool && options.tool->when == Tool::RUN_AFTER_FLAGS) { + // 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); + } + // The build can take up to 2 passes: one to rebuild the manifest, then // another to build the desired target. for (int cycle = 0; cycle < 2; ++cycle) { From 44a0d08a5215f949ae81014f5c5a283918e14a3b Mon Sep 17 00:00:00 2001 From: Demetri Obenour Date: Sun, 1 Jun 2014 11:29:18 -0400 Subject: [PATCH 02/84] Added highlighting in Emacs for ${...} variables --- misc/ninja-mode.el | 1 + 1 file changed, 1 insertion(+) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 9fe6fc8cb5..777d4adfc4 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -31,6 +31,7 @@ '("\\([[:alnum:]_]+\\) =" . (1 font-lock-variable-name-face)) ;; Variable expansion. '("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) + '("\\(${[[:alnum:]_]+}\\)" . (1 font-lock-variable-name-face)) ;; Rule names '("rule \\([[:alnum:]_]+\\)" . (1 font-lock-function-name-face)) )) From 4e0e6c58b4dd43b3f7fa28d66e0f98bda85d7495 Mon Sep 17 00:00:00 2001 From: Demetri Obenour Date: Sun, 1 Jun 2014 13:39:48 -0400 Subject: [PATCH 03/84] Removed tab and added . in variable name regexp --- misc/ninja-mode.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 777d4adfc4..f1ebe2ab63 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -31,7 +31,7 @@ '("\\([[:alnum:]_]+\\) =" . (1 font-lock-variable-name-face)) ;; Variable expansion. '("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) - '("\\(${[[:alnum:]_]+}\\)" . (1 font-lock-variable-name-face)) + '("\\(${[[:alnum:]._]+}\\)" . (1 font-lock-variable-name-face)) ;; Rule names '("rule \\([[:alnum:]_]+\\)" . (1 font-lock-function-name-face)) )) From 781aa24ba5251e6225c72a3cc74b77b7a50504b1 Mon Sep 17 00:00:00 2001 From: donkopotamus Date: Fri, 27 Jun 2014 12:26:37 +1200 Subject: [PATCH 04/84] Add highlighting of rule in build statements Highlight the rule being used in a build statement. Also add `.` to acceptable characters in a rule name and relax whitespace matching before the name. --- misc/ninja-mode.el | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 36ada6f7a6..80585d846d 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -32,7 +32,11 @@ ;; Variable expansion. '("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) ;; Rule names - '("rule \\([[:alnum:]_-]+\\)" . (1 font-lock-function-name-face)) + '("rule +\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) + ;; Build Statement - highlight the rule used, allow for escaped $,: in outputs + '("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . + (1 font-lock-function-name-face)) + )) ;;;###autoload From 9138007a916fe96274782dbdd02c45d7759caf8c Mon Sep 17 00:00:00 2001 From: Ryo ONODERA Date: Sat, 19 Jul 2014 18:45:25 +0900 Subject: [PATCH 05/84] Add NetBSD support. It works fine under NetBSD/amd64 6.99.47. --- platform_helper.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/platform_helper.py b/platform_helper.py index bc3a12525f..eab82626b8 100644 --- a/platform_helper.py +++ b/platform_helper.py @@ -19,7 +19,7 @@ def platforms(): return ['linux', 'darwin', 'freebsd', 'openbsd', 'solaris', 'sunos5', - 'mingw', 'msvc', 'gnukfreebsd', 'bitrig'] + 'mingw', 'msvc', 'gnukfreebsd', 'bitrig', 'netbsd'] class Platform(object): def __init__(self, platform): @@ -43,6 +43,8 @@ def __init__(self, platform): self._platform = 'msvc' elif self._platform.startswith('bitrig'): self._platform = 'bitrig' + elif self._platform.startswith('netbsd'): + self._platform = 'netbsd' def platform(self): return self._platform @@ -81,3 +83,6 @@ def is_sunos5(self): def is_bitrig(self): return self._platform == 'bitrig' + + def is_netbsd(self): + return self._platform == 'netbsd' From 395f4c8873e784131dab6ee67133f80d03b23e4a Mon Sep 17 00:00:00 2001 From: kwesolowski Date: Fri, 25 Jul 2014 00:05:31 +0200 Subject: [PATCH 06/84] Prepared load (-l N) support for windows. Inspired by: http://stackoverflow.com/questions/23143693/retrieving-cpu-load-percent-total-in-windows-with-c --- src/util.cc | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/src/util.cc b/src/util.cc index 484b0c158a..9d0282fa4a 100644 --- a/src/util.cc +++ b/src/util.cc @@ -415,10 +415,51 @@ int GetProcessorCount() { } #if defined(_WIN32) || defined(__CYGWIN__) +static double CalculateProcessorLoad(uint64_t idleTicks, uint64_t totalTicks) +{ + static uint64_t previousIdleTicks = 0; + static uint64_t previousTotalTicks = 0; + + uint64_t idleTicksSinceLastTime = idleTicks - previousIdleTicks; + uint64_t totalTicksSinceLastTime = totalTicks - previousTotalTicks; + + double load; + if( previousTotalTicks > 0) { + //return error for first call + load = -0.0; + } else if(totalTicksSinceLastTime == 0) { + //return error when load cannot be calculated + load = -0.0; + } else { + double idleToTotalRatio = ((double)idleTicksSinceLastTime) / totalTicksSinceLastTime; + load = 1.0 - idleToTotalRatio; + } + + previousTotalTicks = totalTicks; + previousIdleTicks = idleTicks; + return load; +} + +static uint64_t FileTimeToTickCount(const FILETIME & ft) +{ + uint64_t high = (((uint64_t)(ft.dwHighDateTime)) << 32); + uint64_t low = ft.dwLowDateTime; + return (high | low); +} + double GetLoadAverage() { - // TODO(nicolas.despres@gmail.com): Find a way to implement it on Windows. - // Remember to also update Usage() when this is fixed. - return -0.0f; + FILETIME idleTime, kernelTime, userTime; + BOOL getSystemTimeSucceeded = GetSystemTimes(&idleTime, &kernelTime, &userTime); + + double result; + if (getSystemTimeSucceeded) { + uint64_t idleTicks = FileTimeToTickCount(idleTime); + uint64_t totalTicks = FileTimeToTickCount(kernelTime) + FileTimeToTickCount(userTime); + result = CalculateProcessorLoad(idleTicks, totalTicks); + } else { + result = -0.0; + } + return result; } #else double GetLoadAverage() { From 2d377f62ea0a4989bc8a7f5ed2af53b6cd1cc17d Mon Sep 17 00:00:00 2001 From: kwesolowski Date: Fri, 25 Jul 2014 09:42:33 +0200 Subject: [PATCH 07/84] Fixed/improved -l N documentation For windows On windows "system load" is not so obvious term, added line explaining N argument in -l N --- src/ninja.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ninja.cc b/src/ninja.cc index 5dfcbf4d07..735c449df2 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -194,7 +194,7 @@ void Usage(const BuildConfig& config) { " -j N run N jobs in parallel [default=%d, derived from CPUs available]\n" " -l N do not start new jobs if the load average is greater than N\n" #ifdef _WIN32 -" (not yet implemented on Windows)\n" +" (N can be between 0.0 for idle processors to 1.0 for all processors 100% busy)\n" #endif " -k N keep going until N jobs fail [default=1]\n" " -n dry run (don't run commands but act like they succeeded)\n" From 9eca61629030613591a6b63052c2ccd0310c725b Mon Sep 17 00:00:00 2001 From: kwesolowski Date: Fri, 25 Jul 2014 10:36:21 +0200 Subject: [PATCH 08/84] Fixes for windows CalculateProcessorLoad - Fixed bad logic condition, - Added comment to clarify --- src/util.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util.cc b/src/util.cc index 9d0282fa4a..33cf2045aa 100644 --- a/src/util.cc +++ b/src/util.cc @@ -424,7 +424,7 @@ static double CalculateProcessorLoad(uint64_t idleTicks, uint64_t totalTicks) uint64_t totalTicksSinceLastTime = totalTicks - previousTotalTicks; double load; - if( previousTotalTicks > 0) { + if (previousTotalTicks == 0) { //return error for first call load = -0.0; } else if(totalTicksSinceLastTime == 0) { @@ -454,7 +454,10 @@ double GetLoadAverage() { double result; if (getSystemTimeSucceeded) { uint64_t idleTicks = FileTimeToTickCount(idleTime); + + //kernelTime from GetSystemTimes already includes idleTime uint64_t totalTicks = FileTimeToTickCount(kernelTime) + FileTimeToTickCount(userTime); + result = CalculateProcessorLoad(idleTicks, totalTicks); } else { result = -0.0; From 23f81e0f23c769dfcf78af48aad0a85b35e36983 Mon Sep 17 00:00:00 2001 From: kwesolowski Date: Fri, 25 Jul 2014 11:22:20 +0200 Subject: [PATCH 09/84] Small usage info fix for windows --- src/ninja.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ninja.cc b/src/ninja.cc index 735c449df2..04b3689331 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -194,7 +194,7 @@ void Usage(const BuildConfig& config) { " -j N run N jobs in parallel [default=%d, derived from CPUs available]\n" " -l N do not start new jobs if the load average is greater than N\n" #ifdef _WIN32 -" (N can be between 0.0 for idle processors to 1.0 for all processors 100% busy)\n" +" (N can be between 0.0 for all processors being IDLE to 1.0 for all processors 100%% busy)\n" #endif " -k N keep going until N jobs fail [default=1]\n" " -n dry run (don't run commands but act like they succeeded)\n" From 015d5edcbcaa41057383952d17f2b03a8bdf1c03 Mon Sep 17 00:00:00 2001 From: kwesolowski Date: Fri, 25 Jul 2014 11:24:45 +0200 Subject: [PATCH 10/84] Improved load calculation Added code to gracefully handle: 1. Call to CalculateProcessorLoad with not incremented ticks (fast calls to GetSystemTimes can result same results), 2. Smooth/filter load estimation for consecutive calls. --- src/util.cc | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/util.cc b/src/util.cc index 33cf2045aa..89ed198d21 100644 --- a/src/util.cc +++ b/src/util.cc @@ -419,24 +419,34 @@ static double CalculateProcessorLoad(uint64_t idleTicks, uint64_t totalTicks) { static uint64_t previousIdleTicks = 0; static uint64_t previousTotalTicks = 0; - + static double previousLoad = -0.0f; + uint64_t idleTicksSinceLastTime = idleTicks - previousIdleTicks; uint64_t totalTicksSinceLastTime = totalTicks - previousTotalTicks; + bool firstCall = (previousTotalTicks == 0); + bool ticksNotUpdatedSinceLastCall = (totalTicksSinceLastTime == 0); + double load; - if (previousTotalTicks == 0) { - //return error for first call - load = -0.0; - } else if(totalTicksSinceLastTime == 0) { - //return error when load cannot be calculated - load = -0.0; + if (firstCall || ticksNotUpdatedSinceLastCall) { + load = previousLoad; } else { + //calculate load double idleToTotalRatio = ((double)idleTicksSinceLastTime) / totalTicksSinceLastTime; - load = 1.0 - idleToTotalRatio; + double loadSinceLastCall = 1.0 - idleToTotalRatio; + + //filter/smooth result when possible + if(previousLoad > 0) { + load = 0.9 * previousLoad + 0.1 * loadSinceLastCall; + } else { + load = loadSinceLastCall; + } } - + + previousLoad = load; previousTotalTicks = totalTicks; previousIdleTicks = idleTicks; + return load; } @@ -462,6 +472,7 @@ double GetLoadAverage() { } else { result = -0.0; } + return result; } #else From 43879bb63cab1dd6236f04d1ece5724de283193b Mon Sep 17 00:00:00 2001 From: kwesolowski Date: Sat, 26 Jul 2014 10:01:29 +0200 Subject: [PATCH 11/84] Fixed naming convention in GetLoadAverage support functions. --- src/util.cc | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/util.cc b/src/util.cc index 89ed198d21..66ddcb1029 100644 --- a/src/util.cc +++ b/src/util.cc @@ -415,37 +415,37 @@ int GetProcessorCount() { } #if defined(_WIN32) || defined(__CYGWIN__) -static double CalculateProcessorLoad(uint64_t idleTicks, uint64_t totalTicks) +static double CalculateProcessorLoad(uint64_t idle_ticks, uint64_t total_ticks) { - static uint64_t previousIdleTicks = 0; - static uint64_t previousTotalTicks = 0; - static double previousLoad = -0.0f; + static uint64_t previous_idle_ticks = 0; + static uint64_t previous_total_ticks = 0; + static double previous_load = -0.0; - uint64_t idleTicksSinceLastTime = idleTicks - previousIdleTicks; - uint64_t totalTicksSinceLastTime = totalTicks - previousTotalTicks; + uint64_t idle_ticks_since_last_time = idle_ticks - previous_idle_ticks; + uint64_t total_ticks_since_last_time = total_ticks - previous_total_ticks; - bool firstCall = (previousTotalTicks == 0); - bool ticksNotUpdatedSinceLastCall = (totalTicksSinceLastTime == 0); + bool first_call = (previous_total_ticks == 0); + bool ticks_not_updated_since_last_call = (total_ticks_since_last_time == 0); double load; - if (firstCall || ticksNotUpdatedSinceLastCall) { - load = previousLoad; + if (first_call || ticks_not_updated_since_last_call) { + load = previous_load; } else { //calculate load - double idleToTotalRatio = ((double)idleTicksSinceLastTime) / totalTicksSinceLastTime; - double loadSinceLastCall = 1.0 - idleToTotalRatio; + double idle_to_total_ratio = ((double)idle_ticks_since_last_time) / total_ticks_since_last_time; + double load_since_last_call = 1.0 - idle_to_total_ratio; //filter/smooth result when possible - if(previousLoad > 0) { - load = 0.9 * previousLoad + 0.1 * loadSinceLastCall; + if(previous_load > 0) { + load = 0.9 * previous_load + 0.1 * load_since_last_call; } else { - load = loadSinceLastCall; + load = load_since_last_call; } } - previousLoad = load; - previousTotalTicks = totalTicks; - previousIdleTicks = idleTicks; + previous_load = load; + previous_total_ticks = total_ticks; + previous_idle_ticks = idle_ticks; return load; } @@ -458,17 +458,17 @@ static uint64_t FileTimeToTickCount(const FILETIME & ft) } double GetLoadAverage() { - FILETIME idleTime, kernelTime, userTime; - BOOL getSystemTimeSucceeded = GetSystemTimes(&idleTime, &kernelTime, &userTime); + FILETIME idle_time, kernel_time, user_time; + BOOL get_system_time_succeeded = GetSystemTimes(&idle_time, &kernel_time, &user_time); double result; - if (getSystemTimeSucceeded) { - uint64_t idleTicks = FileTimeToTickCount(idleTime); + if (get_system_time_succeeded) { + uint64_t idle_ticks = FileTimeToTickCount(idle_time); - //kernelTime from GetSystemTimes already includes idleTime - uint64_t totalTicks = FileTimeToTickCount(kernelTime) + FileTimeToTickCount(userTime); + //kernel_time from GetSystemTimes already includes idle_time + uint64_t total_ticks = FileTimeToTickCount(kernel_time) + FileTimeToTickCount(user_time); - result = CalculateProcessorLoad(idleTicks, totalTicks); + result = CalculateProcessorLoad(idle_ticks, total_ticks); } else { result = -0.0; } From 97d709208389dbf17b08311caa197e49e51275d9 Mon Sep 17 00:00:00 2001 From: kwesolowski Date: Sat, 26 Jul 2014 12:38:26 +0200 Subject: [PATCH 12/84] Changed implementation to provide load from 0 to ProcessorCount This makes this implementation more consisten with POSIX load avarage. --- src/util.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/util.cc b/src/util.cc index 66ddcb1029..8b69b71255 100644 --- a/src/util.cc +++ b/src/util.cc @@ -461,19 +461,21 @@ double GetLoadAverage() { FILETIME idle_time, kernel_time, user_time; BOOL get_system_time_succeeded = GetSystemTimes(&idle_time, &kernel_time, &user_time); - double result; + double posix_compatible_load; if (get_system_time_succeeded) { uint64_t idle_ticks = FileTimeToTickCount(idle_time); //kernel_time from GetSystemTimes already includes idle_time uint64_t total_ticks = FileTimeToTickCount(kernel_time) + FileTimeToTickCount(user_time); - result = CalculateProcessorLoad(idle_ticks, total_ticks); + double processor_load = CalculateProcessorLoad(idle_ticks, total_ticks); + posix_compatible_load = processor_load * GetProcessorCount(); + } else { - result = -0.0; + posix_compatible_load = -0.0; } - return result; + return posix_compatible_load; } #else double GetLoadAverage() { From 4b2e578d941be0e5d4e878668e5d5f493b9dc429 Mon Sep 17 00:00:00 2001 From: kwesolowski Date: Sun, 27 Jul 2014 01:17:34 +0200 Subject: [PATCH 13/84] Remove extra info from Usage() As now behavior is similar on on platforms, this can probably be removed --- src/ninja.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ninja.cc b/src/ninja.cc index 04b3689331..4368fabda2 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -193,9 +193,6 @@ void Usage(const BuildConfig& config) { "\n" " -j N run N jobs in parallel [default=%d, derived from CPUs available]\n" " -l N do not start new jobs if the load average is greater than N\n" -#ifdef _WIN32 -" (N can be between 0.0 for all processors being IDLE to 1.0 for all processors 100%% busy)\n" -#endif " -k N keep going until N jobs fail [default=1]\n" " -n dry run (don't run commands but act like they succeeded)\n" " -v show all command lines while building\n" From ac26ba226e970188d71e16910af2faa313f7c583 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 4 Aug 2014 08:21:34 -0700 Subject: [PATCH 14/84] add some parens to silence a gcc warning --- src/disk_interface.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index b170f63ea6..1a79fb0032 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -89,7 +89,7 @@ bool IsWindows7OrLater() { if (!GetVersionEx(&version_info)) Fatal("GetVersionEx: %s", GetLastErrorString().c_str()); return version_info.dwMajorVersion > 6 || - version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 1; + (version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 1); } #pragma warning(pop) From d2f600b4f56b86aa28f0546c67970460dd1e1a2e Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 4 Aug 2014 08:35:56 -0700 Subject: [PATCH 15/84] amend HACKING.md with instructions for Arch Linux --- HACKING.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/HACKING.md b/HACKING.md index 8e1696ac7c..d94882b94b 100644 --- a/HACKING.md +++ b/HACKING.md @@ -167,6 +167,12 @@ Setup on Ubuntu Precise: * `sudo apt-get install gcc-mingw-w64-i686 g++-mingw-w64-i686 wine` * `export CC=i686-w64-mingw32-gcc CXX=i686-w64-mingw32-g++ AR=i686-w64-mingw32-ar` +Setup on Arch: +* Uncomment the `[multilib]` section of `/etc/pacman.conf` and `sudo pacman -Sy`. +* `sudo pacman -S mingw-w64-gcc wine` +* `export CC=x86_64-w64-mingw32-cc CXX=x86_64-w64-mingw32-c++ AR=x86_64-w64-mingw32-ar` +* `export CFLAGS=-I/usr/x86_64-w64-mingw32/include` + Then run: * `./configure.py --platform=mingw --host=linux` * Build `ninja.exe` using a Linux ninja binary: `/path/to/linux/ninja` From e32783165866c1a7e5c3a949d6a340de0327308a Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 3 Sep 2014 19:31:29 -0700 Subject: [PATCH 16/84] Provide an error message on malformed lets. Fixes #807. --- src/manifest_parser.cc | 2 +- src/manifest_parser_test.cc | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 6fa4f7c8b4..55d191fcff 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -191,7 +191,7 @@ bool ManifestParser::ParseRule(string* err) { bool ManifestParser::ParseLet(string* key, EvalString* value, string* err) { if (!lexer_.ReadIdent(key)) - return false; + return lexer_.Error("expected variable name", err); if (!ExpectToken(Lexer::EQUALS, err)) return false; if (!lexer_.ReadVarValue(value, err)) diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 152b965d78..5f4b30abcd 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -549,6 +549,15 @@ TEST_F(ParserTest, Errors) { , err); } + { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n && bar", + &err)); + EXPECT_EQ("input:3: expected variable name\n", err); + } + { State state; ManifestParser parser(&state, NULL); From fb9f17f48c9820ee06abc16a23bd5f34d0b393ff Mon Sep 17 00:00:00 2001 From: Damien Pollet Date: Tue, 16 Sep 2014 14:28:46 +0200 Subject: [PATCH 17/84] Emacs mode: inherit from prog-mode `prog-mode` ensures a final newline when saving files, which is useful since ninja fails otherwise. See `require-final-newline` and `mode-require-final-newline`. --- misc/ninja-mode.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 80585d846d..2c37c6a5c4 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -39,8 +39,8 @@ )) -;;;###autoload -(define-derived-mode ninja-mode fundamental-mode "ninja" +;;;###autoload +(define-derived-mode ninja-mode prog-mode "ninja" (setq comment-start "#") ; Pass extra "t" to turn off syntax-based fontification -- we don't want ; quoted strings highlighted. From b2fe56caaf0bed497ee480003f10486c18d8de9a Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 17 Sep 2014 19:48:26 -0700 Subject: [PATCH 18/84] Use a small, standalone testing framework instead of googletest. Ninja currently uses googletest for testing. That makes building ninja_test somewhat annoying since it requires that one passes --with-gtest PATH to configure. It turns out just implementing the bits of googletest that ninja uses needs about the same amount of code than making the --with-gtest flag in configure.py work and making googletest print test results in a way we want (!) In addition to making configuration simpler, this also makes compiling tests much faster: On my system, touching src/build_test.cc (the slowest file to build in ninja) and rebuilding ninja_tests is twice as fast than without this patch. Building all is noticeably faster too: 5.6s with this patch, 9.1s without this patch (38% faster). The most noticeable things missing: EXPECT_* and ASSERT_* don't support streaming notes to them with operator<<, and for failing tests the lhs and rhs are not printed. That's so that this header does not have to include sstream, which slows down building ninja_test almost 20%. If this turns out to be annoying, we can maybe add it. --- configure.py | 31 ++---------- src/build_test.cc | 4 +- src/depfile_parser_test.cc | 2 +- src/deps_log_test.cc | 5 ++ src/disk_interface_test.cc | 4 +- src/includes_normalize_test.cc | 2 - src/lexer_test.cc | 3 +- src/manifest_parser_test.cc | 5 +- src/msvc_helper_test.cc | 2 - src/ninja_test.cc | 89 +++++++++++++++------------------- src/state_test.cc | 3 +- src/subprocess_test.cc | 10 ++-- src/test.cc | 4 +- src/test.h | 88 ++++++++++++++++++++++++++++++++- 14 files changed, 153 insertions(+), 99 deletions(-) diff --git a/configure.py b/configure.py index 64123a0af7..a307043d3f 100755 --- a/configure.py +++ b/configure.py @@ -44,8 +44,7 @@ parser.add_option('--profile', metavar='TYPE', choices=profilers, help='enable profiling (' + '/'.join(profilers) + ')',) -parser.add_option('--with-gtest', metavar='PATH', - help='use gtest unpacked in directory PATH') +parser.add_option('--with-gtest', metavar='PATH', help='ignored') parser.add_option('--with-python', metavar='EXE', help='use EXE as the Python interpreter', default=os.path.basename(sys.executable)) @@ -319,34 +318,10 @@ def has_re2c(): n.comment('Tests all build into ninja_test executable.') variables = [] -test_cflags = cflags + ['-DGTEST_HAS_RTTI=0'] test_ldflags = None test_libs = libs objs = [] -if options.with_gtest: - path = options.with_gtest - gtest_all_incs = '-I%s -I%s' % (path, os.path.join(path, 'include')) - if platform.is_msvc(): - gtest_cflags = '/nologo /EHsc /Zi /D_VARIADIC_MAX=10 ' - if platform.msvc_needs_fs(): - gtest_cflags += '/FS ' - gtest_cflags += gtest_all_incs - else: - gtest_cflags = '-fvisibility=hidden ' + gtest_all_incs - objs += n.build(built('gtest-all' + objext), 'cxx', - os.path.join(path, 'src', 'gtest-all.cc'), - variables=[('cflags', gtest_cflags)]) - - test_cflags.append('-I%s' % os.path.join(path, 'include')) -else: - # Use gtest from system. - if platform.is_msvc(): - test_libs.extend(['gtest_main.lib', 'gtest.lib']) - else: - test_libs.extend(['-lgtest_main', '-lgtest']) - -n.variable('test_cflags', test_cflags) for name in ['build_log_test', 'build_test', 'clean_test', @@ -362,10 +337,10 @@ def has_re2c(): 'subprocess_test', 'test', 'util_test']: - objs += cxx(name, variables=[('cflags', '$test_cflags')]) + objs += cxx(name) if platform.is_windows(): for name in ['includes_normalize_test', 'msvc_helper_test']: - objs += cxx(name, variables=[('cflags', '$test_cflags')]) + objs += cxx(name) if not platform.is_windows(): test_libs.append('-lpthread') diff --git a/src/build_test.cc b/src/build_test.cc index dad69dc0ea..6336206bf6 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -14,6 +14,8 @@ #include "build.h" +#include + #include "build_log.h" #include "deps_log.h" #include "graph.h" @@ -506,7 +508,7 @@ void BuildTest::RebuildTarget(const string& target, const char* manifest, builder.command_runner_.reset(&command_runner_); if (!builder.AlreadyUpToDate()) { bool build_res = builder.Build(&err); - EXPECT_TRUE(build_res) << "builder.Build(&err)"; + EXPECT_TRUE(build_res); } builder.command_runner_.release(); } diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc index a5f3321409..e67ef794ca 100644 --- a/src/depfile_parser_test.cc +++ b/src/depfile_parser_test.cc @@ -14,7 +14,7 @@ #include "depfile_parser.h" -#include +#include "test.h" struct DepfileParserTest : public testing::Test { bool Parse(const char* input, string* err); diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index e8e5138fc2..4fa4008bd0 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -14,6 +14,11 @@ #include "deps_log.h" +#ifndef _WIN32 +#include +#include +#endif + #include "graph.h" #include "util.h" #include "test.h" diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index b2e8cb5fb3..05d509c00d 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include - +#include +#include #ifdef _WIN32 #include #include diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index 419996f4e4..cf4a4a3de9 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -14,8 +14,6 @@ #include "includes_normalize.h" -#include - #include "test.h" #include "util.h" diff --git a/src/lexer_test.cc b/src/lexer_test.cc index e8a164254e..331d8e1ea9 100644 --- a/src/lexer_test.cc +++ b/src/lexer_test.cc @@ -14,9 +14,8 @@ #include "lexer.h" -#include - #include "eval_env.h" +#include "test.h" TEST(Lexer, ReadVarValue) { Lexer lexer("plain text $var $VaR ${x}\n"); diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 152b965d78..ee87c94c1d 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -17,17 +17,16 @@ #include #include -#include - #include "graph.h" #include "state.h" +#include "test.h" struct ParserTest : public testing::Test, public ManifestParser::FileReader { void AssertParse(const char* input) { ManifestParser parser(&state, this); string err; - ASSERT_TRUE(parser.ParseTest(input, &err)) << err; + EXPECT_TRUE(parser.ParseTest(input, &err)); ASSERT_EQ("", err); } diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 391c04568c..29db65065d 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -14,8 +14,6 @@ #include "msvc_helper.h" -#include - #include "test.h" #include "util.h" diff --git a/src/ninja_test.cc b/src/ninja_test.cc index 989ea5c72e..9499c8bf00 100644 --- a/src/ninja_test.cc +++ b/src/ninja_test.cc @@ -15,9 +15,22 @@ #include #include -#include "gtest/gtest.h" +#include "test.h" #include "line_printer.h" +// This can't be a vector because tests call RegisterTest from static +// initializers and the order static initializers run it isn't specified. So +// the vector constructor isn't guaranteed to run before all of the +// RegisterTest() calls. +static testing::Test* (*tests[10000])(); +testing::Test* g_current_test; +static int ntests; +static LinePrinter printer; + +void RegisterTest(testing::Test* (*factory)()) { + tests[ntests++] = factory; +} + string StringPrintf(const char* format, ...) { const int N = 1024; char buf[N]; @@ -30,59 +43,37 @@ string StringPrintf(const char* format, ...) { return buf; } -/// A test result printer that's less wordy than gtest's default. -struct LaconicPrinter : public testing::EmptyTestEventListener { - LaconicPrinter() : tests_started_(0), test_count_(0), iteration_(0) {} - virtual void OnTestProgramStart(const testing::UnitTest& unit_test) { - test_count_ = unit_test.test_to_run_count(); - } - - virtual void OnTestIterationStart(const testing::UnitTest& test_info, - int iteration) { - tests_started_ = 0; - iteration_ = iteration; - } - - virtual void OnTestStart(const testing::TestInfo& test_info) { - ++tests_started_; - printer_.Print( - StringPrintf("[%d/%d%s] %s.%s", - tests_started_, - test_count_, - iteration_ ? StringPrintf(" iter %d", iteration_).c_str() - : "", - test_info.test_case_name(), - test_info.name()), - LinePrinter::ELIDE); +bool testing::Test::Check(bool condition, const char* file, int line, + const char* error) { + if (!condition) { + printer.PrintOnNewLine( + StringPrintf("*** Failure in %s:%d\n%s\n", file, line, error)); + failed_ = true; } + return condition; +} - virtual void OnTestPartResult( - const testing::TestPartResult& test_part_result) { - if (!test_part_result.failed()) - return; - printer_.PrintOnNewLine(StringPrintf( - "*** Failure in %s:%d\n%s\n", test_part_result.file_name(), - test_part_result.line_number(), test_part_result.summary())); - } +int main(int argc, char **argv) { + int tests_started = 0; - virtual void OnTestProgramEnd(const testing::UnitTest& unit_test) { - printer_.PrintOnNewLine(unit_test.Passed() ? "passed\n" : "failed\n"); - } + bool passed = true; + for (int i = 0; i < ntests; i++) { + ++tests_started; - private: - LinePrinter printer_; - int tests_started_; - int test_count_; - int iteration_; -}; + testing::Test* test = tests[i](); -int main(int argc, char **argv) { - testing::InitGoogleTest(&argc, argv); + printer.Print( + StringPrintf("[%d/%d] %s", tests_started, ntests, test->Name()), + LinePrinter::ELIDE); - testing::TestEventListeners& listeners = - testing::UnitTest::GetInstance()->listeners(); - delete listeners.Release(listeners.default_result_printer()); - listeners.Append(new LaconicPrinter); + test->SetUp(); + test->Run(); + test->TearDown(); + if (test->Failed()) + passed = false; + delete test; + } - return RUN_ALL_TESTS(); + printer.PrintOnNewLine(passed ? "passed\n" : "failed\n"); + return passed ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/src/state_test.cc b/src/state_test.cc index af2bff19b3..a4fafa1e9a 100644 --- a/src/state_test.cc +++ b/src/state_test.cc @@ -12,10 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include - #include "graph.h" #include "state.h" +#include "test.h" namespace { diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index 775a13a6c9..fe53748177 100644 --- a/src/subprocess_test.cc +++ b/src/subprocess_test.cc @@ -18,6 +18,7 @@ #ifndef _WIN32 // SetWithLots need setrlimit. +#include #include #include #include @@ -92,7 +93,7 @@ TEST_F(SubprocessTest, InterruptParent) { return; } - ADD_FAILURE() << "We should have been interrupted"; + ASSERT_FALSE("We should have been interrupted"); } TEST_F(SubprocessTest, Console) { @@ -176,9 +177,10 @@ TEST_F(SubprocessTest, SetWithLots) { // Make sure [ulimit -n] isn't going to stop us from working. rlimit rlim; ASSERT_EQ(0, getrlimit(RLIMIT_NOFILE, &rlim)); - ASSERT_GT(rlim.rlim_cur, kNumProcs) - << "Raise [ulimit -n] well above " << kNumProcs - << " to make this test go"; + if (!EXPECT_GT(rlim.rlim_cur, kNumProcs)) { + printf("Raise [ulimit -n] well above %u to make this test go\n", kNumProcs); + return; + } vector procs; for (size_t i = 0; i < kNumProcs; ++i) { diff --git a/src/test.cc b/src/test.cc index 21015ed34e..ed2b91057e 100644 --- a/src/test.cc +++ b/src/test.cc @@ -24,6 +24,8 @@ #ifdef _WIN32 #include +#else +#include #endif namespace { @@ -90,7 +92,7 @@ Node* StateTestWithBuiltinRules::GetNode(const string& path) { void AssertParse(State* state, const char* input) { ManifestParser parser(state, NULL); string err; - ASSERT_TRUE(parser.ParseTest(input, &err)) << err; + EXPECT_TRUE(parser.ParseTest(input, &err)); ASSERT_EQ("", err); } diff --git a/src/test.h b/src/test.h index f34b877911..be5dcff89b 100644 --- a/src/test.h +++ b/src/test.h @@ -15,12 +15,96 @@ #ifndef NINJA_TEST_H_ #define NINJA_TEST_H_ -#include - #include "disk_interface.h" #include "state.h" #include "util.h" +// A tiny testing framework inspired by googletest, but much simpler and +// faster to compile. It supports most things commonly used from googltest. The +// most noticeable things missing: EXPECT_* and ASSERT_* don't support +// streaming notes to them with operator<<, and for failing tests the lhs and +// rhs are not printed. That's so that this header does not have to include +// sstream, which slows down building ninja_test almost 20%. +namespace testing { +class Test { + bool failed_; + int assertion_failures_; + public: + Test() : failed_(false), assertion_failures_(0) {} + virtual ~Test() {} + virtual void SetUp() {} + virtual void TearDown() {} + virtual void Run() = 0; + virtual const char* Name() const = 0; + + bool Failed() const { return failed_; } + int AssertionFailures() const { return assertion_failures_; } + void AddAssertionFailure() { assertion_failures_++; } + bool Check(bool condition, const char* file, int line, const char* error); +}; +} + +void RegisterTest(testing::Test* (*)()); + +extern testing::Test* g_current_test; +#define TEST_F_(x, y, name) \ + struct y : public x { \ + static testing::Test* Create() { return g_current_test = new y; } \ + virtual void Run(); \ + virtual const char* Name() const { return name; } \ + }; \ + struct Register##y { \ + Register##y() { RegisterTest(y::Create); } \ + }; \ + Register##y g_register_##y; \ + void y::Run() + +#define TEST_F(x, y) TEST_F_(x, x##y, #x "." #y) +#define TEST(x, y) TEST_F_(testing::Test, x##y, #x "." #y) + +#define EXPECT_EQ(a, b) \ + g_current_test->Check(a == b, __FILE__, __LINE__, #a " == " #b) +#define EXPECT_NE(a, b) \ + g_current_test->Check(a != b, __FILE__, __LINE__, #a " != " #b) +#define EXPECT_GT(a, b) \ + g_current_test->Check(a > b, __FILE__, __LINE__, #a " > " #b) +#define EXPECT_LT(a, b) \ + g_current_test->Check(a < b, __FILE__, __LINE__, #a " < " #b) +#define EXPECT_GE(a, b) \ + g_current_test->Check(a >= b, __FILE__, __LINE__, #a " >= " #b) +#define EXPECT_LE(a, b) \ + g_current_test->Check(a <= b, __FILE__, __LINE__, #a " <= " #b) +#define EXPECT_TRUE(a) \ + g_current_test->Check(static_cast(a), __FILE__, __LINE__, #a) +#define EXPECT_FALSE(a) \ + g_current_test->Check(!static_cast(a), __FILE__, __LINE__, #a) + +#define ASSERT_EQ(a, b) \ + if (!EXPECT_EQ(a, b)) { g_current_test->AddAssertionFailure(); return; } +#define ASSERT_NE(a, b) \ + if (!EXPECT_NE(a, b)) { g_current_test->AddAssertionFailure(); return; } +#define ASSERT_GT(a, b) \ + if (!EXPECT_GT(a, b)) { g_current_test->AddAssertionFailure(); return; } +#define ASSERT_LT(a, b) \ + if (!EXPECT_LT(a, b)) { g_current_test->AddAssertionFailure(); return; } +#define ASSERT_GE(a, b) \ + if (!EXPECT_GE(a, b)) { g_current_test->AddAssertionFailure(); return; } +#define ASSERT_LE(a, b) \ + if (!EXPECT_LE(a, b)) { g_current_test->AddAssertionFailure(); return; } +#define ASSERT_TRUE(a) \ + 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; \ + } \ + } + // Support utilites for tests. struct Node; From d5bddeafc6bde19724f666617c1bc07f53d217c8 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 17 Sep 2014 20:35:07 -0700 Subject: [PATCH 19/84] Add support for --gtest_filter. The bots need it, and it is a useful flag. --- src/ninja_test.cc | 105 ++++++++++++++++++++++++++++++++++++++++++---- src/test.h | 6 +-- 2 files changed, 99 insertions(+), 12 deletions(-) diff --git a/src/ninja_test.cc b/src/ninja_test.cc index 9499c8bf00..31c2e1fc53 100644 --- a/src/ninja_test.cc +++ b/src/ninja_test.cc @@ -15,22 +15,35 @@ #include #include +#ifdef _WIN32 +#include "getopt.h" +#else +#include +#endif + #include "test.h" #include "line_printer.h" +struct RegisteredTest { + testing::Test* (*factory)(); + const char *name; + bool should_run; +}; // This can't be a vector because tests call RegisterTest from static // initializers and the order static initializers run it isn't specified. So // the vector constructor isn't guaranteed to run before all of the // RegisterTest() calls. -static testing::Test* (*tests[10000])(); +static RegisteredTest tests[10000]; testing::Test* g_current_test; static int ntests; static LinePrinter printer; -void RegisterTest(testing::Test* (*factory)()) { - tests[ntests++] = factory; +void RegisterTest(testing::Test* (*factory)(), const char* name) { + tests[ntests].factory = factory; + tests[ntests++].name = name; } +namespace { string StringPrintf(const char* format, ...) { const int N = 1024; char buf[N]; @@ -43,6 +56,74 @@ string StringPrintf(const char* format, ...) { return buf; } +void Usage() { + fprintf(stderr, +"usage: ninja_tests [options]\n" +"\n" +"options:\n" +" --gtest_filter=POSTIVE_PATTERNS[-NEGATIVE_PATTERNS]\n" +" Run only the tests whose name matches one of the positive patterns but\n" +" none of the negative patterns. '?' matches any single character; '*'\n" +" matches any substring; ':' separates two patterns.\n"); +} + +bool PatternMatchesString(const char* pattern, const char* str) { + switch (*pattern) { + case '\0': + case '-': + case ':': return *str == '\0'; + case '?': return *str != '\0' && PatternMatchesString(pattern + 1, str + 1); + case '*': return (*str != '\0' && PatternMatchesString(pattern, str + 1)) || + PatternMatchesString(pattern + 1, str); + default: return *pattern == *str && + PatternMatchesString(pattern + 1, str + 1); + } +} + +bool MatchesFilter(const char* name, const char* filter) { + for (const char* cur_pattern = filter - 1; cur_pattern != NULL; + cur_pattern = strchr(cur_pattern, ':')) { + cur_pattern++; // Skip the pattern separator (the ':' character). + if (PatternMatchesString(cur_pattern, name)) + return true; + } + return false; +} + +bool TestMatchesFilter(const char* test, const char* filter) { + // Split --gtest_filter at '-' into positive and negative filters. + const char* const dash = strchr(filter, '-'); + const char* positive = // Treat '-test1' as '*-test1': + dash == filter ? "*" : filter; + const char* negative = dash ? dash + 1 : ""; + return MatchesFilter(test, positive) && !MatchesFilter(test, negative); +} + +bool ReadFlags(int* argc, char*** argv, const char** test_filter) { + enum { OPT_GTEST_FILTER = 1 }; + const option kLongOptions[] = { + { "gtest_filter", required_argument, NULL, OPT_GTEST_FILTER }, + { NULL, 0, NULL, 0 } + }; + + int opt; + while ((opt = getopt_long(*argc, *argv, "h", kLongOptions, NULL)) != -1) { + switch (opt) { + case OPT_GTEST_FILTER: + *test_filter = optarg; + break; + default: + Usage(); + return false; + } + } + *argv += optind; + *argc -= optind; + return true; +} + +} // namespace + bool testing::Test::Check(bool condition, const char* file, int line, const char* error) { if (!condition) { @@ -56,16 +137,24 @@ bool testing::Test::Check(bool condition, const char* file, int line, int main(int argc, char **argv) { int tests_started = 0; + const char* test_filter = "*"; + if (!ReadFlags(&argc, &argv, &test_filter)) + return 1; + + int nactivetests = 0; + for (int i = 0; i < ntests; i++) + if ((tests[i].should_run = TestMatchesFilter(tests[i].name, test_filter))) + ++nactivetests; + bool passed = true; for (int i = 0; i < ntests; i++) { - ++tests_started; - - testing::Test* test = tests[i](); + if (!tests[i].should_run) continue; + ++tests_started; + testing::Test* test = tests[i].factory(); printer.Print( - StringPrintf("[%d/%d] %s", tests_started, ntests, test->Name()), + StringPrintf("[%d/%d] %s", tests_started, nactivetests, tests[i].name), LinePrinter::ELIDE); - test->SetUp(); test->Run(); test->TearDown(); diff --git a/src/test.h b/src/test.h index be5dcff89b..4c152039d7 100644 --- a/src/test.h +++ b/src/test.h @@ -35,7 +35,6 @@ class Test { virtual void SetUp() {} virtual void TearDown() {} virtual void Run() = 0; - virtual const char* Name() const = 0; bool Failed() const { return failed_; } int AssertionFailures() const { return assertion_failures_; } @@ -44,17 +43,16 @@ class Test { }; } -void RegisterTest(testing::Test* (*)()); +void RegisterTest(testing::Test* (*)(), const char*); extern testing::Test* g_current_test; #define TEST_F_(x, y, name) \ struct y : public x { \ static testing::Test* Create() { return g_current_test = new y; } \ virtual void Run(); \ - virtual const char* Name() const { return name; } \ }; \ struct Register##y { \ - Register##y() { RegisterTest(y::Create); } \ + Register##y() { RegisterTest(y::Create, name); } \ }; \ Register##y g_register_##y; \ void y::Run() From 954669e9c53301989af3a05bbd69564a3eb41bec Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 17 Sep 2014 21:22:49 -0700 Subject: [PATCH 20/84] Don't support ? and : for --gtest-filter, a bit simpler. Let me know if you use these! --- src/ninja_test.cc | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/src/ninja_test.cc b/src/ninja_test.cc index 31c2e1fc53..54d87844b6 100644 --- a/src/ninja_test.cc +++ b/src/ninja_test.cc @@ -61,18 +61,15 @@ void Usage() { "usage: ninja_tests [options]\n" "\n" "options:\n" -" --gtest_filter=POSTIVE_PATTERNS[-NEGATIVE_PATTERNS]\n" -" Run only the tests whose name matches one of the positive patterns but\n" -" none of the negative patterns. '?' matches any single character; '*'\n" -" matches any substring; ':' separates two patterns.\n"); +" --gtest_filter=POSTIVE_PATTERN[-NEGATIVE_PATTERN]\n" +" Run tests whose names match the positive but not the negative pattern.\n" +" '*' matches any substring. (gtest's ':', '?' are not implemented).\n"); } bool PatternMatchesString(const char* pattern, const char* str) { switch (*pattern) { case '\0': - case '-': - case ':': return *str == '\0'; - case '?': return *str != '\0' && PatternMatchesString(pattern + 1, str + 1); + case '-': return *str == '\0'; case '*': return (*str != '\0' && PatternMatchesString(pattern, str + 1)) || PatternMatchesString(pattern + 1, str); default: return *pattern == *str && @@ -80,23 +77,12 @@ bool PatternMatchesString(const char* pattern, const char* str) { } } -bool MatchesFilter(const char* name, const char* filter) { - for (const char* cur_pattern = filter - 1; cur_pattern != NULL; - cur_pattern = strchr(cur_pattern, ':')) { - cur_pattern++; // Skip the pattern separator (the ':' character). - if (PatternMatchesString(cur_pattern, name)) - return true; - } - return false; -} - bool TestMatchesFilter(const char* test, const char* filter) { // Split --gtest_filter at '-' into positive and negative filters. const char* const dash = strchr(filter, '-'); - const char* positive = // Treat '-test1' as '*-test1': - dash == filter ? "*" : filter; - const char* negative = dash ? dash + 1 : ""; - return MatchesFilter(test, positive) && !MatchesFilter(test, negative); + const char* pos = dash == filter ? "*" : filter; //Treat '-test1' as '*-test1' + const char* neg = dash ? dash + 1 : ""; + return PatternMatchesString(pos, test) && !PatternMatchesString(neg, test); } bool ReadFlags(int* argc, char*** argv, const char** test_filter) { @@ -110,8 +96,10 @@ bool ReadFlags(int* argc, char*** argv, const char** test_filter) { while ((opt = getopt_long(*argc, *argv, "h", kLongOptions, NULL)) != -1) { switch (opt) { case OPT_GTEST_FILTER: - *test_filter = optarg; - break; + if (strchr(optarg, '?') == NULL && strchr(optarg, ':') == NULL) { + *test_filter = optarg; + break; + } // else fall through. default: Usage(); return false; From 0fc8e0c2ca0b164d33039a723475c90952704307 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 17 Sep 2014 22:18:08 -0700 Subject: [PATCH 21/84] Don't need to install and configure with gtest any more --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index d7bee6f54f..6cb1b5c9c6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,5 +4,4 @@ compiler: - clang before_install: - sudo apt-get update -qq - - sudo apt-get install libgtest-dev -script: ./bootstrap.py && ./configure.py --with-gtest=/usr/src/gtest && ./ninja ninja_test && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots +script: ./bootstrap.py && ./configure.py && ./ninja ninja_test && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots From d13eb33fc3e6eedefd0840ed19dc85901fba0d4b Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 18 Sep 2014 08:15:31 -0700 Subject: [PATCH 22/84] fix warning --- src/subprocess_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index fe53748177..8a0787c987 100644 --- a/src/subprocess_test.cc +++ b/src/subprocess_test.cc @@ -172,7 +172,7 @@ TEST_F(SubprocessTest, SetWithMulti) { TEST_F(SubprocessTest, SetWithLots) { // Arbitrary big number; needs to be over 1024 to confirm we're no longer // hostage to pselect. - const size_t kNumProcs = 1025; + const unsigned kNumProcs = 1025; // Make sure [ulimit -n] isn't going to stop us from working. rlimit rlim; From a6b44033c42b7bb4c98d9a66fbb72ad6fc092c18 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 18 Sep 2014 09:14:20 -0700 Subject: [PATCH 23/84] remove more things not needed without gtest --- .travis.yml | 2 -- HACKING.md | 20 -------------------- 2 files changed, 22 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6cb1b5c9c6..4a6dfb01f6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,6 +2,4 @@ language: cpp compiler: - gcc - clang -before_install: - - sudo apt-get update -qq script: ./bootstrap.py && ./configure.py && ./ninja ninja_test && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots diff --git a/HACKING.md b/HACKING.md index d94882b94b..ef521e1490 100644 --- a/HACKING.md +++ b/HACKING.md @@ -50,26 +50,6 @@ patch. ## Testing -### Installing gtest - -The `ninja_test` binary, containing all the tests, depends on the -googletest (gtest) library. - -* On older Ubuntus it'll install as libraries into `/usr/lib`: - - apt-get install libgtest - -* On newer Ubuntus it's only distributed as source - - apt-get install libgtest-dev - ./configure.py --with-gtest=/usr/src/gtest - -* Otherwise you need to download it, unpack it, and pass - `--with-gtest` to `configure.py`. Get it from [its downloads - page](http://code.google.com/p/googletest/downloads/list); [this - direct download link might work - too](http://googletest.googlecode.com/files/gtest-1.6.0.zip). - ### Test-driven development Set your build command to From ac047b1f7efe3e90c2759864b57fd7c0d9404313 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 18 Sep 2014 16:08:25 -0700 Subject: [PATCH 24/84] Fix building tests on Windows again. Turns out gtest was pulling in sys/stat.h, and we were using stat() through that in tests. This doesn't work with old MSVCs, so we should probably replace that with RealDiskInterface in a follow-up. --- src/build_log_test.cc | 2 +- src/deps_log_test.cc | 2 +- src/getopt.c | 4 ++-- src/getopt.h | 6 +++--- src/includes_normalize_test.cc | 2 ++ src/test.cc | 13 ++++++++----- 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/build_log_test.cc b/src/build_log_test.cc index 6738c7b5dc..2c41ba6e8f 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -17,12 +17,12 @@ #include "util.h" #include "test.h" +#include #ifdef _WIN32 #include #include #else #include -#include #include #endif diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 4fa4008bd0..30cada23ed 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -14,9 +14,9 @@ #include "deps_log.h" +#include #ifndef _WIN32 #include -#include #endif #include "graph.h" diff --git a/src/getopt.c b/src/getopt.c index 75ef99cfb4..3350fb9ce5 100644 --- a/src/getopt.c +++ b/src/getopt.c @@ -299,7 +299,7 @@ getopt_internal (int argc, char **argv, char *shortopts, return (optopt = '?'); } has_arg = ((cp[1] == ':') - ? ((cp[2] == ':') ? OPTIONAL_ARG : REQUIRED_ARG) : no_argument); + ? ((cp[2] == ':') ? OPTIONAL_ARG : required_argument) : no_argument); possible_arg = argv[optind] + optwhere + 1; optopt = *cp; } @@ -318,7 +318,7 @@ getopt_internal (int argc, char **argv, char *shortopts, else optarg = NULL; break; - case REQUIRED_ARG: + case required_argument: if (*possible_arg == '=') possible_arg++; if (*possible_arg != '\0') diff --git a/src/getopt.h b/src/getopt.h index ead9878b9a..b4247fb4b7 100644 --- a/src/getopt.h +++ b/src/getopt.h @@ -4,9 +4,9 @@ /* include files needed by this include file */ /* macros defined by this include file */ -#define no_argument 0 -#define REQUIRED_ARG 1 -#define OPTIONAL_ARG 2 +#define no_argument 0 +#define required_argument 1 +#define OPTIONAL_ARG 2 /* types defined by this include file */ diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index cf4a4a3de9..d9d21bf30f 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -14,6 +14,8 @@ #include "includes_normalize.h" +#include + #include "test.h" #include "util.h" diff --git a/src/test.cc b/src/test.cc index ed2b91057e..560ef3ad78 100644 --- a/src/test.cc +++ b/src/test.cc @@ -12,22 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. +#ifdef _WIN32 +#include // Has to be before util.h is included. +#endif + #include "test.h" #include #include - -#include "build_log.h" -#include "manifest_parser.h" -#include "util.h" - #ifdef _WIN32 #include #else #include #endif +#include "build_log.h" +#include "manifest_parser.h" +#include "util.h" + namespace { #ifdef _WIN32 From 213c44a51c1309f0b03a7c2763a7599c8cb9a41f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 18 Sep 2014 21:25:32 -0700 Subject: [PATCH 25/84] Don't print "Recompacting..." message from tests. Now tests don't print anything. Non-test behavior is unchanged. --- src/build_log.cc | 5 +++-- src/build_log.h | 2 ++ src/build_log_test.cc | 1 + src/deps_log.cc | 3 ++- src/deps_log.h | 4 +++- src/deps_log_test.cc | 2 ++ 6 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/build_log.cc b/src/build_log.cc index 3f24c16d38..b6f9874578 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -102,7 +102,7 @@ BuildLog::LogEntry::LogEntry(const string& output, uint64_t command_hash, {} BuildLog::BuildLog() - : log_file_(NULL), needs_recompaction_(false) {} + : log_file_(NULL), needs_recompaction_(false), quiet_(false) {} BuildLog::~BuildLog() { Close(); @@ -354,7 +354,8 @@ bool BuildLog::WriteEntry(FILE* f, const LogEntry& entry) { bool BuildLog::Recompact(const string& path, const BuildLogUser& user, string* err) { METRIC_RECORD(".ninja_log recompact"); - printf("Recompacting log...\n"); + if (!quiet_) + printf("Recompacting log...\n"); Close(); string temp_path = path + ".recompact"; diff --git a/src/build_log.h b/src/build_log.h index fe81a851f4..db0cfe0ebd 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -80,6 +80,7 @@ struct BuildLog { /// Rewrite the known log entries, throwing away old data. bool Recompact(const string& path, const BuildLogUser& user, string* err); + void set_quiet(bool quiet) { quiet_ = quiet; } typedef ExternalStringHashMap::Type Entries; const Entries& entries() const { return entries_; } @@ -88,6 +89,7 @@ struct BuildLog { Entries entries_; FILE* log_file_; bool needs_recompaction_; + bool quiet_; }; #endif // NINJA_BUILD_LOG_H_ diff --git a/src/build_log_test.cc b/src/build_log_test.cc index 2c41ba6e8f..7ea2117ba0 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -290,6 +290,7 @@ TEST_F(BuildLogRecompactTest, Recompact) { ASSERT_TRUE(log2.LookupByOutput("out")); ASSERT_TRUE(log2.LookupByOutput("out2")); // ...and force a recompaction. + log2.set_quiet(true); EXPECT_TRUE(log2.OpenForWrite(kTestFilename, *this, &err)); log2.Close(); diff --git a/src/deps_log.cc b/src/deps_log.cc index 61df387e0d..39de1805dc 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -302,7 +302,8 @@ DepsLog::Deps* DepsLog::GetDeps(Node* node) { bool DepsLog::Recompact(const string& path, string* err) { METRIC_RECORD(".ninja_deps recompact"); - printf("Recompacting deps...\n"); + if (!quiet_) + printf("Recompacting deps...\n"); Close(); string temp_path = path + ".recompact"; diff --git a/src/deps_log.h b/src/deps_log.h index cec0257cef..9b81bc1565 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -64,7 +64,7 @@ struct State; /// wins, allowing updates to just be appended to the file. A separate /// repacking step can run occasionally to remove dead records. struct DepsLog { - DepsLog() : needs_recompaction_(false), file_(NULL) {} + DepsLog() : needs_recompaction_(false), quiet_(false), file_(NULL) {} ~DepsLog(); // Writing (build-time) interface. @@ -87,6 +87,7 @@ struct DepsLog { /// Rewrite the known log entries, throwing away old data. bool Recompact(const string& path, string* err); + void set_quiet(bool quiet) { quiet_ = quiet; } /// Returns if the deps entry for a node is still reachable from the manifest. /// @@ -108,6 +109,7 @@ struct DepsLog { bool RecordId(Node* node); bool needs_recompaction_; + bool quiet_; FILE* file_; /// Maps id -> Node. diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 30cada23ed..60ff48f34a 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -252,6 +252,7 @@ TEST_F(DepsLogTest, Recompact) { ASSERT_EQ("foo.h", deps->nodes[0]->path()); ASSERT_EQ("baz.h", deps->nodes[1]->path()); + log.set_quiet(true); ASSERT_TRUE(log.Recompact(kTestFilename, &err)); // The in-memory deps graph should still be valid after recompaction. @@ -301,6 +302,7 @@ TEST_F(DepsLogTest, Recompact) { ASSERT_EQ("foo.h", deps->nodes[0]->path()); ASSERT_EQ("baz.h", deps->nodes[1]->path()); + log.set_quiet(true); ASSERT_TRUE(log.Recompact(kTestFilename, &err)); // The previous entries should have been removed. From 13dfea4f8ddb38dae127e023d5d25292c4fefb14 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 19 Sep 2014 08:49:00 -0700 Subject: [PATCH 26/84] Make auto-reconfiguring work if CFLAGS contains more than one flag. When using an open-source clang on OS X, one has to pass an isysroot flag so that it can find system headers (stdio.h), like so: CXX=path/to/clang++ CFLAGS="-isysroot $(xcrun -show-sdk-path)" ./configure.py Previously, configure.py wouldn't quote envvars containing spaces, so it'd rerun this as CXX=path/to/clang++ CFLAGS=-isysroot /sysroot/path ./configure.py which would then die because /sysroot/path wasn't excecutable. --- configure.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/configure.py b/configure.py index a307043d3f..01fd30176a 100755 --- a/configure.py +++ b/configure.py @@ -23,6 +23,7 @@ from optparse import OptionParser import os +import pipes import sys import platform_helper sys.path.insert(0, 'misc') @@ -77,7 +78,8 @@ env_keys = set(['CXX', 'AR', 'CFLAGS', 'LDFLAGS']) configure_env = dict((k, os.environ[k]) for k in os.environ if k in env_keys) if configure_env: - config_str = ' '.join([k + '=' + configure_env[k] for k in configure_env]) + config_str = ' '.join([k + '=' + pipes.quote(configure_env[k]) + for k in configure_env]) n.variable('configure_env', config_str + '$ ') n.newline() From 2cb9d7c2d6df859ab26b22472adbf979e28aeedd Mon Sep 17 00:00:00 2001 From: tzik Date: Fri, 19 Sep 2014 17:07:29 +0900 Subject: [PATCH 27/84] Throttle the number of pending commands by the parallelism configuration --- src/build.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/build.cc b/src/build.cc index 64bcea3786..fb1cc74834 100644 --- a/src/build.cc +++ b/src/build.cc @@ -488,7 +488,9 @@ void RealCommandRunner::Abort() { } bool RealCommandRunner::CanRunMore() { - return ((int)subprocs_.running_.size()) < config_.parallelism + size_t subproc_number = + subprocs_.running_.size() + subprocs_.finished_.size(); + return (int)subproc_number < config_.parallelism && ((subprocs_.running_.empty() || config_.max_load_average <= 0.0f) || GetLoadAverage() < config_.max_load_average); } From 6c554dbe4466d4485bab6f2aeecd24b8b828a766 Mon Sep 17 00:00:00 2001 From: Danny Date: Sun, 28 Sep 2014 05:52:08 -0400 Subject: [PATCH 28/84] Fix unknown pragma warnings --- src/disk_interface.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 1a79fb0032..9810708b1e 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -82,8 +82,10 @@ TimeStamp StatSingleFile(const string& path, bool quiet) { return TimeStampFromFileTime(attrs.ftLastWriteTime); } +#ifdef _MSC_VER #pragma warning(push) #pragma warning(disable: 4996) // GetVersionExA is deprecated post SDK 8.1. +#endif bool IsWindows7OrLater() { OSVERSIONINFO version_info = { sizeof(version_info) }; if (!GetVersionEx(&version_info)) @@ -91,7 +93,9 @@ bool IsWindows7OrLater() { return version_info.dwMajorVersion > 6 || (version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 1); } +#ifdef _MSC_VER #pragma warning(pop) +#endif bool StatAllFilesInDir(const string& dir, map* stamps, bool quiet) { From a73e6931aebdc30a66e194e199bc662bf1cea9cb Mon Sep 17 00:00:00 2001 From: Fraser Cormack Date: Fri, 24 Oct 2014 20:34:06 +0100 Subject: [PATCH 29/84] Add zsh completion for targets in conjunction with -C zsh can now list completions for targets in the directory specified by the -C option --- misc/zsh-completion | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/misc/zsh-completion b/misc/zsh-completion index 2fe16fb049..af24f89b26 100644 --- a/misc/zsh-completion +++ b/misc/zsh-completion @@ -17,7 +17,14 @@ # . path/to/ninja/misc/zsh-completion __get_targets() { - ninja -t targets 2>/dev/null | while read -r a b; do echo $a | cut -d ':' -f1; done; + dir="." + if [ -n "${opt_args[-C]}" ]; + then + eval dir="${opt_args[-C]}" + fi + targets_command="ninja -C \"${dir}\" -t targets" + eval ${targets_command} 2>/dev/null | while read -r a b; do echo $a | cut -d ':' -f1; done; + } __get_tools() { From 439f2738cf2ba2f30727971f6b352c9f5e440dc2 Mon Sep 17 00:00:00 2001 From: ppbrown Date: Mon, 27 Oct 2014 13:36:37 -0700 Subject: [PATCH 30/84] Update line_printer.cc Include POSIX termios.h --- src/line_printer.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/line_printer.cc b/src/line_printer.cc index ef1609c28f..813f63eb5c 100644 --- a/src/line_printer.cc +++ b/src/line_printer.cc @@ -21,6 +21,7 @@ #else #include #include +#include #include #endif From 1480a96fb49c0e2c35826a6483dfd15c66afe332 Mon Sep 17 00:00:00 2001 From: Taylor Braun-Jones Date: Sun, 26 Oct 2014 16:36:36 -0400 Subject: [PATCH 31/84] Mention optional files in installation instructions --- README | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README b/README index 733ccb39b5..37e03a4b8a 100644 --- a/README +++ b/README @@ -10,7 +10,9 @@ source files together, then re-builds Ninja using itself. You should end up with a 'ninja' binary in the source root. Run './ninja -h' for help. -There is no installation step. The only file of interest to a user -is the resulting ninja binary. +Installation is not necessary because the only required file is is the +resulting ninja binary. However, to enable features like Bash +completion and Emacs and Vim editing modes, some files in misc/ must be +copied to appropriate locations. If you're interested in making changes to Ninja, read HACKING.md first. From 65151e7eefce514febd0a9676c5363c6d4c54b66 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Thu, 30 Oct 2014 15:34:25 -0700 Subject: [PATCH 32/84] CanonicalizePath handles \ on Windows --- doc/manual.asciidoc | 4 +- misc/write_fake_manifests.py | 2 +- src/build_test.cc | 6 +- src/includes_normalize-win32.cc | 20 +- src/includes_normalize_test.cc | 30 +- src/lexer.cc | 493 ++++++++++++++++++-------------- src/lexer.in.cc | 25 +- src/manifest_parser_test.cc | 22 +- src/util.cc | 40 ++- src/util_test.cc | 66 +++++ 10 files changed, 431 insertions(+), 277 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index fcf3db309d..3757c86e96 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -696,9 +696,7 @@ Lexical syntax Ninja is mostly encoding agnostic, as long as the bytes Ninja cares about (like slashes in paths) are ASCII. This means e.g. UTF-8 or -ISO-8859-1 input files ought to work. (To simplify some code, tabs -and carriage returns are currently disallowed; this could be fixed if -it really mattered to you.) +ISO-8859-1 input files ought to work. Comments begin with `#` and extend to the end of the line. diff --git a/misc/write_fake_manifests.py b/misc/write_fake_manifests.py index 837007e603..ca49535857 100644 --- a/misc/write_fake_manifests.py +++ b/misc/write_fake_manifests.py @@ -182,7 +182,7 @@ def FileWriter(path): def random_targets(): - num_targets = 800 + num_targets = 1500 gen = GenRandom() # N-1 static libraries, and 1 executable depending on all of them. diff --git a/src/build_test.cc b/src/build_test.cc index 6336206bf6..a5b09724e0 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -755,7 +755,7 @@ TEST_F(BuildTest, MakeDirs) { #ifdef _WIN32 ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build subdir\\dir2\\file: cat in1\n")); - EXPECT_TRUE(builder_.AddTarget("subdir\\dir2\\file", &err)); + EXPECT_TRUE(builder_.AddTarget("subdir/dir2/file", &err)); #else ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build subdir/dir2/file: cat in1\n")); @@ -767,11 +767,7 @@ TEST_F(BuildTest, MakeDirs) { ASSERT_EQ("", err); ASSERT_EQ(2u, fs_.directories_made_.size()); EXPECT_EQ("subdir", fs_.directories_made_[0]); -#ifdef _WIN32 - EXPECT_EQ("subdir\\dir2", fs_.directories_made_[1]); -#else EXPECT_EQ("subdir/dir2", fs_.directories_made_[1]); -#endif } TEST_F(BuildTest, DepFileMissing) { diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index 05ce75d190..0d230f8377 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -68,12 +68,15 @@ string IncludesNormalize::ToLower(const string& s) { string IncludesNormalize::AbsPath(StringPiece s) { char result[_MAX_PATH]; GetFullPathName(s.AsString().c_str(), sizeof(result), result, NULL); + for (char* c = result; *c; ++c) + if (*c == '\\') + *c = '/'; return result; } string IncludesNormalize::Relativize(StringPiece path, const string& start) { - vector start_list = Split(AbsPath(start), '\\'); - vector path_list = Split(AbsPath(path), '\\'); + vector start_list = Split(AbsPath(start), '/'); + vector path_list = Split(AbsPath(path), '/'); int i; for (i = 0; i < static_cast(min(start_list.size(), path_list.size())); ++i) { @@ -88,7 +91,7 @@ string IncludesNormalize::Relativize(StringPiece path, const string& start) { rel_list.push_back(path_list[j]); if (rel_list.size() == 0) return "."; - return Join(rel_list, '\\'); + return Join(rel_list, '/'); } string IncludesNormalize::Normalize(const string& input, @@ -96,19 +99,16 @@ string IncludesNormalize::Normalize(const string& input, char copy[_MAX_PATH]; size_t len = input.size(); strncpy(copy, input.c_str(), input.size() + 1); - for (size_t j = 0; j < len; ++j) - if (copy[j] == '/') - copy[j] = '\\'; string err; - if (!CanonicalizePath(copy, &len, &err)) { - Warning("couldn't canonicalize '%s: %s\n", input.c_str(), err.c_str()); - } + if (!CanonicalizePath(copy, &len, &err)) + Warning("couldn't canonicalize '%s': %s\n", input.c_str(), err.c_str()); + StringPiece partially_fixed(copy, len); + string curdir; if (!relative_to) { curdir = AbsPath("."); relative_to = curdir.c_str(); } - StringPiece partially_fixed(copy, len); if (!SameDrive(partially_fixed, relative_to)) return partially_fixed.AsString(); return Relativize(partially_fixed, relative_to); diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index d9d21bf30f..c4c247630e 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -22,8 +22,8 @@ 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)); + EXPECT_EQ("a/b", IncludesNormalize::Normalize("a\\.\\b", NULL)); + EXPECT_EQ("a/b", IncludesNormalize::Normalize("a\\./b", NULL)); } namespace { @@ -42,21 +42,21 @@ TEST(IncludesNormalize, WithRelative) { EXPECT_EQ("c", IncludesNormalize::Normalize("a/b/c", "a/b")); EXPECT_EQ("a", IncludesNormalize::Normalize(IncludesNormalize::AbsPath("a"), NULL)); - EXPECT_EQ(string("..\\") + currentdir + string("\\a"), + EXPECT_EQ(string("../") + currentdir + string("/a"), IncludesNormalize::Normalize("a", "../b")); - EXPECT_EQ(string("..\\") + currentdir + string("\\a\\b"), + EXPECT_EQ(string("../") + currentdir + string("/a/b"), IncludesNormalize::Normalize("a/b", "../c")); - EXPECT_EQ("..\\..\\a", IncludesNormalize::Normalize("a", "b/c")); + EXPECT_EQ("../../a", IncludesNormalize::Normalize("a", "b/c")); EXPECT_EQ(".", IncludesNormalize::Normalize("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("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)); } TEST(IncludesNormalize, Join) { @@ -92,13 +92,13 @@ TEST(IncludesNormalize, DifferentDrive) { IncludesNormalize::Normalize("p:\\vs08\\stuff.h", "p:\\vs08")); EXPECT_EQ("stuff.h", IncludesNormalize::Normalize("P:\\Vs08\\stuff.h", "p:\\vs08")); - EXPECT_EQ("p:\\vs08\\stuff.h", + EXPECT_EQ("p:/vs08/stuff.h", IncludesNormalize::Normalize("p:\\vs08\\stuff.h", "c:\\vs08")); - EXPECT_EQ("P:\\vs08\\stufF.h", + EXPECT_EQ("P:/vs08/stufF.h", IncludesNormalize::Normalize("P:\\vs08\\stufF.h", "D:\\stuff/things")); - EXPECT_EQ("P:\\vs08\\stuff.h", + EXPECT_EQ("P:/vs08/stuff.h", IncludesNormalize::Normalize("P:/vs08\\stuff.h", "D:\\stuff/things")); - // TODO: this fails; fix it. - //EXPECT_EQ("P:\\wee\\stuff.h", - // IncludesNormalize::Normalize("P:/vs08\\../wee\\stuff.h", "D:\\stuff/things")); + EXPECT_EQ("P:/wee/stuff.h", + IncludesNormalize::Normalize("P:/vs08\\../wee\\stuff.h", + "D:\\stuff/things")); } diff --git a/src/lexer.cc b/src/lexer.cc index 685fe818fc..37b867885c 100644 --- a/src/lexer.cc +++ b/src/lexer.cc @@ -103,8 +103,6 @@ const char* Lexer::TokenErrorHint(Token expected) { string Lexer::DescribeLastError() { if (last_token_) { switch (last_token_[0]) { - case '\r': - return "carriage returns are not allowed, use newlines"; case '\t': return "tabs are not allowed, use spaces"; } @@ -129,7 +127,7 @@ Lexer::Token Lexer::ReadToken() { unsigned int yyaccept = 0; static const unsigned char yybm[] = { 0, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 0, 64, 64, 0, 64, 64, + 64, 64, 0, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 192, 64, 64, 64, 64, 64, 64, 64, @@ -163,56 +161,66 @@ Lexer::Token Lexer::ReadToken() { }; yych = *p; - if (yych <= '^') { - if (yych <= ',') { - if (yych <= 0x1F) { - if (yych <= 0x00) goto yy22; - if (yych == '\n') goto yy6; - goto yy24; + if (yych <= 'Z') { + if (yych <= '#') { + if (yych <= '\f') { + if (yych <= 0x00) goto yy23; + if (yych == '\n') goto yy7; + goto yy25; } else { - if (yych <= ' ') goto yy2; - if (yych == '#') goto yy4; - goto yy24; + if (yych <= 0x1F) { + if (yych <= '\r') goto yy6; + goto yy25; + } else { + if (yych <= ' ') goto yy2; + if (yych <= '"') goto yy25; + goto yy4; + } } } else { - if (yych <= ':') { - if (yych == '/') goto yy24; - if (yych <= '9') goto yy21; - goto yy15; + if (yych <= '9') { + if (yych <= ',') goto yy25; + if (yych == '/') goto yy25; + goto yy22; } else { - if (yych <= '=') { - if (yych <= '<') goto yy24; - goto yy13; + if (yych <= '<') { + if (yych <= ':') goto yy16; + goto yy25; } else { - if (yych <= '@') goto yy24; - if (yych <= 'Z') goto yy21; - goto yy24; + if (yych <= '=') goto yy14; + if (yych <= '@') goto yy25; + goto yy22; } } } } else { if (yych <= 'i') { - if (yych <= 'b') { - if (yych == '`') goto yy24; - if (yych <= 'a') goto yy21; - goto yy8; + if (yych <= 'a') { + if (yych == '_') goto yy22; + if (yych <= '`') goto yy25; + goto yy22; } else { - if (yych == 'd') goto yy12; - if (yych <= 'h') goto yy21; - goto yy19; + if (yych <= 'c') { + if (yych <= 'b') goto yy9; + goto yy22; + } else { + if (yych <= 'd') goto yy13; + if (yych <= 'h') goto yy22; + goto yy20; + } } } else { if (yych <= 'r') { - if (yych == 'p') goto yy10; - if (yych <= 'q') goto yy21; - goto yy11; + if (yych == 'p') goto yy11; + if (yych <= 'q') goto yy22; + goto yy12; } else { if (yych <= 'z') { - if (yych <= 's') goto yy20; - goto yy21; + if (yych <= 's') goto yy21; + goto yy22; } else { - if (yych == '|') goto yy17; - goto yy24; + if (yych == '|') goto yy18; + goto yy25; } } } @@ -220,192 +228,203 @@ Lexer::Token Lexer::ReadToken() { yy2: yyaccept = 0; yych = *(q = ++p); - goto yy70; + goto yy73; yy3: { token = INDENT; break; } yy4: yyaccept = 1; yych = *(q = ++p); - if (yych <= 0x00) goto yy5; - if (yych != '\r') goto yy65; + if (yych >= 0x01) goto yy68; yy5: { token = ERROR; break; } yy6: - ++p; + yych = *++p; + if (yych == '\n') goto yy65; + goto yy5; yy7: - { token = NEWLINE; break; } -yy8: ++p; - if ((yych = *p) == 'u') goto yy59; - goto yy26; +yy8: + { token = NEWLINE; break; } yy9: - { token = IDENT; break; } + ++p; + if ((yych = *p) == 'u') goto yy60; + goto yy27; yy10: - yych = *++p; - if (yych == 'o') goto yy55; - goto yy26; + { token = IDENT; break; } yy11: yych = *++p; - if (yych == 'u') goto yy51; - goto yy26; + if (yych == 'o') goto yy56; + goto yy27; yy12: yych = *++p; - if (yych == 'e') goto yy44; - goto yy26; + if (yych == 'u') goto yy52; + goto yy27; yy13: + yych = *++p; + if (yych == 'e') goto yy45; + goto yy27; +yy14: ++p; { token = EQUALS; break; } -yy15: +yy16: ++p; { token = COLON; break; } -yy17: +yy18: ++p; - if ((yych = *p) == '|') goto yy42; + if ((yych = *p) == '|') goto yy43; { token = PIPE; break; } -yy19: - yych = *++p; - if (yych == 'n') goto yy35; - goto yy26; yy20: yych = *++p; - if (yych == 'u') goto yy27; - goto yy26; + if (yych == 'n') goto yy36; + goto yy27; yy21: yych = *++p; - goto yy26; + if (yych == 'u') goto yy28; + goto yy27; yy22: + yych = *++p; + goto yy27; +yy23: ++p; { token = TEOF; break; } -yy24: +yy25: yych = *++p; goto yy5; -yy25: +yy26: ++p; yych = *p; -yy26: +yy27: if (yybm[0+yych] & 32) { - goto yy25; + goto yy26; } - goto yy9; -yy27: + goto yy10; +yy28: yych = *++p; - if (yych != 'b') goto yy26; + if (yych != 'b') goto yy27; yych = *++p; - if (yych != 'n') goto yy26; + if (yych != 'n') goto yy27; yych = *++p; - if (yych != 'i') goto yy26; + if (yych != 'i') goto yy27; yych = *++p; - if (yych != 'n') goto yy26; + if (yych != 'n') goto yy27; yych = *++p; - if (yych != 'j') goto yy26; + if (yych != 'j') goto yy27; yych = *++p; - if (yych != 'a') goto yy26; + if (yych != 'a') goto yy27; ++p; if (yybm[0+(yych = *p)] & 32) { - goto yy25; + goto yy26; } { token = SUBNINJA; break; } -yy35: +yy36: yych = *++p; - if (yych != 'c') goto yy26; + if (yych != 'c') goto yy27; yych = *++p; - if (yych != 'l') goto yy26; + if (yych != 'l') goto yy27; yych = *++p; - if (yych != 'u') goto yy26; + if (yych != 'u') goto yy27; yych = *++p; - if (yych != 'd') goto yy26; + if (yych != 'd') goto yy27; yych = *++p; - if (yych != 'e') goto yy26; + if (yych != 'e') goto yy27; ++p; if (yybm[0+(yych = *p)] & 32) { - goto yy25; + goto yy26; } { token = INCLUDE; break; } -yy42: +yy43: ++p; { token = PIPE2; break; } -yy44: +yy45: yych = *++p; - if (yych != 'f') goto yy26; + if (yych != 'f') goto yy27; yych = *++p; - if (yych != 'a') goto yy26; + if (yych != 'a') goto yy27; yych = *++p; - if (yych != 'u') goto yy26; + if (yych != 'u') goto yy27; yych = *++p; - if (yych != 'l') goto yy26; + if (yych != 'l') goto yy27; yych = *++p; - if (yych != 't') goto yy26; + if (yych != 't') goto yy27; ++p; if (yybm[0+(yych = *p)] & 32) { - goto yy25; + goto yy26; } { token = DEFAULT; break; } -yy51: +yy52: yych = *++p; - if (yych != 'l') goto yy26; + if (yych != 'l') goto yy27; yych = *++p; - if (yych != 'e') goto yy26; + if (yych != 'e') goto yy27; ++p; if (yybm[0+(yych = *p)] & 32) { - goto yy25; + goto yy26; } { token = RULE; break; } -yy55: +yy56: yych = *++p; - if (yych != 'o') goto yy26; + if (yych != 'o') goto yy27; yych = *++p; - if (yych != 'l') goto yy26; + if (yych != 'l') goto yy27; ++p; if (yybm[0+(yych = *p)] & 32) { - goto yy25; + goto yy26; } { token = POOL; break; } -yy59: +yy60: yych = *++p; - if (yych != 'i') goto yy26; + if (yych != 'i') goto yy27; yych = *++p; - if (yych != 'l') goto yy26; + if (yych != 'l') goto yy27; yych = *++p; - if (yych != 'd') goto yy26; + if (yych != 'd') goto yy27; ++p; if (yybm[0+(yych = *p)] & 32) { - goto yy25; + goto yy26; } { token = BUILD; break; } -yy64: +yy65: + ++p; + { token = NEWLINE; break; } +yy67: ++p; yych = *p; -yy65: +yy68: if (yybm[0+yych] & 64) { - goto yy64; + goto yy67; } - if (yych <= 0x00) goto yy66; - if (yych <= '\f') goto yy67; -yy66: + if (yych >= 0x01) goto yy70; +yy69: p = q; if (yyaccept <= 0) { goto yy3; } else { goto yy5; } -yy67: +yy70: ++p; { continue; } -yy69: +yy72: yyaccept = 0; q = ++p; yych = *p; -yy70: +yy73: if (yybm[0+yych] & 128) { - goto yy69; + goto yy72; + } + if (yych <= '\f') { + if (yych != '\n') goto yy3; + } else { + if (yych <= '\r') goto yy75; + if (yych == '#') goto yy67; + goto yy3; } - if (yych == '\n') goto yy71; - if (yych == '#') goto yy64; - goto yy3; -yy71: + yych = *++p; + goto yy8; +yy75: ++p; - yych = *p; - goto yy7; + if ((yych = *p) == '\n') goto yy65; + goto yy69; } } @@ -427,6 +446,7 @@ bool Lexer::PeekToken(Token token) { void Lexer::EatWhitespace() { const char* p = ofs_; + const char* q; for (;;) { ofs_ = p; @@ -468,39 +488,48 @@ void Lexer::EatWhitespace() { }; yych = *p; if (yych <= ' ') { - if (yych <= 0x00) goto yy78; - if (yych <= 0x1F) goto yy80; + if (yych <= 0x00) goto yy82; + if (yych <= 0x1F) goto yy84; } else { - if (yych == '$') goto yy76; - goto yy80; + if (yych == '$') goto yy80; + goto yy84; } ++p; yych = *p; - goto yy84; -yy75: + goto yy92; +yy79: { continue; } -yy76: - ++p; - if ((yych = *p) == '\n') goto yy81; -yy77: +yy80: + yych = *(q = ++p); + if (yych == '\n') goto yy85; + if (yych == '\r') goto yy87; +yy81: { break; } -yy78: +yy82: ++p; { break; } -yy80: +yy84: yych = *++p; - goto yy77; -yy81: + goto yy81; +yy85: ++p; { continue; } -yy83: +yy87: + yych = *++p; + if (yych == '\n') goto yy89; + p = q; + goto yy81; +yy89: + ++p; + { continue; } +yy91: ++p; yych = *p; -yy84: +yy92: if (yybm[0+yych] & 128) { - goto yy83; + goto yy91; } - goto yy75; + goto yy79; } } @@ -550,40 +579,40 @@ bool Lexer::ReadIdent(string* out) { yych = *p; if (yych <= '@') { if (yych <= '.') { - if (yych <= ',') goto yy89; + if (yych <= ',') goto yy97; } else { - if (yych <= '/') goto yy89; - if (yych >= ':') goto yy89; + if (yych <= '/') goto yy97; + if (yych >= ':') goto yy97; } } else { if (yych <= '_') { - if (yych <= 'Z') goto yy87; - if (yych <= '^') goto yy89; + if (yych <= 'Z') goto yy95; + if (yych <= '^') goto yy97; } else { - if (yych <= '`') goto yy89; - if (yych >= '{') goto yy89; + if (yych <= '`') goto yy97; + if (yych >= '{') goto yy97; } } -yy87: +yy95: ++p; yych = *p; - goto yy92; -yy88: + goto yy100; +yy96: { out->assign(start, p - start); break; } -yy89: +yy97: ++p; { return false; } -yy91: +yy99: ++p; yych = *p; -yy92: +yy100: if (yybm[0+yych] & 128) { - goto yy91; + goto yy99; } - goto yy88; + goto yy96; } } @@ -638,29 +667,36 @@ bool Lexer::ReadEvalString(EvalString* eval, bool path, string* err) { yych = *p; if (yych <= ' ') { if (yych <= '\n') { - if (yych <= 0x00) goto yy101; - if (yych >= '\n') goto yy97; + if (yych <= 0x00) goto yy110; + if (yych >= '\n') goto yy107; } else { - if (yych == '\r') goto yy103; - if (yych >= ' ') goto yy97; + if (yych == '\r') goto yy105; + if (yych >= ' ') goto yy107; } } else { if (yych <= '9') { - if (yych == '$') goto yy99; + if (yych == '$') goto yy109; } else { - if (yych <= ':') goto yy97; - if (yych == '|') goto yy97; + if (yych <= ':') goto yy107; + if (yych == '|') goto yy107; } } ++p; yych = *p; - goto yy126; -yy96: + goto yy140; +yy104: { eval->AddText(StringPiece(start, p - start)); continue; } -yy97: +yy105: + ++p; + if ((yych = *p) == '\n') goto yy137; + { + last_token_ = start; + return Error(DescribeLastError(), err); + } +yy107: ++p; { if (path) { @@ -673,137 +709,152 @@ bool Lexer::ReadEvalString(EvalString* eval, bool path, string* err) { continue; } } -yy99: - ++p; - if ((yych = *p) <= '/') { - if (yych <= ' ') { - if (yych == '\n') goto yy115; - if (yych <= 0x1F) goto yy104; - goto yy106; +yy109: + yych = *++p; + if (yych <= '-') { + if (yych <= 0x1F) { + if (yych <= '\n') { + if (yych <= '\t') goto yy112; + goto yy124; + } else { + if (yych == '\r') goto yy114; + goto yy112; + } } else { - if (yych <= '$') { - if (yych <= '#') goto yy104; - goto yy108; + if (yych <= '#') { + if (yych <= ' ') goto yy115; + goto yy112; } else { - if (yych == '-') goto yy110; - goto yy104; + if (yych <= '$') goto yy117; + if (yych <= ',') goto yy112; + goto yy119; } } } else { - if (yych <= '^') { - if (yych <= ':') { - if (yych <= '9') goto yy110; - goto yy112; + if (yych <= 'Z') { + if (yych <= '9') { + if (yych <= '/') goto yy112; + goto yy119; } else { - if (yych <= '@') goto yy104; - if (yych <= 'Z') goto yy110; - goto yy104; + if (yych <= ':') goto yy121; + if (yych <= '@') goto yy112; + goto yy119; } } else { if (yych <= '`') { - if (yych <= '_') goto yy110; - goto yy104; + if (yych == '_') goto yy119; + goto yy112; } else { - if (yych <= 'z') goto yy110; - if (yych <= '{') goto yy114; - goto yy104; + if (yych <= 'z') goto yy119; + if (yych <= '{') goto yy123; + goto yy112; } } } -yy100: - { - last_token_ = start; - return Error(DescribeLastError(), err); - } -yy101: +yy110: ++p; { last_token_ = start; return Error("unexpected EOF", err); } -yy103: - yych = *++p; - goto yy100; -yy104: +yy112: ++p; -yy105: +yy113: { last_token_ = start; return Error("bad $-escape (literal $ must be written as $$)", err); } -yy106: +yy114: + yych = *++p; + if (yych == '\n') goto yy134; + goto yy113; +yy115: ++p; { eval->AddText(StringPiece(" ", 1)); continue; } -yy108: +yy117: ++p; { eval->AddText(StringPiece("$", 1)); continue; } -yy110: +yy119: ++p; yych = *p; - goto yy124; -yy111: + goto yy133; +yy120: { eval->AddSpecial(StringPiece(start + 1, p - start - 1)); continue; } -yy112: +yy121: ++p; { eval->AddText(StringPiece(":", 1)); continue; } -yy114: +yy123: yych = *(q = ++p); if (yybm[0+yych] & 32) { - goto yy118; + goto yy127; } - goto yy105; -yy115: + goto yy113; +yy124: ++p; yych = *p; if (yybm[0+yych] & 16) { - goto yy115; + goto yy124; } { continue; } -yy118: +yy127: ++p; yych = *p; if (yybm[0+yych] & 32) { - goto yy118; + goto yy127; } - if (yych == '}') goto yy121; + if (yych == '}') goto yy130; p = q; - goto yy105; -yy121: + goto yy113; +yy130: ++p; { eval->AddSpecial(StringPiece(start + 2, p - start - 3)); continue; } -yy123: +yy132: ++p; yych = *p; -yy124: +yy133: if (yybm[0+yych] & 64) { - goto yy123; + goto yy132; } - goto yy111; -yy125: + goto yy120; +yy134: ++p; yych = *p; -yy126: + if (yych == ' ') goto yy134; + { + continue; + } +yy137: + ++p; + { + if (path) + p = start; + break; + } +yy139: + ++p; + yych = *p; +yy140: if (yybm[0+yych] & 128) { - goto yy125; + goto yy139; } - goto yy96; + goto yy104; } } diff --git a/src/lexer.in.cc b/src/lexer.in.cc index 93d5540c87..f861239089 100644 --- a/src/lexer.in.cc +++ b/src/lexer.in.cc @@ -102,8 +102,6 @@ const char* Lexer::TokenErrorHint(Token expected) { string Lexer::DescribeLastError() { if (last_token_) { switch (last_token_[0]) { - case '\r': - return "carriage returns are not allowed, use newlines"; case '\t': return "tabs are not allowed, use spaces"; } @@ -132,8 +130,9 @@ Lexer::Token Lexer::ReadToken() { simple_varname = [a-zA-Z0-9_-]+; varname = [a-zA-Z0-9_.-]+; - [ ]*"#"[^\000\r\n]*"\n" { continue; } - [ ]*[\n] { token = NEWLINE; break; } + [ ]*"#"[^\000\n]*"\n" { continue; } + [ ]*"\r\n" { token = NEWLINE; break; } + [ ]*"\n" { token = NEWLINE; break; } [ ]+ { token = INDENT; break; } "build" { token = BUILD; break; } "pool" { token = POOL; break; } @@ -168,13 +167,15 @@ bool Lexer::PeekToken(Token token) { void Lexer::EatWhitespace() { const char* p = ofs_; + const char* q; for (;;) { ofs_ = p; /*!re2c - [ ]+ { continue; } - "$\n" { continue; } - nul { break; } - [^] { break; } + [ ]+ { continue; } + "$\r\n" { continue; } + "$\n" { continue; } + nul { break; } + [^] { break; } */ } } @@ -207,6 +208,11 @@ bool Lexer::ReadEvalString(EvalString* eval, bool path, string* err) { eval->AddText(StringPiece(start, p - start)); continue; } + "\r\n" { + if (path) + p = start; + break; + } [ :|\n] { if (path) { p = start; @@ -226,6 +232,9 @@ bool Lexer::ReadEvalString(EvalString* eval, bool path, string* err) { eval->AddText(StringPiece(" ", 1)); continue; } + "$\r\n"[ ]* { + continue; + } "$\n"[ ]* { continue; } diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 20168dfc7f..aa3e9e4024 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -870,22 +870,18 @@ TEST_F(ParserTest, UTF8) { " description = compilaci\xC3\xB3\n")); } -// We might want to eventually allow CRLF to be nice to Windows developers, -// but for now just verify we error out with a nice message. TEST_F(ParserTest, CRLF) { State state; ManifestParser parser(&state, NULL); string err; - EXPECT_FALSE(parser.ParseTest("# comment with crlf\r\n", - &err)); - EXPECT_EQ("input:1: lexing error\n", - err); - - EXPECT_FALSE(parser.ParseTest("foo = foo\nbar = bar\r\n", - &err)); - EXPECT_EQ("input:2: carriage returns are not allowed, use newlines\n" - "bar = bar\r\n" - " ^ near here", - err); + EXPECT_TRUE(parser.ParseTest("# comment with crlf\r\n", &err)); + EXPECT_TRUE(parser.ParseTest("foo = foo\nbar = bar\r\n", &err)); + EXPECT_TRUE(parser.ParseTest( + "pool link_pool\r\n" + " depth = 15\r\n\r\n" + "rule xyz\r\n" + " command = something$expand \r\n" + " description = YAY!\r\n", + &err)); } diff --git a/src/util.cc b/src/util.cc index 8b69b71255..66ef113667 100644 --- a/src/util.cc +++ b/src/util.cc @@ -106,6 +106,12 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { return false; } +#ifdef _WIN32 + for (char* c = path; *c; ++c) + if (*c == '\\') + *c = '/'; +#endif + const int kMaxPathComponents = 30; char* components[kMaxPathComponents]; int component_count = 0; @@ -280,7 +286,38 @@ void GetWin32EscapedString(const string& input, string* result) { } int ReadFile(const string& path, string* contents, string* err) { - FILE* f = fopen(path.c_str(), "r"); +#ifdef _WIN32 + // This makes a ninja run on a set of 1500 manifest files about 4% faster + // than using the generic fopen code below. + err->clear(); + HANDLE f = ::CreateFile(path.c_str(), + GENERIC_READ, + FILE_SHARE_READ, + NULL, + OPEN_EXISTING, + FILE_FLAG_SEQUENTIAL_SCAN, + NULL); + if (f == INVALID_HANDLE_VALUE) { + err->assign(GetLastErrorString()); + return -ENOENT; + } + + for (;;) { + DWORD len; + char buf[64 << 10]; + if (!::ReadFile(f, buf, sizeof(buf), &len, NULL)) { + err->assign(GetLastErrorString()); + contents->clear(); + return -1; + } + if (len == 0) + break; + contents->append(buf, len); + } + ::CloseHandle(f); + return 0; +#else + FILE* f = fopen(path.c_str(), "rb"); if (!f) { err->assign(strerror(errno)); return -errno; @@ -299,6 +336,7 @@ int ReadFile(const string& path, string* contents, string* err) { } fclose(f); return 0; +#endif } void SetCloseOnExec(int fd) { diff --git a/src/util_test.cc b/src/util_test.cc index b58d15e8ce..e82f227869 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -84,6 +84,72 @@ TEST(CanonicalizePath, PathSamples) { EXPECT_EQ("", path); } +#ifdef _WIN32 +TEST(CanonicalizePath, PathSamplesWindows) { + string path; + string err; + + EXPECT_FALSE(CanonicalizePath(&path, &err)); + EXPECT_EQ("empty path", err); + + path = "foo.h"; err = ""; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("foo.h", path); + + path = ".\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("foo.h", path); + + path = ".\\foo\\.\\bar.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("foo/bar.h", path); + + path = ".\\x\\foo\\..\\bar.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("x/bar.h", path); + + path = ".\\x\\foo\\..\\..\\bar.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("bar.h", path); + + path = "foo\\\\bar"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("foo/bar", path); + + path = "foo\\\\.\\\\..\\\\\\bar"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("bar", path); + + path = ".\\x\\..\\foo\\..\\..\\bar.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("../bar.h", path); + + path = "foo\\.\\."; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("foo", path); + + path = "foo\\bar\\.."; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("foo", path); + + path = "foo\\.hidden_bar"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("foo/.hidden_bar", path); + + path = "\\foo"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("/foo", path); + + path = "\\\\foo"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("//foo", path); + + path = "\\"; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("", path); +} +#endif + TEST(CanonicalizePath, EmptyResult) { string path; string err; From 95e087bdd959a6609b81359a68df4bd966ee2f0c Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Thu, 30 Oct 2014 16:52:55 -0700 Subject: [PATCH 33/84] Use strchr in \ conversion in CanonicalizePath on Windows --- src/util.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util.cc b/src/util.cc index 66ef113667..cb8adf1350 100644 --- a/src/util.cc +++ b/src/util.cc @@ -107,9 +107,8 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { } #ifdef _WIN32 - for (char* c = path; *c; ++c) - if (*c == '\\') - *c = '/'; + for (char* c = path; (c = strchr(c, '\\')) != NULL;) + *c = '/'; #endif const int kMaxPathComponents = 30; From fa62cd4c47e3df4da80342e03741b4dda8181ede Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 1 Nov 2014 12:20:42 -0700 Subject: [PATCH 34/84] Fix building on Solaris. "SunOS" and "Solaris" are the same thing these days, so make them go down the same code paths. In particular, the browse feature was omitted on solaris but not sunos5, causing trouble for some folks (see #838). --- configure.py | 2 +- platform_helper.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/configure.py b/configure.py index 01fd30176a..70c4be5f9c 100755 --- a/configure.py +++ b/configure.py @@ -159,7 +159,7 @@ def binary(name): if platform.is_mingw(): cflags.remove('-fvisibility=hidden'); ldflags.append('-static') -elif platform.is_sunos5(): +elif platform.is_solaris(): cflags.remove('-fvisibility=hidden') elif platform.is_msvc(): pass diff --git a/platform_helper.py b/platform_helper.py index eab82626b8..035a671953 100644 --- a/platform_helper.py +++ b/platform_helper.py @@ -35,7 +35,7 @@ def __init__(self, platform): self._platform = 'freebsd' elif self._platform.startswith('openbsd'): self._platform = 'openbsd' - elif self._platform.startswith('solaris'): + elif self._platform.startswith('solaris') or self._platform == 'sunos5': self._platform = 'solaris' elif self._platform.startswith('mingw'): self._platform = 'mingw' @@ -78,9 +78,6 @@ def is_freebsd(self): def is_openbsd(self): return self._platform == 'openbsd' - def is_sunos5(self): - return self._platform == 'sunos5' - def is_bitrig(self): return self._platform == 'bitrig' From e24d31901fc79aa7348be46bef5dea8d0dce6c4b Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 7 Nov 2014 22:20:34 -0800 Subject: [PATCH 35/84] track back->forward conversions in a bitmask --- src/util.cc | 52 ++++++++++++++++++++++++++++++---- src/util.h | 6 ++++ src/util_test.cc | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 6 deletions(-) diff --git a/src/util.cc b/src/util.cc index cb8adf1350..6a9079e580 100644 --- a/src/util.cc +++ b/src/util.cc @@ -86,18 +86,40 @@ void Error(const char* msg, ...) { } bool CanonicalizePath(string* path, string* err) { + unsigned int unused; + return CanonicalizePath(path, err, &unused); +} + +bool CanonicalizePath(string* path, string* err, unsigned int* slash_bits) { METRIC_RECORD("canonicalize str"); size_t len = path->size(); char* str = 0; if (len > 0) str = &(*path)[0]; - if (!CanonicalizePath(str, &len, err)) + if (!CanonicalizePath(str, &len, err, slash_bits)) return false; path->resize(len); return true; } bool CanonicalizePath(char* path, size_t* len, string* err) { + unsigned int unused; + return CanonicalizePath(path, len, err, &unused); +} + +unsigned int ShiftOverBit(int offset, unsigned int bits) { + // e.g. for |offset| == 2: + // | ... 9 8 7 6 5 4 3 2 1 0 | + // \_________________/ \_/ + // above below + // So we drop the bit at offset and move above "down" into its place. + unsigned int above = bits & ~((1 << (offset + 1)) - 1); + unsigned int below = bits & ((1 << offset) - 1); + return (above >> 1) | below; +} + +bool CanonicalizePath(char* path, size_t* len, string* err, + unsigned int* slash_bits) { // WARNING: this function is performance-critical; please benchmark // any changes you make to it. METRIC_RECORD("canonicalize path"); @@ -106,15 +128,22 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { return false; } -#ifdef _WIN32 - for (char* c = path; (c = strchr(c, '\\')) != NULL;) - *c = '/'; -#endif - const int kMaxPathComponents = 30; char* components[kMaxPathComponents]; int component_count = 0; +#ifdef _WIN32 + // kMaxPathComponents protects this from overflowing. + unsigned int bits = 0; + int bits_offset = 0; + for (char* c = path; (c = strpbrk(c, "/\\")) != NULL;) { + bits |= (*c == '\\') << bits_offset; + *c++ = '/'; + bits_offset++; + } + bits_offset = 0; +#endif + char* start = path; char* dst = start; const char* src = start; @@ -122,10 +151,12 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { if (*src == '/') { #ifdef _WIN32 + bits_offset++; // network path starts with // if (*len > 1 && *(src + 1) == '/') { src += 2; dst += 2; + bits_offset++; } else { ++src; ++dst; @@ -141,6 +172,7 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { if (src + 1 == end || src[1] == '/') { // '.' component; eliminate. src += 2; + bits = ShiftOverBit(bits_offset, bits); continue; } else if (src[1] == '.' && (src + 2 == end || src[2] == '/')) { // '..' component. Back up if possible. @@ -148,6 +180,9 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { dst = components[component_count - 1]; src += 3; --component_count; + bits = ShiftOverBit(bits_offset, bits); + bits_offset--; + bits = ShiftOverBit(bits_offset, bits); } else { *dst++ = *src++; *dst++ = *src++; @@ -159,6 +194,7 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { if (*src == '/') { src++; + bits_offset++; continue; } @@ -169,6 +205,7 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { while (*src != '/' && src != end) *dst++ = *src++; + bits_offset++; *dst++ = *src++; // Copy '/' or final \0 character as well. } @@ -178,6 +215,9 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { } *len = dst - start - 1; +#ifdef _WIN32 + *slash_bits = bits; +#endif return true; } diff --git a/src/util.h b/src/util.h index 71017703ff..36f31f39a1 100644 --- a/src/util.h +++ b/src/util.h @@ -45,6 +45,12 @@ bool CanonicalizePath(string* path, string* err); bool CanonicalizePath(char* path, size_t* len, string* err); +/// |slash_bits| has bits set starting from lowest for a backslash that was +/// normalized to a forward slash. (only used on Windows) +bool CanonicalizePath(string* path, string* err, unsigned int* slash_bits); +bool CanonicalizePath(char* path, size_t* len, string* err, + unsigned int* slash_bits); + /// Appends |input| to |*result|, escaping according to the whims of either /// Bash, or Win32's CommandLineToArgvW(). /// Appends the string directly to |result| without modification if we can diff --git a/src/util_test.cc b/src/util_test.cc index e82f227869..36f212e936 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -148,6 +148,78 @@ TEST(CanonicalizePath, PathSamplesWindows) { EXPECT_TRUE(CanonicalizePath(&path, &err)); EXPECT_EQ("", path); } + +TEST(CanonicalizePath, SlashTracking) { + string path; + string err; + unsigned int slash_bits; + + path = "foo.h"; err = ""; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("foo.h", path); + EXPECT_EQ(0, slash_bits); + + path = "a\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a/bcd/efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/bcd/efh/foo.h", path); + EXPECT_EQ(4, slash_bits); + + path = "a\\bcd/efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/bcd/efh/foo.h", path); + EXPECT_EQ(5, slash_bits); + + path = "a\\bcd\\efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/bcd/efh/foo.h", path); + EXPECT_EQ(7, slash_bits); + + path = "a/bcd/efh/foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/bcd/efh/foo.h", path); + EXPECT_EQ(0, slash_bits); + + path = "a\\./efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/efh/foo.h", path); + EXPECT_EQ(3, slash_bits); + + path = "a\\../efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("efh/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a\\b\\c\\d\\e\\f\\g\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/b/c/d/e/f/g/foo.h", path); + EXPECT_EQ(127, slash_bits); + + path = "a\\b\\c\\..\\..\\..\\g\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("g/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a\\b/c\\../../..\\g\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("g/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a\\b/c\\./../..\\g\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/g/foo.h", path); + EXPECT_EQ(3, slash_bits); + + path = "a\\b/c\\./../..\\g/foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/g/foo.h", path); + EXPECT_EQ(1, slash_bits); +} + #endif TEST(CanonicalizePath, EmptyResult) { From aacfd606f463bc3a7c2cc92d961dbded7979051c Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 7 Nov 2014 22:38:59 -0800 Subject: [PATCH 36/84] wip on adding tests at higher level, some not right --- src/graph.h | 8 ++++++- src/manifest_parser.cc | 10 ++++---- src/manifest_parser_test.cc | 48 +++++++++++++++++++++++++++++++++++++ src/state.cc | 14 +++++++---- src/state.h | 5 ++-- src/state_test.cc | 6 ++--- 6 files changed, 76 insertions(+), 15 deletions(-) diff --git a/src/graph.h b/src/graph.h index 66e31b530c..5c4112b4d8 100644 --- a/src/graph.h +++ b/src/graph.h @@ -33,8 +33,9 @@ struct State; /// Information about a node in the dependency graph: the file, whether /// it's dirty, mtime, etc. struct Node { - explicit Node(const string& path) + Node(const string& path, unsigned int slash_bits) : path_(path), + slash_bits_(slash_bits), mtime_(-1), dirty_(false), in_edge_(NULL), @@ -71,6 +72,7 @@ struct Node { } const string& path() const { return path_; } + unsigned int slash_bits() const { return slash_bits_; } TimeStamp mtime() const { return mtime_; } bool dirty() const { return dirty_; } @@ -90,6 +92,10 @@ struct Node { private: string path_; + + /// XXX See CanonicalizePath. + unsigned int slash_bits_; + /// Possible values of mtime_: /// -1: file hasn't been examined /// 0: we looked, and file doesn't exist diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 55d191fcff..81a191e21b 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -323,16 +323,18 @@ bool ManifestParser::ParseEdge(string* err) { for (vector::iterator i = ins.begin(); i != ins.end(); ++i) { string path = i->Evaluate(env); string path_err; - if (!CanonicalizePath(&path, &path_err)) + unsigned int slash_bits; + if (!CanonicalizePath(&path, &path_err, &slash_bits)) return lexer_.Error(path_err, err); - state_->AddIn(edge, path); + state_->AddIn(edge, path, slash_bits); } for (vector::iterator i = outs.begin(); i != outs.end(); ++i) { string path = i->Evaluate(env); string path_err; - if (!CanonicalizePath(&path, &path_err)) + unsigned int slash_bits; + if (!CanonicalizePath(&path, &path_err, &slash_bits)) return lexer_.Error(path_err, err); - state_->AddOut(edge, path); + state_->AddOut(edge, path, slash_bits); } edge->implicit_deps_ = implicit; edge->order_only_deps_ = order_only; diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index aa3e9e4024..1998a776b5 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -269,6 +269,26 @@ TEST_F(ParserTest, CanonicalizeFile) { EXPECT_FALSE(state.LookupNode("in//2")); } +#ifdef _WIN32 +TEST_F(ParserTest, CanonicalizeFileBackslashes) { + ASSERT_NO_FATAL_FAILURE(AssertParse( +"rule cat\n" +" command = cat $in > $out\n" +"build out: cat in\\1 in\\\\2\n" +"build in\\1: cat\n" +"build in\\2: cat\n")); + + Node* node = state.LookupNode("in/1");; + EXPECT_TRUE(node); + EXPECT_EQ(1, node->slash_bits()); + node = state.LookupNode("in/2"); + EXPECT_TRUE(node); + EXPECT_EQ(1, node->slash_bits()); + EXPECT_FALSE(state.LookupNode("in//1")); + EXPECT_FALSE(state.LookupNode("in//2")); +} +#endif + TEST_F(ParserTest, PathVariables) { ASSERT_NO_FATAL_FAILURE(AssertParse( "rule cat\n" @@ -292,6 +312,34 @@ TEST_F(ParserTest, CanonicalizePaths) { EXPECT_TRUE(state.LookupNode("bar/foo.cc")); } +#ifdef _WIN32 +TEST_F(ParserTest, CanonicalizePathsBackslashes) { + ASSERT_NO_FATAL_FAILURE(AssertParse( +"rule cat\n" +" command = cat $in > $out\n" +"build ./out.o: cat ./bar/baz/../foo.cc\n" +"build .\\out2.o: cat .\\bar/baz\\..\\foo.cc\n" +"build .\\out3.o: cat .\\bar/baz\\..\\foo3.cc\n" +)); + + EXPECT_FALSE(state.LookupNode("./out.o")); + EXPECT_FALSE(state.LookupNode(".\\out2.o")); + EXPECT_FALSE(state.LookupNode(".\\out3.o")); + EXPECT_TRUE(state.LookupNode("out.o")); + EXPECT_TRUE(state.LookupNode("out2.o")); + EXPECT_TRUE(state.LookupNode("out3.o")); + EXPECT_FALSE(state.LookupNode("./bar/baz/../foo.cc")); + EXPECT_FALSE(state.LookupNode(".\\bar/baz\\..\\foo.cc")); + EXPECT_FALSE(state.LookupNode(".\\bar/baz\\..\\foo3.cc")); + Node* node = state.LookupNode("bar/foo.cc"); + EXPECT_TRUE(node); + EXPECT_EQ(0, node->slash_bits()); + node = state.LookupNode("bar/foo3.cc"); + EXPECT_TRUE(node); + EXPECT_EQ(1, node->slash_bits()); // First seen determines slashes. +} +#endif + TEST_F(ParserTest, ReservedWords) { ASSERT_NO_FATAL_FAILURE(AssertParse( "rule build\n" diff --git a/src/state.cc b/src/state.cc index 7258272056..b6c29ffb2f 100644 --- a/src/state.cc +++ b/src/state.cc @@ -112,10 +112,14 @@ Edge* State::AddEdge(const Rule* rule) { } Node* State::GetNode(StringPiece path) { + return GetNode(path, 0); +} + +Node* State::GetNode(StringPiece path, unsigned int slash_bits) { Node* node = LookupNode(path); if (node) return node; - node = new Node(path.AsString()); + node = new Node(path.AsString(), slash_bits); paths_[node->path()] = node; return node; } @@ -145,14 +149,14 @@ Node* State::SpellcheckNode(const string& path) { return result; } -void State::AddIn(Edge* edge, StringPiece path) { - Node* node = GetNode(path); +void State::AddIn(Edge* edge, StringPiece path, unsigned int slash_bits) { + Node* node = GetNode(path, slash_bits); edge->inputs_.push_back(node); node->AddOutEdge(edge); } -void State::AddOut(Edge* edge, StringPiece path) { - Node* node = GetNode(path); +void State::AddOut(Edge* edge, StringPiece path, unsigned int slash_bits) { + Node* node = GetNode(path, slash_bits); edge->outputs_.push_back(node); if (node->in_edge()) { Warning("multiple rules generate %s. " diff --git a/src/state.h b/src/state.h index c382dc014c..9da6d1bf8c 100644 --- a/src/state.h +++ b/src/state.h @@ -96,11 +96,12 @@ struct State { Edge* AddEdge(const Rule* rule); Node* GetNode(StringPiece path); + Node* GetNode(StringPiece path, unsigned int slash_bits); Node* LookupNode(StringPiece path) const; Node* SpellcheckNode(const string& path); - void AddIn(Edge* edge, StringPiece path); - void AddOut(Edge* edge, StringPiece path); + void AddIn(Edge* edge, StringPiece path, unsigned int slash_bits); + void AddOut(Edge* edge, StringPiece path, unsigned int slash_bits); bool AddDefault(StringPiece path, string* error); /// Reset state. Keeps all nodes and edges, but restores them to the diff --git a/src/state_test.cc b/src/state_test.cc index a4fafa1e9a..85c65137b5 100644 --- a/src/state_test.cc +++ b/src/state_test.cc @@ -32,9 +32,9 @@ TEST(State, Basic) { state.AddRule(rule); Edge* edge = state.AddEdge(rule); - state.AddIn(edge, "in1"); - state.AddIn(edge, "in2"); - state.AddOut(edge, "out"); + state.AddIn(edge, "in1", 0); + state.AddIn(edge, "in2", 0); + state.AddOut(edge, "out", 0); EXPECT_EQ("cat in1 in2 > out", edge->EvaluateCommand()); From 3fb18496c4c2642742df152974d78756d1c9df8a Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Sat, 8 Nov 2014 11:25:03 -0800 Subject: [PATCH 37/84] fix multiple sequential slashes --- src/manifest_parser_test.cc | 4 ++-- src/util.cc | 2 +- src/util_test.cc | 15 +++++++++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 1998a776b5..0668668664 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -319,7 +319,7 @@ TEST_F(ParserTest, CanonicalizePathsBackslashes) { " command = cat $in > $out\n" "build ./out.o: cat ./bar/baz/../foo.cc\n" "build .\\out2.o: cat .\\bar/baz\\..\\foo.cc\n" -"build .\\out3.o: cat .\\bar/baz\\..\\foo3.cc\n" +"build .\\out3.o: cat .\\bar\\baz\\..\\foo3.cc\n" )); EXPECT_FALSE(state.LookupNode("./out.o")); @@ -336,7 +336,7 @@ TEST_F(ParserTest, CanonicalizePathsBackslashes) { EXPECT_EQ(0, node->slash_bits()); node = state.LookupNode("bar/foo3.cc"); EXPECT_TRUE(node); - EXPECT_EQ(1, node->slash_bits()); // First seen determines slashes. + EXPECT_EQ(1, node->slash_bits()); } #endif diff --git a/src/util.cc b/src/util.cc index 6a9079e580..9cf736edaf 100644 --- a/src/util.cc +++ b/src/util.cc @@ -194,7 +194,7 @@ bool CanonicalizePath(char* path, size_t* len, string* err, if (*src == '/') { src++; - bits_offset++; + bits = ShiftOverBit(bits_offset, bits); continue; } diff --git a/src/util_test.cc b/src/util_test.cc index 36f212e936..d047d9c9a8 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -218,6 +218,21 @@ TEST(CanonicalizePath, SlashTracking) { EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); EXPECT_EQ("a/g/foo.h", path); EXPECT_EQ(1, slash_bits); + + path = "a\\\\\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a/\\\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/foo.h", path); + EXPECT_EQ(0, slash_bits); + + path = "a\\//foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_EQ("a/foo.h", path); + EXPECT_EQ(1, slash_bits); } #endif From 8177085f4d3adf78b9709069a9c3ce5fe442867a Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Sat, 8 Nov 2014 11:47:22 -0800 Subject: [PATCH 38/84] path decanonicalization when building command --- src/build_test.cc | 3 +-- src/graph.cc | 14 +++++++++++++- src/graph.h | 5 ++++- src/graph_test.cc | 21 +++++++++++++++++++++ 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/build_test.cc b/src/build_test.cc index a5b09724e0..5d6c4e9a3f 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -755,12 +755,11 @@ TEST_F(BuildTest, MakeDirs) { #ifdef _WIN32 ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build subdir\\dir2\\file: cat in1\n")); - EXPECT_TRUE(builder_.AddTarget("subdir/dir2/file", &err)); #else ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build subdir/dir2/file: cat in1\n")); - EXPECT_TRUE(builder_.AddTarget("subdir/dir2/file", &err)); #endif + EXPECT_TRUE(builder_.AddTarget("subdir/dir2/file", &err)); EXPECT_EQ("", err); EXPECT_TRUE(builder_.Build(&err)); diff --git a/src/graph.cc b/src/graph.cc index aa9c0e8c5e..5bc5c0048a 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -256,7 +256,7 @@ string EdgeEnv::MakePathList(vector::iterator begin, for (vector::iterator i = begin; i != end; ++i) { if (!result.empty()) result.push_back(sep); - const string& path = (*i)->path(); + const string& path = (*i)->PathDecanonicalized(); if (escape_in_out_ == kShellEscape) { #if _WIN32 GetWin32EscapedString(path, &result); @@ -328,6 +328,18 @@ bool Edge::use_console() const { return pool() == &State::kConsolePool; } +string Node::PathDecanonicalized() const { + string result = path_; + unsigned int mask = 1; + for (char* c = &result[0]; (c = strpbrk(c, "/\\")) != NULL;) { + if (slash_bits_ & mask) + *c = '\\'; + c++; + mask <<= 1; + } + return result; +} + void Node::Dump(const char* prefix) const { printf("%s <%s 0x%p> mtime: %d%s, (:%s), ", prefix, path().c_str(), this, diff --git a/src/graph.h b/src/graph.h index 5c4112b4d8..9aada38562 100644 --- a/src/graph.h +++ b/src/graph.h @@ -72,6 +72,8 @@ struct Node { } const string& path() const { return path_; } + /// Get |path()| but use slash_bits to convert back to original slash styles. + string PathDecanonicalized() const; unsigned int slash_bits() const { return slash_bits_; } TimeStamp mtime() const { return mtime_; } @@ -93,7 +95,8 @@ struct Node { private: string path_; - /// XXX See CanonicalizePath. + /// Set bits starting from lowest for backslashes that were normalized to + /// forward slashes by CanonicalizePath. See |PathDecanonicalized|. unsigned int slash_bits_; /// Possible values of mtime_: diff --git a/src/graph_test.cc b/src/graph_test.cc index 14dc6780b5..382d352def 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -251,3 +251,24 @@ TEST_F(GraphTest, NestedPhonyPrintsDone) { EXPECT_EQ(0, plan_.command_edge_count()); ASSERT_FALSE(plan_.more_to_do()); } + +#ifdef _WIN32 +TEST_F(GraphTest, Decanonicalize) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build out\\out1: cat src\\in1\n" +"build out\\out2/out3\\out4: cat mid1\n" +"build out3 out4\\foo: cat mid1\n")); + + string err; + vector root_nodes = state_.RootNodes(&err); + EXPECT_EQ(4u, root_nodes.size()); + EXPECT_EQ(root_nodes[0]->path(), "out/out1"); + EXPECT_EQ(root_nodes[1]->path(), "out/out2/out3/out4"); + EXPECT_EQ(root_nodes[2]->path(), "out3"); + EXPECT_EQ(root_nodes[3]->path(), "out4/foo"); + EXPECT_EQ(root_nodes[0]->PathDecanonicalized(), "out\\out1"); + EXPECT_EQ(root_nodes[1]->PathDecanonicalized(), "out\\out2/out3\\out4"); + EXPECT_EQ(root_nodes[2]->PathDecanonicalized(), "out3"); + EXPECT_EQ(root_nodes[3]->PathDecanonicalized(), "out4\\foo"); +} +#endif From 559389de082406f44ae752a2494e53000d31a7df Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Sat, 8 Nov 2014 22:49:21 -0800 Subject: [PATCH 39/84] remove CanonicalizePath overloads, test for toplevel behaviour --- src/build.cc | 6 ++++-- src/build_test.cc | 29 +++++++++++++++++++++++++++++ src/canon_perftest.cc | 3 ++- src/graph.cc | 11 +++++++++-- src/includes_normalize-win32.cc | 3 ++- src/manifest_parser.cc | 3 ++- src/ninja.cc | 6 ++++-- src/util.cc | 10 ---------- src/util.h | 4 ---- src/util_test.cc | 14 ++++++++++++-- 10 files changed, 64 insertions(+), 25 deletions(-) diff --git a/src/build.cc b/src/build.cc index fb1cc74834..a5f5fe89cd 100644 --- a/src/build.cc +++ b/src/build.cc @@ -850,9 +850,11 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, deps_nodes->reserve(deps.ins_.size()); for (vector::iterator i = deps.ins_.begin(); i != deps.ins_.end(); ++i) { - if (!CanonicalizePath(const_cast(i->str_), &i->len_, err)) + unsigned int slash_bits; + if (!CanonicalizePath(const_cast(i->str_), &i->len_, err, + &slash_bits)) return false; - deps_nodes->push_back(state_->GetNode(*i)); + deps_nodes->push_back(state_->GetNode(*i, slash_bits)); } if (disk_interface_->RemoveFile(depfile) < 0) { diff --git a/src/build_test.cc b/src/build_test.cc index 5d6c4e9a3f..f25dd5fa6f 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -937,6 +937,35 @@ TEST_F(BuildTest, RebuildOrderOnlyDeps) { ASSERT_EQ("cc oo.h.in", command_runner_.commands_ran_[0]); } +#ifdef _WIN32 +TEST_F(BuildTest, DepFileCanonicalize) { + string err; + int orig_edges = state_.edges_.size(); + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule cc\n command = cc $in\n depfile = $out.d\n" +"build gen/stuff/foo.o: cc foo.c\n")); + Edge* edge = state_.edges_.back(); + + fs_.Create("foo.c", ""); + GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. + // Note, different slashes from manifest. + fs_.Create("gen/stuff/foo.o.d", "gen\\stuff\\foo.o: blah.h bar.h\n"); + EXPECT_TRUE(builder_.AddTarget("gen/stuff/foo.o", &err)); + ASSERT_EQ("", err); + ASSERT_EQ(1u, fs_.files_read_.size()); + EXPECT_EQ("gen/stuff/foo.o.d", fs_.files_read_[0]); + + // Expect three new edges: one generating foo.o, and two more from + // loading the depfile. + ASSERT_EQ(orig_edges + 3, (int)state_.edges_.size()); + // Expect our edge to now have three inputs: foo.c and two headers. + ASSERT_EQ(3u, edge->inputs_.size()); + + // Expect the command line we generate to only use the original input. + ASSERT_EQ("cc foo.c", edge->EvaluateCommand()); +} +#endif + TEST_F(BuildTest, Phony) { string err; ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, diff --git a/src/canon_perftest.cc b/src/canon_perftest.cc index 59bd18f452..8defa91cbb 100644 --- a/src/canon_perftest.cc +++ b/src/canon_perftest.cc @@ -33,8 +33,9 @@ int main() { for (int j = 0; j < 5; ++j) { const int kNumRepetitions = 2000000; int64_t start = GetTimeMillis(); + unsigned int slash_bits; for (int i = 0; i < kNumRepetitions; ++i) { - CanonicalizePath(buf, &len, &err); + CanonicalizePath(buf, &len, &err, &slash_bits); } int delta = (int)(GetTimeMillis() - start); times.push_back(delta); diff --git a/src/graph.cc b/src/graph.cc index 5bc5c0048a..8666f50db7 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -391,6 +391,11 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, return false; } + unsigned int unused; + if (!CanonicalizePath(const_cast(depfile.out_.str_), + &depfile.out_.len_, err, &unused)) + return false; + // Check that this depfile matches the edge's output. Node* first_output = edge->outputs_[0]; StringPiece opath = StringPiece(first_output->path()); @@ -407,10 +412,12 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, // Add all its in-edges. for (vector::iterator i = depfile.ins_.begin(); i != depfile.ins_.end(); ++i, ++implicit_dep) { - if (!CanonicalizePath(const_cast(i->str_), &i->len_, err)) + unsigned int slash_bits; + if (!CanonicalizePath(const_cast(i->str_), &i->len_, err, + &slash_bits)) return false; - Node* node = state_->GetNode(*i); + Node* node = state_->GetNode(*i, slash_bits); *implicit_dep = node; node->AddOutEdge(edge); CreatePhonyInEdge(node); diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index 0d230f8377..7ea2813d5d 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -100,7 +100,8 @@ string IncludesNormalize::Normalize(const string& input, size_t len = input.size(); strncpy(copy, input.c_str(), input.size() + 1); string err; - if (!CanonicalizePath(copy, &len, &err)) + unsigned int slash_bits; + if (!CanonicalizePath(copy, &len, &err, &slash_bits)) Warning("couldn't canonicalize '%s': %s\n", input.c_str(), err.c_str()); StringPiece partially_fixed(copy, len); diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 81a191e21b..16214f1d88 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -209,7 +209,8 @@ bool ManifestParser::ParseDefault(string* err) { do { string path = eval.Evaluate(env_); string path_err; - if (!CanonicalizePath(&path, &path_err)) + unsigned int slash_bits; // Unused because this only does lookup. + if (!CanonicalizePath(&path, &path_err, &slash_bits)) return lexer_.Error(path_err, err); if (!state_->AddDefault(path, &path_err)) return lexer_.Error(path_err, err); diff --git a/src/ninja.cc b/src/ninja.cc index 4368fabda2..5831e26905 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -228,7 +228,8 @@ struct RealFileReader : public ManifestParser::FileReader { /// Returns true if the manifest was rebuilt. bool NinjaMain::RebuildManifest(const char* input_file, string* err) { string path = input_file; - if (!CanonicalizePath(&path, err)) + unsigned int slash_bits; // Unused because this path is only used for lookup. + if (!CanonicalizePath(&path, err, &slash_bits)) return false; Node* node = state_.LookupNode(path); if (!node) @@ -250,7 +251,8 @@ bool NinjaMain::RebuildManifest(const char* input_file, string* err) { Node* NinjaMain::CollectTarget(const char* cpath, string* err) { string path = cpath; - if (!CanonicalizePath(&path, err)) + unsigned int slash_bits; // Unused because this path is only used for lookup. + if (!CanonicalizePath(&path, err, &slash_bits)) return NULL; // Special syntax: "foo.cc^" means "the first output of foo.cc". diff --git a/src/util.cc b/src/util.cc index 9cf736edaf..cccf59c4fa 100644 --- a/src/util.cc +++ b/src/util.cc @@ -85,11 +85,6 @@ void Error(const char* msg, ...) { fprintf(stderr, "\n"); } -bool CanonicalizePath(string* path, string* err) { - unsigned int unused; - return CanonicalizePath(path, err, &unused); -} - bool CanonicalizePath(string* path, string* err, unsigned int* slash_bits) { METRIC_RECORD("canonicalize str"); size_t len = path->size(); @@ -102,11 +97,6 @@ bool CanonicalizePath(string* path, string* err, unsigned int* slash_bits) { return true; } -bool CanonicalizePath(char* path, size_t* len, string* err) { - unsigned int unused; - return CanonicalizePath(path, len, err, &unused); -} - unsigned int ShiftOverBit(int offset, unsigned int bits) { // e.g. for |offset| == 2: // | ... 9 8 7 6 5 4 3 2 1 0 | diff --git a/src/util.h b/src/util.h index 36f31f39a1..cb0de094bf 100644 --- a/src/util.h +++ b/src/util.h @@ -41,10 +41,6 @@ void Warning(const char* msg, ...); void Error(const char* msg, ...); /// Canonicalize a path like "foo/../bar.h" into just "bar.h". -bool CanonicalizePath(string* path, string* err); - -bool CanonicalizePath(char* path, size_t* len, string* err); - /// |slash_bits| has bits set starting from lowest for a backslash that was /// normalized to a forward slash. (only used on Windows) bool CanonicalizePath(string* path, string* err, unsigned int* slash_bits); diff --git a/src/util_test.cc b/src/util_test.cc index d047d9c9a8..82db6d257d 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -16,6 +16,15 @@ #include "test.h" +namespace { + +bool CanonicalizePath(string* path, string* err) { + unsigned int unused; + return ::CanonicalizePath(path, err, &unused); +} + +} // namespace + TEST(CanonicalizePath, PathSamples) { string path; string err; @@ -275,16 +284,17 @@ TEST(CanonicalizePath, NotNullTerminated) { string path; string err; size_t len; + unsigned int unused; path = "foo/. bar/."; len = strlen("foo/."); // Canonicalize only the part before the space. - EXPECT_TRUE(CanonicalizePath(&path[0], &len, &err)); + EXPECT_TRUE(CanonicalizePath(&path[0], &len, &err, &unused)); EXPECT_EQ(strlen("foo"), len); EXPECT_EQ("foo/. bar/.", string(path)); path = "foo/../file bar/."; len = strlen("foo/../file"); - EXPECT_TRUE(CanonicalizePath(&path[0], &len, &err)); + EXPECT_TRUE(CanonicalizePath(&path[0], &len, &err, &unused)); EXPECT_EQ(strlen("file"), len); EXPECT_EQ("file ./file bar/.", string(path)); } From 4ee1cb54d77d902a0223ebde12dff3b3efec8a8d Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Sat, 8 Nov 2014 23:01:23 -0800 Subject: [PATCH 40/84] fix order of args to CanonicalizePath --- src/build.cc | 4 ++-- src/canon_perftest.cc | 2 +- src/graph.cc | 6 +++--- src/includes_normalize-win32.cc | 2 +- src/manifest_parser.cc | 6 +++--- src/ninja.cc | 4 ++-- src/util.cc | 8 +++---- src/util.h | 6 +++--- src/util_test.cc | 38 ++++++++++++++++----------------- 9 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/build.cc b/src/build.cc index a5f5fe89cd..a346028978 100644 --- a/src/build.cc +++ b/src/build.cc @@ -851,8 +851,8 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, for (vector::iterator i = deps.ins_.begin(); i != deps.ins_.end(); ++i) { unsigned int slash_bits; - if (!CanonicalizePath(const_cast(i->str_), &i->len_, err, - &slash_bits)) + if (!CanonicalizePath(const_cast(i->str_), &i->len_, &slash_bits, + err)) return false; deps_nodes->push_back(state_->GetNode(*i, slash_bits)); } diff --git a/src/canon_perftest.cc b/src/canon_perftest.cc index 8defa91cbb..389ac24636 100644 --- a/src/canon_perftest.cc +++ b/src/canon_perftest.cc @@ -35,7 +35,7 @@ int main() { int64_t start = GetTimeMillis(); unsigned int slash_bits; for (int i = 0; i < kNumRepetitions; ++i) { - CanonicalizePath(buf, &len, &err, &slash_bits); + CanonicalizePath(buf, &len, &slash_bits, &err); } int delta = (int)(GetTimeMillis() - start); times.push_back(delta); diff --git a/src/graph.cc b/src/graph.cc index 8666f50db7..f85a1f62c9 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -393,7 +393,7 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, unsigned int unused; if (!CanonicalizePath(const_cast(depfile.out_.str_), - &depfile.out_.len_, err, &unused)) + &depfile.out_.len_, &unused, err)) return false; // Check that this depfile matches the edge's output. @@ -413,8 +413,8 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, for (vector::iterator i = depfile.ins_.begin(); i != depfile.ins_.end(); ++i, ++implicit_dep) { unsigned int slash_bits; - if (!CanonicalizePath(const_cast(i->str_), &i->len_, err, - &slash_bits)) + if (!CanonicalizePath(const_cast(i->str_), &i->len_, &slash_bits, + err)) return false; Node* node = state_->GetNode(*i, slash_bits); diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index 7ea2813d5d..1e88a0ab65 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -101,7 +101,7 @@ string IncludesNormalize::Normalize(const string& input, strncpy(copy, input.c_str(), input.size() + 1); string err; unsigned int slash_bits; - if (!CanonicalizePath(copy, &len, &err, &slash_bits)) + if (!CanonicalizePath(copy, &len, &slash_bits, &err)) Warning("couldn't canonicalize '%s': %s\n", input.c_str(), err.c_str()); StringPiece partially_fixed(copy, len); diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 16214f1d88..388b5bc679 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -210,7 +210,7 @@ bool ManifestParser::ParseDefault(string* err) { string path = eval.Evaluate(env_); string path_err; unsigned int slash_bits; // Unused because this only does lookup. - if (!CanonicalizePath(&path, &path_err, &slash_bits)) + if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); if (!state_->AddDefault(path, &path_err)) return lexer_.Error(path_err, err); @@ -325,7 +325,7 @@ bool ManifestParser::ParseEdge(string* err) { string path = i->Evaluate(env); string path_err; unsigned int slash_bits; - if (!CanonicalizePath(&path, &path_err, &slash_bits)) + if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); state_->AddIn(edge, path, slash_bits); } @@ -333,7 +333,7 @@ bool ManifestParser::ParseEdge(string* err) { string path = i->Evaluate(env); string path_err; unsigned int slash_bits; - if (!CanonicalizePath(&path, &path_err, &slash_bits)) + if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); state_->AddOut(edge, path, slash_bits); } diff --git a/src/ninja.cc b/src/ninja.cc index 5831e26905..27380c0efc 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -229,7 +229,7 @@ struct RealFileReader : public ManifestParser::FileReader { bool NinjaMain::RebuildManifest(const char* input_file, string* err) { string path = input_file; unsigned int slash_bits; // Unused because this path is only used for lookup. - if (!CanonicalizePath(&path, err, &slash_bits)) + if (!CanonicalizePath(&path, &slash_bits, err)) return false; Node* node = state_.LookupNode(path); if (!node) @@ -252,7 +252,7 @@ bool NinjaMain::RebuildManifest(const char* input_file, string* 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. - if (!CanonicalizePath(&path, err, &slash_bits)) + if (!CanonicalizePath(&path, &slash_bits, err)) return NULL; // Special syntax: "foo.cc^" means "the first output of foo.cc". diff --git a/src/util.cc b/src/util.cc index cccf59c4fa..ed878d1fcc 100644 --- a/src/util.cc +++ b/src/util.cc @@ -85,13 +85,13 @@ void Error(const char* msg, ...) { fprintf(stderr, "\n"); } -bool CanonicalizePath(string* path, string* err, unsigned int* slash_bits) { +bool CanonicalizePath(string* path, unsigned int* slash_bits, string* err) { METRIC_RECORD("canonicalize str"); size_t len = path->size(); char* str = 0; if (len > 0) str = &(*path)[0]; - if (!CanonicalizePath(str, &len, err, slash_bits)) + if (!CanonicalizePath(str, &len, slash_bits, err)) return false; path->resize(len); return true; @@ -108,8 +108,8 @@ unsigned int ShiftOverBit(int offset, unsigned int bits) { return (above >> 1) | below; } -bool CanonicalizePath(char* path, size_t* len, string* err, - unsigned int* slash_bits) { +bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, + string* err) { // WARNING: this function is performance-critical; please benchmark // any changes you make to it. METRIC_RECORD("canonicalize path"); diff --git a/src/util.h b/src/util.h index cb0de094bf..cbdc1a61f5 100644 --- a/src/util.h +++ b/src/util.h @@ -43,9 +43,9 @@ void Error(const char* msg, ...); /// Canonicalize a path like "foo/../bar.h" into just "bar.h". /// |slash_bits| has bits set starting from lowest for a backslash that was /// normalized to a forward slash. (only used on Windows) -bool CanonicalizePath(string* path, string* err, unsigned int* slash_bits); -bool CanonicalizePath(char* path, size_t* len, string* err, - unsigned int* slash_bits); +bool CanonicalizePath(string* path, unsigned int* slash_bits, string* err); +bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, + string* err); /// Appends |input| to |*result|, escaping according to the whims of either /// Bash, or Win32's CommandLineToArgvW(). diff --git a/src/util_test.cc b/src/util_test.cc index 82db6d257d..8ec7a284fa 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -20,7 +20,7 @@ namespace { bool CanonicalizePath(string* path, string* err) { unsigned int unused; - return ::CanonicalizePath(path, err, &unused); + return ::CanonicalizePath(path, &unused, err); } } // namespace @@ -164,82 +164,82 @@ TEST(CanonicalizePath, SlashTracking) { unsigned int slash_bits; path = "foo.h"; err = ""; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("foo.h", path); EXPECT_EQ(0, slash_bits); path = "a\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/foo.h", path); EXPECT_EQ(1, slash_bits); path = "a/bcd/efh\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/bcd/efh/foo.h", path); EXPECT_EQ(4, slash_bits); path = "a\\bcd/efh\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/bcd/efh/foo.h", path); EXPECT_EQ(5, slash_bits); path = "a\\bcd\\efh\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/bcd/efh/foo.h", path); EXPECT_EQ(7, slash_bits); path = "a/bcd/efh/foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/bcd/efh/foo.h", path); EXPECT_EQ(0, slash_bits); path = "a\\./efh\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/efh/foo.h", path); EXPECT_EQ(3, slash_bits); path = "a\\../efh\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("efh/foo.h", path); EXPECT_EQ(1, slash_bits); path = "a\\b\\c\\d\\e\\f\\g\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/b/c/d/e/f/g/foo.h", path); EXPECT_EQ(127, slash_bits); path = "a\\b\\c\\..\\..\\..\\g\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("g/foo.h", path); EXPECT_EQ(1, slash_bits); path = "a\\b/c\\../../..\\g\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("g/foo.h", path); EXPECT_EQ(1, slash_bits); path = "a\\b/c\\./../..\\g\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/g/foo.h", path); EXPECT_EQ(3, slash_bits); path = "a\\b/c\\./../..\\g/foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/g/foo.h", path); EXPECT_EQ(1, slash_bits); path = "a\\\\\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/foo.h", path); EXPECT_EQ(1, slash_bits); path = "a/\\\\foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/foo.h", path); EXPECT_EQ(0, slash_bits); path = "a\\//foo.h"; - EXPECT_TRUE(CanonicalizePath(&path, &err, &slash_bits)); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ("a/foo.h", path); EXPECT_EQ(1, slash_bits); } @@ -288,13 +288,13 @@ TEST(CanonicalizePath, NotNullTerminated) { path = "foo/. bar/."; len = strlen("foo/."); // Canonicalize only the part before the space. - EXPECT_TRUE(CanonicalizePath(&path[0], &len, &err, &unused)); + EXPECT_TRUE(CanonicalizePath(&path[0], &len, &unused, &err)); EXPECT_EQ(strlen("foo"), len); EXPECT_EQ("foo/. bar/.", string(path)); path = "foo/../file bar/."; len = strlen("foo/../file"); - EXPECT_TRUE(CanonicalizePath(&path[0], &len, &err, &unused)); + EXPECT_TRUE(CanonicalizePath(&path[0], &len, &unused, &err)); EXPECT_EQ(strlen("file"), len); EXPECT_EQ("file ./file bar/.", string(path)); } From bbeee64de290de274703c64b81d3704bd0a503e0 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Sat, 8 Nov 2014 23:10:34 -0800 Subject: [PATCH 41/84] assert no slashes in default GetNode --- src/state.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/state.cc b/src/state.cc index b6c29ffb2f..7d1d79d995 100644 --- a/src/state.cc +++ b/src/state.cc @@ -112,6 +112,9 @@ Edge* State::AddEdge(const Rule* rule) { } Node* State::GetNode(StringPiece path) { +#if defined(_WIN32) && !defined(NDEBUG) + assert(strpbrk(path.AsString().c_str(), "/\\") == NULL); +#endif return GetNode(path, 0); } From b01bd31b2576c53114a27426c07c33879e1ee9f2 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Sat, 8 Nov 2014 23:32:21 -0800 Subject: [PATCH 42/84] improve test --- src/build_test.cc | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/build_test.cc b/src/build_test.cc index f25dd5fa6f..912e3ef504 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -943,17 +943,19 @@ TEST_F(BuildTest, DepFileCanonicalize) { int orig_edges = state_.edges_.size(); ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule cc\n command = cc $in\n depfile = $out.d\n" -"build gen/stuff/foo.o: cc foo.c\n")); +"build gen/stuff\\things/foo.o: cc x\\y/z\\foo.c\n")); Edge* edge = state_.edges_.back(); - fs_.Create("foo.c", ""); + fs_.Create("x/y/z/foo.c", ""); GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. // Note, different slashes from manifest. - fs_.Create("gen/stuff/foo.o.d", "gen\\stuff\\foo.o: blah.h bar.h\n"); - EXPECT_TRUE(builder_.AddTarget("gen/stuff/foo.o", &err)); + fs_.Create("gen/stuff\\things/foo.o.d", + "gen\\stuff\\things\\foo.o: blah.h bar.h\n"); + EXPECT_TRUE(builder_.AddTarget("gen/stuff/things/foo.o", &err)); ASSERT_EQ("", err); ASSERT_EQ(1u, fs_.files_read_.size()); - EXPECT_EQ("gen/stuff/foo.o.d", fs_.files_read_[0]); + // The depfile path does not get Canonicalize as it seems unnecessary. + EXPECT_EQ("gen/stuff\\things/foo.o.d", fs_.files_read_[0]); // Expect three new edges: one generating foo.o, and two more from // loading the depfile. @@ -961,8 +963,9 @@ TEST_F(BuildTest, DepFileCanonicalize) { // Expect our edge to now have three inputs: foo.c and two headers. ASSERT_EQ(3u, edge->inputs_.size()); - // Expect the command line we generate to only use the original input. - ASSERT_EQ("cc foo.c", edge->EvaluateCommand()); + // Expect the command line we generate to only use the original input, and + // using the slashes from the manifest. + ASSERT_EQ("cc x\\y/z\\foo.c", edge->EvaluateCommand()); } #endif From fccce3f6731276f369e4454c59556998cdbb22d4 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Sat, 8 Nov 2014 23:36:26 -0800 Subject: [PATCH 43/84] non-win compilation --- src/util.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/util.cc b/src/util.cc index ed878d1fcc..d15837f5ce 100644 --- a/src/util.cc +++ b/src/util.cc @@ -97,6 +97,7 @@ bool CanonicalizePath(string* path, unsigned int* slash_bits, string* err) { return true; } +#ifdef _WIN32 unsigned int ShiftOverBit(int offset, unsigned int bits) { // e.g. for |offset| == 2: // | ... 9 8 7 6 5 4 3 2 1 0 | @@ -107,6 +108,7 @@ unsigned int ShiftOverBit(int offset, unsigned int bits) { unsigned int below = bits & ((1 << offset) - 1); return (above >> 1) | below; } +#endif bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, string* err) { @@ -162,7 +164,9 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, if (src + 1 == end || src[1] == '/') { // '.' component; eliminate. src += 2; +#ifdef _WIN32 bits = ShiftOverBit(bits_offset, bits); +#endif continue; } else if (src[1] == '.' && (src + 2 == end || src[2] == '/')) { // '..' component. Back up if possible. @@ -170,9 +174,11 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, dst = components[component_count - 1]; src += 3; --component_count; +#ifdef _WIN32 bits = ShiftOverBit(bits_offset, bits); bits_offset--; bits = ShiftOverBit(bits_offset, bits); +#endif } else { *dst++ = *src++; *dst++ = *src++; @@ -184,7 +190,9 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, if (*src == '/') { src++; +#ifdef _WIN32 bits = ShiftOverBit(bits_offset, bits); +#endif continue; } @@ -195,7 +203,9 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, while (*src != '/' && src != end) *dst++ = *src++; +#ifdef _WIN32 bits_offset++; +#endif *dst++ = *src++; // Copy '/' or final \0 character as well. } From c47506449cd4f6e44a7d43d9b7ecb33f3b448c16 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 10 Nov 2014 12:00:16 -0800 Subject: [PATCH 44/84] make all GetNode explicit, add DepsLog canonicalize test --- src/build.cc | 6 ++- src/build_test.cc | 68 ++++++++++++++++++++++++++- src/deps_log.cc | 10 +++- src/deps_log_test.cc | 94 ++++++++++++++++++------------------- src/graph.cc | 2 +- src/includes_normalize.h | 3 +- src/manifest_parser_test.cc | 2 +- src/state.cc | 7 --- src/state.h | 1 - src/state_test.cc | 6 +-- src/test.cc | 3 +- 11 files changed, 135 insertions(+), 67 deletions(-) diff --git a/src/build.cc b/src/build.cc index a346028978..3e74131a72 100644 --- a/src/build.cc +++ b/src/build.cc @@ -825,7 +825,11 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, result->output = parser.Parse(result->output, deps_prefix); for (set::iterator i = parser.includes_.begin(); i != parser.includes_.end(); ++i) { - deps_nodes->push_back(state_->GetNode(*i)); + // ~0 is assuming that with MSVC-parsed headers, it's ok to always make + // all backslashes (as some of the slashes will certainly be backslashes + // anyway). This could be fixed if necessary with some additional + // complexity in IncludesNormalize::Relativize. + deps_nodes->push_back(state_->GetNode(*i, ~0u)); } } else #endif diff --git a/src/build_test.cc b/src/build_test.cc index 912e3ef504..bd1cd30cff 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1883,7 +1883,7 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { Edge* edge = state.edges_.back(); - state.GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. + state.GetNode("bar.h", 0)->MarkDirty(); // Mark bar.h as missing. EXPECT_TRUE(builder.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); @@ -1901,6 +1901,72 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { } } +#ifdef _WIN32 +TEST_F(BuildWithDepsLogTest, DepFileDepsLogCanonicalize) { + string err; + const char* manifest = + "rule cc\n command = cc $in\n depfile = $out.d\n deps = gcc\n" + "build a/b\\c\\d/e/fo$ o.o: cc x\\y/z\\foo.c\n"; + + fs_.Create("x/y/z/foo.c", ""); + + { + State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + // Run the build once, everything should be ok. + DepsLog deps_log; + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + EXPECT_TRUE(builder.AddTarget("a/b/c/d/e/fo o.o", &err)); + ASSERT_EQ("", err); + // Note, different slashes from manifest. + fs_.Create("a/b\\c\\d/e/fo o.o.d", + "a\\b\\c\\d\\e\\fo\\ o.o: blah.h bar.h\n"); + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + + deps_log.Close(); + builder.command_runner_.release(); + } + + { + State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + DepsLog deps_log; + ASSERT_TRUE(deps_log.Load("ninja_deps", &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + + Edge* edge = state.edges_.back(); + + state.GetNode("bar.h", 0)->MarkDirty(); // Mark bar.h as missing. + EXPECT_TRUE(builder.AddTarget("a/b/c/d/e/fo o.o", &err)); + ASSERT_EQ("", err); + + // Expect three new edges: one generating fo o.o, and two more from + // loading the depfile. + ASSERT_EQ(3u, state.edges_.size()); + // Expect our edge to now have three inputs: foo.c and two headers. + ASSERT_EQ(3u, edge->inputs_.size()); + + // Expect the command line we generate to only use the original input. + // Note, slashes from manifest, not .d. + ASSERT_EQ("cc x\\y/z\\foo.c", edge->EvaluateCommand()); + + deps_log.Close(); + builder.command_runner_.release(); + } +} +#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 TEST_F(BuildTest, RestatMissingDepfile) { diff --git a/src/deps_log.cc b/src/deps_log.cc index 39de1805dc..d889dfd7b0 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -233,14 +233,20 @@ bool DepsLog::Load(const string& path, State* state, string* err) { if (!UpdateDeps(out_id, deps)) ++unique_dep_record_count; } else { - int path_size = size - 4; + size_t path_size = size - 4; assert(path_size > 0); // CanonicalizePath() rejects empty paths. // There can be up to 3 bytes of padding. if (buf[path_size - 1] == '\0') --path_size; if (buf[path_size - 1] == '\0') --path_size; if (buf[path_size - 1] == '\0') --path_size; + unsigned int slash_bits; + string err; + if (!CanonicalizePath(&buf[0], &path_size, &slash_bits, &err)) { + read_failed = true; + break; + } StringPiece path(buf, path_size); - Node* node = state->GetNode(path); + Node* node = state->GetNode(path, slash_bits); // Check that the expected index matches the actual index. This can only // happen if two ninja processes write to the same deps log concurrently. diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 60ff48f34a..ac2b315f03 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -46,16 +46,16 @@ TEST_F(DepsLogTest, WriteRead) { { vector deps; - deps.push_back(state1.GetNode("foo.h")); - deps.push_back(state1.GetNode("bar.h")); - log1.RecordDeps(state1.GetNode("out.o"), 1, deps); + deps.push_back(state1.GetNode("foo.h", 0)); + deps.push_back(state1.GetNode("bar.h", 0)); + log1.RecordDeps(state1.GetNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state1.GetNode("foo.h")); - deps.push_back(state1.GetNode("bar2.h")); - log1.RecordDeps(state1.GetNode("out2.o"), 2, deps); + deps.push_back(state1.GetNode("foo.h", 0)); + deps.push_back(state1.GetNode("bar2.h", 0)); + log1.RecordDeps(state1.GetNode("out2.o", 0), 2, deps); - DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o")); + DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o", 0)); ASSERT_TRUE(log_deps); ASSERT_EQ(1, log_deps->mtime); ASSERT_EQ(2, log_deps->node_count); @@ -79,7 +79,7 @@ TEST_F(DepsLogTest, WriteRead) { } // Spot-check the entries in log2. - DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out2.o")); + DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out2.o", 0)); ASSERT_TRUE(log_deps); ASSERT_EQ(2, log_deps->mtime); ASSERT_EQ(2, log_deps->node_count); @@ -101,11 +101,11 @@ TEST_F(DepsLogTest, LotsOfDeps) { for (int i = 0; i < kNumDeps; ++i) { char buf[32]; sprintf(buf, "file%d.h", i); - deps.push_back(state1.GetNode(buf)); + deps.push_back(state1.GetNode(buf, 0)); } - log1.RecordDeps(state1.GetNode("out.o"), 1, deps); + log1.RecordDeps(state1.GetNode("out.o", 0), 1, deps); - DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o")); + DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o", 0)); ASSERT_EQ(kNumDeps, log_deps->node_count); } @@ -116,7 +116,7 @@ TEST_F(DepsLogTest, LotsOfDeps) { EXPECT_TRUE(log2.Load(kTestFilename, &state2, &err)); ASSERT_EQ("", err); - DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out.o")); + DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out.o", 0)); ASSERT_EQ(kNumDeps, log_deps->node_count); } @@ -132,9 +132,9 @@ TEST_F(DepsLogTest, DoubleEntry) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); log.Close(); struct stat st; @@ -154,9 +154,9 @@ TEST_F(DepsLogTest, DoubleEntry) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); log.Close(); struct stat st; @@ -186,14 +186,14 @@ TEST_F(DepsLogTest, Recompact) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("baz.h")); - log.RecordDeps(state.GetNode("other_out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("baz.h", 0)); + log.RecordDeps(state.GetNode("other_out.o", 0), 1, deps); log.Close(); @@ -216,8 +216,8 @@ TEST_F(DepsLogTest, Recompact) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); log.Close(); struct stat st; @@ -237,14 +237,14 @@ TEST_F(DepsLogTest, Recompact) { string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); - Node* out = state.GetNode("out.o"); + Node* out = state.GetNode("out.o", 0); DepsLog::Deps* deps = log.GetDeps(out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); ASSERT_EQ(1, deps->node_count); ASSERT_EQ("foo.h", deps->nodes[0]->path()); - Node* other_out = state.GetNode("other_out.o"); + Node* other_out = state.GetNode("other_out.o", 0); deps = log.GetDeps(other_out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); @@ -287,14 +287,14 @@ TEST_F(DepsLogTest, Recompact) { string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); - Node* out = state.GetNode("out.o"); + Node* out = state.GetNode("out.o", 0); DepsLog::Deps* deps = log.GetDeps(out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); ASSERT_EQ(1, deps->node_count); ASSERT_EQ("foo.h", deps->nodes[0]->path()); - Node* other_out = state.GetNode("other_out.o"); + Node* other_out = state.GetNode("other_out.o", 0); deps = log.GetDeps(other_out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); @@ -361,14 +361,14 @@ TEST_F(DepsLogTest, Truncated) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar2.h")); - log.RecordDeps(state.GetNode("out2.o"), 2, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar2.h", 0)); + log.RecordDeps(state.GetNode("out2.o", 0), 2, deps); log.Close(); } @@ -420,14 +420,14 @@ TEST_F(DepsLogTest, TruncatedRecovery) { ASSERT_EQ("", err); vector deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar2.h")); - log.RecordDeps(state.GetNode("out2.o"), 2, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar2.h", 0)); + log.RecordDeps(state.GetNode("out2.o", 0), 2, deps); log.Close(); } @@ -448,16 +448,16 @@ TEST_F(DepsLogTest, TruncatedRecovery) { err.clear(); // The truncated entry should've been discarded. - EXPECT_EQ(NULL, log.GetDeps(state.GetNode("out2.o"))); + EXPECT_EQ(NULL, log.GetDeps(state.GetNode("out2.o", 0))); EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); // Add a new entry. vector deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar2.h")); - log.RecordDeps(state.GetNode("out2.o"), 3, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar2.h", 0)); + log.RecordDeps(state.GetNode("out2.o", 0), 3, deps); log.Close(); } @@ -471,7 +471,7 @@ TEST_F(DepsLogTest, TruncatedRecovery) { EXPECT_TRUE(log.Load(kTestFilename, &state, &err)); // The truncated entry should exist. - DepsLog::Deps* deps = log.GetDeps(state.GetNode("out2.o")); + DepsLog::Deps* deps = log.GetDeps(state.GetNode("out2.o", 0)); ASSERT_TRUE(deps); } } diff --git a/src/graph.cc b/src/graph.cc index f85a1f62c9..930c626fa9 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -331,7 +331,7 @@ bool Edge::use_console() const { string Node::PathDecanonicalized() const { string result = path_; unsigned int mask = 1; - for (char* c = &result[0]; (c = strpbrk(c, "/\\")) != NULL;) { + for (char* c = &result[0]; (c = strchr(c, '/')) != NULL;) { if (slash_bits_ & mask) *c = '\\'; c++; diff --git a/src/includes_normalize.h b/src/includes_normalize.h index 43527af941..634fef3579 100644 --- a/src/includes_normalize.h +++ b/src/includes_normalize.h @@ -29,7 +29,6 @@ struct IncludesNormalize { static string Relativize(StringPiece path, const string& start); /// Normalize by fixing slashes style, fixing redundant .. and . and makes the - /// path relative to |relative_to|. Case is normalized to lowercase on - /// Windows too. + /// path relative to |relative_to|. static string Normalize(const string& input, const char* relative_to); }; diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 0668668664..6909ea9240 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -96,7 +96,7 @@ TEST_F(ParserTest, IgnoreIndentedComments) { ASSERT_EQ(2u, state.rules_.size()); const Rule* rule = state.rules_.begin()->second; EXPECT_EQ("cat", rule->name()); - Edge* edge = state.GetNode("result")->in_edge(); + Edge* edge = state.GetNode("result", 0)->in_edge(); EXPECT_TRUE(edge->GetBindingBool("restat")); EXPECT_FALSE(edge->GetBindingBool("generator")); } diff --git a/src/state.cc b/src/state.cc index 7d1d79d995..6e3e10d65a 100644 --- a/src/state.cc +++ b/src/state.cc @@ -111,13 +111,6 @@ Edge* State::AddEdge(const Rule* rule) { return edge; } -Node* State::GetNode(StringPiece path) { -#if defined(_WIN32) && !defined(NDEBUG) - assert(strpbrk(path.AsString().c_str(), "/\\") == NULL); -#endif - return GetNode(path, 0); -} - Node* State::GetNode(StringPiece path, unsigned int slash_bits) { Node* node = LookupNode(path); if (node) diff --git a/src/state.h b/src/state.h index 9da6d1bf8c..880453284c 100644 --- a/src/state.h +++ b/src/state.h @@ -95,7 +95,6 @@ struct State { Edge* AddEdge(const Rule* rule); - Node* GetNode(StringPiece path); Node* GetNode(StringPiece path, unsigned int slash_bits); Node* LookupNode(StringPiece path) const; Node* SpellcheckNode(const string& path); diff --git a/src/state_test.cc b/src/state_test.cc index 85c65137b5..bd133b60b1 100644 --- a/src/state_test.cc +++ b/src/state_test.cc @@ -38,9 +38,9 @@ TEST(State, Basic) { EXPECT_EQ("cat in1 in2 > out", edge->EvaluateCommand()); - EXPECT_FALSE(state.GetNode("in1")->dirty()); - EXPECT_FALSE(state.GetNode("in2")->dirty()); - EXPECT_FALSE(state.GetNode("out")->dirty()); + EXPECT_FALSE(state.GetNode("in1", 0)->dirty()); + EXPECT_FALSE(state.GetNode("in2", 0)->dirty()); + EXPECT_FALSE(state.GetNode("out", 0)->dirty()); } } // namespace diff --git a/src/test.cc b/src/test.cc index 560ef3ad78..f667fef0f3 100644 --- a/src/test.cc +++ b/src/test.cc @@ -89,7 +89,8 @@ void StateTestWithBuiltinRules::AddCatRule(State* state) { } Node* StateTestWithBuiltinRules::GetNode(const string& path) { - return state_.GetNode(path); + EXPECT_FALSE(strpbrk(path.c_str(), "/\\")); + return state_.GetNode(path, 0); } void AssertParse(State* state, const char* input) { From 7d60548d7bafc7374d6486a5a6b350b4b4a2fa29 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 10 Nov 2014 12:38:55 -0800 Subject: [PATCH 45/84] initialize slash_bits on non-win --- src/util.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util.cc b/src/util.cc index d15837f5ce..eaf720ffa1 100644 --- a/src/util.cc +++ b/src/util.cc @@ -217,6 +217,8 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, *len = dst - start - 1; #ifdef _WIN32 *slash_bits = bits; +#else + *slash_bits = 0; #endif return true; } From aece8b098cb2a5d722edf0221dfd0744a818a8d1 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 10 Nov 2014 14:48:55 -0800 Subject: [PATCH 46/84] fix CanonicalizePath going past StringPiece length + test --- src/util.cc | 2 ++ src/util_test.cc | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/util.cc b/src/util.cc index eaf720ffa1..f89c7ade08 100644 --- a/src/util.cc +++ b/src/util.cc @@ -129,6 +129,8 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, unsigned int bits = 0; int bits_offset = 0; for (char* c = path; (c = strpbrk(c, "/\\")) != NULL;) { + if (static_cast(c - path) >= *len) + break; bits |= (*c == '\\') << bits_offset; *c++ = '/'; bits_offset++; diff --git a/src/util_test.cc b/src/util_test.cc index 8ec7a284fa..0073994dce 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -244,6 +244,16 @@ TEST(CanonicalizePath, SlashTracking) { EXPECT_EQ(1, slash_bits); } +TEST(CanonicalizePath, CanonicalizeNotExceedingLen) { + // Make sure searching \/ doesn't go past supplied len. + char buf[] = "foo/bar\\baz.h\\"; // Last \ past end. + unsigned int slash_bits; + string err; + size_t size = 13; + EXPECT_TRUE(::CanonicalizePath(buf, &size, &slash_bits, &err)); + EXPECT_EQ(0, strncmp("foo/bar/baz.h", buf, size)); + EXPECT_EQ(2, slash_bits); // Not including the trailing one. +} #endif TEST(CanonicalizePath, EmptyResult) { From b6e6564a603b184ebb077ccb7b7a89976ef97a5b Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 10 Nov 2014 16:35:31 -0800 Subject: [PATCH 47/84] no need to Decanonicalize on non-Windows --- src/graph.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/graph.cc b/src/graph.cc index 930c626fa9..223661c432 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -330,6 +330,7 @@ bool Edge::use_console() const { string Node::PathDecanonicalized() const { string result = path_; +#ifdef _WIN32 unsigned int mask = 1; for (char* c = &result[0]; (c = strchr(c, '/')) != NULL;) { if (slash_bits_ & mask) @@ -337,6 +338,7 @@ string Node::PathDecanonicalized() const { c++; mask <<= 1; } +#endif return result; } From d77fea08b70f1ab1b7a981e6a0614fbf029724ed Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 10 Nov 2014 17:18:16 -0800 Subject: [PATCH 48/84] save slash_bits in depslog --- src/deps_log.cc | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index d889dfd7b0..951444673b 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -30,7 +30,7 @@ // The version is stored as 4 bytes after the signature and also serves as a // byte order mark. Signature and version combined are 16 bytes long. const char kFileSignature[] = "# ninjadeps\n"; -const int kCurrentVersion = 3; +const int kCurrentVersion = 4; // Record size is currently limited to less than the full 32 bit, due to // internal buffers having to have this size. @@ -233,19 +233,15 @@ bool DepsLog::Load(const string& path, State* state, string* err) { if (!UpdateDeps(out_id, deps)) ++unique_dep_record_count; } else { - size_t path_size = size - 4; + int path_size = size - 8; assert(path_size > 0); // CanonicalizePath() rejects empty paths. // There can be up to 3 bytes of padding. if (buf[path_size - 1] == '\0') --path_size; if (buf[path_size - 1] == '\0') --path_size; if (buf[path_size - 1] == '\0') --path_size; - unsigned int slash_bits; - string err; - if (!CanonicalizePath(&buf[0], &path_size, &slash_bits, &err)) { - read_failed = true; - break; - } StringPiece path(buf, path_size); + unsigned int slash_bits = + *reinterpret_cast(buf + size - 8); Node* node = state->GetNode(path, slash_bits); // Check that the expected index matches the actual index. This can only @@ -386,7 +382,7 @@ bool DepsLog::RecordId(Node* node) { int path_size = node->path().size(); int padding = (4 - path_size % 4) % 4; // Pad path to 4 byte boundary. - unsigned size = path_size + padding + 4; + unsigned size = path_size + padding + 4 + 4; if (size > kMaxRecordSize) { errno = ERANGE; return false; @@ -399,6 +395,9 @@ bool DepsLog::RecordId(Node* node) { } if (padding && fwrite("\0\0", padding, 1, file_) < 1) return false; + unsigned int slash_bits = node->slash_bits(); + if (fwrite(&slash_bits, 4, 1, file_) < 1) + return false; int id = nodes_.size(); unsigned checksum = ~(unsigned)id; if (fwrite(&checksum, 4, 1, file_) < 1) From 99a72092646537992d57196e5286cc556b1437e5 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 10 Nov 2014 17:55:18 -0800 Subject: [PATCH 49/84] no need to save slash_bits, add comment --- src/deps_log.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 951444673b..fc46497847 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -30,7 +30,7 @@ // The version is stored as 4 bytes after the signature and also serves as a // byte order mark. Signature and version combined are 16 bytes long. const char kFileSignature[] = "# ninjadeps\n"; -const int kCurrentVersion = 4; +const int kCurrentVersion = 3; // Record size is currently limited to less than the full 32 bit, due to // internal buffers having to have this size. @@ -233,16 +233,19 @@ bool DepsLog::Load(const string& path, State* state, string* err) { if (!UpdateDeps(out_id, deps)) ++unique_dep_record_count; } else { - int path_size = size - 8; + int path_size = size - 4; assert(path_size > 0); // CanonicalizePath() rejects empty paths. // There can be up to 3 bytes of padding. if (buf[path_size - 1] == '\0') --path_size; if (buf[path_size - 1] == '\0') --path_size; if (buf[path_size - 1] == '\0') --path_size; StringPiece path(buf, path_size); - unsigned int slash_bits = - *reinterpret_cast(buf + size - 8); - Node* node = state->GetNode(path, slash_bits); + // It is not necessary to pass in a correct slash_bits here. It will + // either be a Node that's in the manifest (in which case it will already + // have a correct slash_bits that GetNode will look up), or it is an + // implicit dependency from a .d which does not affect the build command + // (and so need not have its slashes maintained). + Node* node = state->GetNode(path, 0); // Check that the expected index matches the actual index. This can only // happen if two ninja processes write to the same deps log concurrently. @@ -382,7 +385,7 @@ bool DepsLog::RecordId(Node* node) { int path_size = node->path().size(); int padding = (4 - path_size % 4) % 4; // Pad path to 4 byte boundary. - unsigned size = path_size + padding + 4 + 4; + unsigned size = path_size + padding + 4; if (size > kMaxRecordSize) { errno = ERANGE; return false; @@ -395,9 +398,6 @@ bool DepsLog::RecordId(Node* node) { } if (padding && fwrite("\0\0", padding, 1, file_) < 1) return false; - unsigned int slash_bits = node->slash_bits(); - if (fwrite(&slash_bits, 4, 1, file_) < 1) - return false; int id = nodes_.size(); unsigned checksum = ~(unsigned)id; if (fwrite(&checksum, 4, 1, file_) < 1) From 0c2982d13ea290a451efa8e8ddbaa9af1e7229a1 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 10 Nov 2014 19:38:08 -0800 Subject: [PATCH 50/84] fix not respecting length --- src/util.cc | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/util.cc b/src/util.cc index f89c7ade08..e667992543 100644 --- a/src/util.cc +++ b/src/util.cc @@ -98,7 +98,7 @@ bool CanonicalizePath(string* path, unsigned int* slash_bits, string* err) { } #ifdef _WIN32 -unsigned int ShiftOverBit(int offset, unsigned int bits) { +static unsigned int ShiftOverBit(int offset, unsigned int bits) { // e.g. for |offset| == 2: // | ... 9 8 7 6 5 4 3 2 1 0 | // \_________________/ \_/ @@ -124,25 +124,29 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, char* components[kMaxPathComponents]; int component_count = 0; + char* start = path; + char* dst = start; + const char* src = start; + const char* end = start + *len; + #ifdef _WIN32 // kMaxPathComponents protects this from overflowing. unsigned int bits = 0; - int bits_offset = 0; - for (char* c = path; (c = strpbrk(c, "/\\")) != NULL;) { - if (static_cast(c - path) >= *len) - break; - bits |= (*c == '\\') << bits_offset; - *c++ = '/'; - bits_offset++; + unsigned int bits_mask = 1; + // Convert \ to /, setting a bit in |bits| for each \ encountered. + for (char* c = path; c < end; ++c) { + switch (*c) { + case '\\': + bits |= bits_mask; + *c = '/'; + // Intentional fallthrough. + case '/': + bits_mask <<= 1; + } } - bits_offset = 0; + int bits_offset = 0; #endif - char* start = path; - char* dst = start; - const char* src = start; - const char* end = start + *len; - if (*src == '/') { #ifdef _WIN32 bits_offset++; From cc8b72a87d72a030dc5b2bb212958c0e46213a6b Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 12 Nov 2014 09:16:25 -0800 Subject: [PATCH 51/84] properly guard against slash_bits overflow --- src/util.cc | 7 +++++-- src/util_test.cc | 28 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/util.cc b/src/util.cc index e667992543..4df81dd79a 100644 --- a/src/util.cc +++ b/src/util.cc @@ -130,9 +130,9 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, const char* end = start + *len; #ifdef _WIN32 - // kMaxPathComponents protects this from overflowing. unsigned int bits = 0; unsigned int bits_mask = 1; + int bits_offset = 0; // Convert \ to /, setting a bit in |bits| for each \ encountered. for (char* c = path; c < end; ++c) { switch (*c) { @@ -142,9 +142,12 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, // Intentional fallthrough. case '/': bits_mask <<= 1; + bits_offset++; } } - int bits_offset = 0; + if (bits_offset > 32) + return false; + bits_offset = 0; #endif if (*src == '/') { diff --git a/src/util_test.cc b/src/util_test.cc index 0073994dce..5bbf39726e 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -254,6 +254,34 @@ TEST(CanonicalizePath, CanonicalizeNotExceedingLen) { EXPECT_EQ(0, strncmp("foo/bar/baz.h", buf, size)); EXPECT_EQ(2, slash_bits); // Not including the trailing one. } + +TEST(CanonicalizePath, TooManyComponents) { + string path; + string err; + unsigned int slash_bits; + + // 32 is OK. + path = "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./x.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + + // Backslashes version. + path = + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\." + "\\a\\.\\a\\.\\a\\.\\a\\.\\x.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ(slash_bits, 0xffff); + + // 33 is not. + path = + "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/x.h"; + EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); + + // Backslashes version. + path = + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\." + "\\a\\.\\a\\.\\a\\.\\a\\.\\a\\x.h"; + EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); +} #endif TEST(CanonicalizePath, EmptyResult) { From fb320646918dac633bdb4e449e7d0f372d83d01d Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 12 Nov 2014 12:32:45 -0800 Subject: [PATCH 52/84] set *err when too many components in CanonicalizePath --- src/util.cc | 4 +++- src/util_test.cc | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/util.cc b/src/util.cc index 4df81dd79a..7717ef458c 100644 --- a/src/util.cc +++ b/src/util.cc @@ -145,8 +145,10 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, bits_offset++; } } - if (bits_offset > 32) + if (bits_offset > 32) { + *err = "too many path components"; return false; + } bits_offset = 0; #endif diff --git a/src/util_test.cc b/src/util_test.cc index 5bbf39726e..13730af2a4 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -272,15 +272,19 @@ TEST(CanonicalizePath, TooManyComponents) { EXPECT_EQ(slash_bits, 0xffff); // 33 is not. + err = ""; path = "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/x.h"; EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ(err, "too many path components"); // Backslashes version. + err = ""; path = "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\." "\\a\\.\\a\\.\\a\\.\\a\\.\\a\\x.h"; EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ(err, "too many path components"); } #endif From a023d9923cbb584680fb53760747a75e380b520a Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 12 Nov 2014 12:39:20 -0800 Subject: [PATCH 53/84] whitespace/comment/wrap fixes, no intended functionality change --- src/graph.cc | 2 +- src/util.cc | 35 +++++++++++++++++++---------------- src/util_test.cc | 4 ++-- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/graph.cc b/src/graph.cc index 223661c432..2829669e79 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -131,7 +131,7 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { } bool DependencyScan::RecomputeOutputsDirty(Edge* edge, - Node* most_recent_input) { + Node* most_recent_input) { string command = edge->EvaluateCommand(true); for (vector::iterator i = edge->outputs_.begin(); i != edge->outputs_.end(); ++i) { diff --git a/src/util.cc b/src/util.cc index 4df81dd79a..e0320eb8e9 100644 --- a/src/util.cc +++ b/src/util.cc @@ -508,33 +508,34 @@ static double CalculateProcessorLoad(uint64_t idle_ticks, uint64_t total_ticks) static uint64_t previous_idle_ticks = 0; static uint64_t previous_total_ticks = 0; static double previous_load = -0.0; - + uint64_t idle_ticks_since_last_time = idle_ticks - previous_idle_ticks; uint64_t total_ticks_since_last_time = total_ticks - previous_total_ticks; - + bool first_call = (previous_total_ticks == 0); bool ticks_not_updated_since_last_call = (total_ticks_since_last_time == 0); - + double load; if (first_call || ticks_not_updated_since_last_call) { load = previous_load; } else { - //calculate load - double idle_to_total_ratio = ((double)idle_ticks_since_last_time) / total_ticks_since_last_time; + // Calculate load. + double idle_to_total_ratio = + ((double)idle_ticks_since_last_time) / total_ticks_since_last_time; double load_since_last_call = 1.0 - idle_to_total_ratio; - - //filter/smooth result when possible + + // Filter/smooth result when possible. if(previous_load > 0) { load = 0.9 * previous_load + 0.1 * load_since_last_call; } else { load = load_since_last_call; } } - + previous_load = load; previous_total_ticks = total_ticks; previous_idle_ticks = idle_ticks; - + return load; } @@ -547,22 +548,24 @@ static uint64_t FileTimeToTickCount(const FILETIME & ft) double GetLoadAverage() { FILETIME idle_time, kernel_time, user_time; - BOOL get_system_time_succeeded = GetSystemTimes(&idle_time, &kernel_time, &user_time); - + BOOL get_system_time_succeeded = + GetSystemTimes(&idle_time, &kernel_time, &user_time); + double posix_compatible_load; if (get_system_time_succeeded) { uint64_t idle_ticks = FileTimeToTickCount(idle_time); - - //kernel_time from GetSystemTimes already includes idle_time - uint64_t total_ticks = FileTimeToTickCount(kernel_time) + FileTimeToTickCount(user_time); - + + // kernel_time from GetSystemTimes already includes idle_time. + uint64_t total_ticks = + FileTimeToTickCount(kernel_time) + FileTimeToTickCount(user_time); + double processor_load = CalculateProcessorLoad(idle_ticks, total_ticks); posix_compatible_load = processor_load * GetProcessorCount(); } else { posix_compatible_load = -0.0; } - + return posix_compatible_load; } #else diff --git a/src/util_test.cc b/src/util_test.cc index 5bbf39726e..1ee8f357df 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -339,10 +339,10 @@ TEST(CanonicalizePath, NotNullTerminated) { TEST(PathEscaping, TortureTest) { string result; - + GetWin32EscapedString("foo bar\\\"'$@d!st!c'\\path'\\", &result); EXPECT_EQ("\"foo bar\\\\\\\"'$@d!st!c'\\path'\\\\\"", result); - result.clear(); + result.clear(); GetShellEscapedString("foo bar\"/'$@d!st!c'/path'", &result); EXPECT_EQ("'foo bar\"/'\\''$@d!st!c'\\''/path'\\'''", result); From 2c241d76162532e67ba923175c5c77464593b3a0 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 13 Nov 2014 17:31:37 -0800 Subject: [PATCH 54/84] Remove unused variables. --- configure.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/configure.py b/configure.py index 01fd30176a..518c49916d 100755 --- a/configure.py +++ b/configure.py @@ -319,8 +319,6 @@ def has_re2c(): n.comment('Tests all build into ninja_test executable.') -variables = [] -test_ldflags = None test_libs = libs objs = [] @@ -347,8 +345,7 @@ def has_re2c(): if not platform.is_windows(): test_libs.append('-lpthread') ninja_test = n.build(binary('ninja_test'), 'link', objs, implicit=ninja_lib, - variables=[('ldflags', test_ldflags), - ('libs', test_libs)]) + variables=[('libs', test_libs)]) n.newline() all_targets += ninja_test From a39b9577ba25dc4968bbc5f4afe7d17524325bb0 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 13 Nov 2014 17:53:26 -0800 Subject: [PATCH 55/84] Make sure configure.py and ninja.cc always agree on if -t browse is included. No behavior change on most platforms. On solaris, -t browse was compiled in in ninja.cc but browse.cc wasn't compiled in, which probably means that building on Solaris didn't work. It might be better now. This also makes browse.cc automatically not included in bootstrap builds; previously this was done manually through the NINJA_BOOTSTRAP check. --- configure.py | 6 +++++- src/ninja.cc | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/configure.py b/configure.py index 518c49916d..a5e9323317 100755 --- a/configure.py +++ b/configure.py @@ -175,6 +175,10 @@ def binary(name): not options.force_pselect: cflags.append('-DUSE_PPOLL') +have_browse = not platform.is_windows() and not platform.is_solaris() +if have_browse: + cflags.append('-DNINJA_HAVE_BROWSE') + def shell_escape(str): """Escape str such that it's interpreted as a single argument by the shell.""" @@ -233,7 +237,7 @@ def shell_escape(str): objs = [] -if not platform.is_windows() and not platform.is_solaris(): +if have_browse: n.comment('browse_py.h is used to inline browse.py.') n.rule('inline', command='src/inline.sh $varname < $in > $out', diff --git a/src/ninja.cc b/src/ninja.cc index 27380c0efc..847000ac78 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -365,7 +365,7 @@ int NinjaMain::ToolQuery(int argc, char* argv[]) { return 0; } -#if !defined(_WIN32) && !defined(NINJA_BOOTSTRAP) +#if defined(NINJA_HAVE_BROWSE) int NinjaMain::ToolBrowse(int argc, char* argv[]) { if (argc < 1) { Error("expected a target to browse"); @@ -698,7 +698,7 @@ int NinjaMain::ToolUrtle(int argc, char** argv) { /// Returns a Tool, or NULL if Ninja should exit. const Tool* ChooseTool(const string& tool_name) { static const Tool kTools[] = { -#if !defined(_WIN32) && !defined(NINJA_BOOTSTRAP) +#if defined(NINJA_HAVE_BROWSE) { "browse", "browse dependency graph in a web browser", Tool::RUN_AFTER_LOAD, &NinjaMain::ToolBrowse }, #endif From 2cfb99985da66ff254dccab3e92eacfd8ecdfb81 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 17 Nov 2014 15:15:10 -0800 Subject: [PATCH 56/84] emacs: Remove an empty line, wrap a comment. --- misc/ninja-mode.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 80585d846d..28e2ea5fca 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -33,10 +33,10 @@ '("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) ;; Rule names '("rule +\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) - ;; Build Statement - highlight the rule used, allow for escaped $,: in outputs + ;; Build Statement - highlight the rule used, + ;; allow for escaped $,: in outputs. '("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) - )) ;;;###autoload From 76a95e45bba61b617dfac14e83cb6229a4e4e897 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 18 Nov 2014 07:48:26 -0800 Subject: [PATCH 57/84] add an "expand" function to ninja_syntax Implements basic variable expansion for use in configure.py. --- misc/ninja_syntax.py | 15 +++++++++++++++ misc/ninja_syntax_test.py | 26 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/misc/ninja_syntax.py b/misc/ninja_syntax.py index 14b932ff4c..e200514745 100644 --- a/misc/ninja_syntax.py +++ b/misc/ninja_syntax.py @@ -7,6 +7,7 @@ use Python. """ +import re import textwrap def escape_path(word): @@ -154,3 +155,17 @@ def escape(string): assert '\n' not in string, 'Ninja syntax does not allow newlines' # We only have one special metacharacter: '$'. return string.replace('$', '$$') + + +def expand(string, vars, local_vars={}): + """Expand a string containing $vars as Ninja would. + + Note: doesn't handle the full Ninja variable syntax, but it's enough + to make configure.py's use of it work. + """ + def exp(m): + var = m.group(1) + if var == '$': + return '$' + return local_vars.get(var, vars.get(var, '')) + return re.sub(r'\$(\$|\w*)', exp, string) diff --git a/misc/ninja_syntax_test.py b/misc/ninja_syntax_test.py index 2aef7ff830..36b2e7be8b 100755 --- a/misc/ninja_syntax_test.py +++ b/misc/ninja_syntax_test.py @@ -148,5 +148,31 @@ def test_variables_list(self): ''', self.out.getvalue()) +class TestExpand(unittest.TestCase): + def test_basic(self): + vars = {'x': 'X'} + self.assertEqual('foo', ninja_syntax.expand('foo', vars)) + + def test_var(self): + vars = {'xyz': 'XYZ'} + self.assertEqual('fooXYZ', ninja_syntax.expand('foo$xyz', vars)) + + def test_vars(self): + vars = {'x': 'X', 'y': 'YYY'} + self.assertEqual('XYYY', ninja_syntax.expand('$x$y', vars)) + + def test_space(self): + vars = {} + self.assertEqual('x y z', ninja_syntax.expand('x$ y$ z', vars)) + + def test_locals(self): + vars = {'x': 'a'} + local_vars = {'x': 'b'} + self.assertEqual('a', ninja_syntax.expand('$x', vars)) + self.assertEqual('b', ninja_syntax.expand('$x', vars, local_vars)) + + def test_double(self): + self.assertEqual('a b$c', ninja_syntax.expand('a$ b$$c', {})) + if __name__ == '__main__': unittest.main() From dcd41dcef3020e5c2cbe5c29b5b1d71e581de029 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Fri, 14 Nov 2014 10:53:33 -0800 Subject: [PATCH 58/84] add a --bootstrap mode for configure.py Instead of bootstrapping through a separate script, instead make configure.py able to either generate a build.ninja *or* just execute all the computed commands to build a ninja binary. --- configure.py | 128 ++++++++++++++++++++++++++++++++++++++++--- misc/ninja_syntax.py | 26 +++++---- 2 files changed, 136 insertions(+), 18 deletions(-) diff --git a/configure.py b/configure.py index c2abcde079..b818bfc003 100755 --- a/configure.py +++ b/configure.py @@ -24,14 +24,90 @@ from optparse import OptionParser import os import pipes +import string +import subprocess import sys import platform_helper sys.path.insert(0, 'misc') import ninja_syntax + +class Bootstrap: + """API shim for ninja_syntax.Writer that instead runs the commands. + + Used to bootstrap Ninja from scratch. In --bootstrap mode this + class is used to execute all the commands to build an executable. + It also proxies all calls to an underlying ninja_syntax.Writer, to + behave like non-bootstrap mode. + """ + def __init__(self, writer): + self.writer = writer + # Map of variable name => expanded variable value. + self.vars = {} + # Map of rule name => dict of rule attributes. + self.rules = { + 'phony': {} + } + + def comment(self, text): + return self.writer.comment(text) + + def newline(self): + return self.writer.newline() + + def variable(self, key, val): + self.vars[key] = self._expand(val) + return self.writer.variable(key, val) + + def rule(self, name, **kwargs): + self.rules[name] = kwargs + return self.writer.rule(name, **kwargs) + + def build(self, outputs, rule, inputs=None, **kwargs): + ruleattr = self.rules[rule] + cmd = ruleattr.get('command') + if cmd is None: # A phony rule, for example. + return + + # Implement just enough of Ninja variable expansion etc. to + # make the bootstrap build work. + local_vars = { + 'in': self._expand_paths(inputs), + 'out': self._expand_paths(outputs) + } + for key, val in kwargs.get('variables', []): + local_vars[key] = ' '.join(ninja_syntax.as_list(val)) + + self._run_command(self._expand(cmd, local_vars)) + + return self.writer.build(outputs, rule, inputs, **kwargs) + + def default(self, paths): + return self.writer.default(paths) + + def _expand_paths(self, paths): + """Expand $vars in an array of paths, e.g. from a 'build' block.""" + paths = ninja_syntax.as_list(paths) + return ' '.join(map(self._expand, paths)) + + def _expand(self, str, local_vars={}): + """Expand $vars in a string.""" + return ninja_syntax.expand(str, self.vars, local_vars) + + def _run_command(self, cmdline): + """Run a subcommand, quietly. Prints the full command on error.""" + try: + subprocess.check_call(cmdline, shell=True) + except subprocess.CalledProcessError, e: + print('when running: ', cmdline) + raise + + parser = OptionParser() profilers = ['gmon', 'pprof'] +parser.add_option('--bootstrap', action='store_true', + help='bootstrap a ninja binary from nothing') parser.add_option('--platform', help='target platform (' + '/'.join(platform_helper.platforms()) + ')', @@ -64,8 +140,20 @@ host = platform BUILD_FILENAME = 'build.ninja' -buildfile = open(BUILD_FILENAME, 'w') -n = ninja_syntax.Writer(buildfile) +ninja_writer = ninja_syntax.Writer(open(BUILD_FILENAME, 'w')) +n = ninja_writer + +if options.bootstrap: + # Make the build directory. + try: + os.mkdir('build') + except OSError: + pass + # Wrap ninja_writer with the Bootstrapper, which also executes the + # commands. + print('bootstrapping ninja...') + n = Bootstrap(n) + n.comment('This file is used to build ninja itself.') n.comment('It is generated by ' + os.path.basename(__file__) + '.') n.newline() @@ -74,7 +162,10 @@ n.newline() n.comment('The arguments passed to configure.py, for rerunning it.') -n.variable('configure_args', ' '.join(sys.argv[1:])) +configure_args = sys.argv[1:] +if '--bootstrap' in configure_args: + configure_args.remove('--bootstrap') +n.variable('configure_args', ' '.join(configure_args)) env_keys = set(['CXX', 'AR', 'CFLAGS', 'LDFLAGS']) configure_env = dict((k, os.environ[k]) for k in os.environ if k in env_keys) if configure_env: @@ -114,7 +205,8 @@ def binary(name): n.variable('ar', configure_env.get('AR', 'ar')) if platform.is_msvc(): - cflags = ['/nologo', # Don't print startup banner. + cflags = ['/showIncludes', + '/nologo', # Don't print startup banner. '/Zi', # Create pdb with debug info. '/W4', # Highest warning level. '/WX', # Warnings as errors. @@ -129,6 +221,10 @@ def binary(name): '/DNOMINMAX', '/D_CRT_SECURE_NO_WARNINGS', '/D_VARIADIC_MAX=10', '/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'] @@ -200,9 +296,10 @@ def shell_escape(str): if platform.is_msvc(): n.rule('cxx', - command='$cxx /showIncludes $cflags -c $in /Fo$out', + command='$cxx $cflags -c $in /Fo$out', description='CXX $out', - deps='msvc') + deps='msvc' # /showIncludes is included in $cflags. + ) else: n.rule('cxx', command='$cxx -MMD -MT $out -MF $out.d $cflags -c $in -o $out', @@ -252,7 +349,6 @@ def shell_escape(str): n.comment('the depfile parser and ninja lexers are generated using re2c.') def has_re2c(): - import subprocess try: proc = subprocess.Popen(['re2c', '-V'], stdout=subprocess.PIPE) return int(proc.communicate()[0], 10) >= 1103 @@ -321,6 +417,12 @@ def has_re2c(): n.newline() all_targets += ninja +if options.bootstrap: + # We've built the ninja binary. Don't run any more commands + # through the bootstrap executor, but continue writing the + # build.ninja file. + n = ninja_writer + n.comment('Tests all build into ninja_test executable.') test_libs = libs @@ -434,4 +536,16 @@ def has_re2c(): n.build('all', 'phony', all_targets) +n.close() print('wrote %s.' % BUILD_FILENAME) + +if options.bootstrap: + print('bootstrap complete. rebuilding...') + if platform.is_windows(): + bootstrap_exe = 'ninja.bootstrap.exe' + if os.path.exists(bootstrap_exe): + os.unlink(bootstrap_exe) + os.rename('ninja.exe', bootstrap_exe) + subprocess.check_call('ninja.bootstrap.exe', shell=True) + else: + subprocess.check_call('./ninja', shell=True) diff --git a/misc/ninja_syntax.py b/misc/ninja_syntax.py index e200514745..8673518cfb 100644 --- a/misc/ninja_syntax.py +++ b/misc/ninja_syntax.py @@ -60,16 +60,16 @@ def rule(self, name, command, description=None, depfile=None, def build(self, outputs, rule, inputs=None, implicit=None, order_only=None, variables=None): - outputs = self._as_list(outputs) + outputs = as_list(outputs) out_outputs = [escape_path(x) for x in outputs] - all_inputs = [escape_path(x) for x in self._as_list(inputs)] + all_inputs = [escape_path(x) for x in as_list(inputs)] if implicit: - implicit = [escape_path(x) for x in self._as_list(implicit)] + implicit = [escape_path(x) for x in as_list(implicit)] all_inputs.append('|') all_inputs.extend(implicit) if order_only: - order_only = [escape_path(x) for x in self._as_list(order_only)] + order_only = [escape_path(x) for x in as_list(order_only)] all_inputs.append('||') all_inputs.extend(order_only) @@ -94,7 +94,7 @@ def subninja(self, path): self._line('subninja %s' % path) def default(self, paths): - self._line('default %s' % ' '.join(self._as_list(paths))) + self._line('default %s' % ' '.join(as_list(paths))) def _count_dollars_before_index(self, s, i): """Returns the number of '$' characters right in front of s[i].""" @@ -141,12 +141,16 @@ def _line(self, text, indent=0): self.output.write(leading_space + text + '\n') - def _as_list(self, input): - if input is None: - return [] - if isinstance(input, list): - return input - return [input] + def close(self): + self.output.close() + + +def as_list(input): + if input is None: + return [] + if isinstance(input, list): + return input + return [input] def escape(string): From bca80b12ff2702d0403c31ce34fca90ec7df2d28 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Fri, 14 Nov 2014 14:22:56 -0800 Subject: [PATCH 59/84] drop bootstrap.py We now use configure.py --bootstrap. Direct users of this script to use the other one. --- README | 8 ++- bootstrap.py | 151 ++------------------------------------------------- 2 files changed, 9 insertions(+), 150 deletions(-) diff --git a/README b/README index 733ccb39b5..9faf477e78 100644 --- a/README +++ b/README @@ -5,10 +5,12 @@ See the manual -- http://martine.github.com/ninja/manual.html or doc/manual.asciidoc included in the distribution -- for background and more details. -To build, run ./bootstrap.py. It first blindly compiles all non-test +To build, run ./configure.py --bootstrap. It first compiles all non-test source files together, then re-builds Ninja using itself. You should -end up with a 'ninja' binary in the source root. Run './ninja -h' for -help. +end up with a 'ninja' binary in the source root. + +Run './configure.py --help' for more configuration options. +Run './ninja -h' for Ninja help. There is no installation step. The only file of interest to a user is the resulting ninja binary. diff --git a/bootstrap.py b/bootstrap.py index 026396b5c3..56eab64d18 100755 --- a/bootstrap.py +++ b/bootstrap.py @@ -15,152 +15,9 @@ from __future__ import print_function -from optparse import OptionParser -import sys -import os -import glob -import errno -import shlex -import shutil import subprocess -import platform_helper - -os.chdir(os.path.dirname(os.path.abspath(__file__))) - -parser = OptionParser() - -parser.add_option('--verbose', action='store_true', - help='enable verbose build',) -parser.add_option('--x64', action='store_true', - help='force 64-bit build (Windows)',) -parser.add_option('--platform', - help='target platform (' + - '/'.join(platform_helper.platforms()) + ')', - choices=platform_helper.platforms()) -parser.add_option('--force-pselect', action='store_true', - help='ppoll() is used by default where available, ' - 'but some platforms might need to use pselect instead',) -(options, conf_args) = parser.parse_args() - - -platform = platform_helper.Platform(options.platform) -conf_args.append("--platform=" + platform.platform()) - -def run(*args, **kwargs): - returncode = subprocess.call(*args, **kwargs) - if returncode != 0: - sys.exit(returncode) - -# Compute system-specific CFLAGS/LDFLAGS as used in both in the below -# g++ call as well as in the later configure.py. -cflags = os.environ.get('CFLAGS', '').split() -ldflags = os.environ.get('LDFLAGS', '').split() -if platform.is_freebsd() or platform.is_openbsd() or platform.is_bitrig(): - cflags.append('-I/usr/local/include') - ldflags.append('-L/usr/local/lib') - -print('Building ninja manually...') - -try: - os.mkdir('build') -except OSError: - e = sys.exc_info()[1] - if e.errno != errno.EEXIST: - raise - -sources = [] -for src in glob.glob('src/*.cc'): - if src.endswith('test.cc') or src.endswith('.in.cc'): - continue - if src.endswith('bench.cc'): - continue - - filename = os.path.basename(src) - if filename == 'browse.cc': # Depends on generated header. - continue - - if platform.is_windows(): - if src.endswith('-posix.cc'): - continue - else: - if src.endswith('-win32.cc'): - continue - - sources.append(src) - -if platform.is_windows(): - sources.append('src/getopt.c') - -if platform.is_msvc(): - cl = 'cl' - vcdir = os.environ.get('VCINSTALLDIR') - if vcdir: - if options.x64: - cl = os.path.join(vcdir, 'bin', 'x86_amd64', 'cl.exe') - if not os.path.exists(cl): - cl = os.path.join(vcdir, 'bin', 'amd64', 'cl.exe') - else: - cl = os.path.join(vcdir, 'bin', 'cl.exe') - args = [cl, '/nologo', '/EHsc', '/DNOMINMAX'] -else: - args = shlex.split(os.environ.get('CXX', 'g++')) - cflags.extend(['-Wno-deprecated', - '-DNINJA_PYTHON="' + sys.executable + '"', - '-DNINJA_BOOTSTRAP']) - if platform.is_windows(): - cflags.append('-D_WIN32_WINNT=0x0501') - if options.x64: - cflags.append('-m64') -if (platform.is_linux() or platform.is_openbsd() or platform.is_bitrig()) and \ - not options.force_pselect: - cflags.append('-DUSE_PPOLL') -if options.force_pselect: - conf_args.append("--force-pselect") -args.extend(cflags) -args.extend(ldflags) -binary = 'ninja.bootstrap' -if platform.is_windows(): - binary = 'ninja.bootstrap.exe' -args.extend(sources) -if platform.is_msvc(): - args.extend(['/link', '/out:' + binary]) -else: - args.extend(['-o', binary]) - -if options.verbose: - print(' '.join(args)) - -try: - run(args) -except: - print('Failure running:', args) - raise - -verbose = [] -if options.verbose: - verbose = ['-v'] - -if platform.is_windows(): - print('Building ninja using itself...') - run([sys.executable, 'configure.py'] + conf_args) - run(['./' + binary] + verbose) - - # Copy the new executable over the bootstrap one. - shutil.copyfile('ninja.exe', binary) - - # Clean up. - for obj in glob.glob('*.obj'): - os.unlink(obj) - - print(""" -Done! +import sys -Note: to work around Windows file locking, where you can't rebuild an -in-use binary, to run ninja after making any changes to build ninja -itself you should run ninja.bootstrap instead.""") -else: - print('Building ninja using itself...') - run([sys.executable, 'configure.py'] + conf_args) - run(['./' + binary] + verbose) - os.unlink(binary) - print('Done!') +print('DEPRECATED: this script will be deleted.') +print('use "configure.py --bootstrap" instead.') +subprocess.check_call([sys.executable, 'configure.py', '--bootstrap']) From 741363ef14ca31ecdf886f51a19f20447366e584 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 18 Nov 2014 08:22:42 -0800 Subject: [PATCH 60/84] also test ninja_syntax.py in travis --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4a6dfb01f6..26089952b7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,4 +2,4 @@ language: cpp compiler: - gcc - clang -script: ./bootstrap.py && ./configure.py && ./ninja ninja_test && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots +script: ./bootstrap.py && ./configure.py && ./ninja ninja_test && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots && ./misc/ninja_syntax_test.py From ba1ede49896316534b755f234a52fc182ed70347 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 18 Nov 2014 08:04:23 -0800 Subject: [PATCH 61/84] drop NINJA_BOOTSTRAP define --- src/minidump-win32.cc | 5 ++--- src/ninja.cc | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/minidump-win32.cc b/src/minidump-win32.cc index c79ec0e9cb..c611919672 100644 --- a/src/minidump-win32.cc +++ b/src/minidump-win32.cc @@ -12,12 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef NINJA_BOOTSTRAP +#ifdef _MSC_VER #include #include - #include "util.h" typedef BOOL (WINAPI *MiniDumpWriteDumpFunc) ( @@ -85,4 +84,4 @@ void CreateWin32MiniDump(_EXCEPTION_POINTERS* pep) { Warning("minidump created: %s", temp_file); } -#endif // NINJA_BOOTSTRAP +#endif // _MSC_VER diff --git a/src/ninja.cc b/src/ninja.cc index 847000ac78..2c890c24cc 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -1111,7 +1111,7 @@ int real_main(int argc, char** argv) { } // anonymous namespace int main(int argc, char** argv) { -#if !defined(NINJA_BOOTSTRAP) && defined(_MSC_VER) +#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); From e4d72ef97ce5f509175a6188e0854c121a3032c7 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 18 Nov 2014 09:07:43 -0800 Subject: [PATCH 62/84] switch travis to use new --bootstrap mode for building --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 26089952b7..544db6fe83 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,4 +2,4 @@ language: cpp compiler: - gcc - clang -script: ./bootstrap.py && ./configure.py && ./ninja ninja_test && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots && ./misc/ninja_syntax_test.py +script: ./configure.py --bootstrap && ./ninja ninja_test && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots && ./misc/ninja_syntax_test.py From 1e21e5f44181ccb07e75bc78593714f403836b71 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 18 Nov 2014 09:58:45 -0800 Subject: [PATCH 63/84] drop leftover references to bootstrap.py --- HACKING.md | 9 +++------ misc/packaging/ninja.spec | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/HACKING.md b/HACKING.md index ef521e1490..059a4244a6 100644 --- a/HACKING.md +++ b/HACKING.md @@ -7,10 +7,6 @@ The primary build target of interest is `ninja`, but when hacking on Ninja your changes should be testable so it's more useful to build and run `ninja_test` when developing. -(`./bootstrap.py` creates a bootstrap `ninja` and runs the above -process; it's only necessary to run if you don't have a copy of -`ninja` to build with.) - ### Adjusting build flags Build in "debug" mode while developing (disables optimizations and builds @@ -126,14 +122,15 @@ it's locked while in use. * Install Visual Studio (Express is fine), [Python for Windows][], and (if making changes) googletest (see above instructions) -* In a Visual Studio command prompt: `python bootstrap.py` +* In a Visual Studio command prompt: `python configure.py --bootstrap` [Python for Windows]: http://www.python.org/getit/windows/ ### Via mingw on Windows (not well supported) * Install mingw, msys, and python -* In the mingw shell, put Python in your path, and `python bootstrap.py` +* In the mingw shell, put Python in your path, and + `python configure.py --bootstrap` * To reconfigure, run `python configure.py` * Remember to strip the resulting executable if size matters to you diff --git a/misc/packaging/ninja.spec b/misc/packaging/ninja.spec index f0c46feab5..fa244a6fec 100644 --- a/misc/packaging/ninja.spec +++ b/misc/packaging/ninja.spec @@ -23,7 +23,7 @@ seconds to start building after changing one file. Ninja is under a second. %build echo Building.. -./bootstrap.py +./configure.py --bootstrap ./ninja manual %install From 684bf3815e4ffebcf48f24d9393621522abe6e4c Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 18 Nov 2014 10:09:07 -0800 Subject: [PATCH 64/84] merge platform_helper into configure script With this code all in one place, it's easier to spot unused code and simplification opportunities. --- configure.py | 86 ++++++++++++++++++++++++++++++++++++++++------ platform_helper.py | 85 --------------------------------------------- 2 files changed, 76 insertions(+), 95 deletions(-) delete mode 100644 platform_helper.py diff --git a/configure.py b/configure.py index b818bfc003..524ffca0a2 100755 --- a/configure.py +++ b/configure.py @@ -27,12 +27,75 @@ import string import subprocess import sys -import platform_helper -sys.path.insert(0, 'misc') +sys.path.insert(0, 'misc') import ninja_syntax +class Platform(object): + """Represents a host/target platform and its specific build attributes.""" + def __init__(self, platform): + self._platform = platform + if self._platform is not None: + return + self._platform = sys.platform + if self._platform.startswith('linux'): + self._platform = 'linux' + elif self._platform.startswith('freebsd'): + self._platform = 'freebsd' + elif self._platform.startswith('gnukfreebsd'): + self._platform = 'freebsd' + elif self._platform.startswith('openbsd'): + self._platform = 'openbsd' + elif self._platform.startswith('solaris') or self._platform == 'sunos5': + self._platform = 'solaris' + elif self._platform.startswith('mingw'): + self._platform = 'mingw' + elif self._platform.startswith('win'): + self._platform = 'msvc' + elif self._platform.startswith('bitrig'): + self._platform = 'bitrig' + elif self._platform.startswith('netbsd'): + self._platform = 'netbsd' + + @staticmethod + def known_platforms(): + return ['linux', 'darwin', 'freebsd', 'openbsd', 'solaris', 'sunos5', + 'mingw', 'msvc', 'gnukfreebsd', 'bitrig', 'netbsd'] + + def platform(self): + return self._platform + + def is_linux(self): + return self._platform == 'linux' + + def is_mingw(self): + return self._platform == 'mingw' + + def is_msvc(self): + return self._platform == 'msvc' + + def msvc_needs_fs(self): + import subprocess + popen = subprocess.Popen(['cl', '/nologo', '/?'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + out, err = popen.communicate() + return '/FS ' in str(out) + + def is_windows(self): + return self.is_mingw() or self.is_msvc() + + def is_solaris(self): + return self._platform == 'solaris' + + def uses_usr_local(self): + return self._platform in ('freebsd', 'openbsd', 'bitrig') + + def supports_ppoll(self): + return self._platform in ('linux', 'openbsd', 'bitrig') + + class Bootstrap: """API shim for ninja_syntax.Writer that instead runs the commands. @@ -110,12 +173,12 @@ def _run_command(self, cmdline): help='bootstrap a ninja binary from nothing') parser.add_option('--platform', help='target platform (' + - '/'.join(platform_helper.platforms()) + ')', - choices=platform_helper.platforms()) + '/'.join(Platform.known_platforms()) + ')', + choices=Platform.known_platforms()) parser.add_option('--host', help='host platform (' + - '/'.join(platform_helper.platforms()) + ')', - choices=platform_helper.platforms()) + '/'.join(Platform.known_platforms()) + ')', + choices=Platform.known_platforms()) parser.add_option('--debug', action='store_true', help='enable debugging extras',) parser.add_option('--profile', metavar='TYPE', @@ -133,9 +196,9 @@ def _run_command(self, cmdline): print('ERROR: extra unparsed command-line arguments:', args) sys.exit(1) -platform = platform_helper.Platform(options.platform) +platform = Platform(options.platform) if options.host: - host = platform_helper.Platform(options.host) + host = Platform(options.host) else: host = platform @@ -250,6 +313,10 @@ def binary(name): if platform.is_mingw(): cflags += ['-D_WIN32_WINNT=0x0501'] ldflags = ['-L$builddir'] + if platform.uses_usr_local(): + cflags.append('-I/usr/local/include') + ldflags.append('-L/usr/local/lib') + libs = [] if platform.is_mingw(): @@ -267,8 +334,7 @@ def binary(name): cflags.append('-fno-omit-frame-pointer') libs.extend(['-Wl,--no-as-needed', '-lprofiler']) -if (platform.is_linux() or platform.is_openbsd() or platform.is_bitrig()) and \ - not options.force_pselect: +if platform.supports_ppoll() and not options.force_pselect: cflags.append('-DUSE_PPOLL') have_browse = not platform.is_windows() and not platform.is_solaris() diff --git a/platform_helper.py b/platform_helper.py deleted file mode 100644 index 035a671953..0000000000 --- a/platform_helper.py +++ /dev/null @@ -1,85 +0,0 @@ -#!/usr/bin/env python -# Copyright 2011 Google Inc. -# Copyright 2013 Patrick von Reth -# 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. - -import sys - -def platforms(): - return ['linux', 'darwin', 'freebsd', 'openbsd', 'solaris', 'sunos5', - 'mingw', 'msvc', 'gnukfreebsd', 'bitrig', 'netbsd'] - -class Platform(object): - def __init__(self, platform): - self._platform = platform - if not self._platform is None: - return - self._platform = sys.platform - if self._platform.startswith('linux'): - self._platform = 'linux' - elif self._platform.startswith('freebsd'): - self._platform = 'freebsd' - elif self._platform.startswith('gnukfreebsd'): - self._platform = 'freebsd' - elif self._platform.startswith('openbsd'): - self._platform = 'openbsd' - elif self._platform.startswith('solaris') or self._platform == 'sunos5': - self._platform = 'solaris' - elif self._platform.startswith('mingw'): - self._platform = 'mingw' - elif self._platform.startswith('win'): - self._platform = 'msvc' - elif self._platform.startswith('bitrig'): - self._platform = 'bitrig' - elif self._platform.startswith('netbsd'): - self._platform = 'netbsd' - - def platform(self): - return self._platform - - def is_linux(self): - return self._platform == 'linux' - - def is_mingw(self): - return self._platform == 'mingw' - - def is_msvc(self): - return self._platform == 'msvc' - - def msvc_needs_fs(self): - import subprocess - popen = subprocess.Popen(['cl', '/nologo', '/?'], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - out, err = popen.communicate() - return '/FS ' in str(out) - - def is_windows(self): - return self.is_mingw() or self.is_msvc() - - def is_solaris(self): - return self._platform == 'solaris' - - def is_freebsd(self): - return self._platform == 'freebsd' - - def is_openbsd(self): - return self._platform == 'openbsd' - - def is_bitrig(self): - return self._platform == 'bitrig' - - def is_netbsd(self): - return self._platform == 'netbsd' From e1f1a2cf65182f1c9d31e2eef18ec1d56da9d881 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 20 Nov 2014 16:46:52 -0800 Subject: [PATCH 65/84] Remove duplicate import. No behavior change. --- configure.py | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.py b/configure.py index 524ffca0a2..6d4c4d40bb 100755 --- a/configure.py +++ b/configure.py @@ -76,7 +76,6 @@ def is_msvc(self): return self._platform == 'msvc' def msvc_needs_fs(self): - import subprocess popen = subprocess.Popen(['cl', '/nologo', '/?'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) From 0bf1bd3f8ecb1bc615ae7b79ef7636a9476c89dc Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 20 Nov 2014 16:56:17 -0800 Subject: [PATCH 66/84] Make browse detection consistent with other platform checks. --- configure.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/configure.py b/configure.py index 6d4c4d40bb..05f74fd833 100755 --- a/configure.py +++ b/configure.py @@ -94,6 +94,9 @@ def uses_usr_local(self): def supports_ppoll(self): return self._platform in ('linux', 'openbsd', 'bitrig') + def supports_ninja_browse(self): + return self._platform not in ('windows', 'solaris') + class Bootstrap: """API shim for ninja_syntax.Writer that instead runs the commands. @@ -335,9 +338,7 @@ def binary(name): if platform.supports_ppoll() and not options.force_pselect: cflags.append('-DUSE_PPOLL') - -have_browse = not platform.is_windows() and not platform.is_solaris() -if have_browse: +if platform.supports_ninja_browse(): cflags.append('-DNINJA_HAVE_BROWSE') def shell_escape(str): @@ -399,7 +400,7 @@ def shell_escape(str): objs = [] -if have_browse: +if platform.supports_ninja_browse(): n.comment('browse_py.h is used to inline browse.py.') n.rule('inline', command='src/inline.sh $varname < $in > $out', From 2af5eef7c71131a3cb995d3d8e297378bacf6ac1 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 21 Nov 2014 18:13:48 -0800 Subject: [PATCH 67/84] Stop linking pthread. It was only needed by gtest, which is no longer used. (Intesting note: I checked when the -lpthread flag was added, and it's been around since the first revision of build.ninja, which used to be checked in before configure.py existed. Back then, it looks like '@' was used to dereference built-in variables, and build outputs were also prefixed by '@'!). --- configure.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/configure.py b/configure.py index 6d4c4d40bb..eacf5a045e 100755 --- a/configure.py +++ b/configure.py @@ -513,8 +513,6 @@ def has_re2c(): for name in ['includes_normalize_test', 'msvc_helper_test']: objs += cxx(name) -if not platform.is_windows(): - test_libs.append('-lpthread') ninja_test = n.build(binary('ninja_test'), 'link', objs, implicit=ninja_lib, variables=[('libs', test_libs)]) n.newline() From 9870d3bc73b37dd7fb3cfaa34d8c876f6c4a6734 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 21 Nov 2014 18:18:30 -0800 Subject: [PATCH 68/84] Remove now-unused variable test_libs. --- configure.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/configure.py b/configure.py index eacf5a045e..b38ac866d9 100755 --- a/configure.py +++ b/configure.py @@ -490,7 +490,6 @@ def has_re2c(): n.comment('Tests all build into ninja_test executable.') -test_libs = libs objs = [] for name in ['build_log_test', @@ -514,7 +513,7 @@ def has_re2c(): objs += cxx(name) ninja_test = n.build(binary('ninja_test'), 'link', objs, implicit=ninja_lib, - variables=[('libs', test_libs)]) + variables=[('libs', libs)]) n.newline() all_targets += ninja_test From 13bd84f0ac493b28253f28be26b8ef4ca9bad41d Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 22 Nov 2014 11:43:33 -0800 Subject: [PATCH 69/84] Fix Windows build after #862. Thanks to @harig for the report. --- configure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.py b/configure.py index c58b1e4529..aac4ad4f0a 100755 --- a/configure.py +++ b/configure.py @@ -95,7 +95,7 @@ def supports_ppoll(self): return self._platform in ('linux', 'openbsd', 'bitrig') def supports_ninja_browse(self): - return self._platform not in ('windows', 'solaris') + return not self.is_windows() and not self.is_solaris() class Bootstrap: From 6fc15c47a0b17e627f80d6f1d8f08f03d3ae88d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Wed, 22 Jan 2014 00:33:08 +0200 Subject: [PATCH 70/84] no subshell don't need subshell to send stderr to /dev/null --- misc/bash-completion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/bash-completion b/misc/bash-completion index 6edf4dfea0..0536760172 100644 --- a/misc/bash-completion +++ b/misc/bash-completion @@ -50,7 +50,7 @@ _ninja_target() { esac done; targets_command="eval ninja -C \"${dir}\" -t targets all" - targets=$((${targets_command} 2>/dev/null) | awk -F: '{print $1}') + targets=$(${targets_command} 2>/dev/null | awk -F: '{print $1}') COMPREPLY=($(compgen -W "$targets" -- "$cur")) fi return From 77d5c3a67a5f985981e93424b5e35e840a664ee0 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 18:15:09 +0100 Subject: [PATCH 71/84] Don't leave lone closing parens. --- misc/ninja-mode.el | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index ee5fe4894c..d3b2248eb7 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -37,16 +37,14 @@ ;; Build Statement - highlight the rule used, ;; allow for escaped $,: in outputs. '("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . - (1 font-lock-function-name-face)) - )) + (1 font-lock-function-name-face)))) ;;;###autoload (define-derived-mode ninja-mode prog-mode "ninja" (setq comment-start "#") ; Pass extra "t" to turn off syntax-based fontification -- we don't want ; quoted strings highlighted. - (setq font-lock-defaults '(ninja-keywords t)) - ) + (setq font-lock-defaults '(ninja-keywords t))) ;; Run ninja-mode for files ending in .ninja. ;;;###autoload From 6091cfefcd0f283eef17e256db4991cc7c3daac4 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 18:17:39 +0100 Subject: [PATCH 72/84] Use double semicolon comments. --- misc/ninja-mode.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index d3b2248eb7..0eae72eae1 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -42,8 +42,8 @@ ;;;###autoload (define-derived-mode ninja-mode prog-mode "ninja" (setq comment-start "#") - ; Pass extra "t" to turn off syntax-based fontification -- we don't want - ; quoted strings highlighted. + ;; Pass extra "t" to turn off syntax-based fontification -- we don't want + ;; quoted strings highlighted. (setq font-lock-defaults '(ninja-keywords t))) ;; Run ninja-mode for files ending in .ninja. From a13b64746022ab1f5f47c18b0bd1cc3c909e210e Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 18:15:46 +0100 Subject: [PATCH 73/84] Reindent ninja-mode.el using Emacs. Emacs knows best how to indent Emacs Lisp. --- misc/ninja-mode.el | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 0eae72eae1..ad1722c6b7 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -22,22 +22,22 @@ ;;; Code: (defvar ninja-keywords - (list - '("^#.*" . font-lock-comment-face) - (cons (concat "^" (regexp-opt '("rule" "build" "subninja" "include" - "pool" "default") - 'words)) - font-lock-keyword-face) - '("\\([[:alnum:]_]+\\) =" . (1 font-lock-variable-name-face)) - ;; Variable expansion. - '("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) - '("\\(${[[:alnum:]._]+}\\)" . (1 font-lock-variable-name-face)) - ;; Rule names - '("rule +\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) - ;; Build Statement - highlight the rule used, - ;; allow for escaped $,: in outputs. - '("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . - (1 font-lock-function-name-face)))) + (list + '("^#.*" . font-lock-comment-face) + (cons (concat "^" (regexp-opt '("rule" "build" "subninja" "include" + "pool" "default") + 'words)) + font-lock-keyword-face) + '("\\([[:alnum:]_]+\\) =" . (1 font-lock-variable-name-face)) + ;; Variable expansion. + '("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) + '("\\(${[[:alnum:]._]+}\\)" . (1 font-lock-variable-name-face)) + ;; Rule names + '("rule +\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) + ;; Build Statement - highlight the rule used, + ;; allow for escaped $,: in outputs. + '("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . + (1 font-lock-function-name-face)))) ;;;###autoload (define-derived-mode ninja-mode prog-mode "ninja" From 2f7f4ae945f440744fb5addc39c637e48f5a08f2 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 18:22:11 +0100 Subject: [PATCH 74/84] Use quasi-quoting instead of list+cons. --- misc/ninja-mode.el | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index ad1722c6b7..48709bca10 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -22,21 +22,20 @@ ;;; Code: (defvar ninja-keywords - (list - '("^#.*" . font-lock-comment-face) - (cons (concat "^" (regexp-opt '("rule" "build" "subninja" "include" - "pool" "default") - 'words)) - font-lock-keyword-face) - '("\\([[:alnum:]_]+\\) =" . (1 font-lock-variable-name-face)) - ;; Variable expansion. - '("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) - '("\\(${[[:alnum:]._]+}\\)" . (1 font-lock-variable-name-face)) - ;; Rule names - '("rule +\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) - ;; Build Statement - highlight the rule used, - ;; allow for escaped $,: in outputs. - '("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . + `(("^#.*" . font-lock-comment-face) + (,(concat "^" (regexp-opt '("rule" "build" "subninja" "include" + "pool" "default") + 'words)) + . font-lock-keyword-face) + ("\\([[:alnum:]_]+\\) =" . (1 font-lock-variable-name-face)) + ;; Variable expansion. + ("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) + ("\\(${[[:alnum:]._]+}\\)" . (1 font-lock-variable-name-face) + ;; Rule names + ("rule +\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) + ;; Build Statement - highlight the rule used, + ;; allow for escaped $,: in outputs. + ("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)))) ;;;###autoload From c600a8258959e7338296423bc05cf47534272ae7 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 18:23:01 +0100 Subject: [PATCH 75/84] Set comment-start buffer-locally, not globally. --- misc/ninja-mode.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 48709bca10..10cec3ed1c 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -40,7 +40,7 @@ ;;;###autoload (define-derived-mode ninja-mode prog-mode "ninja" - (setq comment-start "#") + (set (make-local-variable 'comment-start) "#") ;; Pass extra "t" to turn off syntax-based fontification -- we don't want ;; quoted strings highlighted. (setq font-lock-defaults '(ninja-keywords t))) From 327c094596228b8dfdc3715a560ef226cd1403a9 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 18:35:46 +0100 Subject: [PATCH 76/84] Set up a proper syntax table for ninja-mode. Since quotes are not meant to be treated as string delimiters, the syntax table is the place to tell Emacs so. This also means syntactic fontification can be reenabled and the font-lock keyword entry for comments removed. --- misc/ninja-mode.el | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 10cec3ed1c..021ede12ed 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -22,8 +22,7 @@ ;;; Code: (defvar ninja-keywords - `(("^#.*" . font-lock-comment-face) - (,(concat "^" (regexp-opt '("rule" "build" "subninja" "include" + `((,(concat "^" (regexp-opt '("rule" "build" "subninja" "include" "pool" "default") 'words)) . font-lock-keyword-face) @@ -38,12 +37,16 @@ ("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)))) +(defvar ninja-mode-syntax-table + (let ((table (make-syntax-table))) + (modify-syntax-entry ?\" "." table) + table) + "Syntax table used in `ninja-mode'.") + ;;;###autoload (define-derived-mode ninja-mode prog-mode "ninja" (set (make-local-variable 'comment-start) "#") - ;; Pass extra "t" to turn off syntax-based fontification -- we don't want - ;; quoted strings highlighted. - (setq font-lock-defaults '(ninja-keywords t))) + (setq font-lock-defaults '(ninja-keywords))) ;; Run ninja-mode for files ending in .ninja. ;;;###autoload From 931db561cfd2c3fcfe85a5ab8783828137106014 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 18:57:31 +0100 Subject: [PATCH 77/84] Correctly recognize comments. --- misc/ninja-mode.el | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 021ede12ed..fd54b29356 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -43,9 +43,26 @@ table) "Syntax table used in `ninja-mode'.") +(defun ninja-syntax-propertize (start end) + (save-match-data + (save-excursion + (goto-char start) + (while (search-forward "#" end t) + (let ((match-pos (match-beginning 0))) + (when (and + ;; Is it the first non-white character on the line? + (eq match-pos (save-excursion (back-to-indentation) (point))) + ;; Are we *not* continuing the previous line? + (not (eq ?$ (char-before (line-end-position 0))))) + (put-text-property match-pos (1+ match-pos) 'syntax-table '(11)) + (let ((line-end (line-end-position))) + (put-text-property line-end (1+ line-end) 'syntax-table '(12))))))))) + ;;;###autoload (define-derived-mode ninja-mode prog-mode "ninja" (set (make-local-variable 'comment-start) "#") + (set (make-local-variable 'parse-sexp-lookup-properties) t) + (set (make-local-variable 'syntax-propertize-function) #'ninja-syntax-propertize) (setq font-lock-defaults '(ninja-keywords))) ;; Run ninja-mode for files ending in .ninja. From 310532c0b0c4a80f089477e7cdd2fb99b20e7de1 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 18:59:53 +0100 Subject: [PATCH 78/84] Remove unnecessary regexp group. --- misc/ninja-mode.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index fd54b29356..b475edb54f 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -28,8 +28,8 @@ . font-lock-keyword-face) ("\\([[:alnum:]_]+\\) =" . (1 font-lock-variable-name-face)) ;; Variable expansion. - ("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) - ("\\(${[[:alnum:]._]+}\\)" . (1 font-lock-variable-name-face) + ("$[[:alnum:]_]+" . font-lock-variable-name-face) + ("${[[:alnum:]._]+}" . font-lock-variable-name-face) ;; Rule names ("rule +\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) ;; Build Statement - highlight the rule used, From 14d161080c256ac5e7e877058691172a95acda45 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 19:03:23 +0100 Subject: [PATCH 79/84] Don't use dotted list syntax unless necessary. --- misc/ninja-mode.el | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index b475edb54f..20b12378f1 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -26,16 +26,16 @@ "pool" "default") 'words)) . font-lock-keyword-face) - ("\\([[:alnum:]_]+\\) =" . (1 font-lock-variable-name-face)) + ("\\([[:alnum:]_]+\\) =" 1 font-lock-variable-name-face) ;; Variable expansion. ("$[[:alnum:]_]+" . font-lock-variable-name-face) ("${[[:alnum:]._]+}" . font-lock-variable-name-face) ;; Rule names - ("rule +\\([[:alnum:]_.-]+\\)" . (1 font-lock-function-name-face)) + ("rule +\\([[:alnum:]_.-]+\\)" 1 font-lock-function-name-face) ;; Build Statement - highlight the rule used, ;; allow for escaped $,: in outputs. - ("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" . - (1 font-lock-function-name-face)))) + ("build +\\(?:[^:$\n]\\|$[:$]\\)+ *: *\\([[:alnum:]_.-]+\\)" + 1 font-lock-function-name-face))) (defvar ninja-mode-syntax-table (let ((table (make-syntax-table))) From 73f934e88679fee031f971554a7b79d640fb28f5 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sat, 22 Nov 2014 19:25:43 +0100 Subject: [PATCH 80/84] Avoid putting properties past the end of the buffer. --- misc/ninja-mode.el | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 20b12378f1..330eba7cc6 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -56,7 +56,10 @@ (not (eq ?$ (char-before (line-end-position 0))))) (put-text-property match-pos (1+ match-pos) 'syntax-table '(11)) (let ((line-end (line-end-position))) - (put-text-property line-end (1+ line-end) 'syntax-table '(12))))))))) + ;; Avoid putting properties past the end of the buffer. + ;; Otherwise we get an `args-out-of-range' error. + (unless (= line-end (1+ (buffer-size))) + (put-text-property line-end (1+ line-end) 'syntax-table '(12)))))))))) ;;;###autoload (define-derived-mode ninja-mode prog-mode "ninja" From 11377a46d103f788982e9a190c50a8526b9a6b80 Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sun, 23 Nov 2014 16:05:48 +0100 Subject: [PATCH 81/84] Correctly recognize a comment if the previous line is a comment ending in $. --- misc/ninja-mode.el | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 330eba7cc6..4802ef1a50 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -52,8 +52,15 @@ (when (and ;; Is it the first non-white character on the line? (eq match-pos (save-excursion (back-to-indentation) (point))) - ;; Are we *not* continuing the previous line? - (not (eq ?$ (char-before (line-end-position 0))))) + (save-excursion + (goto-char (line-end-position 0)) + (or + ;; If we're continuting the previous line, it's not a + ;; comment. + (not (eq ?$ (char-before))) + ;; Except if the previous line is a comment as well, as the + ;; continuation dollar is ignored then. + (nth 4 (syntax-ppss))))) (put-text-property match-pos (1+ match-pos) 'syntax-table '(11)) (let ((line-end (line-end-position))) ;; Avoid putting properties past the end of the buffer. From f15faca3c35881e5f2ada33a63a8a452bc0f3d1e Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sun, 23 Nov 2014 22:26:54 +0100 Subject: [PATCH 82/84] Add a Emacs 24 requirement. Only Emacs >= 24 has prog-mode. --- misc/ninja-mode.el | 2 ++ 1 file changed, 2 insertions(+) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 4802ef1a50..ac5318d6d5 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -1,5 +1,7 @@ ;;; ninja-mode.el --- Major mode for editing .ninja files +;; Package-Requires: ((emacs "24")) + ;; Copyright 2011 Google Inc. All Rights Reserved. ;; ;; Licensed under the Apache License, Version 2.0 (the "License"); From 40a76d8cd5c77833d31b19311f6a9213c23fd09a Mon Sep 17 00:00:00 2001 From: Fanael Linithien Date: Sun, 23 Nov 2014 22:29:49 +0100 Subject: [PATCH 83/84] Use lexical-binding. There's no reason not to use lexical-binding when supporting only Emacs 24+. Its semantics are just that much saner. --- misc/ninja-mode.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index ac5318d6d5..71825d547b 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -1,4 +1,4 @@ -;;; ninja-mode.el --- Major mode for editing .ninja files +;;; ninja-mode.el --- Major mode for editing .ninja files -*- lexical-binding: t -*- ;; Package-Requires: ((emacs "24")) From b532cab080bbde2068ab49aba814c7176111681f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 24 Nov 2014 09:33:39 -0800 Subject: [PATCH 84/84] mark this 1.5.3.git --- src/version.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/version.cc b/src/version.cc index fc25676cbc..f2d6711993 100644 --- a/src/version.cc +++ b/src/version.cc @@ -18,7 +18,7 @@ #include "util.h" -const char* kNinjaVersion = "1.5.1.git"; +const char* kNinjaVersion = "1.5.3.git"; void ParseVersion(const string& version, int* major, int* minor) { size_t end = version.find('.');