Adios2: Option to link to external dependencies instead of thirdparty copies

[williamfgc]

Why is this feature important?
In preparation to package for major Linux distros repos (Debian and Fedora). Third party copies are ~discouraged~ prohibited if already part of the distribution system, see Fedora and Debian docs on the subject.

What is the potential impact of this feature in the community?
Having Debian and Fedora distributions would ease the integration of adios2 in projects (my current ones included) due to widespread distribution in other distros (e.g. Ubuntu, Centos).

Describe the solution you'd like and potential required effort
Extend ADIOS2_USE_EXTERNAL_* to cover the dependencies of interest, if external dependency not found it might fallback to thirdparty copy inside ADIOS2. I'll be happy to do a PR, might need a new CI container.

Additional context
Current ADIOS2/thirdparty list (empty stays as-is):

EVPath
GTest: Not needed for installation
KWSys: Can Kitware folks @chuckatkins @bradking @KyleFromKitware provide context?
RapidJSON: target
atl
dill
enet: target
ffs
mingw-w64: not needed
nlohmann_json: target
pugixml: target
pybind11: target
yaml-cpp: target

Note: the targeted libraries seem to have a stable API, so making these tasks easier to maintain.
Input is welcome. CC: @ax3l @pnorbert

[ax3l]

Thanks, same is highly encouraged on conda-forge and spack.

[bradking]

KWSys: Can Kitware folks provide context?

KWSys is designed specifically to be configured by host projects into a unique namespace. There are no external installations. It is always deployed as part of a host project, and each such deployment is effectively a distinct library configured (both in name and features) for that host project. The upstream KWSys repository is separate only to de-duplicate source code and maintenance.

[chuckatkins]

W.r.t. KWSys, it is internal-only by design. See https://github.com/ornladios/ADIOS2/issues/2143#issuecomment-616560299

W.r.t. nlohmann_json, pugixml, and pybind11, they are header-only and tend to be very sensitive to version changes. For nlohmann_json and pybind11 specifically we have, on numerous occasions in the past few years, needed to depend on locally patched versions that upstream has not been willing to accept in order to work around compiler issues with XL and PGI. Given that there's no real burden to keeping them strictly internal and that the upstream versions have repeatedly been ones we can't use, I'm not inclined to change change that.

W.r.t. the SST dependencies from the GTKorvo stack, i.e. atl, dill, enet, evpath, ffs, we already support using external versions of those. ADIOS is rarely built that way since it's not often we're in a space where other packages share those dependencies, but it is supported via the -DADIOS2_USE_EXTERNAL_<PACKAGE_NAME> configure flags.

So I really only think this is applicable to yaml-cpp, in which case I agree that we should support an external build.

[williamfgc]

@ax3l @bradking @chuckatkins thanks for you valuable input. I will dig more about requirements, the stability of those packages and see what is acceptable for Debian admins. We are long overdue for sudo apt-get install libadios2-dev

[chuckatkins]

@KyleFromKitware has been doing Debian packaging for other projects and should be able to get it going for ADIOS.

@KyleFromKitware any insights or feedback on this?

[KyleFromKitware]

A few years ago I began work on a tool that can make Debian packaging of CMake projects easier: dh-cmake. Basically it allows you to split up install() components into different output packages, like libadios2 and libadios2-dev. This project is a perfect candidate for it. Early development of the Debian/Ubuntu package can be published on our APT repository so that developers can start using it right away.

Getting it published into Ubuntu proper is going to be trickier. It will first have to go through Debian, where it will undergo rigorous review for conformance to the Debian standards, copyright, etc. This is more of a time commitment than simply doing it ourselves. I don't know how much time, as I have not submitted a package to Debian before, but I am willing to take a crack at it.

[KyleFromKitware]

A quick look at the ADIOS code suggests that we will need to make some slight changes to the installation to make it suitable for dh-cmake, but this shouldn't be difficult at all. I can work on it.

[KyleFromKitware]

@chuckatkins, Debian is less likely to accept adios if we don't allow external dependencies for pugixml, pybind11, etc. We are more likely to get away with keeping atl, ffs, etc. internal. Something to keep in mind if we want this to go into Debian/Ubuntu proper (not a problem if we only host it on our own third-party repository.)

[KyleFromKitware]

More to the above point, it sounds like our locally patched copies are mainly an issue for PGI and XL. This point is obviously moot for Debian packaging, because we will be building with the standard GCC. The main one that worries me is pybind11... ugh. Don't even get me started. We've had a lot of issues with it in other projects that we've had to patch locally. I don't know what we're going to do about that... use our internal version and hope for the best?

[chuckatkins]

@KyleFromKitware pybind11 has been fine for gcc. We've had to patch it for other compilers but gcc is okay. In the service of packaging, it'd be good to go ahead and get them working externally where possible. Of those packages are already available then we'll likely just need to adjust our imported targets to match. It'd be great to have before the feature freeze in mid May so we can target a 2.6.0 package.

[KyleFromKitware]

My point about pybind11 was not about the compiler, but about a myriad of bugs in pybind11 itself that we've found in it that we've had to patch locally. I doubt any of these patches have made it into upstream.

[williamfgc]

I see another challenge with pugixml, the toolkit/query/XMLUtil.h uses the pugixml.hpp header which makes it hard to make an external dependency effectively PRIVATE. There is some trickery in thirdparty/pugixml to allow by providing an intermediate INTERFACE to the pugixml source code. I don't know how to do the same with an external dependency. @guj would you be OK if we can refactor those files to make pugixml effectively a private dependency? This would help official packaging efforts with Debian repos (I think that should be the long term target).

[williamfgc]

BTW, thanks @KyleFromKitware for your input. My take on this is that we should target official Debian repos for the long-term if we want to make adoption easier (those go through a vast amount of testing). It took a while, but getting an official repo and sponsor https://salsa.debian.org/science-team/adios2 was possible thanks to @ax3l .

[williamfgc]

BTW, I don't see RapidJSON being used at all. @chuckatkins should it be removed?

[chuckatkins]

BTW, I don't see RapidJSON being used at all. @chuckatkins should it be removed?

It's currently not being used but was added as a transition path away from nlohnann_json both due to performance reasons and issues with compilers we need to support for which workarounds were in conflict with upstream development policies.

[ax3l]

@KyleFromKitware re:pybind11 as far as I can see, the current copy of pybind11 in ADIOS2 is vanilla upstream with explicit control to install the library 52b42492d79175ae03c6bf52ef1df762bffb0dad.
You might want this one still upstreamed, too? https://github.com/pybind/pybind11/pull/1428 I did the leftover documentation work now that was not added to the original PR: https://github.com/pybind/pybind11/pull/2184

Checking pybind11 in master: there is a PYBIND11_INSTALL control var. Subprojects are furthermore implicitly detected via PYBIND11_MASTER_PROJECT variables in pybind11's CMakeLists.txt. Not sure if anything is missing that you need. https://github.com/pybind/pybind11/blob/f6e543b15ec0aaf39c494936440f91a16f161c9a/CMakeLists.txt#L22-L26
Do you try to install the internal pybind11 with this patch? It's header-only and consumed after a build.

There was one non-merged PGI work-around that is fixed with PGI 18.7 and newer. I reported this upstream for you to PGI, that's why it got fixed: https://github.com/pybind/pybind11/issues/1381#issuecomment-411805050 https://github.com/pybind/pybind11/pull/1447#issuecomment-502041459

What about your two PRs? Anything still applied in ADIOS2 master of those that I overlook? One is WIP, the other one seems not converged to a solution: https://github.com/pybind/pybind11/pulls/KyleFromKitware

Looking at your lingo in our current public thread as well as missing PR descriptions and inappropriate wording when contributing: maybe reduce the amount of aggression in your communication to get your points across.

dh-cmake

Really nice, thank you for this and all your other community work as well. I do appreciate this a lot and can understand things get frustrating at times.

[williamfgc]

It's currently not being used

@chuckatkins it can live in a separate branch until actual integration work is done. It would help simplify packaging efforts if it's not planned to be used in v2.6.0. Let me know, I'd be happy to provide the PR and keep it in my fork. Thanks.

[KyleFromKitware]

I see another challenge with pugixml, the toolkit/query/XMLUtil.h uses the pugixml.hpp header which makes it hard to make an external dependency effectively PRIVATE.

