Re: [cmake-developers] [PATCH] cmFileCommand: sort list of files from glob command

2016-05-16 Thread Reiner Herrmann 
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

2016-05-16 Thread Brad King 
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

2016-05-16 Thread Chuck Atkins 
>
> 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

2016-05-16 Thread Petr Kmoch 
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