Re: [cmake-developers] CPack/NSIS is broken after extended length paths fix

2015-09-21 Thread Clinton Stimpson 
On Monday, September 21, 2015 12:28:37 PM Dmitry Kochkin wrote:
>  Hi Clint,
> 
> >>Actually it's even worse because in current master HEAD I can see that
> >>INST_DIR is empty in generated project: !define INST_DIR ""
> >>(not sure if that's caused by your change)
> >
> >Interesting... Do you know why this happens?  How is this problem related
> >to extended paths (with the \\?\ prefix)?  Do you have an example to
> >demonstrate this problem? That was my mistake, please ignore.>
> >>I've figured out that NSIS was not going to fix this (see 
> >>http://sourceforge.net/p/nsis/feature-requests/241/ )>
> >Maybe they'll accept a patch to fix it there?  Was there any effort to fix
> >it there?
> In the URL I've put they claim you should just use "\\?\" in your NSIS
> project. I guess it's also possible to fix NSIS, but we cannot rely that
> user have latest NSIS.

Using "\\?\" in a NSIS project is a workaround, not a fix.
If a workaround is put into CMake, then you are relying on the latest CMake, 
right?


> >>I've made a small patch to NSIS generator and template to put infamous
> >>\\?\ there.
> >>
> >>I can send directly to you as I guess you know more about these parts of
> >>code.>
> >You can send your patch to this mailing list for review.
> 
> Patch attached. It's a bit hacky, more fixing symptoms I guess. The purpose
> is only to demonstrate the idea. Let me know if it can be done in better
> way.

Yes, it fixes symptoms.  The correct and robust way to fix this is to fix NSIS.

I have concerns with this patch:

1. Are the paths cleaned before placed into the NSIS template?  Only clean 
paths can be prepended with \\?\.  For example, these paths may not contain 
"." or "../" in them.

2. This patch can break UNC paths.  UNC paths must be prepended with a "\\?
\UNC\" prefix instead of "\\?\".  If CPACK_TEMPORARY_DIRECTORY is a UNC path, 
this patch breaks it.

3. For the uninstall code put into the NSIS template, how do you know whether 
INSTDIR is a UNC path or not?  INSTDIR is determined by the end-user when 
installing the software.  Blindly prepending "\\?\" may not work all the time.

Clint
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] CPack/NSIS is broken after extended length paths fix

2015-09-21 Thread Dmitry Kochkin 
 Hi Clint,

>>Actually it's even worse because in current master HEAD I can see that 
>>INST_DIR is empty in generated project:
>>  !define INST_DIR ""
>>(not sure if that's caused by your change)
>Interesting... Do you know why this happens?  How is this problem related to 
>extended paths (with the \\?\ prefix)?  Do you have an example to demonstrate 
>this problem? That was my mistake, please ignore.
>>I've figured out that NSIS was not going to fix this (see  
>>http://sourceforge.net/p/nsis/feature-requests/241/ )
>
>Maybe they'll accept a patch to fix it there?  Was there any effort to fix it 
>there?
In the URL I've put they claim you should just use "\\?\" in your NSIS project.
I guess it's also possible to fix NSIS, but we cannot rely that user have 
latest NSIS.

>
>>
>>I've made a small patch to NSIS generator and template to put infamous \\?\ 
>>there.
>>
>>I can send directly to you as I guess you know more about these parts of code.
>
>You can send your patch to this mailing list for review.
Patch attached. It's a bit hacky, more fixing symptoms I guess. The purpose is 
only to demonstrate the idea.
Let me know if it can be done in better way.

Regards,
Dmitry


From a14fc375593f2d150dba8f93d34f04b954c9365e Mon Sep 17 00:00:00 2001
From: Dmitry Kochkin 
Date: Mon, 21 Sep 2015 11:23:19 +0200
Subject: [PATCH] Fixing NSIS packer for very long filenames on Windows

---
 Modules/NSIS.template.in  |  4 ++--
 Source/CPack/cmCPackNSISGenerator.cxx | 10 --
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/Modules/NSIS.template.in b/Modules/NSIS.template.in
index 76310af..0fe19fe 100644
--- a/Modules/NSIS.template.in
+++ b/Modules/NSIS.template.in
@@ -5,7 +5,7 @@
 
   !define VERSION "@CPACK_PACKAGE_VERSION@"
   !define PATCH  "@CPACK_PACKAGE_VERSION_PATCH@"
-  !define INST_DIR "@CPACK_TEMPORARY_DIRECTORY@"
+  !define INST_DIR "@CPACK_TEMPORARY_DIRECTORY_SAFE@"
 
 ;
 ;Variables
@@ -639,7 +639,7 @@ FunctionEnd
 Section "-Core installation"
   ;Use the entire tree produced by the INSTALL target.  Keep the
   ;list of directories here in sync with the RMDir commands below.
-  SetOutPath "$INSTDIR"
+  SetOutPath "\\?\$INSTDIR"
   @CPACK_NSIS_EXTRA_PREINSTALL_COMMANDS@
   @CPACK_NSIS_FULL_INSTALL@
 
diff --git a/Source/CPack/cmCPackNSISGenerator.cxx b/Source/CPack/cmCPackNSISGenerator.cxx
index 6cdda28..6676d3a 100644
--- a/Source/CPack/cmCPackNSISGenerator.cxx
+++ b/Source/CPack/cmCPackNSISGenerator.cxx
@@ -46,6 +46,12 @@ cmCPackNSISGenerator::~cmCPackNSISGenerator()
 //--
 int cmCPackNSISGenerator::PackageFiles()
 {
+  std::string win_safe_tmp( this->GetOption( "CPACK_TEMPORARY_DIRECTORY" ) );
+#ifdef _WIN32
+  cmSystemTools::ReplaceString(win_safe_tmp, "/", "\\");
+  win_safe_tmp = "?\\" + win_safe_tmp;
+#endif
+  this->SetOptionIfNotSet( "CPACK_TEMPORARY_DIRECTORY_SAFE", win_safe_tmp.c_str( ) );
   // TODO: Fix nsis to force out file name
 
   std::string nsisInFileName = this->FindTemplate("NSIS.template.in");
@@ -83,7 +89,7 @@ int cmCPackNSISGenerator::PackageFiles()
   fileN = fileN.substr(fileN.find('/')+1, std::string::npos);
   }
 cmSystemTools::ReplaceString(fileN, "/", "\\");
-str << "  Delete \"$INSTDIR\\" << fileN << "\"" << std::endl;
+str << "  Delete \"?\\$INSTDIR\\" << fileN << "\"" << std::endl;
 }
   cmCPackLogger(cmCPackLog::LOG_DEBUG, "Uninstall Files: "
 << str.str() << std::endl);
