-
Notifications
You must be signed in to change notification settings - Fork 44
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
Use pkg-config to find libraries and include files #1502
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ |
The mintaka test seems not to find libmongoc-1.0.pc. Strange, as https://github.com/mongodb/mongo-c-driver/tree/master/src/libmongoc/src clearly shows that it is being installed. Are you sure mongo-c-driver is properly installed on the test system, with the usual "make install"? [Edit:] The 1.25.0 release removes libmongoc-1.0.pc (I tested with an earlier version), and I don't know why. Asked here: |
8aa1d2e
to
eaf5896
Compare
Ok, now libmongoc-ssl-1.0 is not installed... |
eaf5896
to
3b5b88e
Compare
Ok, the recent 1.25.0 release of mongo-c-driver made things more complicated. Need to port to that version and find out how things are meant to be used there. I'm marking this PR as "draft" for the moment. |
3b5b88e
to
bb5b2b9
Compare
Ok, I tested with mongo-c-driver-1.25.0 now, and while removing libmongoc-1.0.pc.in from the source, it properly generates a libmongoc-1.0.pc file and pkg-config works as expected. So something must be wrong in your CI setup. |
2819b75
to
bbf8bef
Compare
I found the pc files for some of the depencencies in non-standard locations, currently libmongo-client.pc is still missing. I continue investigating ... |
a116a26
to
cfbb127
Compare
Ok, libmongo-client comes from that same old rotten mongo-cxx-driver that has been deprecated back in 2016. It really is a lost case. I leave that one as it is. |
cfbb127
to
b58f0a4
Compare
Hello there, and thank you for the contribution! I'm perfectly fine with this type of changes. One important thing though, very few people compile their own broker, but, afaik. those that do use Ubuntu. As long as you can guarantee those four working, I'm perfectly fine with the suggested changes. |
Hi Ken,
That's how the build system code looks like ;-)
Thanks for that valuable information, will test those platforms in more detail once I'm finished with the outstanding issues. |
This PR is getting stale. |
b58f0a4
to
8eeb204
Compare
Thanks for the heads-up, I have rebased the patches to the develop branch and will look at the ci fallout. |
5c4c664
to
f44157f
Compare
The unit tests are fixed now: The PKG_CONFIG_PATH had to be extended with /usr/local/lib/pkgconfig (due to the unusual installation mechanism for libs in /usr/local - the whole library installation in the CI setup is a mess). Additionally, in the last patch series I missed adding the pkg-config derived ${LIBMICROHTTPD_LIBRARIES} to the unitTest. With these fixed, the "Deploy Baseimage" is now the last check that fails. I'll have a look. |
The failing "Deploy Baseimage / deploy-quay" checks fail because I don't have the permissions to push there. Seems like there is nothing I can do against this. Ken, I think this patch series is ready now, I'll undraft it. Please have a look. |
Yes, I believe you're right. Think we had a case of that not long ago. |
Could you re-trigger the tests and check if it works then? |
Won't solve a thing. |
Hold on, I found something to improve... |
For correct usage of header file inclusion and linking, it is more correct to use pkg-config instead of ad-hoc methods. With pkg-config, also special situations like out of tree building, setting a sysroot or cross compiling can be handled properly. The actual usage of pkg-config will follow in subsequent patches. Signed-off-by: Robert Schwebel <[email protected]>
Find Boost and its components by asking pkg-config. This does also take care of selecting the proper multithreaded variant for the actual distro. Signed-off-by: Robert Schwebel <[email protected]>
Now that all boost details are determined by pkg-config, remove the ifdeffery that differentiated between the distros. Signed-off-by: Robert Schwebel <[email protected]>
Use pkg-config to find out include- and library requirements for libmongoc. Signed-off-by: Robert Schwebel <[email protected]>
Use pkg-config to find out include- and library requirements for libbson. Signed-off-by: Robert Schwebel <[email protected]>
Use pkg-config to find out include- and library requirements for libmicrohttpd. Signed-off-by: Robert Schwebel <[email protected]>
Use pkg-config to find out include- and library requirements for gnutls.
47a9458
to
5eb067f
Compare
Several prerequisite packages install with different --prefix settings, so let pkg-config look in those locations to find .pc files. Signed-off-by: Robert Schwebel <[email protected]>
5eb067f
to
e9cc365
Compare
I've now pushed the fixed patch series. Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you for contributing!!!
Currently, the CMakeLists.txt files open-code how to find header files and libraries. As we have pkg-config as a mechanism to find these files in a generic way, it should be possible to get rid of most of the ifdeffery and thus the differentiation for all those different distributions.
In addition, pkg-config takes care of things like sysroot (when not building natively but in a cross enviroment).
It also makes it possible to get rid of the fact that orion-ld currently makes the arbitrary assumption that some components are built into /usr/local, which might not be the case. With pkg-config, it doesn't matter if those other packages are installed into /usr or /usr/local or elsewhere, as long as they have been properly installed and pkg-config can actually find them.
Note that I havn't been able to test these changes on all the different distributions, so a closer look might be necessary if it really works there.