From 47993664be821df46b009d3e48d251e6266cefc9 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 13 May 2014 12:35:52 -0700 Subject: wip for console pool on windows --- doc/manual.asciidoc | 2 -- src/manifest_parser.cc | 4 ---- src/subprocess-posix.cc | 2 ++ src/subprocess-win32.cc | 24 +++++++++++++++--------- src/subprocess.h | 5 +---- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 5b0c1fe..18760dd 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -664,8 +664,6 @@ While a task in the `console` pool is running, Ninja's regular output (such as progress status and output from concurrent tasks) is buffered until it completes. -This feature is not yet available on Windows. - Ninja file reference -------------------- diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index a566eda..6fa4f7c 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -317,10 +317,6 @@ bool ManifestParser::ParseEdge(string* err) { Pool* pool = state_->LookupPool(pool_name); if (pool == NULL) return lexer_.Error("unknown pool name '" + pool_name + "'", err); -#ifdef _WIN32 - if (pool == &State::kConsolePool) - return lexer_.Error("console pool unsupported on Windows", err); -#endif edge->pool_ = pool; } diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 793d48f..7311f64 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -84,6 +84,8 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { error_pipe = 2; close(output_pipe[1]); } + // In the console case, child_pipe is still inherited by the child and + // closed when the subprocess finishes, which then notifies ninja. execl("/bin/sh", "/bin/sh", "-c", command.c_str(), (char *) NULL); } while (false); diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index c9607e1..c71f95b 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -21,7 +21,9 @@ #include "util.h" -Subprocess::Subprocess() : child_(NULL) , overlapped_(), is_reading_(false) { +Subprocess::Subprocess(bool use_console) : child_(NULL) , overlapped_(), + is_reading_(false), + use_console_(use_console) { } Subprocess::~Subprocess() { @@ -87,10 +89,14 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { STARTUPINFOA startup_info; memset(&startup_info, 0, sizeof(startup_info)); startup_info.cb = sizeof(STARTUPINFO); - startup_info.dwFlags = STARTF_USESTDHANDLES; - startup_info.hStdInput = nul; - startup_info.hStdOutput = child_pipe; - startup_info.hStdError = child_pipe; + if (!use_console_) { + startup_info.dwFlags = STARTF_USESTDHANDLES; + startup_info.hStdInput = nul; + startup_info.hStdOutput = child_pipe; + startup_info.hStdError = child_pipe; + } + // In the console case, child_pipe is still inherited by the child and closed + // when the subprocess finishes, which then notifies ninja. PROCESS_INFORMATION process_info; memset(&process_info, 0, sizeof(process_info)); @@ -215,9 +221,7 @@ BOOL WINAPI SubprocessSet::NotifyInterrupted(DWORD dwCtrlType) { } Subprocess *SubprocessSet::Add(const string& command, bool use_console) { - assert(!use_console); // We don't support this yet on Windows. - - Subprocess *subprocess = new Subprocess; + Subprocess *subprocess = new Subprocess(use_console); if (!subprocess->Start(this, command)) { delete subprocess; return 0; @@ -269,7 +273,9 @@ Subprocess* SubprocessSet::NextFinished() { void SubprocessSet::Clear() { for (vector::iterator i = running_.begin(); i != running_.end(); ++i) { - if ((*i)->child_) { + // Since the foreground process is in our process group, it will receive a + // SIGINT at the same time as us. XXX is this true on windows? + if ((*i)->child_ && !(*i)->use_console_) { if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId((*i)->child_))) { Win32Fatal("GenerateConsoleCtrlEvent"); diff --git a/src/subprocess.h b/src/subprocess.h index 6ea6f62..b7a1a4c 100644 --- a/src/subprocess.h +++ b/src/subprocess.h @@ -44,14 +44,13 @@ struct Subprocess { const string& GetOutput() const; private: + Subprocess(bool use_console); bool Start(struct SubprocessSet* set, const string& command); void OnPipeReady(); string buf_; #ifdef _WIN32 - Subprocess(); - /// Set up pipe_ as the parent-side pipe of the subprocess; return the /// other end of the pipe, usable in the child process. HANDLE SetupPipe(HANDLE ioport); @@ -62,8 +61,6 @@ struct Subprocess { char overlapped_buf_[4 << 10]; bool is_reading_; #else - Subprocess(bool use_console); - int fd_; pid_t pid_; #endif -- cgit v1.2.1 From c57b771cc3e86f80bf16d36eb1b1f3ab2ddde1de Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 13 May 2014 12:47:08 -0700 Subject: win console wip: enable test --- src/build_test.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/build_test.cc b/src/build_test.cc index 119521e..c414c88 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -245,7 +245,6 @@ TEST_F(PlanTest, PoolWithDepthOne) { "build out2: poolcat in\n"); } -#ifndef _WIN32 TEST_F(PlanTest, ConsolePool) { TestPoolWithDepthOne( "rule poolcat\n" @@ -254,7 +253,6 @@ TEST_F(PlanTest, ConsolePool) { "build out1: poolcat in\n" "build out2: poolcat in\n"); } -#endif TEST_F(PlanTest, PoolsWithDepthTwo) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, @@ -1944,12 +1942,12 @@ TEST_F(BuildTest, Console) { "rule console\n" " command = console\n" " pool = console\n" -"build con: console in.txt\n")); +"build cons: console in.txt\n")); fs_.Create("in.txt", ""); string err; - EXPECT_TRUE(builder_.AddTarget("con", &err)); + EXPECT_TRUE(builder_.AddTarget("cons", &err)); ASSERT_EQ("", err); EXPECT_TRUE(builder_.Build(&err)); EXPECT_EQ("", err); -- cgit v1.2.1 From 6d5cdede695cd08064f9b7e76b7e4b77a5a78c0c Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 13 May 2014 12:58:01 -0700 Subject: win console wip: ctrl-c should reach commands running in console pools --- src/subprocess-win32.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index c71f95b..59b2d37 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -101,10 +101,13 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { PROCESS_INFORMATION process_info; memset(&process_info, 0, sizeof(process_info)); + // Ninja handles ctrl-c, except for subprocesses in console pools. + DWORD process_flags = use_console_ ? 0 : CREATE_NEW_PROCESS_GROUP; + // Do not prepend 'cmd /c' on Windows, this breaks command // lines greater than 8,191 chars. if (!CreateProcessA(NULL, (char*)command.c_str(), NULL, NULL, - /* inherit handles */ TRUE, CREATE_NEW_PROCESS_GROUP, + /* inherit handles */ TRUE, process_flags, NULL, NULL, &startup_info, &process_info)) { DWORD error = GetLastError(); -- cgit v1.2.1 From 4213308cb98c809190424cb43052f8d1cd264ac5 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 13 May 2014 23:51:51 +0200 Subject: vim syntax: Correctly highlight $$a as ($$)a instead of $($a). --- misc/ninja.vim | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/misc/ninja.vim b/misc/ninja.vim index d813267..9e6ee5c 100644 --- a/misc/ninja.vim +++ b/misc/ninja.vim @@ -1,8 +1,8 @@ " ninja build file syntax. " Language: ninja build file as described at " http://martine.github.com/ninja/manual.html -" Version: 1.3 -" Last Change: 2013/04/16 +" Version: 1.4 +" Last Change: 2014/05/13 " Maintainer: Nicolas Weber " Version 1.3 of this script is in the upstream vim repository and will be " included in the next vim release. If you change this, please send your change @@ -55,6 +55,7 @@ syn keyword ninjaPoolCommand contained depth " $simple_varname -> variable " ${varname} -> variable +syn match ninjaDollar "\$\$" syn match ninjaWrapLineOperator "\$$" syn match ninjaSimpleVar "\$[a-zA-Z0-9_-]\+" syn match ninjaVar "\${[a-zA-Z0-9_.-]\+}" @@ -70,6 +71,7 @@ hi def link ninjaComment Comment hi def link ninjaKeyword Keyword hi def link ninjaRuleCommand Statement hi def link ninjaPoolCommand Statement +hi def link ninjaDollar ninjaOperator hi def link ninjaWrapLineOperator ninjaOperator hi def link ninjaOperator Operator hi def link ninjaSimpleVar ninjaVar -- cgit v1.2.1 From c246b586a4191d8eca21761b15466b047008489f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 14 May 2014 03:00:34 -0700 Subject: win console wip: resolve FIXME --- src/subprocess-posix.cc | 1 + src/subprocess-win32.cc | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 7311f64..6e4b47c 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -65,6 +65,7 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { break; if (!use_console_) { + // Put the child in its own process group, so ctrl-c won't reach it. if (setpgid(0, 0) < 0) break; diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index 59b2d37..4fa24c6 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -277,7 +277,7 @@ void SubprocessSet::Clear() { for (vector::iterator i = running_.begin(); i != running_.end(); ++i) { // Since the foreground process is in our process group, it will receive a - // SIGINT at the same time as us. XXX is this true on windows? + // CTRL_BREAK_EVENT at the same time as us. if ((*i)->child_ && !(*i)->use_console_) { if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId((*i)->child_))) { -- cgit v1.2.1 From b0ada0efd9ee1d80d38c781fa15d73bece4a3d69 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 17 May 2014 17:20:20 -0700 Subject: win console wip: Fix comments based on review feedback. --- src/subprocess-posix.cc | 2 +- src/subprocess-win32.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 6e4b47c..743e406 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -85,7 +85,7 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { error_pipe = 2; close(output_pipe[1]); } - // In the console case, child_pipe is still inherited by the child and + // In the console case, output_pipe is still inherited by the child and // closed when the subprocess finishes, which then notifies ninja. execl("/bin/sh", "/bin/sh", "-c", command.c_str(), (char *) NULL); diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index 4fa24c6..fad66e8 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -277,7 +277,7 @@ void SubprocessSet::Clear() { for (vector::iterator i = running_.begin(); i != running_.end(); ++i) { // Since the foreground process is in our process group, it will receive a - // CTRL_BREAK_EVENT at the same time as us. + // CTRL_C_EVENT or CTRL_BREAK_EVENT at the same time as us. if ((*i)->child_ && !(*i)->use_console_) { if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId((*i)->child_))) { -- cgit v1.2.1 From 9c3cada3bf6e56f5ab202689dd61bfff845795e8 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 21 May 2014 14:16:18 -0700 Subject: CleanTest cleanups: * $in only makes sense on rules, not edges (see issue #687) * Remove unneccesary clear() line at end of test --- src/clean_test.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/clean_test.cc b/src/clean_test.cc index 04cff73..4a00fd8 100644 --- a/src/clean_test.cc +++ b/src/clean_test.cc @@ -286,8 +286,7 @@ TEST_F(CleanTest, CleanRspFile) { " rspfile = $rspfile\n" " rspfile_content=$in\n" "build out1: cc in1\n" -" rspfile = cc1.rsp\n" -" rspfile_content=$in\n")); +" rspfile = cc1.rsp\n")); fs_.Create("out1", ""); fs_.Create("cc1.rsp", ""); @@ -307,10 +306,9 @@ TEST_F(CleanTest, CleanRsp) { "build out1: cat in1\n" "build in2: cat_rsp src2\n" " rspfile=in2.rsp\n" -" rspfile_content=$in\n" "build out2: cat_rsp in2\n" " rspfile=out2.rsp\n" -" rspfile_content=$in\n")); +)); fs_.Create("in1", ""); fs_.Create("out1", ""); fs_.Create("in2.rsp", ""); @@ -336,8 +334,6 @@ TEST_F(CleanTest, CleanRsp) { EXPECT_EQ(0, fs_.Stat("out2")); EXPECT_EQ(0, fs_.Stat("in2.rsp")); EXPECT_EQ(0, fs_.Stat("out2.rsp")); - - fs_.files_removed_.clear(); } TEST_F(CleanTest, CleanFailure) { -- cgit v1.2.1 From bc239cca4f3f0757ba34d0306fbc2a2b75d067a2 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 21 May 2014 15:07:47 -0700 Subject: Make "depfile=$out.d" work if $out contains escaped characters, rspfile too. Fixes #730. This has always been broken, but due to #690 more paths are now escaped (e.g. paths containing + characters, like file.c++). Also see discussion in #689. The approach is to give EdgeEnv an enum deciding on whether or not to escape file names, and provide functions that evaluate depfile and rspfile with that set that to kNoEscape. (depfile=$out.d doesn't make sense on edges with multiple outputs.) This should be relatively safe, as $in and $out can't be used on edges, only on rules (#687). --- doc/manual.asciidoc | 6 ++++-- src/build.cc | 10 +++++----- src/build_test.cc | 45 ++++++++++++++++++++++++++++----------------- src/clean.cc | 4 ++-- src/clean_test.cc | 28 ++++++++++++++++++++++++++++ src/graph.cc | 31 +++++++++++++++++++++++++------ src/graph.h | 6 ++++++ 7 files changed, 98 insertions(+), 32 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 18760dd..aea1784 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -783,7 +783,8 @@ keys. `depfile`:: path to an optional `Makefile` that contains extra _implicit dependencies_ (see <>). This is explicitly to support C/C++ header - dependencies; see <>. + dependencies; see <>. `out`, `in`, and + `in_newline` are not shell-quoted when used to set `depfile`. `deps`:: _(Available since Ninja 1.3.)_ if present, must be one of `gcc` or `msvc` to specify special dependency processing. See @@ -830,7 +831,8 @@ keys. response file for the given command, i.e. write the selected string (`rspfile_content`) to the given file (`rspfile`) before calling the command and delete the file after successful execution of the - command. + command. `out`, `in`, and `in_newline` are not shell-quoted when used to set + `rspfile`. + This is particularly useful on Windows OS, where the maximal length of a command line is limited and response files must be used instead. diff --git a/src/build.cc b/src/build.cc index 91f1754..64bcea3 100644 --- a/src/build.cc +++ b/src/build.cc @@ -541,7 +541,7 @@ void Builder::Cleanup() { for (vector::iterator i = active_edges.begin(); i != active_edges.end(); ++i) { - string depfile = (*i)->GetBinding("depfile"); + string depfile = (*i)->GetUnescapedDepfile(); for (vector::iterator ni = (*i)->outputs_.begin(); ni != (*i)->outputs_.end(); ++ni) { // Only delete this output if it was actually modified. This is @@ -696,7 +696,7 @@ bool Builder::StartEdge(Edge* edge, string* err) { // Create response file, if needed // XXX: this may also block; do we care? - string rspfile = edge->GetBinding("rspfile"); + string rspfile = edge->GetUnescapedRspfile(); if (!rspfile.empty()) { string content = edge->GetBinding("rspfile_content"); if (!disk_interface_->WriteFile(rspfile, content)) @@ -772,7 +772,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { restat_mtime = input_mtime; } - string depfile = edge->GetBinding("depfile"); + string depfile = edge->GetUnescapedDepfile(); if (restat_mtime != 0 && deps_type.empty() && !depfile.empty()) { TimeStamp depfile_mtime = disk_interface_->Stat(depfile); if (depfile_mtime > restat_mtime) @@ -788,7 +788,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { plan_.EdgeFinished(edge); // Delete any left over response file. - string rspfile = edge->GetBinding("rspfile"); + string rspfile = edge->GetUnescapedRspfile(); if (!rspfile.empty() && !g_keep_rsp) disk_interface_->RemoveFile(rspfile); @@ -828,7 +828,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, } else #endif if (deps_type == "gcc") { - string depfile = result->edge->GetBinding("depfile"); + string depfile = result->edge->GetUnescapedDepfile(); if (depfile.empty()) { *err = string("edge with deps=gcc but no depfile makes no sense"); return false; diff --git a/src/build_test.cc b/src/build_test.cc index c414c88..dad69dc 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -521,6 +521,7 @@ bool FakeCommandRunner::StartCommand(Edge* edge) { commands_ran_.push_back(edge->EvaluateCommand()); if (edge->rule().name() == "cat" || edge->rule().name() == "cat_rsp" || + edge->rule().name() == "cat_rsp_out" || edge->rule().name() == "cc" || edge->rule().name() == "touch" || edge->rule().name() == "touch-interrupt") { @@ -775,13 +776,13 @@ TEST_F(BuildTest, DepFileMissing) { string err; ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule cc\n command = cc $in\n depfile = $out.d\n" -"build foo.o: cc foo.c\n")); +"build fo$ o.o: cc foo.c\n")); fs_.Create("foo.c", ""); - EXPECT_TRUE(builder_.AddTarget("foo.o", &err)); + EXPECT_TRUE(builder_.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); ASSERT_EQ(1u, fs_.files_read_.size()); - EXPECT_EQ("foo.o.d", fs_.files_read_[0]); + EXPECT_EQ("fo o.o.d", fs_.files_read_[0]); } TEST_F(BuildTest, DepFileOK) { @@ -1302,14 +1303,20 @@ TEST_F(BuildTest, RspFileSuccess) " command = cat $rspfile > $out\n" " rspfile = $rspfile\n" " rspfile_content = $long_command\n" + "rule cat_rsp_out\n" + " command = cat $rspfile > $out\n" + " rspfile = $out.rsp\n" + " rspfile_content = $long_command\n" "build out1: cat in\n" "build out2: cat_rsp in\n" - " rspfile = out2.rsp\n" + " rspfile = out 2.rsp\n" + " long_command = Some very long command\n" + "build out$ 3: cat_rsp_out in\n" " long_command = Some very long command\n")); fs_.Create("out1", ""); fs_.Create("out2", ""); - fs_.Create("out3", ""); + fs_.Create("out 3", ""); fs_.Tick(); @@ -1320,20 +1327,24 @@ TEST_F(BuildTest, RspFileSuccess) ASSERT_EQ("", err); EXPECT_TRUE(builder_.AddTarget("out2", &err)); ASSERT_EQ("", err); + EXPECT_TRUE(builder_.AddTarget("out 3", &err)); + ASSERT_EQ("", err); size_t files_created = fs_.files_created_.size(); size_t files_removed = fs_.files_removed_.size(); EXPECT_TRUE(builder_.Build(&err)); - ASSERT_EQ(2u, command_runner_.commands_ran_.size()); // cat + cat_rsp + ASSERT_EQ(3u, command_runner_.commands_ran_.size()); - // The RSP file was created - ASSERT_EQ(files_created + 1, fs_.files_created_.size()); - ASSERT_EQ(1u, fs_.files_created_.count("out2.rsp")); + // The RSP files were created + ASSERT_EQ(files_created + 2, fs_.files_created_.size()); + ASSERT_EQ(1u, fs_.files_created_.count("out 2.rsp")); + ASSERT_EQ(1u, fs_.files_created_.count("out 3.rsp")); - // The RSP file was removed - ASSERT_EQ(files_removed + 1, fs_.files_removed_.size()); - ASSERT_EQ(1u, fs_.files_removed_.count("out2.rsp")); + // The RSP files were removed + ASSERT_EQ(files_removed + 2, fs_.files_removed_.size()); + ASSERT_EQ(1u, fs_.files_removed_.count("out 2.rsp")); + ASSERT_EQ(1u, fs_.files_removed_.count("out 3.rsp")); } // Test that RSP file is created but not removed for commands, which fail @@ -1804,7 +1815,7 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { string err; const char* manifest = "rule cc\n command = cc $in\n depfile = $out.d\n deps = gcc\n" - "build foo.o: cc foo.c\n"; + "build fo$ o.o: cc foo.c\n"; fs_.Create("foo.c", ""); @@ -1819,9 +1830,9 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { Builder builder(&state, config_, NULL, &deps_log, &fs_); builder.command_runner_.reset(&command_runner_); - EXPECT_TRUE(builder.AddTarget("foo.o", &err)); + EXPECT_TRUE(builder.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); - fs_.Create("foo.o.d", "foo.o: blah.h bar.h\n"); + fs_.Create("fo o.o.d", "fo\\ o.o: blah.h bar.h\n"); EXPECT_TRUE(builder.Build(&err)); EXPECT_EQ("", err); @@ -1844,10 +1855,10 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { Edge* edge = state.edges_.back(); state.GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. - EXPECT_TRUE(builder.AddTarget("foo.o", &err)); + EXPECT_TRUE(builder.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); - // Expect three new edges: one generating foo.o, and two more from + // 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. diff --git a/src/clean.cc b/src/clean.cc index 5d1974e..98c638c 100644 --- a/src/clean.cc +++ b/src/clean.cc @@ -80,11 +80,11 @@ bool Cleaner::IsAlreadyRemoved(const string& path) { } void Cleaner::RemoveEdgeFiles(Edge* edge) { - string depfile = edge->GetBinding("depfile"); + string depfile = edge->GetUnescapedDepfile(); if (!depfile.empty()) Remove(depfile); - string rspfile = edge->GetBinding("rspfile"); + string rspfile = edge->GetUnescapedRspfile(); if (!rspfile.empty()) Remove(rspfile); } diff --git a/src/clean_test.cc b/src/clean_test.cc index 4a00fd8..5869bbb 100644 --- a/src/clean_test.cc +++ b/src/clean_test.cc @@ -368,3 +368,31 @@ TEST_F(CleanTest, CleanPhony) { EXPECT_EQ(2, cleaner.cleaned_files_count()); EXPECT_NE(0, fs_.Stat("phony")); } + +TEST_F(CleanTest, CleanDepFileAndRspFileWithSpaces) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule cc_dep\n" +" command = cc $in > $out\n" +" depfile = $out.d\n" +"rule cc_rsp\n" +" command = cc $in > $out\n" +" rspfile = $out.rsp\n" +" rspfile_content = $in\n" +"build out$ 1: cc_dep in$ 1\n" +"build out$ 2: cc_rsp in$ 1\n" +)); + fs_.Create("out 1", ""); + fs_.Create("out 2", ""); + fs_.Create("out 1.d", ""); + fs_.Create("out 2.rsp", ""); + + Cleaner cleaner(&state_, config_, &fs_); + EXPECT_EQ(0, cleaner.CleanAll()); + EXPECT_EQ(4, cleaner.cleaned_files_count()); + EXPECT_EQ(4u, fs_.files_removed_.size()); + + EXPECT_EQ(0, fs_.Stat("out 1")); + EXPECT_EQ(0, fs_.Stat("out 2")); + EXPECT_EQ(0, fs_.Stat("out 1.d")); + EXPECT_EQ(0, fs_.Stat("out 2.rsp")); +} diff --git a/src/graph.cc b/src/graph.cc index 7121342..aa9c0e8 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -215,7 +215,10 @@ bool Edge::AllInputsReady() const { /// An Env for an Edge, providing $in and $out. struct EdgeEnv : public Env { - explicit EdgeEnv(Edge* edge) : edge_(edge) {} + enum EscapeKind { kShellEscape, kDoNotEscape }; + + explicit EdgeEnv(Edge* edge, EscapeKind escape) + : edge_(edge), escape_in_out_(escape) {} virtual string LookupVariable(const string& var); /// Given a span of Nodes, construct a list of paths suitable for a command @@ -225,6 +228,7 @@ struct EdgeEnv : public Env { char sep); Edge* edge_; + EscapeKind escape_in_out_; }; string EdgeEnv::LookupVariable(const string& var) { @@ -250,13 +254,18 @@ string EdgeEnv::MakePathList(vector::iterator begin, char sep) { string result; for (vector::iterator i = begin; i != end; ++i) { - if (!result.empty()) result.push_back(sep); + if (!result.empty()) + result.push_back(sep); const string& path = (*i)->path(); + if (escape_in_out_ == kShellEscape) { #if _WIN32 - GetWin32EscapedString(path, &result); + GetWin32EscapedString(path, &result); #else - GetShellEscapedString(path, &result); + GetShellEscapedString(path, &result); #endif + } else { + result.append(path); + } } return result; } @@ -272,7 +281,7 @@ string Edge::EvaluateCommand(bool incl_rsp_file) { } string Edge::GetBinding(const string& key) { - EdgeEnv env(this); + EdgeEnv env(this, EdgeEnv::kShellEscape); return env.LookupVariable(key); } @@ -280,6 +289,16 @@ bool Edge::GetBindingBool(const string& key) { return !GetBinding(key).empty(); } +string Edge::GetUnescapedDepfile() { + EdgeEnv env(this, EdgeEnv::kDoNotEscape); + return env.LookupVariable("depfile"); +} + +string Edge::GetUnescapedRspfile() { + EdgeEnv env(this, EdgeEnv::kDoNotEscape); + return env.LookupVariable("rspfile"); +} + void Edge::Dump(const char* prefix) const { printf("%s[ ", prefix); for (vector::const_iterator i = inputs_.begin(); @@ -331,7 +350,7 @@ bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) { if (!deps_type.empty()) return LoadDepsFromLog(edge, err); - string depfile = edge->GetBinding("depfile"); + string depfile = edge->GetUnescapedDepfile(); if (!depfile.empty()) return LoadDepFile(edge, depfile, err); diff --git a/src/graph.h b/src/graph.h index 6cd7f25..66e31b5 100644 --- a/src/graph.h +++ b/src/graph.h @@ -146,9 +146,15 @@ struct Edge { /// full contents of a response file (if applicable) string EvaluateCommand(bool incl_rsp_file = false); + /// Returns the shell-escaped value of |key|. string GetBinding(const string& key); bool GetBindingBool(const string& key); + /// Like GetBinding("depfile"), but without shell escaping. + string GetUnescapedDepfile(); + /// Like GetBinding("rspfile"), but without shell escaping. + string GetUnescapedRspfile(); + void Dump(const char* prefix="") const; const Rule* rule_; -- cgit v1.2.1 From 385b7f39f5eb602dd90d9aedee0f6147b695c762 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 24 May 2014 15:52:52 -0700 Subject: reword manual for depfile/rspfile escaping change --- doc/manual.asciidoc | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index aea1784..fcf3db3 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -783,8 +783,7 @@ keys. `depfile`:: path to an optional `Makefile` that contains extra _implicit dependencies_ (see <>). This is explicitly to support C/C++ header - dependencies; see <>. `out`, `in`, and - `in_newline` are not shell-quoted when used to set `depfile`. + dependencies; see <>. `deps`:: _(Available since Ninja 1.3.)_ if present, must be one of `gcc` or `msvc` to specify special dependency processing. See @@ -807,9 +806,9 @@ keys. rebuilt if the command line changes; and secondly, they are not cleaned by default. -`in`:: the shell-quoted space-separated list of files provided as - inputs to the build line referencing this `rule`. (`$in` is provided - solely for convenience; if you need some subset or variant of this +`in`:: the space-separated list of files provided as inputs to the build line + referencing this `rule`, shell-quoted if it appears in commands. (`$in` is + provided solely for convenience; if you need some subset or variant of this list of files, just construct a new variable with that list and use that instead.) @@ -818,8 +817,8 @@ keys. `$rspfile_content`; this works around a bug in the MSVC linker where it uses a fixed-size buffer for processing input.) -`out`:: the shell-quoted space-separated list of files provided as - outputs to the build line referencing this `rule`. +`out`:: the space-separated list of files provided as outputs to the build line + referencing this `rule`, shell-quoted if it appears in commands. `restat`:: if present, causes Ninja to re-stat the command's outputs after execution of the command. Each output whose modification time @@ -831,8 +830,7 @@ keys. response file for the given command, i.e. write the selected string (`rspfile_content`) to the given file (`rspfile`) before calling the command and delete the file after successful execution of the - command. `out`, `in`, and `in_newline` are not shell-quoted when used to set - `rspfile`. + command. + This is particularly useful on Windows OS, where the maximal length of a command line is limited and response files must be used instead. -- cgit v1.2.1 From 8d84ba41dee869935243da46e4bf61fc84117676 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 30 May 2014 03:14:48 +0200 Subject: Allow + in filenames without escaping Due to #690, file.c++ used to be escaped. + seems as safe as -, so allow it to not be escaped, to keep compile command lines with a fairly common extension slightly cleaner. --- src/util.cc | 1 + src/util_test.cc | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util.cc b/src/util.cc index 24d231f..484b0c1 100644 --- a/src/util.cc +++ b/src/util.cc @@ -183,6 +183,7 @@ static inline bool IsKnownShellSafeCharacter(char ch) { switch (ch) { case '_': + case '+': case '-': case '.': case '/': diff --git a/src/util_test.cc b/src/util_test.cc index f827e5a..b58d15e 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -148,7 +148,7 @@ TEST(PathEscaping, TortureTest) { } TEST(PathEscaping, SensiblePathsAreNotNeedlesslyEscaped) { - const char* path = "some/sensible/path/without/crazy/characters.cc"; + const char* path = "some/sensible/path/without/crazy/characters.c++"; string result; GetWin32EscapedString(path, &result); @@ -160,7 +160,7 @@ TEST(PathEscaping, SensiblePathsAreNotNeedlesslyEscaped) { } TEST(PathEscaping, SensibleWin32PathsAreNotNeedlesslyEscaped) { - const char* path = "some\\sensible\\path\\without\\crazy\\characters.cc"; + const char* path = "some\\sensible\\path\\without\\crazy\\characters.c++"; string result; GetWin32EscapedString(path, &result); -- cgit v1.2.1 From 8324fcc7425cb2bc285b042c357a954e7a1a2013 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Fri, 30 May 2014 22:59:02 +0200 Subject: Use unversioned gnukfreebsd platform. --- platform_helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform_helper.py b/platform_helper.py index de102b5..bc3a125 100644 --- a/platform_helper.py +++ b/platform_helper.py @@ -19,7 +19,7 @@ import sys def platforms(): return ['linux', 'darwin', 'freebsd', 'openbsd', 'solaris', 'sunos5', - 'mingw', 'msvc', 'gnukfreebsd8', 'bitrig'] + 'mingw', 'msvc', 'gnukfreebsd', 'bitrig'] class Platform(object): def __init__(self, platform): @@ -31,7 +31,7 @@ class Platform(object): self._platform = 'linux' elif self._platform.startswith('freebsd'): self._platform = 'freebsd' - elif self._platform.startswith('gnukfreebsd8'): + elif self._platform.startswith('gnukfreebsd'): self._platform = 'freebsd' elif self._platform.startswith('openbsd'): self._platform = 'openbsd' -- cgit v1.2.1