Add openpmix, prrte

[minrk]

currently bundled as dependencies of openmpi

related to #27988 which adds libfabric, also currently bundled as part of multiple mpi providers.

Once these and libfabric are packaged and built on all platforms, we can add them as dependencies of openmpi.

Each library has 3 outputs:

• libpmix, libprrte have libraries, most files (config in etc, help/error messages in share)
• pmix-bin, prrte have executables in bin/
• libpmix-devel, libprrte-devel have headers. I don't normally separate these, but both packages install a disproportionate number of headers in e.g. include/pmix/src

openmpi will depend on prrte-bin and libpmix, but not pmix-bin and not (directly) libprrte.
Currently, the openmpi package installs as if all of these packages are present (bin, lib, devel).

FWIW, ubuntu has libpmix, libpmix-bin, and libpmix-dev. Ubuntu does not appear to package prrte separately from openmpi. Fedora has pmix (libs), pmix-tools (bin), and pmix-devel, and prrte (bin), prrte-libs (lib), and prrte-devel.

xref conda-forge/openmpi-feedstock#178

cc @leofang @dalcinl @jakirkham @j34ni who have been participating in related discussions, if anyone wants to chime in or join as a maintainer

[github-actions]

Hi! This is the staged-recipes linter and your PR looks excellent! 🚀

[conda-forge-admin]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/prrte/recipe.yaml, recipes/openpmix/recipe.yaml) and found it was in an excellent condition.

[j34ni]

Hi @minrk ,
I would be happy to contribute
In fact I did work on a PMIx recipe before realizing that someone had already done https://github.com/openpmix/pmix-feedstock but apparently not more in the last 8 months, a bit weird isn't it?

[j34ni]

@minrk If there is no known use case to have the binaries, then I would suggest to have a package with only the libraries, and to reflect that explicitly in the name (like libpmix)

[j34ni]

@minrk I cloned your repo and I am trying to fix the recipe to add missing dependencies but I get a lot errors when building locally using python3 build-locally.py (about syntax with the $ and % that should not be there, the recipe.yaml that should be called meta.yaml, something about CI not being defined, etc.): can I ask what you are using to build since you managed to make it go that far whereas when for me it fails straight away?

[minrk]

@j34ni that's great, thank you! I've been trying out rattler-build for new recipes, so to build linux64, run:

rattler-build build -r recipes/openpmix -m .ci_support/linux64.yaml

But I've discovered working on this one that it doesn't yet have sufficient support for split-output recipes (at least until conda-forge/conda-smithy#2057 is addressed so we can use this, so I'll switch back to conda-build if we want to separate lib from bin packages, which I think makes sense.

[conda-forge-admin]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/prrte/meta.yaml, recipes/openpmix/meta.yaml) and found it was in an excellent condition.

[conda-forge-admin]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/openpmix/meta.yaml, recipes/prrte/meta.yaml) and found it was in an excellent condition.

[j34ni]

@minrk I do not understand: without automake and autoconf in the requirements my local build fails, anyway is {{ compiler('cxx') }} of any use?

[minrk]

Good catch, I removed cxx, I just assumed it would be needed.

autotools should not generally be needed to build ~anything from source unless you need to ship patches for autoconf files. I did have to add this touch to prevent autotools being invoked because there appears to be something wrong with the timestamps in the pmix tarball. But the builds here on CI and my local builds don't invoke autotools. I'm not sure what would cause your builds to trigger it. Can you share output?

[j34ni]

Do not bother if it builds on Azure as it is
The error I have without automake/autoconf is about the build process failing because the aclocal-1.16 command is not found

[minrk]

Yeah, that's exactly the issue fixed by the touch in these builds here. The dependency check is deciding (incorrectly) that autotools needs to regenerate configure or some other files, which it does based on the timestamps of the generated files and the files used to generate them. A touch on the generated files should convince it the files are up-to-date and prevent the invocation of autotools. Unfortunately, I don't know why it's coming to a different decision some places and not others. If you do a ls -la in the checkout directory, you might see which files have the wrong timestamps on them.

[danielnachun]

The overall structure here looks good to me - I just made some minor suggestions for improvements.

[danielnachun]

Comments should be removed from the meta.yaml - if there are TODOs to track, open an issue when the feedstock is created to track these. Other wise the recipe becomes difficult to parse programmatically and contributors have to open the meta.yaml to see what needs to be done instead of seeing a separate issue.

[minrk]

This is the only place we have to write a todo because the feedstock doesn't exist to open issues, I can remove it once the feedstock is created, since it is only here to workaround a bug in staged recipes, but there's nowhere else to put it for now.

Can I also ask where the idea that all comments should be discouraged in meta.yaml is coming from? I don't think it's true or a good practice and I haven't seen it suggested before. Comments help make clear, maintainable recipes, and should generally be encouraged.

[danielnachun]

I guess there isn't a clear consensus (at least going by documentation) about the use of comments in meta.yaml specifically. Certainly in build.sh/bld.bat scripts comments are strongly encouraged as those files are in procedural languages (shell scripting and Windows batch scripting) and can be very complex and variable. But in principle meta.yaml is supposed to be "declarative" following a very clear, well documented specification that should hypothetically not require much or any commenting to understand.

Having said that I very much appreciate your perspective on this! I'm trying to compile more complete documentation on recipe best "practices" and an important point of discussion should be the role of comments in meta.yaml or the new recipe.yaml for v2 recipes.

[minrk]

Thanks! Hopefully those best practices will encourage code comments for clear, maintainable recipes :)

[danielnachun]

While these kinds of comments may be helpful in development, they should be removed for the final meta.yaml.

