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

libschedutil: export library for use by flux-sched and others #2380

Merged
merged 2 commits into from Sep 18, 2019

Conversation

@SteVwonder
Copy link
Member

commented Sep 16, 2019

  • installs the compiled schedutil shared library
  • adds a libschedutil map file for ld
  • installs the schedutil headers
  • creates a schedutil .pc file
  • prefixes all public function typedefs with schedutil_

Closes #2377

@grondo
grondo approved these changes Sep 17, 2019
Copy link
Contributor

left a comment

LGTM. Just one typo inline (which I'm sure you would have found anyway.

etc/flux-schedutil.pc.in Outdated Show resolved Hide resolved
@SteVwonder SteVwonder force-pushed the SteVwonder:schedutil-export branch from eb3490b to cf92573 Sep 18, 2019
SteVwonder added 2 commits Sep 16, 2019
Install the library as libflux-schedutil.so, with a flux-schedutil.pc
file for pkgconfig.
prefix with `schedutil_` and add `_cb_` to signify that it is a
callback function (e.g., `hello_f` -> `schedutil_hello_cb_f`).
@SteVwonder SteVwonder force-pushed the SteVwonder:schedutil-export branch from cf92573 to 64f2466 Sep 18, 2019
@SteVwonder SteVwonder marked this pull request as ready for review Sep 18, 2019
@SteVwonder

This comment has been minimized.

Copy link
Member Author

commented Sep 18, 2019

Just finished the corresponding flux-sched PR on my local machine and everything seems to work just fine. Thanks @grondo for the review. Once Travis goes green, I'm happy with this PR.

@SteVwonder

This comment has been minimized.

Copy link
Member Author

commented Sep 18, 2019

The clang builder failed with:

+ flux job attach 333514276864
+ flux job info 333514276864 eventlog jobspec R
==30549== 
==30549== HEAP SUMMARY:
==30549==     in use at exit: 22,719 bytes in 58 blocks
==30549==   total heap usage: 80,679 allocs, 80,621 frees, 56,210,722 bytes allocated
==30549== 
==30549== LEAK SUMMARY:
==30549==    definitely lost: 0 bytes in 0 blocks
==30549==    indirectly lost: 0 bytes in 0 blocks
==30549==      possibly lost: 0 bytes in 0 blocks
==30549==    still reachable: 22,719 bytes in 58 blocks
==30549==         suppressed: 0 bytes in 0 blocks
==30549== Reachable blocks (those to which a pointer was found) are not shown.
==30549== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==30549== 
==30549== For counts of detected and suppressed errors, rerun with: -v
==30549== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
flux-start: 0 (pid 30548) Killed
not ok 1 - valgrind reports no new errors on 2 broker run

I'm a little suspicious that a different compiler would result in a leak. I'm guessing something hung and the test timed out? I'll try restarting it and maybe it'll pass.

@codecov-io

This comment has been minimized.

Copy link

commented Sep 18, 2019

Codecov Report

Merging #2380 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2380      +/-   ##
==========================================
+ Coverage   80.92%   80.92%   +<.01%     
==========================================
  Files         221      221              
  Lines       34819    34819              
==========================================
+ Hits        28176    28178       +2     
+ Misses       6643     6641       -2
Impacted Files Coverage Δ
src/modules/sched-simple/sched.c 73.52% <ø> (ø) ⬆️
src/common/libschedutil/init.c 86.36% <ø> (ø) ⬆️
src/common/libschedutil/hello.c 53.12% <100%> (ø) ⬆️
src/common/libflux/message.c 80.36% <0%> (-0.14%) ⬇️
src/common/libflux/mrpc.c 88.97% <0%> (+1.18%) ⬆️
@garlick

This comment has been minimized.

Copy link
Member

commented Sep 18, 2019

LGTM! Thanks!

@garlick garlick merged commit 956fef3 into flux-framework:master Sep 18, 2019
4 checks passed
4 checks passed
Summary 1 potential rule
Details
codecov/patch 100% of diff hit (target 80.92%)
Details
codecov/project 80.92% (+<.01%) compared to a6d362b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@SteVwonder SteVwonder deleted the SteVwonder:schedutil-export branch Sep 19, 2019
@SteVwonder

This comment has been minimized.

Copy link
Member Author

commented Sep 19, 2019

Thanks @garlick!

@garlick garlick referenced this pull request Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.