@@ -117,7 +123,7 @@ int cmCPackNSISGenerator::PackageFiles()
 }
   }
 cmSystemTools::ReplaceString(fileN, "/", "\\");
-dstr << "  RMDir \"$INSTDIR\\" << fileN << "\"" << std::endl;
+dstr << "  RMDir \"?\\$INSTDIR\\" << fileN << "\"" << std::endl;
 if (!componentName.empty())
   {
   this->Components[componentName].Directories.push_back(fileN);
-- 
2.5.1.windows.1

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

[cmake-developers] CPack/NSIS is broken after extended length paths fix

2015-09-18 Thread Dmitry Kochkin 
 Hi Clinton,

I was looking into an issue that we have in CMake on Windows with extended 
paths and later on realized that you've fixed it.
However I've realized that you fixed it only in cmSystemTools, which fixes e.g. 
INSTALL, but does not fix cpack.

Actually it's even worse because in current master HEAD I can see that INST_DIR 
is empty in generated project:
  !define INST_DIR ""
(not sure if that's caused by your change)

I've figured out that NSIS was not going to fix this (see  
http://sourceforge.net/p/nsis/feature-requests/241/ )
I've made a small patch to NSIS generator and template to put infamous \\?\ 
there.

I can send directly to you as I guess you know more about these parts of code.

Best regards,
Dmitry Kochkin

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] CPack/NSIS is broken after extended length paths fix

2015-09-18 Thread clinton 
- On Sep 18, 2015, at 6:07 AM, Dmitry Kochkin  wrote: 

> Hi Clinton,

> I was looking into an issue that we have in CMake on Windows with extended 
> paths
> and later on realized that you've fixed it.
> However I've realized that you fixed it only in cmSystemTools, which fixes 
> e.g.
> INSTALL, but does not fix cpack.
Yes, I was fixing the 'install' to work with paths > 260 characters, and was 
using the cpack archive generator at the time. 
With the path to the build tree, plus the "_CPackPackage/", some of these 
paths can become quite long. 

> Actually it's even worse because in current master HEAD I can see that 
> INST_DIR
> is empty in generated project:
> !define INST_DIR ""
> (not sure if that's caused by your change)
Interesting... Do you know why this happens? How is this problem related to 
extended paths (with the \\?\ prefix)? Do you have an example to demonstrate 
this problem? 

> I've figured out that NSIS was not going to fix this (see
> http://sourceforge.net/p/nsis/feature-requests/241/ )
Maybe they'll accept a patch to fix it there? Was there any effort to fix it 
there? 

> I've made a small patch to NSIS generator and template to put infamous \\?\
> there.

> I can send directly to you as I guess you know more about these parts of code.

You can send your patch to this mailing list for review. 

Clint 
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers