facebookarchive / libphenom

An eventing framework for building high performance and high scalability systems in C.
http://facebook.github.io/libphenom
Apache License 2.0
1.66k stars 362 forks source link

Unbundle Concurrency Kit #60

Closed brucespang closed 10 years ago

brucespang commented 10 years ago

This removes the thirdparty/ck directory and uses pkg-config to specify the concurrency-kit dependency. It forces ck to be <= 0.3.2, as that's the last compatible release with the current libphenom.

For more reference, see #46 and the comments in #59.

brucespang commented 10 years ago

With regard to using the correct ck version, I've had luck with checking out 0.3.2 in the ck git repo and building from source.

wez commented 10 years ago

I've been playing with this tonight; it looks good; thanks for doing this!

I think we need the changes in this gist thought to wrap things up:

https://gist.github.com/wez/11076200

What this does:

The pkg_config_libs function does some mild trickery to make the rpath work sanely for folks that have installed libs outside of the default load or search paths; this is really just a slight refactoring of the equivalent logic I had in place for libc-ares. I've needed this in the past but haven't been able to prove that this is strictly needed in my linux VM as it appears to default to static linkage... when linking against the .so, it is desirable to bake the rpath in so that we don't need to set LD_LIBRARY_PATH to find CK.

This all stemmed from having this problem on OS/X; the solution for OS/X is different because we would need to modify CK to set the install_name to use a prefix of @rpath (or just set it to the full install location). Since we can't go back in time and fix that for CK 0.3.2, I installed CK globally to test this and ignored the problem for now.

The travis stuff is untested on travis, but seems to work in my ubuntu VM; to test it locally:

./travis/deps.sh
./travis/run.sh

Do you think you could amend the gist into your pull request and update it?

That should trigger a travis build so we can prove that this is all good; travis is supposed to show that we're green on the pull requests, but for whatever reason, it doesn't show that the build is bad on this request.

The pull request page itself shows that the commits so far don't build though:

https://travis-ci.org/facebook/libphenom/pull_requests

wez commented 10 years ago

Tested the travis build bits part of this myself: looks good here: https://travis-ci.org/facebook/libphenom/jobs/23342467 I'll take your changes and push them with tweaks; thanks!

wez commented 10 years ago

Pushed; thanks!

brucespang commented 10 years ago

Great, thanks!