Page MenuHomePhabricator

[cmake] Link to libnetwork on Haiku in llvm-jitlink
ClosedPublic

Authored by nielx on Mar 11 2021, 2:48 AM.

Details

Summary

The system's network API is in libnetwork.so, so we explicitly need to link to them on Haiku. This patch is similar to https://reviews.llvm.org/D97633.

Diff Detail

Event Timeline

nielx created this revision.Mar 11 2021, 2:48 AM
nielx requested review of this revision.Mar 11 2021, 2:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2021, 2:48 AM

Adding reviewer ASDenysPetrov (you recently reviewed some other work on this tool)

Adding reviewer ASDenysPetrov (you recently reviewed some other work on this tool)

Sorry, I have to refuse your invitation. I have no competence in this. I was a reviewer for other revisions just because I've accidentally discovered fails of several tests on my environment (Windows+GCC+MSYS2), found a causer and raised an issue. I was just testing those patches using my specific environment and giving feedbacks, but not verifing the correctness.

You may need to do the same change in llvm/tools/llvm-jitlink/CMakeLists.txt, which makes me think it may valuable to implement a decent check à la autoconf to check for the actual symbol you're looking for, probably based on a sequence of CHECK_LIBRARY_EXISTS. Would you be ok to do that change?

nielx added a comment.Apr 7 2021, 11:33 PM

You may need to do the same change in llvm/tools/llvm-jitlink/CMakeLists.txt, which makes me think it may valuable to implement a decent check à la autoconf to check for the actual symbol you're looking for, probably based on a sequence of CHECK_LIBRARY_EXISTS. Would you be ok to do that change?

Thank you for having a look at the patch, I appreciate it!

Not sure whether I fully understand you, but I made the change both in llvm/tools/llvm-jitlink/CMakeLists.txt and llvm/tools/llvm-jitlink/llvm-jitlink-executor/CMakeLists.txt, is there another place that links to the network symbols?

About making it more generic, I would be okay looking into that, but I am slightly doubtful of the added value. The Haiku ABI is stable, and there is no version in existence that has these symbols in another place. I therefore don't know what the added value would be of doing a dynamic check for something that's by definition static.

lhames accepted this revision.Wed, May 5, 8:49 AM

If we acquire too many of these special cases then we should consider creating some more generic CMake infrastructure for them, but I think for now this is fine as-is.

Do you have commit access? If not please let me know and I can commit on your behalf.

This revision is now accepted and ready to land.Wed, May 5, 8:49 AM
nielx added a comment.Wed, May 5, 8:51 AM

Do you have commit access? If not please let me know and I can commit on your behalf.

I do not have commit access, so I appreciate the help there.

This revision was automatically updated to reflect the committed changes.