[minrk]

Since you mentioned documenting best practices, I'll provide a bit of background on why I think these (and other similar comments) should stay, and indeed be actively encouraged in best practices for recipes across conda-forge, in case it's helpful to your writing.

Most recipes are maintained in a tiny fraction of maintainers' time, and are often interacted with very infrequently, sometimes months or years between changes. Any deviation from the simplest recipes or bits that prompt "why is this here?" should have comments explaining what's going on. In this way, meta.yaml the same as any other code or config. Many times over the years, I've seen (and made!) what look like innocuous 'clean up' changes to recipes because something odd or unusual in meta.yaml was not explained, its rationale not remembered and not self-evident, and then removed or changed in a way which ultimately broke the recipe (examples include host dependencies that aren't linked, but required for build to succeed, parts of the recipe where changes must be kept in sync with another part of the recipe, etc.). Comments help explain these things and help maintainers keep recipes in good health, since it should not be expected that maintainers have all the history and context of the recipe in mind when it's time to make a contribution.

As a specific example, this particular comment helps maintainers and reviewers understand what should be packaged in which output vs excluded entirely, and what needs to be kept in sync. For example, if these exclusions are changed it is likely the -bin output inclusions are likely to need changes as well.

[conda-forge-admin]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/prrte/meta.yaml, recipes/openpmix/meta.yaml) and found it was in an excellent condition.

[carterbox]

The upstream project seems to be aware of sonames and provides separate ABI and API versions. However, the sonames of the binaries built here don't match what upstream claims they should be in their VERSION files. Maybe the project is configured incorrectly?

https://github.com/openpmix/openpmix/blob/20ad9b16fe49ede4a76e7489bdf0e2ea05c3de95/VERSION#L122
https://github.com/openpmix/prrte/blob/v3.0.7/VERSION#L92C1-L93C1

Also, I think the comments added to this recipe are appropriate. Comments are valid yaml, so I would expect the only time they conflict with parsing of the recipe is if the comments are in-line like selectors instead of above-line like you have here.

[minrk]

However, the sonames of the binaries built here don't match what upstream claims they should be in their VERSION files.

What version mismatch do you see? pmix is producing libpmix.2.13.4.so, which is what you get from the (arcane) libtool version input of 15:4:13 -> (15-13).13.4, and same for prrte where 3:7:0 gives (3-0).0.7.

[carterbox]

What version mismatch do you see?

Maybe I don't understand libtool versioning. I thought that the sonames should be literally:

libprrte_so_version=3:7:0
libpmix_so_version=15:4:13

But I will defer to you.

Can we use the soname in the package name instead of pinning run_exports to the minor API version? e.g. libprrt3 and libpmix2.

[dalcinl]

Maybe I don't understand libtool versioning. I thought that the sonames should be literally:

That's definitely NOT the way libtool versioning works.

Can we use the soname in the package name instead of pinning run_exports to the minor API version? e.g. libprrt3 and libpmix2.

That may be a good idea for lib* packages, I believe Debian uses a similar approach. However, pinning is still needed. When new APIs are added in a backward compatible way, the SONAME version does not change. How do you prevent an old incompatible version from being installed if not by pinning?

Then, what's the point of adding the soname to the package name? IMHO the the only reason to have libpmix2 is to be able to install both libpmix1 and libpmix2 in the same conda environment. Such thing may be a reasonable expectation for a Linux distro installing multiple libraries system-wide, but I think such feature is less useful within the context of throw away conda environments. I'm not opposed to it, but I wanted to point out it may not be that useful in the end.

[carterbox]

How do you prevent an old incompatible version from being installed if not by pinning?

By "instead of pinning [...] to the minor API version" I meant letting max_pin be the major version (which is the default behavior) instead of restricting max_pin to the minor version. That wasn't very clear of me, sorry.

In my experience, adding the soname to the package name lets you use looser pinning on the API version because the package name change will protect you from ABI breaks if a library breaks the ABI without breaking the API. A concrete example, one could break an ABI by changing the memory layout of a struct without breaking the API which (in my experience) is something that image/video codec libraries tend to do.

[minrk]

Yeah, libtool has wild versioning, but in short the libtool version triplet is (current, revision, age), which maps to filenames/semver as (current-age, age, revision).

I appreciate the ABI package name suggestion. I do think it would have to contain libpmix.so.2 and not libpmix.so, which would move to devel. I've been meaning to try to pursue a documented strategy of more consistent, explicit ABI tracking across conda-forge, but that's a bigger discussion and I'm reticent to start adopting a whole new ABI-tracking standard in this recipe that I think has no precedent across conda-forge (or are there libsomethingN packages already? libgfortran5 is the only one I can recall). I think the far more likely case for these particular packages that take care to consider their ABI is that a major package release doesn't break the ABI than a minor release does, as is about to happen with libfabric 2.0.

Other packages like cpython track the abi as its own metapackage, like python_abi and pybind11-abi.

[carterbox]

I do think it would have to contain libpmix.so.2 and not libpmix.so, which would move to devel.

Yes. That's exactly correct.

I think this PR is ready to merge. Please post an explicit decision about whether you want libpmix2 before merging. Since, if you decide to do so later, you probably need a repo patch.

[minrk]

Yeah, I don't want to do the soversion package until there's a clearer agreement from conda-forge core on how to approach ABI tracking. I opened conda-forge/conda-forge.github.io#2401 to start that discussion.

Depending on what approach we take, repodata patches may or may not be needed, but I don't anticipate it happening any time soon. If we go with libpmix2, I don't think any patches would be needed.