We can make libadios-dev depend on libpugixml-dev, which would make this less of an issue.

@guj would you be OK if we can refactor those files to make pugixml effectively a private dependency?

I can do this, it doesn't look too hard.

[KyleFromKitware]

Actually, it doesn't even look like it will be an issue. toolkit/query/XMLUtil.h is not installed in the install tree, and I don't see any targets that have pugixml or pugixml_private as a PUBLIC or INTERFACE dependency.

[williamfgc]

@KyleFromKitware I found the issue when trying to link privately an external pugixml library. See my branch here . Compilation stops at that header, since pugixml is linked under PRIVATE. Perhaps is my lack of cmake understanding, but I tend to keep PRIVATE targets outside headers. BTW, the libpugixml found on macOS from Homebrew is static.

[KyleFromKitware]

Thanks @williamfgc. I see you referenced a Launchpad bug in your code - it looks like it's been fixed in Debian Sid, and so will probably be fixed in Ubuntu 20.04.

[KyleFromKitware]

@williamfgc I just tried your branch in Ubuntu 18.04 and Debian Sid. In Ubuntu it fails due to a dependency on pugi::xml_document::xml_document(const pugi::xml_document&), but in Debian Sid it compiles fine. The pugixml version in Ubuntu 18.04 is 1.8, while in Debian Sid it's 1.10. Between that, the botched CMake export file in 1.8 (which was fixed by the time of 1.10), and the fact that we're vendoring version 1.10 anyway, makes me think that we should just require 1.10 or later.

[KyleFromKitware]

I just checked out the latest pugixml master - it looks like they made even more CMake improvements but also broke some things too. I'm going to work on fixing this.

[guj]

Sorry William, just saw this. Yes, don't worry about query part.
I will accommodate when your change is done.

I see another challenge with pugixml, the toolkit/query/XMLUtil.h uses the pugixml.hpp header which makes it hard to make an external dependency effectively PRIVATE. There is some trickery in thirdparty/pugixml to allow by providing an intermediate INTERFACE to the pugixml source code. I don't know how to do the same with an external dependency. @guj would you be OK if we can refactor those files to make pugixml effectively a private dependency? This would help official packaging efforts with Debian repos (I think that should be the long term target).

[KyleFromKitware]

Please see zeux/pugixml#349. I just fixed some issues with the CMake code and added some improvments to the install() commands.

[KyleFromKitware]

Please see #2174. This take a slightly different find_package() approach - it requires pugixml's own CMake package file to be present and correct, but adds version information since pugixml didn't export it to CMake until recently.

Basically, the approach I think we should take is to recommend that people always build ADIOS with the built-in vendored libraries except when packaging ADIOS for a distribution. That way, if the distro ships a bad CMake package file (like Ubuntu 18.04 seems to do) the user should just use the built-in pugixml. However, if they're packaging for a distro, then the distro should fix the CMake package file (as Debian Sid has already done.)

[williamfgc]

@KyleFromKitware thanks a lot. It's looking great, I appreciate you're putting the time on this. I have a PR #2172 but your approach is better as I'm far from a cmake expert, I can drop it. For the most part those libraries are becoming stable over time as they are widely used.

@guj sounds great, thanks! I might as well move things to a helper as it's the same functions as in adiosXML.cpp

[williamfgc]

Basically, the approach I think we should take is to recommend that people always build ADIOS with the built-in vendored libraries except when packaging ADIOS for a distribution.

Correct, the main driver is packaging for standard distributions. Users building from source or master should stick with the current model.

[chuckatkins]

@KyleFromKitware Before the set of PRs addressing this are merged I'd like to get a serial and MPI CI build in place for whatever distro we're targeting with all of the external dependencies installed as system packages in the image. That way as we enable them externally the configurations will be adequately tested.

W.R.T. the build configuration of those images, given the extent of test coverage in the other CI builds I think it's fine to restrict those builds to just the interface and install tests as to not bog down our CI resources.

[KyleFromKitware]

@chuckatkins In that case I will combine all of those PRs into one and add the CI build there as well.

[williamfgc]

@KyleFromKitware @chuckatkins if it helps, we are targeting debian, homebrew, spack (on Linux and macOS) and conda-forge (Linux, macOS and Windows, see here ). We currently don't have anything going on Fedora rpm based systems, but would be nice to add in the near future.

[KyleFromKitware]

@eisenhauer How does the release process work for the GTkorvo stack? Do you cut releases for them at the same time you cut an ADIOS release? It looks like we'll probably be able to bend Debian's vendoring rules for the GTkorvo stack, but I would like to have a contingency plan for packaging them separately.

[eisenhauer]

Since we have had them pulled into ADIOS, I haven't worried about a separate release for GTkorvo correlated with the ADIOS release, but happy to do whatever is appropriate..

[KyleFromKitware]

I see you referenced a Launchpad bug in your code - it looks like it's been fixed in Debian Sid, and so will probably be fixed in Ubuntu 20.04.

Confirmed. The fix is in 20.04.

[KyleFromKitware]

Something occurred to me last night. Because the Debian build will build ADIOS three times, that means three builds of atl, enet, ffs, etc. Therefore, I think they will need to be split into separate packages after all. I'm going to start working on packaging them as-is, but please be prepared to cut a release of them just before the next ADIOS release.

[KyleFromKitware]

@eisenhauer, can you give me a run-down of what you changed in the GTkorvo fork of enet (besides the obvious addition of the CMake buildsystem)? I need to know so I can justify its inclusion into Debian.

[eisenhauer]

Sure:
1) added a call, enet_host_get_sock_fd() to expose the internal FD (for use in external select()).
2) modified enet to support dynamic sizing of the internal peer array (previous was fixed at init time).
3) changes to protocol to support 24-bit peer IDs.

[KyleFromKitware]

1. changes to protocol to support 24-bit peer IDs.

So, in that case, the fork is not compatible with the original, and can't act as a replacement. Good to know, thanks for the insight.

[KyleFromKitware]

@ax3l I've been told that you got a mentors repo on Debian for ADIOS. Could you point me to this, and possibly get me in touch with someone at Debian willing to sponsor this? I've done a bunch of packaging work already, you can find it at https://gitlab.kitware.com/debian/adios2.

[ax3l]

Hi @KyleFromKitware, William got in contact with the current ADIOS maintainer in Debian (Alastair McKinstry) to sponsor our new package.

A repo from which we can draft PRs to Debian's science team is set up here: https://salsa.debian.org/science-team/adios2

So far, Drew Parsons from the Debian science team helped us with the setup on their gitlab. Do you have an account on https://salsa.debian.org so we can add you to the repo for the PR to their mainline?

[KyleFromKitware]

William got in contact with the current ADIOS maintainer in Debian (Alastair McKinstry)

Oh, that's the same guy who's sponsoring dh-cmake for me.

Do you have an account on https://salsa.debian.org so we can add you to the repo for the PR to their mainline?

I'll get one set up.

[KyleFromKitware]

A repo from which we can draft PRs

To be honest, it will probably be easier if we simply take all of the Git history from gitlab.kitware.com and migrate it to Salsa. Salsa appears to only have a readme at the moment, so we wouldn't lose anything.

[ax3l]

Absolutely, due to repo rights we just had issues creating new branches and asked for an explicit one.

[ax3l]

Ah, I just remembered: the modus operandi is that you do a PR to https://salsa.debian.org/science-team/adios2 .

Only debian science has access to this repo. Just fork it and MR against them. That's why we needed a branch, because we cannot open MRs without branches in the mainline...

[KyleFromKitware]

Looks like we raced here... I just opened https://salsa.debian.org/science-team/adios2/-/issues/1 to request migration from gitlab.kitware.com.

[ax3l]

That would be even better. I'll ping again the folks that helped us.

[williamfgc]

@KyleFromKitware please feel free to contribute to the salsa repo. Do you think the current adios2 upstream is ready for Debian packaging? It took a while to get that repo going thanks to @ax3l. Please keep us in the communication loop with the Debian admins. We want to get this going and we'll need their full support as none of us is an admin. Let's accommodate to their requests. Thanks!

[KyleFromKitware]

Do you think the current adios2 upstream is ready for Debian packaging?

I think upstream is ready. Worst case scenario, if I run into issues, I can patch them in the packaging and then contribute the patches here so they're ready in the next release.