search

conda-forge / openmpi-feedstock

A conda-smithy repository for openmpi.
BSD 3-Clause "New" or "Revised" License
9 stars 22 forks source link

add wrapper compiler flags in CONDA_BUILD, install .mod in include #158

Closed minrk closed 4 weeks ago

minrk commented 1 month ago

openmpi appears to be the only fortran package that puts .mod files in $PREFIX/lib, which is not found by default, so move them to include like everybody else, so gfortran finds them with standard include flags.

Without this, some situations (only found so far in mac cross-compile) need -I$PREFIX/lib to find fortran modules.

also ensures that OMPI_CC=$CC and friends are set in conda-build, which is generally necessary for cross-compilation and should now be more likely to work by default.

found via https://github.com/conda-forge/scalapack-feedstock/pull/34

conda-forge-webservices[bot] commented 1 month ago

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 (recipe) and found it was in an excellent condition.

dalcinl commented 1 month ago

~@minrk What about LDFLAGS ?~

I should have read the full diff... sorry for the noise.

msarahan commented 3 weeks ago

I hate to be "that guy" who comes in and says "this broke me," but I'm even worse: "that guy, who's coming in on behalf of others, saying "this broke them."" Cugraph is having to pin build numbers to avoid this change: https://github.com/rapidsai/cugraph/issues/4474

It looks to me like this change affects CMake somehow, which is what breaks cugraph. Does it make sense to try to add a test in this recipe that has a simple CMakeLists.txt project that calls FindMPI from an env with this package in it?

We can contribute this if you'd like, but I admit I'm not super familiar here and it may take me some time.

leofang commented 3 weeks ago

Contribution is welcome! Although it's unclear yet to me what's causing the issue. By a quick glimpse of FindMPI, it seems it's indeed expected that .mod files should be found in the include dir: https://github.com/Kitware/CMake/blob/48827cd746895da872e3f5834ed749439e83e77e/Modules/FindMPI.cmake#L1184-L1189 so it could be something else.

leofang commented 3 weeks ago

@minrk i noticed the new build activate script (that's supposed to be run only in conda-forge CI) is also now run in RAPIDS CI (because they use mambabuild). Can we tighten the check condition to check more than CONDA_BUILD? Maybe it's causing issues. https://github.com/rapidsai/cugraph/actions/runs/9402241384/job/25895993301#step:7:722

minrk commented 3 weeks ago

No apologies necessary, sorry for causing trouble and thanks for reporting!

If you can share debug output, that would help. I can't actually find my way to what's actually failing with all the indirection and abstraction in that CI setup, so I don't know what's causing it to fail.

Are any of the failing builds cross compiling?

I think passing through the compiler flags may have been the wrong choice, and puts way more in the base compiler wrapper command than is there by default. Perhaps we should roll those back and leave just the compiler envs themselves and OPAL_PREFIX, which I think are very unlikely to cause a problem, are sensible defaults, and easily overridden (or unset). None of those are conda-forge specific, I think.

Is it possible to test if unsetting :

unset OMPI_CPPFLAGS
unset OMPI_CFLAGS
unset OMPI_CXXFLAGS
unset OMPI_FCFLAGS
unset OMPI_LDFLAGS

fixes it?

I think perhaps the compiler env should only be set for cross-compilation, where we know the default values are wrong. THat's what #159 does.