Re: [cmake-developers] [PATCH] cmFileCommand: sort list of files from glob command
On 05/16/2016 05:23 PM, Reiner Herrmann wrote: > Unfortunately many projects still use this for finding source files [1], > without considering sorting. > Without sorting it in file(GLOB), most of those packages have to be > patched individually. Would sorting by default really have such a > noticable performance impact? I can't imagine there are many projects > where sorting time isn't negligible. I also doubt the in-memory sorting time is noticeable. If it turns out to be a problem we can look at more efficient implementations. There is an open issue about this too: https://cmake.org/Bug/view.php?id=14491 Even when file(GLOB) is used as a primitive for a purpose other than collecting a list of source files, deterministic results are useful. I've decided to apply the patch and merge to 'next' for testing: file: Sort GLOB results to make it deterministic (#14491) https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=edcccde7 I also updated the test suite to cover it. I revised the wording of the documentation to not promise sorting, only determinism. When multiple globbing patterns are used we still sort only the results of each individual pattern, so overall the results are not necessarily sorted. 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] cmFileCommand: sort list of files from glob command
On Mon, May 16, 2016 at 09:27:57AM -0400, Brad King wrote: > This has been proposed several times and never accepted because file(GLOB) > is a primitive. As Petr said our documentation explicitly discourages > the use case in question. Those projects that wish to ignore our advice > can use list(SORT) themselves to get reproducible source file ordering > after file(GLOB). Thanks for the documentation hint, I didn't see this warning. Unfortunately many projects still use this for finding source files [1], without considering sorting. Without sorting it in file(GLOB), most of those packages have to be patched individually. Would sorting by default really have such a noticable performance impact? I can't imagine there are many projects where sorting time isn't negligible. What about having another glob mode for the few cornercases that really need this bit of extra-performance, but have the sorting enabled by default for file(GLOB)? This would automatically help many CMake projects with reproducibility. Regards, Reiner [1] https://codesearch.debian.net/perpackage-results/path%3ACMakeLists.txt%20(file%7CFILE).*(glob%7CGLOB).*%5C*%5C.c/2/page_0 signature.asc Description: Digital signature -- 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] cmFileCommand: sort list of files from glob command
On 05/16/2016 03:06 AM, Petr Kmoch wrote: > I'd like to express my concerns about the proposed change. > CMake strongly discourages using `file(GLOB)` to find source files [snip] > On 14 May 2016 at 12:30, Reiner Herrmann wrote: > @@ -1026,6 +1026,7 @@ bool > cmFileCommand::HandleGlobCommand(std::vector const& args, > > std::vector::size_type cc; > std::vector& files = g.GetFiles(); > +std::sort(files.begin(), files.end()); This has been proposed several times and never accepted because file(GLOB) is a primitive. As Petr said our documentation explicitly discourages the use case in question. Those projects that wish to ignore our advice can use list(SORT) themselves to get reproducible source file ordering after file(GLOB). -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] cmFileCommand: sort list of files from glob command
> > I'd like to express my concerns about the proposed change. CMake strongly > discourages using `file(GLOB)` to find source files, since file additions > then do not automatically trigger a buildsystem regeneration. > I second this. The intent of the patch is to address an issue that is only present when using a practice explicitly not recommended in CMake documentation: https://cmake.org/cmake/help/v3.5/command/file.html Note: We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate. -- 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] cmFileCommand: sort list of files from glob command
Hi all. I'd like to express my concerns about the proposed change. CMake strongly discourages using `file(GLOB)` to find source files, since file additions then do not automatically trigger a buildsystem regeneration. So well-behaved projects, which do not feed such outputs to add_*() commands, will now pay a performance penalty for a feature only introduced for the sake of the ill-behaved ones. In our setup, we have a large CMake-language framework built on top of "vanilla" CMake (partly for historical reasons, but partly to support our particular workflow), and we're always struggling with keeping the time of CMake processing down. Adding unnecessary overhead would be bad news for us. Can we instead make the feature optional? Even opt-out would be good enough, even thought there technically already is an opt-in, namely `list(SORT)` (which is even mentioned in the docs). Petr On 14 May 2016 at 12:30, Reiner Herrmann wrote: > file(GLOB ...) is often used to find source files. As it uses > readdir(), the list of files will be unsorted. > This list is often passed directly to add_executable / add_library. > Linking binaries with an unsorted list will make it unreproducible, > which means that the produced binary will differ depending on the > unpredictable readdir() order. > > To solve those reproducibility issues in a lot of programs (which > don't explicitely sort the list manually), this change sorts the > resulting list of the file GLOB command. > > A more detailed rationale about reproducible builds is available at: > https://reproducible-builds.org/ > --- > Help/command/file.rst| 3 +-- > Source/cmFileCommand.cxx | 1 + > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Help/command/file.rst b/Help/command/file.rst > index 96ac6c7..a502134 100644 > --- a/Help/command/file.rst > +++ b/Help/command/file.rst > @@ -103,8 +103,7 @@ Generate a list of files that match the > and > store it into the . Globbing expressions are similar to > regular expressions, but much simpler. If ``RELATIVE`` flag is > specified, the results will be returned as relative paths to the given > -path. No specific order of results is defined. If order is important > then > -sort the list explicitly (e.g. using the :command:`list(SORT)` command). > +path. The file list will be sorted. > > By default ``GLOB`` lists directories - directories are omited in result > if > ``LIST_DIRECTORIES`` is set to false. > diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx > index 6b6b913..4efa550 100644 > --- a/Source/cmFileCommand.cxx > +++ b/Source/cmFileCommand.cxx > @@ -1026,6 +1026,7 @@ bool > cmFileCommand::HandleGlobCommand(std::vector const& args, > > std::vector::size_type cc; > std::vector& files = g.GetFiles(); > +std::sort(files.begin(), files.end()); > for ( cc = 0; cc < files.size(); cc ++ ) >{ >if ( !first ) > -- > 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 > -- 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] [PATCH] cmFileCommand: sort list of files from glob command
file(GLOB ...) is often used to find source files. As it uses readdir(), the list of files will be unsorted. This list is often passed directly to add_executable / add_library. Linking binaries with an unsorted list will make it unreproducible, which means that the produced binary will differ depending on the unpredictable readdir() order. To solve those reproducibility issues in a lot of programs (which don't explicitely sort the list manually), this change sorts the resulting list of the file GLOB command. A more detailed rationale about reproducible builds is available at: https://reproducible-builds.org/ --- Help/command/file.rst| 3 +-- Source/cmFileCommand.cxx | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Help/command/file.rst b/Help/command/file.rst index 96ac6c7..a502134 100644 --- a/Help/command/file.rst +++ b/Help/command/file.rst @@ -103,8 +103,7 @@ Generate a list of files that match the and store it into the . Globbing expressions are similar to regular expressions, but much simpler. If ``RELATIVE`` flag is specified, the results will be returned as relative paths to the given -path. No specific order of results is defined. If order is important then -sort the list explicitly (e.g. using the :command:`list(SORT)` command). +path. The file list will be sorted. By default ``GLOB`` lists directories - directories are omited in result if ``LIST_DIRECTORIES`` is set to false. diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index 6b6b913..4efa550 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -1026,6 +1026,7 @@ bool cmFileCommand::HandleGlobCommand(std::vector const& args, std::vector::size_type cc; std::vector& files = g.GetFiles(); +std::sort(files.begin(), files.end()); for ( cc = 0; cc < files.size(); cc ++ ) { if ( !first ) -- 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