Re: [cmake-developers] QtAutogen 3.6.0 directory nesting overflow patch
Am 08.08.2016 um 19:40 schrieb Brad King: On 08/06/2016 09:42 AM, Sebastian Holtermann wrote: Ok, here is a new set of patches based on the current git master branch. Thanks! +outStream << "#include \"" + << cmsys::SystemTools::ConvertToOutputPath(it->second) + << "\"\n"; The ConvertToOutputPath method may also add its own double quotes. It is not just about slash conversion. Please use another approach to convert slashes. I dropped this patch for now. Ok, I just checked http://stackoverflow.com/questions/5790161/is-the-backslash-acceptable-in-c-and-c-include-directives It is even invalid to use backslashes in an #include statements. The paths are generated with a forward slash so it should be good to just use them as they are. Meanwhile I've applied the rest: cmFilePathUuid: Add class to generate deterministic unique file names https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=ecb6df52 QtAutogen: Use std:: instead of ::std:: https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=06217388 QtAutogen: Allow multiple moc files with the same name https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=3e2cd04b QtAutogen: Allow multiple qrc files with the same name https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=71d0308a Tests/QtAutogen: Test same moc/qrc source names in different directories https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=20f0028c and merged to `next` for testing. Great, thanks. One of the problems reported in https://gitlab.kitware.com/cmake/cmake/issues/16209 with the previous approach was that the symbol name created by autorcc changed from what it was in CMake <= 3.5. Is it now preserved? It is back to what it was in 3.5 for the all-qrc-names-unique case. So yes, existing projects get the same symbol name they used to get. New projects that now can use non unique file names may get a symbol name based on the checksum extended file name - but that one is also length limited and should be safe. But I found another issue. The checksum string generator may generate a '-' in the file name. This is allowed for file names but not for symbol names. I have attached a small patch to fix this. -Sebastian >From 35aa83b165fd6b69694c9cfe264b9d18d86de3ab Mon Sep 17 00:00:00 2001 From: Sebastian Holtermann Date: Mon, 8 Aug 2016 20:58:21 +0200 Subject: [PATCH] QtAutogen: Replace invalid character for symbol names --- Source/cmQtAutoGenerators.cxx | 4 1 file changed, 4 insertions(+) diff --git a/Source/cmQtAutoGenerators.cxx b/Source/cmQtAutoGenerators.cxx index 6a95615..063c355 100644 --- a/Source/cmQtAutoGenerators.cxx +++ b/Source/cmQtAutoGenerators.cxx @@ -1331,6 +1331,10 @@ bool cmQtAutoGenerators::GenerateQrc(const std::string& qrcInputFile, // Remove "qrc_" at string begin symbolName.erase(0, 4); } + // Replace '-' with '_'. The former is valid for + // file names but not for symbol names. + std::replace( symbolName.begin(), symbolName.end(), '-', '_' ); + const std::string qrcBuildFile = this->Builddir + qrcOutputFile; int sourceNewerThanQrc = 0; -- 2.8.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
Re: [cmake-developers] Export files in the build tree have a total path length which is unnecessarily long
On 2016-08-08 13:40-0400 Brad King wrote: On 08/07/2016 01:39 AM, Alan W. Irwin wrote: why is the install location of these export files included in the pathname for both export files? That significantly increases the pathlength of these files to no purpose since the install location of these files is already known to CMake by other means. These files are generated by the install(EXPORT) command. Their location is a staging area for "make install" to copy to the actual install destination. The install destination is used in their staging location to make sure there are no collisions. Normally the install(EXPORT) command is given a relative DESTINATION so the path is not very long. Are you giving it an absolute destination? Why? PLplot (including the test_fortran subproject to be consistent with the principal plplot project) has always used absolute install locations. It sounds like I could work around the issue by moving PLplot over to relative install locations, but that is a fairly intrusive change to our build system so I am a bit reluctant to make that change. Furthermore, I doubt this problem is unique to the PLplot build system since presumably there are other projects that use absolute install locations as well because such install locations are typically well supported by CMake. The only exception to that support that I am aware of at this time is this particular install(EXPORT) case where the (encoded) absolute staging area potentially adds so much to the total length of the pathname. Is this the only case where you have a staging area in the build tree that corresponds to an install pathname? For all such staging areas, would it be straightforward to hash the paths for the cmake-generated EXPORT files in the build tree to substantially reduce their pathlengths for the absolute install location case while still avoiding name collisions? Alan __ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); the Time Ephemerides project (timeephem.sf.net); PLplot scientific plotting software package (plplot.sf.net); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __ Linux-powered Science __ -- 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] [PATCH] Use full path for all source files in ninja build.
Thanks. But to address Florent's concern, are there tests that cover the windows command line limit? I guess to test if this breaks we'd need a source file whose compile command is under the limit if using relative path, but over the limit if using absolute path. On Mon, Aug 8, 2016 at 11:04 AM, Brad King wrote: > On 08/08/2016 01:42 PM, Chaoren Lin wrote: > >> I don't think this hunk is needed anymore, correct? > > > > That hunk is the opposite now > > Oops, right. That actually fixes the existing RC bug I mentioned > earlier in this thread. > > With your patch the use of IN_ABS breaks builds with spaces in the > path. The reason is that Ninja handles quoting of paths when > replacing the `$in` placeholder but does nothing special for > `$IN_ABS`. CMake will have to generate the right path in the value. > > I've applied the patch with the appropriate modification for that: > > Ninja: Use full path for all source files > https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=955c2a63 > > Thanks, > -Brad > > -- 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] [PATCH] Use full path for all source files in ninja build.
On 08/08/2016 01:42 PM, Chaoren Lin wrote: >> I don't think this hunk is needed anymore, correct? > > That hunk is the opposite now Oops, right. That actually fixes the existing RC bug I mentioned earlier in this thread. With your patch the use of IN_ABS breaks builds with spaces in the path. The reason is that Ninja handles quoting of paths when replacing the `$in` placeholder but does nothing special for `$IN_ABS`. CMake will have to generate the right path in the value. I've applied the patch with the appropriate modification for that: Ninja: Use full path for all source files https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=955c2a63 Thanks, -Brad -- 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] QtAutogen 3.6.0 directory nesting overflow patch
On 08/06/2016 09:42 AM, Sebastian Holtermann wrote: > Ok, here is a new set of patches based on the current git master branch. Thanks! > +outStream << "#include \"" > + << cmsys::SystemTools::ConvertToOutputPath(it->second) > + << "\"\n"; The ConvertToOutputPath method may also add its own double quotes. It is not just about slash conversion. Please use another approach to convert slashes. I dropped this patch for now. Meanwhile I've applied the rest: cmFilePathUuid: Add class to generate deterministic unique file names https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=ecb6df52 QtAutogen: Use std:: instead of ::std:: https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=06217388 QtAutogen: Allow multiple moc files with the same name https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=3e2cd04b QtAutogen: Allow multiple qrc files with the same name https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=71d0308a Tests/QtAutogen: Test same moc/qrc source names in different directories https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=20f0028c and merged to `next` for testing. One of the problems reported in https://gitlab.kitware.com/cmake/cmake/issues/16209 with the previous approach was that the symbol name created by autorcc changed from what it was in CMake <= 3.5. Is it now preserved? Thanks, -Brad -- 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] [PATCH] Use full path for all source files in ninja build.
On 08/08/2016 09:19 AM, Ben Boeckel wrote: > On Sun, Aug 07, 2016 at 04:54:52 +0200, Florent Castelli wrote: >> I'd say that you shouldn't do this unless you have tested it extensively >> with very long command lines, making sure that there is a response file >> fallback if it grows too much. >> There are issues in CMake already on Windows with long command lines and >> having even longer ones is not going to help. > > Agreed. RC files compilation already has issues in some projects without > absolute paths as is. > > https://gitlab.kitware.com/cmake/cmake/issues/16171 RC files used absolute paths prior to this patch. The patch is for other languages. Also in out-of-source builds we already use absolute paths to files in the source tree. If there are problems with command line lengths they can be addressed separately. -Brad -- 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] [PATCH] Use full path for all source files in ninja build.
> I don't think this hunk is needed anymore, correct? That hunk is the opposite now, or does RC still need that hack for some reason? On Mon, Aug 8, 2016 at 10:41 AM, Brad King wrote: > On 08/05/2016 07:51 PM, Chaoren Lin wrote: > > - std::string const sourceFileName = > > -language == "RC" ? source->GetFullPath() : this->GetSourceFilePath( > source); > > + std::string const sourceFileName = this->GetSourceFilePath(source); > > I don't think this hunk is needed anymore, correct? > > Thanks, > -Brad > -- 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] [PATCH] Use full path for all source files in ninja build.
On 08/05/2016 07:51 PM, Chaoren Lin wrote: > - std::string const sourceFileName = > -language == "RC" ? source->GetFullPath() : > this->GetSourceFilePath(source); > + std::string const sourceFileName = this->GetSourceFilePath(source); I don't think this hunk is needed anymore, correct? Thanks, -Brad -- 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] Export files in the build tree have a total path length which is unnecessarily long
On 08/07/2016 01:39 AM, Alan W. Irwin wrote: > why is the install location of these export files included in the > pathname for both export files? > That significantly increases the pathlength of these files to no purpose > since the > install location of these files is already known to CMake by other means. These files are generated by the install(EXPORT) command. Their location is a staging area for "make install" to copy to the actual install destination. The install destination is used in their staging location to make sure there are no collisions. Normally the install(EXPORT) command is given a relative DESTINATION so the path is not very long. Are you giving it an absolute destination? Why? -Brad -- 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] [PATCH] Use full path for all source files in ninja build.
On Sun, Aug 07, 2016 at 04:54:52 +0200, Florent Castelli wrote: > I'd say that you shouldn't do this unless you have tested it extensively > with very long command lines, making sure that there is a response file > fallback if it grows too much. > There are issues in CMake already on Windows with long command lines and > having even longer ones is not going to help. Agreed. RC files compilation already has issues in some projects without absolute paths as is. https://gitlab.kitware.com/cmake/cmake/issues/16171 --Ben -- 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] [Patch 4/5] Improved WIX support
On 07/20/2016 03:58 PM, Stuermer, Michael SP/HZA-ZSEP wrote: Using the patchfile support I managed to implement the service installation issue I had, so the unnecessary features from the last patch are removed now. I tested all patches separately and hope they work now. looking forward for feedback, best regrads, Michael I fixed a minor whitespace error (did you forget |./Utilities/SetupForDevelopment.sh ?), added a release note, extended the documentation, reworded the commit message and merged to "next" for testing. The feature itself seems to work as advertised though I am not too fond of the semantics which is why my contribution to the documentation is primarily a cautionary note. Nils | -- 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
