-
Notifications
You must be signed in to change notification settings - Fork 59
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
fix compile of slurm plugin against Slurm >= 23.x #153
Conversation
Problem: Slurm exports a few generic container typedefs via slurm.h like `List` and `ListIterator` which conflict with pdsh internal definitions of the same types. To work around this, pdsh currently defines `__list_datatypes_defined` in the slurm plugin to avoid Slurm exporting those typedefs. However, in Slurm >23.x, a new typedef is added, `list_t`, and the list handling functions are updated to use this new type (which is actually just a new name for the same type). Now pdsh can no longer use the `__list_datatypes_defined` because then the necessary `list_t` data is not defined so including the `slurm.h` header fails. However, without `__list_datatypes_defined` compilation also fails because the `List` typedef is redefined. Use the only available option and force the definitions of typedef struct xlist list_t; typedef struct listIterator list_itr_t; in the pdsh slurm module before including `slurm.h`. This allows the `__list_datatypes_defined` define to remain enabled, avoiding the redefinition of `List` and `ListIterator`, while also ensuring that the new names for these structs are defined. This may be brittle in the long term, but is the only solution available now. Fixes chaos#152
Problem: GitHub complains that checkout@v2 is deprecated: The following actions uses node12 which is deprecated and will be forced to run on node16: actions/checkout@v2. Update to checkout@v3.
Problem: Github complains that the codecov-action@v1 is deprecated: The following actions uses node12 which is deprecated and will be forced to run on node16: codecov/codecov-action@v1. Update to codecov-action@v3.
Problem: The slurm plugin is not even compiled in Github CI. Install libslurm-dev to the list of packages to install in the ci container and add --with-slurm to the list of configure options.
Codecov ReportAll modified lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #153 +/- ##
==========================================
- Coverage 64.20% 64.17% -0.04%
==========================================
Files 27 27
Lines 5588 5940 +352
==========================================
+ Hits 3588 3812 +224
- Misses 2000 2128 +128 ☔ View full report in Codecov by Sentry. |
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.
LGTM!
Requesting approval from @ryanday36 as well. |
It looks reasonable to me too. Mark Schmitz at Sandia said that he would test building with it. Don't know if you want to wait for that or not. |
Mark S. says that it worked him. |
Great, thanks @ryanday36! |
I don't think we have mergify here, so @garlick can you press the button when you have time? |
Thanks! |
This PR fixes #152 and also includes some updates to CI configuration.