JuliaPackaging / Yggdrasil

Collection of builder repositories for BinaryBuilder.jl
https://binarybuilder.org
Other
308 stars 554 forks source link

Incorrect MPI augmentation if binary == "system" #6893

Open simonbyrne opened 1 year ago

simonbyrne commented 1 year ago

For libraries which have an MPI dependency, we augment based in the MPIPreferences abi key: https://github.com/JuliaBinaryWrappers/HDF5_jll.jl/blob/b96de8ada558f8d70e27b5561d4f5df815b01ebf/.pkg/platform_augmentation.jl#L13

However we augment by loading the corresponding JLL. e.g. the augmentation for abi = "openmpi" always loads OpenMPI_jll: https://github.com/JuliaBinaryWrappers/HDF5_jll.jl/blob/main/src/wrappers/x86_64-linux-gnu-libgfortran5-cxx03-mpi%2Bopenmpi.jl#L9

The problem is if someone uses binary = "system": then MPI.jl will load the system binary, but downstream dependencies (e.g. HDF5_jll) will load the corresponding MPI JLL binary, causing problems: https://github.com/JuliaIO/HDF5.jl/issues/1079

There are a couple of potential fixes here:

  1. Drop support for binary = "system" and require everyone to use MPItrampoline?
  2. Override the corresponding JLLs instead (https://docs.binarybuilder.org/stable/jll/#Overriding-specific-products)
    • the problem is that JLLs expect specific .so names, so it's not clear that this would work generally (since MPI libraries can be arbitrarily named)
  3. Build deps without MPI support if binary == "system"
    • this is feasible if MPI is optional (e.g. HDF5), but maybe not for all.

cc: @giordano @staticfloat @mkitti @vchuravy

mkitti commented 1 year ago

My sense is that option 2 is the path of least resistance at the moment. So name issues can be addressed by overriding a consistent set of files and symlinks.

giordano commented 1 year ago

I'm not sure option 2 is going to work well given the problem with the soname

vchuravy commented 1 year ago

Yeah my stance on this (which is not satisfying) is that the user of system must also use preferences to change the libraries of their dependency.

Long-term goal was to get some decent Spack integration going to make this feasible...

staticfloat commented 1 year ago

Option 2 works as long as one of the following two options is true:

  1. All relevant JLLs are overridden to system versions. You could make sure this is the case by getting a list of all JLLs in a manifest, but filtering them to only include transient dependees of the JLL you're interested in, then ensuring that they are all overridden.
  2. All BB-provided JLLs link against a sufficiently generic SONAME. This is the case for many of our dependencies, but it's hard to say without a more detailed analysis. If someone wants to actually look into this, it's not too hard to collect a bunch of JLLs, download them, then sic ObjectFile's DynamicLinks() function to figure out what they actually link against.

Fundamentally, a change in SONAME means that the ABI has a decent chance of being incompatible, so I'm not sure it makes sense to try and work around that, I think it's kind of playing with fire. So the best option, in my mind, is to build up tooling for setting preferences en-masse to picking up system library versions.

If someone wants to look into this, I'm happy to give some guidance on how I think it should work.

simonbyrne commented 1 year ago

Could we change this: https://github.com/JuliaBinaryWrappers/HDF5_jll.jl/blob/b11cbc14cd22353ed44891395858fbcf514811bd/src/wrappers/x86_64-linux-gnu-libgfortran5-cxx03-mpi%2Bopenmpi.jl#L9 to something like

using MPIPreferences
if MPIPreferences.binary == "system"
    using MPIPreferences.System
else
    using OpenMPI_jll
end