Skip to content
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

integration: add GLOB with CONFIGURE_DEPENDS to pick up toml files #196

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

JakeHillion
Copy link
Contributor

Summary

Currently integration test configs have to be specified manually in the CMakeLists.txt file. This changes to using a CMake GLOB_RECURSE with the flag CONFIGURE_DEPENDS (available since 3.12) which causes the build to check the glob each time and re-configure CMake if it has changed.

Test plan

$ cp test/integration/std_string{,_2}.toml && make test
[0/2] Re-checking globbed directories...
-- GLOB mismatch!
[1/2] Re-running CMake...
...

Otherwise:

$ make test
[0/2] Re-checking globbed directories...
[0/11] Building drgn
...

Copy link
Contributor

@ajor ajor left a comment

Choose a reason for hiding this comment

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

The "Adding Tests" section of the docs will need updating:

Just quoting this section from the CMake docs for future reference:

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.

They recommend against this method, but it seems to be fairly widely used and works with our current generators. We can always switch back if we encounter problems in the future.

@JakeHillion JakeHillion merged commit 5c3bb26 into facebookexperimental:main Jun 29, 2023
@JakeHillion JakeHillion deleted the glob-tests branch June 29, 2023 14:47
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.

3 participants