From f4c5eade787f4f0a6e33fe029c2816580db06041 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 10 Oct 2014 09:57:07 -0400 Subject: [PATCH] Ninja: Fix RC include directories regression Changes in commit b9aa5041 (cmLocalGenerator: Simplify GetIncludeFlags output formatting, 2014-03-04) caused Windows Resource Compiler include directories to be computed as relative paths in the Ninja generator. This breaks the cmcldeps handling of include paths. The reason for the regression is that several cmLocalGenerator::GetIncludeFlags callers treated the fourth "bool forResponseFile" argument as if it controlled whether include directories were a full path. It actually did control that by accident until the above commit. Add an explicit "bool forceFullPaths" argument to GetIncludeFlags and thread the value through ConvertToIncludeReference as needed. Update GetIncludeFlags call sites that really wanted to control the forResponseFile setting to be aware of the new argument. Extend the VSResource test to cover this case. --- Source/cmLocalGenerator.cxx | 9 ++++++--- Source/cmLocalGenerator.h | 4 +++- Source/cmLocalNinjaGenerator.cxx | 5 +++-- Source/cmLocalNinjaGenerator.h | 3 ++- Source/cmMakefileTargetGenerator.cxx | 2 +- Tests/VSResource/CMakeLists.txt | 14 +++++++++++++- Tests/VSResource/include.rc.in | 1 + Tests/VSResource/test.rc | 3 +++ 8 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 Tests/VSResource/include.rc.in diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx index 4bd9191f0..50e279b48 100644 --- a/Source/cmLocalGenerator.cxx +++ b/Source/cmLocalGenerator.cxx @@ -1287,9 +1287,11 @@ cmLocalGenerator::ConvertToOutputForExisting(RelativeRoot remote, //---------------------------------------------------------------------------- std::string cmLocalGenerator::ConvertToIncludeReference(std::string const& path, - OutputFormat format) + OutputFormat format, + bool forceFullPaths) { - return this->ConvertToOutputForExisting(path, START_OUTPUT, format); + return this->ConvertToOutputForExisting( + path, forceFullPaths? FULL : START_OUTPUT, format); } //---------------------------------------------------------------------------- @@ -1297,6 +1299,7 @@ std::string cmLocalGenerator::GetIncludeFlags( const std::vector &includes, cmGeneratorTarget* target, const std::string& lang, + bool forceFullPaths, bool forResponseFile, const std::string& config) { @@ -1401,7 +1404,7 @@ std::string cmLocalGenerator::GetIncludeFlags( flagUsed = true; } std::string includePath = - this->ConvertToIncludeReference(*i, shellFormat); + this->ConvertToIncludeReference(*i, shellFormat, forceFullPaths); if(quotePaths && includePath.size() && includePath[0] != '\"') { includeFlags << "\""; diff --git a/Source/cmLocalGenerator.h b/Source/cmLocalGenerator.h index b25b9abb6..3a9d5be80 100644 --- a/Source/cmLocalGenerator.h +++ b/Source/cmLocalGenerator.h @@ -160,6 +160,7 @@ public: std::string GetIncludeFlags(const std::vector &includes, cmGeneratorTarget* target, const std::string& lang, + bool forceFullPaths = false, bool forResponseFile = false, const std::string& config = ""); @@ -215,7 +216,8 @@ public: OutputFormat format = SHELL); virtual std::string ConvertToIncludeReference(std::string const& path, - OutputFormat format = SHELL); + OutputFormat format = SHELL, + bool forceFullPaths = false); /** Called from command-line hook to clear dependencies. */ virtual void ClearDependencies(cmMakefile* /* mf */, diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index 5522e0d28..398b55a45 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -151,9 +151,10 @@ cmLocalNinjaGenerator::ConvertToLinkReference(std::string const& lib, std::string cmLocalNinjaGenerator::ConvertToIncludeReference(std::string const& path, - OutputFormat format) + OutputFormat format, + bool forceFullPaths) { - return this->Convert(path, HOME_OUTPUT, format); + return this->Convert(path, forceFullPaths? FULL : HOME_OUTPUT, format); } //---------------------------------------------------------------------------- diff --git a/Source/cmLocalNinjaGenerator.h b/Source/cmLocalNinjaGenerator.h index c787ac6a2..1d27224f5 100644 --- a/Source/cmLocalNinjaGenerator.h +++ b/Source/cmLocalNinjaGenerator.h @@ -108,7 +108,8 @@ public: protected: virtual std::string ConvertToIncludeReference(std::string const& path, - OutputFormat format = SHELL); + OutputFormat format = SHELL, + bool forceFullPaths = false); private: diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx index 7849d1282..1f8f68678 100644 --- a/Source/cmMakefileTargetGenerator.cxx +++ b/Source/cmMakefileTargetGenerator.cxx @@ -1962,7 +1962,7 @@ void cmMakefileTargetGenerator::AddIncludeFlags(std::string& flags, std::string includeFlags = this->LocalGenerator->GetIncludeFlags(includes, this->GeneratorTarget, - lang, useResponseFile); + lang, false, useResponseFile); if(includeFlags.empty()) { return; diff --git a/Tests/VSResource/CMakeLists.txt b/Tests/VSResource/CMakeLists.txt index c5cb336ae..17eb04136 100644 --- a/Tests/VSResource/CMakeLists.txt +++ b/Tests/VSResource/CMakeLists.txt @@ -18,6 +18,11 @@ if(CMAKE_RC_COMPILER MATCHES windres) message(STATUS "CMAKE_RC_COMPILER MATCHES windres") add_definitions(/DCMAKE_RCDEFINE=test.txt) add_definitions(/DCMAKE_RCDEFINE_NO_QUOTED_STRINGS) + if(MSYS AND CMAKE_CURRENT_BINARY_DIR MATCHES " ") + # windres cannot handle spaces in include dir, and + # for the MSys shell we do not convert to shortpath. + set(CMAKE_RC_NO_INCLUDE 1) + endif() elseif(MSVC60) # VS6 rc compiler does not deal well with spaces in a "/D" value, but it can # handle the quoting @@ -30,10 +35,17 @@ else() set(TEXTFILE_FROM_SOURCE_DIR "textfile, spaces in name, from binary dir") configure_file(${CMAKE_CURRENT_SOURCE_DIR}/test.txt "${CMAKE_CURRENT_BINARY_DIR}/test with spaces.txt" @ONLY) - include_directories(${CMAKE_CURRENT_BINARY_DIR}) add_definitions(/DCMAKE_RCDEFINE="test with spaces.txt") endif() +if(CMAKE_RC_NO_INCLUDE) + add_definitions(/DCMAKE_RC_NO_INCLUDE) +else() + configure_file(${CMAKE_CURRENT_SOURCE_DIR}/include.rc.in + "${CMAKE_CURRENT_BINARY_DIR}/include.rc" @ONLY) + include_directories(${CMAKE_CURRENT_BINARY_DIR}) +endif() + add_executable(VSResource main.cpp test.rc) set_property(TARGET VSResource diff --git a/Tests/VSResource/include.rc.in b/Tests/VSResource/include.rc.in new file mode 100644 index 000000000..f0f685980 --- /dev/null +++ b/Tests/VSResource/include.rc.in @@ -0,0 +1 @@ +// This file should be included. diff --git a/Tests/VSResource/test.rc b/Tests/VSResource/test.rc index 4ce4b5312..0de468386 100644 --- a/Tests/VSResource/test.rc +++ b/Tests/VSResource/test.rc @@ -1,4 +1,7 @@ #ifdef CMAKE_RCDEFINE +# ifndef CMAKE_RC_NO_INCLUDE +# include +# endif // This line can compile with either an unquoted or a quoted string 1025 TEXTFILE CMAKE_RCDEFINE