MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0014754CMakeCMakepublic2014-02-13 06:432016-06-10 14:31
ReporterAndreas Pakulat 
Assigned ToKitware Robot 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionmoved 
PlatformOSWindowsOS Versionall
Product Version 
Target VersionFixed in Version 
Summary0014754: fixing up bundles fails in short windows path names
DescriptionI've set CPACK_PACKAGE_DIRECTORY to a temporary directory to avoid the dreaded maximum path length on Windows. The API used to get the temp dir yields a short path name and I have no influence on that. That is, the path is something like C:/Users/mylong~1/AppData/Local/Temp/packagingDir instead of the long path C:/Users/mylongusername/AppData/Local/Temp/packagingDir.

Since verify_app generates an absolute path to the dependencies of an executable using CMake API which yields a long path the two paths passed to gp_resolved_file_type are different when compared using STREQUAL. This lets verify_app think the bundling was not successful which in turn aborts the build.

I could fix this with the attached path which converts both the original_file and the resolved_file parameters in gp_resolved_file_type to 'real paths' before further using them. This converts the short pathname passed in through CPACK_PACKAGE_DIRECTORY to a long one.
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
Attached Filesdiff fix_gp_resolved_file_type_long_vs_short_path.diff (709) 2014-02-13 06:43
https://public.kitware.com/Bug/file/5072/fix_gp_resolved_file_type_long_vs_short_path.diff
Issue History
Date ModifiedUsernameFieldChange
2014-02-13 06:43Andreas PakulatNew Issue
2014-02-13 06:43Andreas PakulatFile Added: fix_gp_resolved_file_type_long_vs_short_path.diff
2014-02-13 09:22Clinton StimpsonNote Added: 0035099
2014-02-13 09:31Brad KingNote Added: 0035100
2014-02-13 13:42Andreas PakulatNote Added: 0035103
2014-02-17 09:38Brad KingNote Added: 0035126
2014-02-17 10:59Andreas PakulatNote Added: 0035130
2014-02-17 11:05Brad KingNote Added: 0035132
2016-06-10 14:29Kitware RobotNote Added: 0042486
2016-06-10 14:29Kitware RobotStatusnew => resolved
2016-06-10 14:29Kitware RobotResolutionopen => moved
2016-06-10 14:29Kitware RobotAssigned To => Kitware Robot
2016-06-10 14:31Kitware RobotStatusresolved => closed

Notes
(0035099)
Clinton Stimpson   
2014-02-13 09:22   
I'm curious if you can convert the path to a long path before passing it in to cmake?
Is it a CMake API giving you a short path name? If so, that should be fixed.

I can imagine there would be many other places to add this kind of a patch to have cmake support mixed use of 8.3 and normal file names.

I question the value of supporting short file names that were used on no longer supported Windows versions, and making all CMake users on modern Windows versions pay for that.
(0035100)
Brad King   
2014-02-13 09:31   
Re 0014754:0035099: FWIW, CMake has never supported 8.3-only filesystems. We use shortpath not for compatibility with old Windows/DOS versions, but to get past limitations of some tools that do not do standard command-line parsing and cannot handle quoted paths with spaces. Its use in such contexts may have leaked out into unnecessary places though.
(0035103)
Andreas Pakulat   
2014-02-13 13:42   
Fixing the caller site requires calling native Windows API in a language that has no bindings for that by default. I'm doing that at the moment as a more local workaround than patching cmake on multiple machines, but its something I'd like to drop at some point in the future. (for the curious, I'm using a python script and tempfile.gettempdir() returns the path mentioned).

My patch was just a as-local-as-possible fix. You're right that any other places relying on two paths obtained from differing sources (one possibly being path provided by the user) being the same string would need fixing.

Maybe the proper fix for at least my use-case would be to let cpack fixup the path passed in via CPACK_PACKAGE_DIRECTORY to be a long pathname if CMake is supposed to use long pathnames everywhere. That would eliminate the problem too, since thats where the short-path comes into the system. I didn't further debug how the other path is assembled.

I'm also wondering wether GetPrerequisites would be better off when it had a 'ISSAMEFILE' comparison in if(), then the comparison would succeed as well since both paths do point to the same directory.
(0035126)
Brad King   
2014-02-17 09:38   
I'm not particularly familiar with GetPrerequisites but it looks like there is logic that checks IS_ABSOLUTE on resolved_file. I'm also concerned that the path transforms may have other effects. To be as local as possible shouldn't the transformation be done only as an implementation detail of the path comparison?
(0035130)
Andreas Pakulat   
2014-02-17 10:59   
I don't see what the IS_ABSOLUTE checks have to do with this. The problem is that one of the paths (can't recall which one, but I think original_file) starts with something like C:\Users\mylong~\AppData\Local\Temp\packagingDir\. That is a proper absolute path on Windows, but if the other starts as C:\Users\mylongusername\AppData\Local\Temp\packagingDir\ then the check will fail even if both paths point to the same file.

Doing the conversion further down in the code, closer to the actual string comparison would only work on Windows since the code compares lower-case versions of the strings. Using that with REALPATH would fail if the actual path is not all-lowercase on case-sensitive filesystems.

In fact there's another flaw in the logics, it assumes that the original paths and their lower-case version are pointing to the same file if they are the same string. Thats not necessarily the case on case sensitive filesystems.
(0035132)
Brad King   
2014-02-17 11:05   
Re 0014754:0035130: If a path wasn't absolute before then REALPATH may make it so. That will change the following logic when IS_ABSOLUTE is reached.
(0042486)
Kitware Robot   
2016-06-10 14:29   
Resolving issue as `moved`.

This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.