View Issue Details [ Jump to Notes ] | [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0015642 | CMake | (No Category) | public | 2015-07-08 10:13 | 2016-02-01 09:10 | ||||
Reporter | Martin Sander | ||||||||
Assigned To | Brad King | ||||||||
Priority | normal | Severity | major | Reproducibility | always | ||||
Status | closed | Resolution | fixed | ||||||
Platform | OS | Windows 8 | OS Version | ||||||
Product Version | |||||||||
Target Version | CMake 3.4 | Fixed in Version | CMake 3.4 | ||||||
Summary | 0015642: Source file properties lost if case of drive letter changes | ||||||||
Description | If 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 | ||||||||
Tags | No tags attached. | ||||||||
Attached Files | Archive.zip [^] (193,140 bytes) 2015-07-08 10:13 | ||||||||
Relationships | ||||||
|
Relationships |
Notes | |
(0039091) Brad King (manager) 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 (reporter) 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 (reporter) 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 (manager) 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 (reporter) 2015-07-08 11:50 |
Yes, thanks. I'll try tomorrow. Kids are waiting... |
(0039099) Martin Sander (reporter) 2015-07-08 18:44 edited on: 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 (reporter) 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 (reporter) 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 (reporter) 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 (manager) 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 (manager) 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 (manager) 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 (manager) 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 (manager) 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 (manager) 2015-07-09 16:12 edited on: 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 (manager) 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 (manager) 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 (manager) 2016-02-01 09:10 |
Closing resolved issues that have not been updated in more than 4 months. |
Notes |
Issue History | |||
Date Modified | Username | Field | Change |
2015-07-08 10:13 | Martin Sander | New Issue | |
2015-07-08 10:13 | Martin Sander | File Added: Archive.zip | |
2015-07-08 11:00 | Brad King | Note Added: 0039091 | |
2015-07-08 11:22 | Martin Sander | Note Added: 0039092 | |
2015-07-08 11:34 | Martin Sander | Note Added: 0039093 | |
2015-07-08 11:47 | Brad King | Note Added: 0039095 | |
2015-07-08 11:50 | Martin Sander | Note Added: 0039096 | |
2015-07-08 18:44 | Martin Sander | Note Added: 0039099 | |
2015-07-08 18:44 | Martin Sander | Note Edited: 0039099 | |
2015-07-09 06:39 | Martin Sander | Note Added: 0039100 | |
2015-07-09 06:41 | Martin Sander | Note Added: 0039101 | |
2015-07-09 08:46 | Martin Sander | Note Added: 0039103 | |
2015-07-09 10:28 | Brad King | Note Added: 0039105 | |
2015-07-09 10:29 | Brad King | Note Added: 0039106 | |
2015-07-09 10:30 | Brad King | Relationship added | related to 0015366 |
2015-07-09 10:32 | Brad King | Note Added: 0039107 | |
2015-07-09 14:22 | Brad King | Note Added: 0039111 | |
2015-07-09 14:50 | Brad King | Note Added: 0039115 | |
2015-07-09 16:12 | Brad King | Note Added: 0039118 | |
2015-07-09 16:12 | Brad King | Note Edited: 0039118 | |
2015-07-10 09:49 | Brad King | Note Added: 0039122 | |
2015-07-13 09:26 | Brad King | Note Added: 0039132 | |
2015-07-13 09:27 | Brad King | Assigned To | => Brad King |
2015-07-13 09:27 | Brad King | Status | new => resolved |
2015-07-13 09:27 | Brad King | Resolution | open => fixed |
2015-07-13 09:27 | Brad King | Fixed in Version | => CMake 3.4 |
2015-07-13 09:27 | Brad King | Target Version | => CMake 3.4 |
2015-07-13 09:27 | Brad King | Summary | <ClCompile Include> empty if case of drive letter changes => Source file properties lost if case of drive letter changes |
2016-02-01 09:10 | Robert Maynard | Note Added: 0040399 | |
2016-02-01 09:10 | Robert Maynard | Status | resolved => closed |
Issue History |
Copyright © 2000 - 2018 MantisBT Team |