From 07bd1e2f088c55dfb01609e73febc08cedf854ad Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 11:59:35 -0700 Subject: doc that ReadFile reads in text mode on Windows --- src/util.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util.h b/src/util.h index ab1882b..7e30368 100644 --- a/src/util.h +++ b/src/util.h @@ -41,7 +41,8 @@ bool CanonicalizePath(string* path, string* err); bool CanonicalizePath(char* path, size_t* len, string* err); -/// Read a file to a string. +/// Read a file to a string (in text mode: with CRLF conversion +/// on Windows). /// Returns -errno and fills in \a err on error. int ReadFile(const string& path, string* contents, string* err); -- cgit v0.12 From edaf99e9bda420696b2173b44f0abfc63184aaae Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 12:01:38 -0700 Subject: use correct path separator for --with-gtest source --- configure.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.py b/configure.py index 76abc72..f4881a7 100755 --- a/configure.py +++ b/configure.py @@ -291,10 +291,10 @@ if options.with_gtest: else: gtest_cflags = '-fvisibility=hidden ' + gtest_all_incs objs += n.build(built('gtest-all' + objext), 'cxx', - os.path.join(path, 'src/gtest-all.cc'), + os.path.join(path, 'src', 'gtest-all.cc'), variables=[('cflags', gtest_cflags)]) objs += n.build(built('gtest_main' + objext), 'cxx', - os.path.join(path, 'src/gtest_main.cc'), + os.path.join(path, 'src', 'gtest_main.cc'), variables=[('cflags', gtest_cflags)]) test_cflags = cflags + ['-DGTEST_HAS_RTTI=0', -- cgit v0.12 From 5cc5615211b1cf83605a0fb7565cc640f4d7698e Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 12:12:01 -0700 Subject: add a module for working with MSVC (cl.exe) behavior This will be needed for performant builds on Windows. --- configure.py | 3 +++ src/msvc_helper-win32.cc | 35 +++++++++++++++++++++++++++++++++++ src/msvc_helper.h | 29 +++++++++++++++++++++++++++++ src/msvc_helper_test.cc | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+) create mode 100644 src/msvc_helper-win32.cc create mode 100644 src/msvc_helper.h create mode 100644 src/msvc_helper_test.cc diff --git a/configure.py b/configure.py index f4881a7..331e027 100755 --- a/configure.py +++ b/configure.py @@ -249,6 +249,7 @@ for name in ['build', if platform in ('mingw', 'windows'): objs += cxx('subprocess-win32') if platform == 'windows': + objs += cxx('msvc_helper-win32') objs += cxx('minidump-win32') objs += cc('getopt') else: @@ -318,6 +319,8 @@ for name in ['build_log_test', 'test', 'util_test']: objs += cxx(name, variables=[('cflags', test_cflags)]) +if platform == 'windows': + objs += cxx('msvc_helper_test', variables=[('cflags', test_cflags)]) if platform != 'mingw' and platform != 'windows': test_libs.append('-lpthread') diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc new file mode 100644 index 0000000..ff18e80 --- /dev/null +++ b/src/msvc_helper-win32.cc @@ -0,0 +1,35 @@ +// Copyright 2011 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "msvc_helper.h" + +#include + +#include "string_piece.h" + +// static +string CLWrapper::FilterShowIncludes(const string& line) { + static const char kMagicPrefix[] = "Note: including file: "; + const char* in = line.c_str(); + const char* end = in + line.size(); + + if (end - in > (int)sizeof(kMagicPrefix) - 1 && + memcmp(in, kMagicPrefix, sizeof(kMagicPrefix) - 1) == 0) { + in += sizeof(kMagicPrefix) - 1; + while (*in == ' ') + ++in; + return line.substr(in - line.c_str()); + } + return ""; +} diff --git a/src/msvc_helper.h b/src/msvc_helper.h new file mode 100644 index 0000000..e7c2b1a --- /dev/null +++ b/src/msvc_helper.h @@ -0,0 +1,29 @@ +// Copyright 2011 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +using namespace std; + +struct StringPiece; + +/// Visual Studio's cl.exe requires some massaging to work with Ninja; +/// for example, it emits include information on stderr in a funny +/// format when building with /showIncludes. This class wraps a CL +/// process and parses that output to extract the file list. +struct CLWrapper { + /// Parse a line of cl.exe output and extract /showIncludes info. + /// If a dependency is extracted, returns a nonempty string. + static string FilterShowIncludes(const string& line); +}; diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc new file mode 100644 index 0000000..9eb3c89 --- /dev/null +++ b/src/msvc_helper_test.cc @@ -0,0 +1,32 @@ +// Copyright 2011 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "msvc_helper.h" + +#include + +#include "test.h" +#include "util.h" + +TEST(MSVCHelperTest, ShowIncludes) { + ASSERT_EQ("", CLWrapper::FilterShowIncludes("")); + + ASSERT_EQ("", CLWrapper::FilterShowIncludes("Sample compiler output")); + ASSERT_EQ("c:\\Some Files\\foobar.h", + CLWrapper::FilterShowIncludes("Note: including file: " + "c:\\Some Files\\foobar.h")); + ASSERT_EQ("c:\\initspaces.h", + CLWrapper::FilterShowIncludes("Note: including file: " + "c:\\initspaces.h")); +} -- cgit v0.12 From 5024ada1786143f4946a574f4f0bbe3da7b93520 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 13:57:35 -0700 Subject: move Win32Fatal into util --- src/subprocess-win32.cc | 8 -------- src/util.cc | 4 ++++ src/util.h | 3 +++ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index 38b6957..4b103a5 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -20,14 +20,6 @@ #include "util.h" -namespace { - -void Win32Fatal(const char* function) { - Fatal("%s: %s", function, GetLastErrorString().c_str()); -} - -} // anonymous namespace - Subprocess::Subprocess() : child_(NULL) , overlapped_(), is_reading_(false) { } diff --git a/src/util.cc b/src/util.cc index 90a1d45..14f6265 100644 --- a/src/util.cc +++ b/src/util.cc @@ -247,6 +247,10 @@ string GetLastErrorString() { LocalFree(msg_buf); return msg; } + +void Win32Fatal(const char* function) { + Fatal("%s: %s", function, GetLastErrorString().c_str()); +} #endif static bool islatinalpha(int c) { diff --git a/src/util.h b/src/util.h index 7e30368..3a658fb 100644 --- a/src/util.h +++ b/src/util.h @@ -82,6 +82,9 @@ string ElideMiddle(const string& str, size_t width); #ifdef _WIN32 /// Convert the value returned by GetLastError() into a string. string GetLastErrorString(); + +/// Calls Fatal() with a function name and GetLastErrorString. +void Win32Fatal(const char* function); #endif #endif // NINJA_UTIL_H_ -- cgit v0.12 From 031237133e33ee4eb5e9641396d6b0f566b575c8 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 14:07:38 -0700 Subject: doc some cl.exe flags in the configure script --- configure.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/configure.py b/configure.py index 331e027..1cdab49 100755 --- a/configure.py +++ b/configure.py @@ -108,8 +108,13 @@ else: n.variable('ar', configure_env.get('AR', 'ar')) if platform == 'windows': - cflags = ['/nologo', '/Zi', '/W4', '/WX', '/wd4530', '/wd4100', '/wd4706', - '/wd4512', '/wd4800', '/wd4702', '/wd4819', '/GR-', + cflags = ['/nologo', # Don't print startup banner. + '/Zi', # Create pdb with debug info. + '/W4', # Highest warning level. + '/WX', # Warnings as errors. + '/wd4530', '/wd4100', '/wd4706', + '/wd4512', '/wd4800', '/wd4702', '/wd4819', + '/GR-', # Disable RTTI. # Disable size_t -> int truncation warning. # We never have strings or arrays larger than 2**31. '/wd4267', -- cgit v0.12 From 1843f550d9b8b6d271cefdfb5fffd150bb8ef069 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 14:52:18 -0700 Subject: add subprocess-spawning to msvc_helper Rather than using subprocess.h, reimplement the subprocess code. This allows: 1) using anonymous (instead of named) pipes 2) not using all the completion port craziness 3) printing the output as it happens 4) further variation, like adjusting the environment (in a forthcoming change) without affecting the main subprocess code --- src/msvc_helper-win32.cc | 98 +++++++++++++++++++++++++++++++++++++++++++++++- src/msvc_helper.h | 9 +++++ src/msvc_helper_test.cc | 10 +++++ 3 files changed, 116 insertions(+), 1 deletion(-) diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index ff18e80..ac957e4 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -15,8 +15,9 @@ #include "msvc_helper.h" #include +#include -#include "string_piece.h" +#include "util.h" // static string CLWrapper::FilterShowIncludes(const string& line) { @@ -33,3 +34,98 @@ string CLWrapper::FilterShowIncludes(const string& line) { } return ""; } + +int CLWrapper::Run(const string& command, string* extra_output) { + SECURITY_ATTRIBUTES security_attributes = {}; + security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); + security_attributes.bInheritHandle = TRUE; + + // Must be inheritable so subprocesses can dup to children. + HANDLE nul = CreateFile("NUL", GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | + FILE_SHARE_DELETE, + &security_attributes, OPEN_EXISTING, 0, NULL); + if (nul == INVALID_HANDLE_VALUE) + Fatal("couldn't open nul"); + + HANDLE stdout_read, stdout_write; + if (!CreatePipe(&stdout_read, &stdout_write, &security_attributes, 0)) + Win32Fatal("CreatePipe"); + + if (!SetHandleInformation(stdout_read, HANDLE_FLAG_INHERIT, 0)) + Win32Fatal("SetHandleInformation"); + + PROCESS_INFORMATION process_info = {}; + STARTUPINFO startup_info = {}; + startup_info.cb = sizeof(STARTUPINFO); + startup_info.hStdInput = nul; + startup_info.hStdError = stdout_write; + startup_info.hStdOutput = stdout_write; + startup_info.dwFlags |= STARTF_USESTDHANDLES; + + if (!CreateProcessA(NULL, (char*)command.c_str(), NULL, NULL, + /* inherit handles */ TRUE, 0, + NULL, NULL, + &startup_info, &process_info)) { + Win32Fatal("CreateProcess"); + } + + if (!CloseHandle(nul) || + !CloseHandle(stdout_write)) { + Win32Fatal("CloseHandle"); + } + + // Read output of the subprocess and parse it. + string output; + DWORD read_len = 1; + while (read_len) { + char buf[64 << 10]; + read_len = 0; + if (!::ReadFile(stdout_read, buf, sizeof(buf), &read_len, NULL) && + GetLastError() != ERROR_BROKEN_PIPE) { + Win32Fatal("ReadFile"); + } + output.append(buf, read_len); + + // Loop over all lines in the output and process them. + for (;;) { + size_t ofs = output.find_first_of("\r\n"); + if (ofs == string::npos) + break; + string line = output.substr(0, ofs); + + string include = FilterShowIncludes(line); + if (!include.empty()) { + includes_.push_back(include); + } else { + if (extra_output) { + extra_output->append(line); + extra_output->append("\n"); + } else { + printf("%s\n", line.c_str()); + } + } + + if (ofs < output.size() && output[ofs] == '\r') + ++ofs; + if (ofs < output.size() && output[ofs] == '\n') + ++ofs; + output = output.substr(ofs); + } + } + + if (WaitForSingleObject(process_info.hProcess, INFINITE) == WAIT_FAILED) + Win32Fatal("WaitForSingleObject"); + + DWORD exit_code = 0; + if (!GetExitCodeProcess(process_info.hProcess, &exit_code)) + Win32Fatal("GetExitCodeProcess"); + + if (!CloseHandle(stdout_read) || + !CloseHandle(process_info.hProcess) || + !CloseHandle(process_info.hThread)) { + Win32Fatal("CloseHandle"); + } + + return exit_code; +} diff --git a/src/msvc_helper.h b/src/msvc_helper.h index e7c2b1a..5a657be 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -23,7 +23,16 @@ struct StringPiece; /// format when building with /showIncludes. This class wraps a CL /// process and parses that output to extract the file list. struct CLWrapper { + /// Start a process and parse its output. Returns its exit code. + /// Any non-parsed output is buffered into \a extra_output if provided, + /// otherwise it is printed to stdout while the process runs. + /// Crashes (calls Fatal()) on error. + int Run(const string& command, string* extra_output=NULL); + /// Parse a line of cl.exe output and extract /showIncludes info. /// If a dependency is extracted, returns a nonempty string. + /// Exposed for testing. static string FilterShowIncludes(const string& line); + + vector includes_; }; diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 9eb3c89..8db4c69 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -30,3 +30,13 @@ TEST(MSVCHelperTest, ShowIncludes) { CLWrapper::FilterShowIncludes("Note: including file: " "c:\\initspaces.h")); } + +TEST(MSVCHelperTest, Run) { + CLWrapper cl; + string output; + cl.Run("cmd /c \"echo foo&& echo Note: including file: foo.h&&echo bar\"", + &output); + ASSERT_EQ("foo\nbar\n", output); + ASSERT_EQ(1u, cl.includes_.size()); + ASSERT_EQ("foo.h", cl.includes_[0]); +} -- cgit v0.12 From c963c4834d0ab05ce8ecf341c74db6ded379fa8a Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 15:09:33 -0700 Subject: msvc helper: attempt to filter out when it prints the input filename This is a heuristic but it appears to work for the Chrome build. --- src/msvc_helper-win32.cc | 23 +++++++++++++++++++++++ src/msvc_helper.h | 6 ++++++ src/msvc_helper_test.cc | 18 ++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index ac957e4..f6b2a22 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -19,6 +19,16 @@ #include "util.h" +namespace { + +/// Return true if \a input ends with \a needle. +bool EndsWith(const string& input, const string& needle) { + return (input.size() >= needle.size() && + input.substr(input.size() - needle.size()) == needle); +} + +} // anonymous namespace + // static string CLWrapper::FilterShowIncludes(const string& line) { static const char kMagicPrefix[] = "Note: including file: "; @@ -35,6 +45,15 @@ string CLWrapper::FilterShowIncludes(const string& line) { return ""; } +// static +bool CLWrapper::FilterInputFilename(const string& line) { + // TODO: other extensions, like .asm? + return EndsWith(line, ".c") || + EndsWith(line, ".cc") || + EndsWith(line, ".cxx") || + EndsWith(line, ".cpp"); +} + int CLWrapper::Run(const string& command, string* extra_output) { SECURITY_ATTRIBUTES security_attributes = {}; security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); @@ -97,6 +116,10 @@ int CLWrapper::Run(const string& command, string* extra_output) { string include = FilterShowIncludes(line); if (!include.empty()) { includes_.push_back(include); + } else if (FilterInputFilename(line)) { + // Drop it. + // TODO: if we support compiling multiple output files in a single + // cl.exe invocation, we should stash the filename. } else { if (extra_output) { extra_output->append(line); diff --git a/src/msvc_helper.h b/src/msvc_helper.h index 5a657be..5bf9787 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -34,5 +34,11 @@ struct CLWrapper { /// Exposed for testing. static string FilterShowIncludes(const string& line); + /// Parse a line of cl.exe output and return true if it looks like + /// it's printing an input filename. This is a heuristic but it appears + /// to be the best we can do. + /// Exposed for testing. + static bool FilterInputFilename(const string& line); + vector includes_; }; diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 8db4c69..a0bab90 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -31,6 +31,16 @@ TEST(MSVCHelperTest, ShowIncludes) { "c:\\initspaces.h")); } +TEST(MSVCHelperTest, FilterInputFilename) { + ASSERT_TRUE(CLWrapper::FilterInputFilename("foobar.cc")); + ASSERT_TRUE(CLWrapper::FilterInputFilename("foo bar.cc")); + ASSERT_TRUE(CLWrapper::FilterInputFilename("baz.c")); + + ASSERT_FALSE(CLWrapper::FilterInputFilename( + "src\\cl_helper.cc(166) : fatal error C1075: end " + "of file found ...")); +} + TEST(MSVCHelperTest, Run) { CLWrapper cl; string output; @@ -40,3 +50,11 @@ TEST(MSVCHelperTest, Run) { ASSERT_EQ(1u, cl.includes_.size()); ASSERT_EQ("foo.h", cl.includes_[0]); } + +TEST(MSVCHelperTest, RunFilenameFilter) { + CLWrapper cl; + string output; + cl.Run("cmd /c \"echo foo.cc&& echo cl: warning\"", + &output); + ASSERT_EQ("cl: warning\n", output); +} -- cgit v0.12 From c08766e86d10f0ef5417f6c6accbff37706b08c4 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 15:23:31 -0700 Subject: add .ilk (incremental linker) to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index b7a7ab8..1872263 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ *.pyc *.exe *.pdb +*.ilk TAGS /build /build.ninja -- cgit v0.12 From 0af67f1babe08c7e00ba194ccb47c4a0c60fa52a Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Sun, 12 Aug 2012 15:51:21 -0700 Subject: add functions for normalizing win32 include paths (Note from Evan: this is landing Scott's code more or less verbatim without a lot of analysis; it could maybe be simplified and reduced, but it's only intended to be used in the MSVC helper so it's fine to be experimental.) --- configure.py | 4 +- src/includes_normalize-win32.cc | 116 ++++++++++++++++++++++++++++++++++++++++ src/includes_normalize.h | 35 ++++++++++++ src/includes_normalize_test.cc | 98 +++++++++++++++++++++++++++++++++ 4 files changed, 252 insertions(+), 1 deletion(-) create mode 100644 src/includes_normalize-win32.cc create mode 100644 src/includes_normalize.h create mode 100644 src/includes_normalize_test.cc diff --git a/configure.py b/configure.py index 1cdab49..ed05031 100755 --- a/configure.py +++ b/configure.py @@ -254,6 +254,7 @@ for name in ['build', if platform in ('mingw', 'windows'): objs += cxx('subprocess-win32') if platform == 'windows': + objs += cxx('includes_normalize-win32') objs += cxx('msvc_helper-win32') objs += cxx('minidump-win32') objs += cc('getopt') @@ -325,7 +326,8 @@ for name in ['build_log_test', 'util_test']: objs += cxx(name, variables=[('cflags', test_cflags)]) if platform == 'windows': - objs += cxx('msvc_helper_test', variables=[('cflags', test_cflags)]) + for name in ['includes_normalize_test', 'msvc_helper_test']: + objs += cxx(name, variables=[('cflags', test_cflags)]) if platform != 'mingw' and platform != 'windows': test_libs.append('-lpthread') diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc new file mode 100644 index 0000000..cf398ae --- /dev/null +++ b/src/includes_normalize-win32.cc @@ -0,0 +1,116 @@ +// Copyright 2012 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "includes_normalize.h" + +#include "string_piece.h" +#include "util.h" + +#include +#include +#include + +#include + +namespace { + +/// Return true if paths a and b are on the same Windows drive. +bool SameDrive(StringPiece a, StringPiece b) { + char a_absolute[_MAX_PATH]; + char b_absolute[_MAX_PATH]; + GetFullPathName(a.AsString().c_str(), sizeof(a_absolute), a_absolute, NULL); + GetFullPathName(b.AsString().c_str(), sizeof(b_absolute), b_absolute, NULL); + char a_drive[_MAX_DIR]; + char b_drive[_MAX_DIR]; + _splitpath(a_absolute, a_drive, NULL, NULL, NULL); + _splitpath(b_absolute, b_drive, NULL, NULL, NULL); + return _stricmp(a_drive, b_drive) == 0; +} + +} // anonymous namespace + +string IncludesNormalize::Join(const vector& list, char sep) { + string ret; + for (size_t i = 0; i < list.size(); ++i) { + ret += list[i]; + if (i != list.size() - 1) + ret += sep; + } + return ret; +} + +vector IncludesNormalize::Split(const string& input, char sep) { + vector elems; + stringstream ss(input); + string item; + while (getline(ss, item, sep)) + elems.push_back(item); + return elems; +} + +string IncludesNormalize::ToLower(const string& s) { + string ret; + transform(s.begin(), s.end(), back_inserter(ret), tolower); + return ret; +} + +string IncludesNormalize::AbsPath(StringPiece s) { + char result[_MAX_PATH]; + GetFullPathName(s.AsString().c_str(), sizeof(result), result, NULL); + return result; +} + +string IncludesNormalize::Relativize(StringPiece path, const string& start) { + 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) { + if (ToLower(start_list[i]) != ToLower(path_list[i])) + break; + } + + vector rel_list; + for (int j = 0; j < static_cast(start_list.size() - i); ++j) + rel_list.push_back(".."); + for (int j = i; j < static_cast(path_list.size()); ++j) + rel_list.push_back(path_list[j]); + if (rel_list.size() == 0) + return "."; + return Join(rel_list, '\\'); +} + +string IncludesNormalize::Normalize(StringPiece input, + const char* relative_to) { + char copy[_MAX_PATH]; + size_t len = input.len_; + strncpy(copy, input.str_, input.len_ + 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.len_, input.str_, + err.c_str()); + } + 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 ToLower(Relativize(partially_fixed, relative_to)); +} diff --git a/src/includes_normalize.h b/src/includes_normalize.h new file mode 100644 index 0000000..458613a --- /dev/null +++ b/src/includes_normalize.h @@ -0,0 +1,35 @@ +// Copyright 2012 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +using namespace std; + +struct StringPiece; + +/// Utility functions for normalizing include paths on Windows. +/// TODO: this likely duplicates functionality of CanonicalizePath; refactor. +struct IncludesNormalize { + // Internal utilities made available for testing, maybe useful otherwise. + static string Join(const vector& list, char sep); + static vector Split(const string& input, char sep); + static string ToLower(const string& s); + static string AbsPath(StringPiece s); + 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. + static string Normalize(StringPiece input, const char* relative_to); +}; diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc new file mode 100644 index 0000000..97f7174 --- /dev/null +++ b/src/includes_normalize_test.cc @@ -0,0 +1,98 @@ +// Copyright 2012 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "includes_normalize.h" + +#include + +#include "test.h" +#include "util.h" + +TEST(IncludesNormalize, Simple) { + EXPECT_EQ("b", IncludesNormalize::Normalize("a\\..\\b", NULL)); + EXPECT_EQ("b", IncludesNormalize::Normalize("a\\../b", NULL)); + EXPECT_EQ("a\\b", IncludesNormalize::Normalize("a\\.\\b", NULL)); + EXPECT_EQ("a\\b", IncludesNormalize::Normalize("a\\./b", NULL)); +} + +namespace { + +string GetCurDir() { + char buf[_MAX_PATH]; + _getcwd(buf, sizeof(buf)); + vector parts = IncludesNormalize::Split(string(buf), '\\'); + return parts[parts.size() - 1]; +} + +} // namespace + +TEST(IncludesNormalize, WithRelative) { + string currentdir = IncludesNormalize::ToLower(GetCurDir()); + 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"), + IncludesNormalize::Normalize("a", "../b")); + EXPECT_EQ(string("..\\") + currentdir + string("\\a\\b"), + IncludesNormalize::Normalize("a/b", "../c")); + EXPECT_EQ("..\\..\\a", IncludesNormalize::Normalize("a", "b/c")); + EXPECT_EQ(".", IncludesNormalize::Normalize("a", "a")); +} + +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)); +} + +TEST(IncludesNormalize, Join) { + vector x; + EXPECT_EQ("", IncludesNormalize::Join(x, ':')); + x.push_back("alpha"); + EXPECT_EQ("alpha", IncludesNormalize::Join(x, ':')); + x.push_back("beta"); + x.push_back("gamma"); + EXPECT_EQ("alpha:beta:gamma", IncludesNormalize::Join(x, ':')); +} + +TEST(IncludesNormalize, Split) { + EXPECT_EQ("", IncludesNormalize::Join(IncludesNormalize::Split("", '/'), ':')); + EXPECT_EQ("a", IncludesNormalize::Join(IncludesNormalize::Split("a", '/'), ':')); + EXPECT_EQ("a:b:c", IncludesNormalize::Join(IncludesNormalize::Split("a/b/c", '/'), ':')); +} + +TEST(IncludesNormalize, ToLower) { + EXPECT_EQ("", IncludesNormalize::ToLower("")); + EXPECT_EQ("stuff", IncludesNormalize::ToLower("Stuff")); + EXPECT_EQ("stuff and things", IncludesNormalize::ToLower("Stuff AND thINGS")); + EXPECT_EQ("stuff 3and thin43gs", IncludesNormalize::ToLower("Stuff 3AND thIN43GS")); +} + +TEST(IncludesNormalize, DifferentDrive) { + EXPECT_EQ("stuff.h", + 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", + IncludesNormalize::Normalize("P:\\vs08\\stuff.h", "c:\\vs08")); + EXPECT_EQ("P:\\vs08\\stuff.h", + IncludesNormalize::Normalize("P:\\vs08\\stuff.h", "D:\\stuff/things")); + EXPECT_EQ("P:\\vs08\\stuff.h", + IncludesNormalize::Normalize("P:/vs08\\stuff.h", "D:\\stuff/things")); + // TODO: this fails; fix it. + //EXPECT_EQ("P:\\wee\\stuff.h", + // IncludesNormalize::Normalize("P:/vs08\\../wee\\stuff.h", "D:\\stuff/things")); +} -- cgit v0.12 From de3f570cfe0bfdd564913a1431dc95fd17ff44eb Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 16:27:58 -0700 Subject: includes_normalize: also lowercase cross-drive includes It seems to me inconsistent to normalize one but not the other. --- src/includes_normalize-win32.cc | 11 +++++------ src/includes_normalize.h | 2 +- src/includes_normalize_test.cc | 6 +++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index cf398ae..134cfb8 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -91,18 +91,17 @@ string IncludesNormalize::Relativize(StringPiece path, const string& start) { return Join(rel_list, '\\'); } -string IncludesNormalize::Normalize(StringPiece input, +string IncludesNormalize::Normalize(const string& input, const char* relative_to) { char copy[_MAX_PATH]; - size_t len = input.len_; - strncpy(copy, input.str_, input.len_ + 1); + 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.len_, input.str_, - err.c_str()); + Warning("couldn't canonicalize '%s: %s\n", input.c_str(), err.c_str()); } string curdir; if (!relative_to) { @@ -111,6 +110,6 @@ string IncludesNormalize::Normalize(StringPiece input, } StringPiece partially_fixed(copy, len); if (!SameDrive(partially_fixed, relative_to)) - return partially_fixed.AsString(); + return ToLower(partially_fixed.AsString()); return ToLower(Relativize(partially_fixed, relative_to)); } diff --git a/src/includes_normalize.h b/src/includes_normalize.h index 458613a..43527af 100644 --- a/src/includes_normalize.h +++ b/src/includes_normalize.h @@ -31,5 +31,5 @@ struct IncludesNormalize { /// Normalize by fixing slashes style, fixing redundant .. and . and makes the /// path relative to |relative_to|. Case is normalized to lowercase on /// Windows too. - static string Normalize(StringPiece input, const char* relative_to); + static string Normalize(const string& input, const char* relative_to); }; diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index 97f7174..77b5b3b 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -86,11 +86,11 @@ 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", -- cgit v0.12 From 449b62075c81431c58ab9fef0054d61513d9d656 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 16:29:43 -0700 Subject: msvc helper: drop system includes Drop any #includes that look like they're referencing system headers. This reduces the dependency information considerably. --- src/msvc_helper-win32.cc | 12 +++++++++++- src/msvc_helper.h | 7 +++++-- src/msvc_helper_test.cc | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index f6b2a22..624bc2d 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -17,6 +17,7 @@ #include #include +#include "includes_normalize.h" #include "util.h" namespace { @@ -46,6 +47,13 @@ string CLWrapper::FilterShowIncludes(const string& line) { } // static +bool CLWrapper::IsSystemInclude(const string& path) { + // TODO: this is a heuristic, perhaps there's a better way? + return (path.find("program files") != string::npos || + path.find("microsoft visual studio") != string::npos); +} + +// static bool CLWrapper::FilterInputFilename(const string& line) { // TODO: other extensions, like .asm? return EndsWith(line, ".c") || @@ -115,7 +123,9 @@ int CLWrapper::Run(const string& command, string* extra_output) { string include = FilterShowIncludes(line); if (!include.empty()) { - includes_.push_back(include); + include = IncludesNormalize::Normalize(include, NULL); + if (!IsSystemInclude(include)) + includes_.push_back(include); } else if (FilterInputFilename(line)) { // Drop it. // TODO: if we support compiling multiple output files in a single diff --git a/src/msvc_helper.h b/src/msvc_helper.h index 5bf9787..41b9f9b 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -16,8 +16,6 @@ #include using namespace std; -struct StringPiece; - /// Visual Studio's cl.exe requires some massaging to work with Ninja; /// for example, it emits include information on stderr in a funny /// format when building with /showIncludes. This class wraps a CL @@ -34,6 +32,11 @@ struct CLWrapper { /// Exposed for testing. static string FilterShowIncludes(const string& line); + /// Return true if a mentioned include file is a system path. + /// Expects the path to already by normalized (including lower case). + /// Filtering these out reduces dependency information considerably. + static bool IsSystemInclude(const string& path); + /// Parse a line of cl.exe output and return true if it looks like /// it's printing an input filename. This is a heuristic but it appears /// to be the best we can do. diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index a0bab90..b65d66f 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -58,3 +58,17 @@ TEST(MSVCHelperTest, RunFilenameFilter) { &output); ASSERT_EQ("cl: warning\n", output); } + +TEST(MSVCHelperTest, RunSystemInclude) { + CLWrapper cl; + string output; + cl.Run("cmd /c \"echo Note: including file: c:\\Program Files\\foo.h&&" + "echo Note: including file: d:\\Microsoft Visual Studio\\bar.h&&" + "echo Note: including file: path.h\"", + &output); + // We should have dropped the first two includes because they look like + // system headers. + ASSERT_EQ("", output); + ASSERT_EQ(1u, cl.includes_.size()); + ASSERT_EQ("path.h", cl.includes_[0]); +} -- cgit v0.12 From 356f31abb92c87b3fa32700d3b44e49f1321cdb2 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Sun, 12 Aug 2012 18:43:42 -0700 Subject: create phony rules for all binaries --- configure.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/configure.py b/configure.py index ed05031..bdf4613 100755 --- a/configure.py +++ b/configure.py @@ -97,7 +97,9 @@ def cxx(name, **kwargs): return n.build(built(name + objext), 'cxx', src(name + '.cc'), **kwargs) def binary(name): if platform in ('mingw', 'windows'): - return name + '.exe' + exe = name + '.exe' + n.build(name, 'phony', exe) + return exe return name n.variable('builddir', 'build') @@ -277,8 +279,6 @@ n.comment('Main executable is library plus main() function.') objs = cxx('ninja') ninja = n.build(binary('ninja'), 'link', objs, implicit=ninja_lib, variables=[('libs', libs)]) -if 'ninja' not in ninja: - n.build('ninja', 'phony', ninja) n.newline() all_targets += ninja @@ -334,8 +334,6 @@ if platform != 'mingw' and platform != 'windows': ninja_test = n.build(binary('ninja_test'), 'link', objs, implicit=ninja_lib, variables=[('ldflags', test_ldflags), ('libs', test_libs)]) -if 'ninja_test' not in ninja_test: - n.build('ninja_test', 'phony', ninja_test) n.newline() all_targets += ninja_test -- cgit v0.12