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

Add missing librt on linux via NATS_EXTRA_LIB var #63

Merged
merged 3 commits into from
Apr 6, 2017

Conversation

film42
Copy link
Contributor

@film42 film42 commented Apr 5, 2017

I was working on loading cnats via the ffi gem with jruby and noticed that I was having issues loading libnats because...

libnats.so: undefined symbol: clock_gettime.

This loads just fine under MRI.

I noticed we set the NATS_EXTRA_LIB var in the main CMakeLists.txt and that we use it for tests, but we we're not using it in src/CMakeLists.txt. Adding it back fixes the problem.

Here's a failing job in travis: https://travis-ci.org/abrandoned/protobuf-nats/jobs/218995507#L761

@film42
Copy link
Contributor Author

film42 commented Apr 5, 2017

Btw, here's the same jruby job pointed at my cnats branch: https://travis-ci.org/abrandoned/protobuf-nats/jobs/219008594

@kozlovic
Copy link
Member

kozlovic commented Apr 5, 2017

Thanks for your contribution!

@abrandoned actually added NATS_BUILD_WITHOUT_CLOCK_MONOTONIC in src/CMakefile.txt to avoid this problem I believe. To be fair, this whole thing need revisiting because I used defined CLOCK_MONOTONIC but then only use CLOCK_REALTIME.

If the real issue was that rt was missing, I am also questioning if adding it in building nats library is the right thing to do. Maybe it should be added to the application linking to libnats.so? (like I do for the test and examples).

Let me experiment a bit and get back to you.

PS: At any rate, not sure it is doing anything to add it to nats_static.

@film42
Copy link
Contributor Author

film42 commented Apr 5, 2017

Oh yeah, I totally forgot about NATS_BUILD_WITHOUT_CLOCK_MONOTONIC! And btw, you're right about nats_static.

@kozlovic
Copy link
Member

kozlovic commented Apr 5, 2017

Yes, but let me think more about it. I think that what @abrandoned did was actually making such that libnats would not use clock_gettime at all. I think that your fix would be better. Let me experiment and I will get back to you.

@kozlovic
Copy link
Member

kozlovic commented Apr 5, 2017

@film42 So I have verified that even if we add "rt" to nats target, the examples still need to be linked with "rt". Without adding "rt" in nats, you can see from Travis that the build of the library works fine.

So I wonder if adding "rt" to your build (without changing anything to libnats) would not be enough. That being said, it seems that it would not cause a problem adding it to libnats? And if so, maybe we could remove the CMake flag that we added to disable use of clock_gettime. Could you please experiment and see if:

  • adding "rt" in your build is enough
  • if not and you add it to libnats, remove NATS_BUILD_WITHOUT_CLOCK_MONOTONIC and see if @abrandoned issue is no longer relevant?

Thanks again!

@film42
Copy link
Contributor Author

film42 commented Apr 6, 2017

@kozlovic I found that I can set my own cmake global vars with master branch, so I might not need this PR after all:

cmake .. -DCMAKE_SHARED_LINKER_FLAGS="-lrt"
make VERBOSE=1

[ 37%] Linking C shared library libnats.so
...
/usr/bin/cc  -fPIC   -std=c99 -pedantic -pthread  ... -lrt ...

So this might be good enough for me since it seems to be specific to loading in the JVM.

@abrandoned are working on the same thing, so we might not end up using NATS_BUILD_WITHOUT_CLOCK_MONOTONIC. So we can remove this and add the librt to the shared object if you prefer. Might be better to do this?

EDIT: I'm running travis with cmake global vars to make sure this is correct.
EDIT 2: While this seems to be adding the linker flags, something is still wrong. Travis is failing.

@film42
Copy link
Contributor Author

film42 commented Apr 6, 2017

@kozlovic I ran travis with NATS_BUILD_WITHOUT_CLOCK_MONOTONIC and it passed, and ran with the fix in this branch and it worked, so I went ahead and removed NATS_BUILD_WITHOUT_CLOCK_MONOTONIC.

@kozlovic
Copy link
Member

kozlovic commented Apr 6, 2017

Thank you for your contribution!

@kozlovic kozlovic merged commit a3948d7 into nats-io:master Apr 6, 2017
@kozlovic kozlovic removed the pending label Apr 6, 2017
@film42 film42 deleted the gt/fix_librt branch April 6, 2017 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants