MantisBT - CMake
View Issue Details
0011662CMakeCMakepublic2011-01-06 10:512011-05-02 14:45
kentwilliams 
Brad King 
normalminoralways
closedsuspended 
MacProOS X10.6
CMake 2.8.3 
 
0011662: tab characters in paths causes errors in initial configure step
I was testing a problem with building FFTW as an external project in ITK -- specifically that FFTW can't configure if the source or build directories have whitespace characters in the directory path.

I created a build directory with space characters in it, and CMake had no problem with configuration, and I verified my test for paths w/whitespace characters worked properly.

I then created a build directory with tab characters in it. This seemed to break the initial CMake configure run, in the CMakeBackwardCompatibilityC module.
#!/bin/sh
# This script will produce the behavior I observed
mkdir -p cmake_tabs_bug/cmake_tabs_bug
cd cmake_tabs_bug/cmake_tabs_bug
cat > CMakeLists.txt <<EOF
project(cmake_tabs_bug)
cmake_minimum_required(VERSION 2.8.2)
include(CMakeBackwardCompatibilityC)
include(CMakeBackwardCompatibilityCXX)
EOF
cd ..
# this path should be 'build\ttabs'
mkdir 'build tabs'
cd 'build tabs'
cmake ../cmake_tabs_bug
No tags attached.
Issue History
2011-01-06 10:51kentwilliamsNew Issue
2011-01-06 11:14Brad KingNote Added: 0024459
2011-01-06 11:19Brad KingNote Added: 0024460
2011-01-06 11:23Brad KingNote Added: 0024461
2011-01-06 11:25Brad KingNote Added: 0024462
2011-01-06 11:25Brad KingStatusnew => resolved
2011-01-06 11:25Brad KingResolutionopen => suspended
2011-01-06 11:25Brad KingAssigned To => Brad King
2011-05-02 14:45David ColeNote Added: 0026314
2011-05-02 14:45David ColeStatusresolved => closed

Notes
(0024459)
Brad King   
2011-01-06 11:14   
Small CMakeLists.txt that fails:

  cmake_minimum_required(VERSION 2.8.2)
  project(FOO C)
  include(CheckTypeSize)
  check_type_size(int SIZEOF_INT)

CMake reports no error, but the check_type_size incorrectly fails.
(0024460)
Brad King   
2011-01-06 11:19   
Here is a patch that fixes the test case mentioned in 0011662:0024459:

diff --git a/Source/kwsys/SystemTools.cxx b/Source/kwsys/SystemTools.cxx
index cef2de6..fc8aac9 100644
--- a/Source/kwsys/SystemTools.cxx
+++ b/Source/kwsys/SystemTools.cxx
@@ -1647,14 +1647,14 @@ kwsys_stl::string SystemTools::ConvertToUnixOutputPath(const char* path)
     ret.erase(pos, 1);
     }
   // escape spaces and () in the path
- if(ret.find_first_of(" ") != kwsys_stl::string::npos)
+ if(ret.find_first_of(" \t") != kwsys_stl::string::npos)
     {
     kwsys_stl::string result = "";
     char lastch = 1;
     for(const char* ch = ret.c_str(); *ch != '\0'; ++ch)
       {
         // if it is already escaped then don't try to escape it again
- if((*ch == ' ') && lastch != '\\')
+ if((*ch == ' ' || *ch == '\t') && lastch != '\\')
         {
         result += '\\';
         }
(0024461)
Brad King   
2011-01-06 11:23   
The code path that leads to SystemTools::ConvertToUnixOutputPath for Makefile generation is:

  cmLocalUnixMakefileGenerator3::WriteMakeRule
  cmLocalGenerator::Convert
  cmLocalGenerator::ConvertToOutputFormat (output == MAKEFILE)
  cmSystemTools::ConvertToOutputPath => SystemTools::ConvertToOutputPath
  kwsys_stl::string SystemTools::ConvertToOutputPath(const char* path)
  {
  #if defined(_WIN32) && !defined(__CYGWIN__)
    return SystemTools::ConvertToWindowsOutputPath(path);
  #else
    return SystemTools::ConvertToUnixOutputPath(path);
  #endif
  }

The meaning of "UnixOutputPath" is a historical misnomer. It really means "make dependency path".

The patch in 0011662:0024460 is a hack. Really the above code path needs to be updated to not use the ancient SystemTools method (which has bad code like guessing whether to escape something based on the context). The replacement needs to know how to escape all kinds of charaters in Makefile dependencies, not just space and TAB.
(0024462)
Brad King   
2011-01-06 11:25   
This seems like a big effort for an extremely obscure use case that no one ever uses in practice. I'm going to suspend this issue for now. If anyone wants to submit a patch for discussion we can consider it.
(0026314)
David Cole   
2011-05-02 14:45   
Closing resolved issues that have not been updated in more than 3 months.