Skip to content

Use explicit GLOB command for globbing #867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

bgs99
Copy link
Contributor

@bgs99 bgs99 commented Aug 13, 2024

As described in #865, this is the simplest approach for fixing CROW_AMALGAMATE with CMake Ninja generator.

@gittiver
Copy link
Member

gittiver commented Aug 16, 2024

What about removing glowing explicitly and list the files in a variable without globbing?
I would tend to make the list of files explicitly, what speaks against it?

@bgs99
Copy link
Contributor Author

bgs99 commented Aug 17, 2024

What about removing glowing explicitly and list the files in a variable without globbing? I would tend to make the list of files explicitly, what speaks against it?

Without globbing it would be possible to create a new header file and forget to add it to the list, assuming there are no tests/examples that attempt to use it.

I personally don't feel strongly for either approach - this one just maintains the current tradeoffs.

Should I make a new MR with headers listed explicitly instead?

@gittiver
Copy link
Member

I personally don't feel strongly for either approach - this one just maintains the current tradeoffs.

Should I make a new MR with headers listed explicitly instead?

If you would be so kind, let's try with explicit listing.

cmake docs state on globbing:
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. 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.

@gittiver
Copy link
Member

replaced by #871 list the headers manually instead of globbing.

@gittiver gittiver closed this Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants