MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0015642CMake(No Category)public2015-07-08 10:132016-02-01 09:10
ReporterMartin Sander 
Assigned ToBrad King 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSWindows 8OS Version
Product Version 
Target VersionCMake 3.4Fixed in VersionCMake 3.4 
Summary0015642: Source file properties lost if case of drive letter changes
DescriptionIf cmake is used within cmd.exe and a dependency in add_custom_command(...) has an upper case drive letter and this files occurs otherwise with a lower case drive letter, all <ClCompile Include> tags refererring to the containing directory are empty.

The difference is produced with Archive.zip:cmakeBugHunting/build/ttt.bat and visible in Archive.zip:diff.png
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
related to 0015366closed Brad King set_source_files_properties OBJECT_DEPENDS broken after normalization 
Attached Fileszip Archive.zip (193,140) 2015-07-08 10:13
https://public.kitware.com/Bug/file/5485/Archive.zip
Issue History
Date ModifiedUsernameFieldChange
2015-07-08 10:13Martin SanderNew Issue
2015-07-08 10:13Martin SanderFile Added: Archive.zip
2015-07-08 11:00Brad KingNote Added: 0039091
2015-07-08 11:22Martin SanderNote Added: 0039092
2015-07-08 11:34Martin SanderNote Added: 0039093
2015-07-08 11:47Brad KingNote Added: 0039095
2015-07-08 11:50Martin SanderNote Added: 0039096
2015-07-08 18:44Martin SanderNote Added: 0039099
2015-07-08 18:44Martin SanderNote Edited: 0039099bug_revision_view_page.php?bugnote_id=39099#r1839
2015-07-09 06:39Martin SanderNote Added: 0039100
2015-07-09 06:41Martin SanderNote Added: 0039101
2015-07-09 08:46Martin SanderNote Added: 0039103
2015-07-09 10:28Brad KingNote Added: 0039105
2015-07-09 10:29Brad KingNote Added: 0039106
2015-07-09 10:30Brad KingRelationship addedrelated to 0015366
2015-07-09 10:32Brad KingNote Added: 0039107
2015-07-09 14:22Brad KingNote Added: 0039111
2015-07-09 14:50Brad KingNote Added: 0039115
2015-07-09 16:12Brad KingNote Added: 0039118
2015-07-09 16:12Brad KingNote Edited: 0039118bug_revision_view_page.php?bugnote_id=39118#r1843
2015-07-10 09:49Brad KingNote Added: 0039122
2015-07-13 09:26Brad KingNote Added: 0039132
2015-07-13 09:27Brad KingAssigned To => Brad King
2015-07-13 09:27Brad KingStatusnew => resolved
2015-07-13 09:27Brad KingResolutionopen => fixed
2015-07-13 09:27Brad KingFixed in Version => CMake 3.4
2015-07-13 09:27Brad KingTarget Version => CMake 3.4
2015-07-13 09:27Brad KingSummary<ClCompile Include> empty if case of drive letter changes => Source file properties lost if case of drive letter changes
2016-02-01 09:10Robert MaynardNote Added: 0040399
2016-02-01 09:10Robert MaynardStatusresolved => closed

Notes
(0039091)
Brad King   
2015-07-08 11:00   
Strange. Thanks for the detailed report and script. However, I cannot reproduce the issue. I made minimal modifications to the sample to fix paths to reference my system. Then I ran ttt.bat and got wgui_*.vcxproj files that are all identical and have the expected content.

I don't have a "D:" mounted (as shown in your diff.png) so I had to run everything under "C:". Can you reproduce it under a single drive letter?

For reference, many paths within CMake on Windows are sent through GetActualCaseForPath:

 http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Source/kwsys/SystemTools.cxx;hb=v3.3.0-rc1#l3742 [^]

which normalizes the drive letter to uppercase and takes the case preserved/recorded on the filesystem for the rest of the path. Perhaps there is some code path in the dependency analysis that misses this.
(0039092)
Martin Sander   
2015-07-08 11:22   
colleague tried D:\Archive\cmakeBugHunting as base
I tried D:\cmakeBugHunting and C:\DEV\sparbuch\main\cmakeBugHunting

tryind different cmake versions now
(0039093)
Martin Sander   
2015-07-08 11:34   
cmake-2.8.12.2-win32-x86 "Visual Studio 10" OK
cmake-3.0.0-rc1-win32-x86 "Visual Studio 12 2013" OK
cmake-3.0.2-win32-x86 "Visual Studio 10 2010" OK

cmake-3.1.0-rc1-win32-x86 "Visual Studio 12 2013" fail
cmake-3.1.3-win32-x86 "Visual Studio 12 2013" fail
cmake-3.3.0-rc3-win32-x86 "Visual Studio 10 2010" fail
cmake-3.3.0-rc3-win32-x86 "Visual Studio 12 2013" fail
(0039095)
Brad King   
2015-07-08 11:47   
Re 0015642:0039093: Thanks. Would you be able to build from source and "git bisect" to find the culprit?
(0039096)
Martin Sander   
2015-07-08 11:50   
Yes, thanks. I'll try tomorrow. Kids are waiting...
(0039099)
Martin Sander   
2015-07-08 18:44   
Uh, that will take some hours... bisect started with 11 steps, now down to 7

After midnight here now, I'll continue tomorrow as promised.
(0039100)
Martin Sander   
2015-07-09 06:39   
git bisect:
c4af46b4443374f0a0a64bb7db87750454cc3dac

-------------------------------------------------------------------------------
std::vector<std::string> const& cmCustomCommandGenerator::GetDepends() const
{
  if (!this->DependsDone)
    {
    this->DependsDone = true;
    std::vector<std::string> depends = this->CC.GetDepends();
    for(std::vector<std::string>::const_iterator
          i = depends.begin();
        i != depends.end(); ++i)
      {
      cmsys::auto_ptr<cmCompiledGeneratorExpression> cge
                                              = this->GE->Parse(*i);
      std::vector<std::string> result;
      cmSystemTools::ExpandListArgument(
                  cge->Evaluate(this->Makefile, this->Config), result);
      for (std::vector<std::string>::iterator it = result.begin();
          it != result.end(); ++it)
        {
        if (cmSystemTools::FileIsFullPath(it->c_str()))
          {
          *it = cmSystemTools::CollapseFullPath(*it); // <----------------- without this line it works
          }
        }
      this->Depends.insert(this->Depends.end(), result.begin(), result.end());
      }
    }
  return this->Depends;
}
-------------------------------------------------------------------------------
(0039101)
Martin Sander   
2015-07-09 06:41   
we tried 5 different machines, on all the error was reproducible

Each were German Windows 8.1 64bit (mine were 8.1 Pro and Enterprise, each fully patched)
(0039103)
Martin Sander   
2015-07-09 08:46   
Also reproducible in a vmware with an English Windows 8.1:

en_windows_8.1_with_update_x64_dvd_6051480.iso
en_visual_studio_professional_2013_with_update_4_x86_dvd_5935322.iso
npp.6.7.9.2.Installer.exe
cmake-3.3.0-rc3-win32-x86.exe
cygwin (for grep)
(0039105)
Brad King   
2015-07-09 10:28   
Thanks for bisecting! For reference, the commit referenced in 0015642:0039100 is:

 add_custom_command: Normalize OUTPUT and DEPENDS paths.
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=c4af46b4 [^]
(0039106)
Brad King   
2015-07-09 10:29   
For reference, here is a fix to another issue created by the same commit:

 Normalize OBJECT_DEPENDS paths to match custom commands
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9259d778 [^]
(0039107)
Brad King   
2015-07-09 10:32   
Re 0015642:0039100: The new CollapseFullPath call refers to this method:

 http://www.cmake.org/gitweb?p=cmake.git;a=blob;f=Source/kwsys/SystemTools.cxx;hb=v3.3.0-rc3#l3464 [^]

which ends in a call to GetActualCaseForPath as linked in 0015642:0039091.
(0039111)
Brad King   
2015-07-09 14:22   
I've reproduced this and narrowed it down to this example:

cmake_minimum_required(VERSION 3.2)
project(Issue15642 C)
string(SUBSTRING ${CMAKE_CURRENT_SOURCE_DIR} 0  1 DRIVE)
string(SUBSTRING ${CMAKE_CURRENT_SOURCE_DIR} 2 -1 src)
string(TOLOWER "${DRIVE}" d)
string(TOUPPER "${DRIVE}" D)

add_custom_target(custom DEPENDS ${D}:${src}/sub/src.txt)

add_custom_command(
  OUTPUT  out.txt
  COMMAND ${CMAKE_COMMAND} -E touch out.txt
  )

add_executable(main
  ${d}:${src}/sub/src.c
  ${d}:${src}/sub/src.txt
  out.txt
  )

set_property(SOURCE ${d}:${src}/sub/src.c PROPERTY COMPILE_DEFINITIONS SRC)


Both sources have to be in a sub/ directory. The upper-case dependency has to be in a separate target. When this is added to the test suite later we can make "src.c" not compile unless "SRC" is defined as a way to verify the behavior.
(0039115)
Brad King   
2015-07-09 14:50   
Another related change:

 cmSourceFileLocation: Collapse full path for directory comparisons.
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=4959f341 [^]
(0039118)
Brad King   
2015-07-09 16:12   
The GetActualCaseForPath implementation linked in 0015642:0039091 is buggy. The drive letter case conversion is not done on the output value! It is only done on the memoization map key. In the case discussed here this leads to two cmSourceFile instances for the same source file, each with a different case for the drive letter. The source file property gets set on one of them but the other one ends up getting used for generation. I have a fix locally and will report more after further testing.
(0039122)
Brad King   
2015-07-10 09:49   
Re 0015642:0039118: I've submitted a fix to upstream KWSys for GetActualCaseForPath drive letter case handling here:

 http://review.source.kitware.com/19985 [^]
(0039132)
Brad King   
2015-07-13 09:26   
The change from 0015642:0039122 has been merged to upstream KWSys. I've updated CMake's copy of KWSys to include the change here:

 KWSys 2015-07-10 (c9336bcf)
 http://cmake.org/gitweb?p=cmake.git;a=commit;h=dc822da8 [^]
(0040399)
Robert Maynard   
2016-02-01 09:10   
Closing resolved issues that have not been updated in more than 4 months.