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

Include cmake or pkg-config modules #582

Closed
bbockelm opened this issue Sep 17, 2017 · 11 comments
Closed

Include cmake or pkg-config modules #582

bbockelm opened this issue Sep 17, 2017 · 11 comments
Assignees

Comments

@bbockelm
Copy link
Contributor

Many other C++ projects actually ship suggested CMake modules for compiling/linking against their projects. It provides a standard technical point-of-reference for using the project's modules.

For example, if I need the boost python module, I simply have to add the following to my CMakeLists.txt:

find_package( Boost REQUIRED COMPONENTS python )

and I can then reference Boost_PYTHON_LIBRARY and BOOST_INCLUDE_DIRS as needed. I'd love to see something similar provided by Xrootd.

I use standard macros for libraries like boost or python, but for Xrootd I find myself copy / pasting from older projects into my newer projects. That means I copy/paste bugs and don't get any improvements from upstream -- for example, I'm certain I'm not handling the library suffix (-4) correctly.

Now, there's a reasonable argument for-and-against the use of CMake specifically (for: it's likely the dominant C++ build system and covers ROOT and EOS; against: don't want to impose upon externals). In that case, the pkg-config is the standard way of doing things -- and hooks into almost every build system out there. It just isn't as expressive or useful for C++ projects.

@xrootd-dev
Copy link

xrootd-dev commented Sep 18, 2017 via email

@simonmichal
Copy link
Contributor

Hi guys,

I do agree that this would give our project a nice touch :-)

IMHO it would be enough to do a CMake module, but if there is a strong desire to use pkg-config it's also OK with me ;-)

@bbockelm : would you be so kind to prepare a PR? ;-)

Cheers,
Michal

@ljanyst
Copy link
Contributor

ljanyst commented Sep 20, 2017

I actually wrote one when I started writing the client. It's extremely simplistic and adding COMPONENTS support and plugin suffixes would be a nice touch :)

https://github.com/ljanyst/xrdclient/blob/master/cmake/FindXRootD.cmake

@bbockelm
Copy link
Contributor Author

bbockelm commented Oct 2, 2017

Yeah - I think that one might have even been the one I originally forked a few years ago (and have copy / pasted ever since)! Here's the latest revision on my end:

https://github.com/bbockelm/xrootd-multiuser/blob/master/cmake/FindXrootd.cmake

@simonmichal - I'm starting to think what a PR would look like. One big conceptual question: do we want the COMPONENTS arguments of the macro to map to source code components (OFS / ACC / XRDCL etc) or packaging components (SERVER / CLIENT / CORE / PRIVATE)?

The former would likely be more meaningful to developers, as they are likely looking at the source code. The latter would help us stay sane (less components!) because it would better match the existing packaging structures.

Just for reference - here's the upstream docs: https://cmake.org/Wiki/CMake:How_To_Find_Libraries. Seems that there's a reasonable set of helper macros we can either use directly or copy/paste (probably the latter, given the vintage of CMake we use...).

Note: I'm on vacation for approximately the next week - may be a bit before I can return to this.

@simonmichal
Copy link
Contributor

@bbockelm : good question. If we go for source code, we will end up with about 19 components, which is a lot ;-) On the other hand, if we go with packaging we will have about 4, which sounds to me much more sane ;-) I don't think there is much harm in grouping several source code components into a single cmake component. In the worst case cmake will set more variables than the developer needs, and that's it.

@bbockelm
Copy link
Contributor Author

bbockelm commented Oct 3, 2017

Although - if we want to do the "more sane" option (which I would like!), what's the correct value of ${XROOTD_SERVER_LIB}.

That would evaluate to something like -lXrdServer -lXrdHttp-4 -lXrdSomethingelse - i.e., listing all the libraries in the server package. Is the linker smart enough to skip unnecessary links? If not, that might force us to do all 19 sub-components.

@simonmichal
Copy link
Contributor

That's a good point. When I wrote my previous comment what I had in mind was INCLUDE_DIRS : |
I have to think it through.

@simonmichal
Copy link
Contributor

I suppose if we use -Wl,--as-needed then we should be OK with the "more sane" option.

@simonmichal
Copy link
Contributor

@bbockelm : are you still interested in implementing the cmake/ pkg-config modules?

@bbockelm
Copy link
Contributor Author

bbockelm commented Feb 9, 2018

Hi @simonmichal -

I'm still interested in seeing this -- but at this point, I think I must admit defeat and say I don't have time to do it myself.

So, feel free to keep it open if it's worthwhile to reassign elsewhere... otherwise, maybe just close it for now?

Brian

@simonmichal
Copy link
Contributor

@bbockelm: I pushed following commit 43124f7 that is an attempt to implement a XRootD find module. Let me know if you have any comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants