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

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

Merged
merged 10 commits into from
Jun 6, 2024

Conversation

minrk
Copy link
Member

@minrk minrk commented Jun 5, 2024

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 conda-forge/scalapack-feedstock#34

minrk added 2 commits June 5, 2024 11:35
set OMPI_ compiler wrapper flags from conda-build compiler flags
like every other fortran package
@conda-forge-webservices
Copy link
Contributor

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.

recipe/run_test.sh Outdated Show resolved Hide resolved
minrk added 4 commits June 5, 2024 13:41
@dalcinl
Copy link
Contributor

dalcinl commented Jun 5, 2024

@minrk What about LDFLAGS ?

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

recipe/build-mpi.sh Outdated Show resolved Hide resolved
@minrk minrk force-pushed the activate-flags branch from 9aeca5f to f61a0b2 Compare June 6, 2024 07:22
@minrk minrk merged commit 99059c0 into conda-forge:main Jun 6, 2024
10 checks passed
@minrk minrk deleted the activate-flags branch June 6, 2024 08:47
@msarahan
Copy link
Member

msarahan commented Jun 7, 2024

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: rapidsai/cugraph#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
Copy link
Member

leofang commented Jun 7, 2024

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
Copy link
Member

leofang commented Jun 8, 2024

@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

Comment on lines +3 to +14
# build-time variables
for _var in CC CXX FC CPPFLAGS CFLAGS CXXFLAGS FCFLAGS LDFLAGS; do
if [[ ! -z "${!_var:-}" ]]; then
echo "OMPI_${_var}=${!_var}"
export OMPI_${_var}="${!_var}"
fi
if [[ -z "${OMPI_FCFLAGS}" && ! -z "${FFLAGS}" ]]; then
echo OMPI_FCFLAGS="-I$PREFIX/include $FFLAGS"
export OMPI_FCFLAGS="-I$PREFIX/include $FFLAGS"
fi
done
export OPAL_PREFIX="$PREFIX"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these are causing issues?

@minrk
Copy link
Member Author

minrk commented Jun 8, 2024

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.

raydouglass pushed a commit to rapidsai/cugraph that referenced this pull request Jun 10, 2024
Addresses #4474 

Currently `openmpi=5.0.3-hfd7b305_105` is blocking our CI `cpp_build`
job.

Most likely introduced by this PR:
conda-forge/openmpi-feedstock#158

This PR will unblock cugraph development until the issues are fixed.
Once that happens, the version pinning should be removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants