Skip to content

test/CMakeLists.txt: Use an explicit list instead of GLOB - #1779

Merged
nlohmann merged 1 commit into
nlohmann:developfrom
t-b:avoid-using-glob-in-cmake
Oct 9, 2019
Merged

nlohmann merged 1 commit into
nlohmann:developfrom
t-b:avoid-using-glob-in-cmake

Conversation

@t-b

@t-b t-b commented Oct 7, 2019

Copy link
Copy Markdown
Contributor

Using GLOB is slow and considered bad practice.

From https://cmake.org/cmake/help/latest/command/file.html:

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. The CONFIGURE_DEPENDS flag may not work reliably on
all generators, or if a new generator is added in the future that cannot
support it, projects using it will be stuck. Even if CONFIGURE_DEPENDS
works reliably, there is still a cost to perform the check on every
rebuild.

Close #1686.

Using GLOB is slow and considered bad practice.

From https://cmake.org/cmake/help/latest/command/file.html:

> 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. The CONFIGURE_DEPENDS flag may not work reliably on
> all generators, or if a new generator is added in the future that cannot
> support it, projects using it will be stuck. Even if CONFIGURE_DEPENDS
> works reliably, there is still a cost to perform the check on every
> rebuild.
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling eb6fe42 on t-b:avoid-using-glob-in-cmake into d187488 on nlohmann:develop.

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann self-assigned this Oct 9, 2019
@nlohmann nlohmann added this to the Release 3.7.1 milestone Oct 9, 2019
@nlohmann
nlohmann merged commit ed55414 into nlohmann:develop Oct 9, 2019
@nlohmann

nlohmann commented Oct 9, 2019

Copy link
Copy Markdown
Owner

Thansk!

@nlohmann

nlohmann commented Oct 9, 2019

Copy link
Copy Markdown
Owner

🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

  • ✨ New Feature
  • 🐛 Bug Fix
  • ⚡️ Improvement
  • 🔨 Further Change
  • 🔥 Deprecated function

Description

  • Avoid usage of GLOB in CMake

@t-b
t-b deleted the avoid-using-glob-in-cmake branch October 9, 2019 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider listing files explicitly instead of using GLOB

3 participants