-
Notifications
You must be signed in to change notification settings - Fork 198
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
CMake/CTest: Opt-in Disable Signal Handling #5550
base: development
Are you sure you want to change the base?
Conversation
In IDEs, we want to attach debuggers to CTest runs. This needs an option to disable signal handling from AMReX to work.
CMakeLists.txt
Outdated
mark_as_advanced(WarpX_TEST_FPETRAP) | ||
|
||
# Advanced option to run CI tests with the -g compile option | ||
option(WarpX_TEST_DEBUG "Run CI tests with the -g compile option" OFF) | ||
option(WarpX_TEST_DEBUG "Compile tests with -g1 for symbols (for CI)" OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like to rename WarpX_TEST_DEBUG
in this PR or in a separate one? If you would like to do it in this PR, feel free to let me know if you want me to do the change (after discussing the new name with you, of course), in case you have other more important stuff on your plate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the docs at https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html, maybe we could rename this simply WarpX_TEST_DEBUG_INFO
, more verbose but less generic than WarpX_TEST_DEBUG
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about WarpX_TEST_DEBUG_SYMBOLS
?
Technically, it is even WarpX_DEBUG_SYMBOLS
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment was marked as resolved.
This comment was marked as resolved.
I think this replaces #5259. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me now. @ax3l, feel free to review my latest changes and confirm if this looks good to you too, so we can enable automerge.
In IDEs, we want to attach debuggers to CTest runs. This needs an option to disable signal handling from AMReX to work.