fix macos portability issues (second batch) #6468
Conversation
garlick
changed the title
| @@ -564,6 +564,12 @@ struct signalfd_siginfo | |||
| # endif | |||
| #endif | |||
|
|
|||
| /* disable timerfd on macos */ | |||
src/common/libfluxutil/method.c
Outdated
| #ifdef RUSAGE_THREAD | ||
| int who = RUSAGE_THREAD; | ||
| #else | ||
| int who = RUSAGE_SELF; | ||
| #endif | ||
| if (flux_request_decode (msg, NULL, NULL) < 0 | ||
| || getrusage (RUSAGE_THREAD, &ru) < 0) | ||
| || getrusage (who, &ru) < 0) |
There was a problem hiding this comment.
Hmmm. Would it better to return an error instead of using RUSAGE_SELF? B/c the wrong data would probably be returned.
Or perhaps there should be a RUSAGE_THREAD and RUSAGE_SELF variant. SELF for the broker usage (which I guess includes all modules) and RUSAGE_THREAD for the module threads, and the RUSAGE_THREAD one doesn't work if RUSAGE_THREAD not supported. This could be for a different issue/PR of course.
There was a problem hiding this comment.
Well I hate to complicate this since it's not used much, but I think your idea of a variant (or passing a flag) makes the most sense. Then when a variant/flag is not supported, it could return an appropriate error.
There was a problem hiding this comment.
OK, I just pushed a change that adds an optional who key to the RPC. The default is to use RUSAGE_SELF but the other who values can be requested. Then I changed flux module stats --rusage to work as follows
$ flux module stats --rusage=[self|children|thread]
so there is now an optional argument. This is now documented it in flux-module(1).
On systems where RUSAGE_THREAD is unsupported, you'll get an error like this:
$ flux module stats --rusage=thread connector-local
flux-module: connector-local.rusage: thread is unsupported
Since this is little used and the documentation before this was ambiguous, I think this change is probably not disruptive to anything.
There was a problem hiding this comment.
LGTM, maybe wanna split off those 4 commits to a separate PR since it is more than just a simple portability fix at this point.
garlick
force-pushed
the
macos2
branch
2 times, most recently
from
0a0b12b to
ebab9b5
Compare
|
Sorry, noticed a typo in the rusage service side and forced a push. |
|
@chu11 I think this is ready for a final review if you have a moment. |
|
Good point, thanks. OK, I split that out and I'll set MWP here. |
Problem: macos build fails with struct itimerspec undefined. Disable timerfd on macos to avoid requiring this in libev.
Problem: compilation fails in various places on macos due to homebrew installing headers in a non-standard location. Fix AM_CPPFLAGS where needed.
Problem: compilation fails on macos when %ju or %jd is used with non-matching integer types. Cast them to [u]intmax_t.
Problem: some function prototypes that are not used outside of GNU C are not fully specified, so compilation fails on macos. Add full specs.
Problem: envz.h includes features.h and malloc.h but those don't exist on macos. Drop features.h and replace malloc.h wtih stdlib.h.
Problem: compilation fails on macos with 'environ' undefined. Just define it locally where needed.
Problem: compilation fails in various places on macos due to undefined kill(2) and killpg(2). Include signal.h where needed.
Problem: compilation fails on macos in various places because SOCK_CLOEXEC is not defined. Set SO_CLOEXEC with fcntl(2) after socket creation if SOCK_CLOEXEC is undefined.
Problem: SO_PEERCRED is not defined on macos so compilation fails. Borrowing from the munge project, implement unix socket authentication using LOCAL_PEERCRED instead, if defined. If neither is defined, throw a preprocessor error. See also: https://github.com/dun/munge/blob/master/src/munged/auth_recv.c#L315
Problem: KVS module does not compile on macos because EBADE is not a valid signal name there. Use EINVAL if EBADE is undefined.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6468 +/- ##
==========================================
- Coverage 83.62% 83.61% -0.01%
==========================================
Files 523 523
Lines 87590 87600 +10
==========================================
+ Hits 73243 73247 +4
- Misses 14347 14353 +6
|
Problem: flux doesn't build on macos
This resolves a few portability issues, though it is not sufficient to get Flux working.
I have more fixes to post but some unexplained test failures to run down. I thought it might be best to post the first chunk and get that reviewed while I debug then next one.