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

openmpi v5.0.4 #169

Merged
merged 7 commits into from
Jul 30, 2024
Merged

openmpi v5.0.4 #169

merged 7 commits into from
Jul 30, 2024

Conversation

regro-cf-autotick-bot
Copy link
Contributor

@regro-cf-autotick-bot regro-cf-autotick-bot commented Jul 19, 2024

closes #168

It is very likely that the current package version for this feedstock is out of date.

Checklist before merging this PR:

  • Dependencies have been updated if changed: see upstream
  • Tests have passed
  • Updated license if changed and license_file is packaged

Information about this PR:

  1. Feel free to push to the bot's branch to update this PR if needed.
  2. The bot will almost always only open one PR per version.
  3. The bot will stop issuing PRs if more than 3 version bump PRs generated by the bot are open. If you don't want to package a particular version please close the PR.
  4. If you want these PRs to be merged automatically, make an issue with @conda-forge-admin,please add bot automerge in the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.
  5. If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

Pending Dependency Version Updates

Here is a list of all the pending dependency version updates for this repo. Please double check all dependencies before merging.

Name Upstream Version Current Version
libnl 3.10.0 Anaconda-Server Badge
openmpi 5.0.4 Anaconda-Server Badge

Dependency Analysis

We couldn't run dependency analysis due to an internal error in the bot, depfinder, or grayskull. :/ Help is very welcome!

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by - please use this URL for debugging.

@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/meta.yaml) and found it was in an excellent condition.

@dalcinl
Copy link
Contributor

dalcinl commented Jul 21, 2024

The broken build in ARM64 needs this patch https://patch-diff.githubusercontent.com/raw/open-mpi/ompi/pull/12694.patch

cf. open-mpi/ompi#12694

@regro-cf-autotick-bot regro-cf-autotick-bot mentioned this pull request Jul 23, 2024
3 tasks
@dalcinl
Copy link
Contributor

dalcinl commented Jul 23, 2024

@isuruf @minrk I'm having issues in macOS arm64. The issue may be related to gfortran, I'm seeing the following version mix in the logs, is this correct?

2024-07-23T06:53:54.3522480Z The following NEW packages will be INSTALLED:
2024-07-23T06:53:54.3523880Z 
...
2024-07-23T06:53:54.3541070Z     gfortran_impl_osx-64:               13.2.0-h2bc304d_3          conda-forge
2024-07-23T06:53:54.3541720Z     gfortran_impl_osx-arm64:            12.3.0-h4f4d140_3          conda-forge
2024-07-23T06:53:54.3542900Z     gfortran_osx-arm64:                 12.3.0-h64a2375_1          conda-forge
...
2024-07-23T06:53:54.3551040Z     libgfortran-devel_osx-64:           13.2.0-h80d4556_3          conda-forge
2024-07-23T06:53:54.3553300Z     libgfortran-devel_osx-arm64:        12.3.0-hc62be1c_3          conda-forge
2024-07-23T06:53:54.3553850Z     libgfortran5:                       13.2.0-h2873a65_3          conda-forge
...

@minrk
Copy link
Member

minrk commented Jul 29, 2024

I'm not sure what's changed or where the fix is, but the test for how to link pthreads with gfortran is not respecting LDFLAGS:

configure:121920: checking if Fortran compiler and POSIX threads work with -pthread
configure:122008: arm64-apple-darwin20.0.0-clang -DNDEBUG -ftree-vectorize -fPIC -fstack-protector-strong -O2 -pipe -isystem /Users/runner/miniforge3/conda-bld/openmpi-mpi_1721717413781/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -fdebug-prefix-map=/Users/runner/miniforge3/conda-bld/openmpi-mpi_1721717413781/work=/usr/local/src/conda/openmpi-5.0.4 -fdebug-prefix-map=/Users/runner/miniforge3/conda-bld/openmpi-mpi_1721717413781/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho=/usr/local/src/conda-prefix -isysroot /Applications/Xcode_14.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -finline-functions -I. -c conftest.c
configure:122011: $? = 0
configure:122015: arm64-apple-darwin20.0.0-gfortran  -pthread conftestf.f conftest.o -o conftest  -Wl,-commons,use_dylibs 
ld: library not found for -lgfortran

which should have -L$PREFIX/lib in there (it is present in $LDFLAGS). I'm not sure where this is happening or how to override it, but somehow LDFLAGS is getting overridden to:

LDFLAGS=' -Wl,-commons,use_dylibs'

for some step, which causes all gfortran linking to fail.

Finding where that override is and why it's clobbering $LDFLAGS rather than extending it might help us figure out if there's another env variable to set or if there's a bug in openmpi configure. The logs of the empty string and space make me thing there could be something like an env variable typo where something is expecting "$LDFLAGS -Wl,-commons,use_dylibs" buti it's actually "$LD_TYPO -Wl,-commons,use_dylibs" or something like that.

I think it makes sense that only cross compilation is affected, since non-cross compilation will tend to find libgfortran even if these args are missing.

@minrk
Copy link
Member

minrk commented Jul 29, 2024

I think there's a typo in open-mpi/ompi#12650, specifically LDFLAGS is saved in LDFLAGS_save_xcode and restored from LDFLAGS_xcode_save, which would have the equivalent effect of setting $LDFLAGS='', which looks exactly like what we are seeing. Testing a patch in #170. If that fixes it, I'll submit it upstream.

@dalcinl
Copy link
Contributor

dalcinl commented Jul 29, 2024

@minrk Yes! I just found it too, but you beat me to announce the discovery.

@dalcinl
Copy link
Contributor

dalcinl commented Jul 29, 2024

@minrk Can we make make all the needed changes to get this version published?

@minrk
Copy link
Member

minrk commented Jul 29, 2024

This now has the patch and hopefully the right dependencies. I left out the moduledir part, to try to focus on just one change at a time. Not sure when you think is the right time for that one.

@dalcinl
Copy link
Contributor

dalcinl commented Jul 29, 2024

You can add the moduledir thing here too, if it is OK with you.

official, no longer need symlinks
@minrk
Copy link
Member

minrk commented Jul 29, 2024

added moduledir patch as well

recipe/build-mpi.sh Outdated Show resolved Hide resolved
Co-authored-by: Lisandro Dalcin <dalcinl@gmail.com>
dalcinl
dalcinl previously approved these changes Jul 30, 2024
@dalcinl dalcinl dismissed their stale review July 30, 2024 09:46

change my mind

recipe/build-mpi.sh Outdated Show resolved Hide resolved
recipe/build-mpi.sh Show resolved Hide resolved
@minrk minrk merged commit 89f6315 into conda-forge:main Jul 30, 2024
10 checks passed
@regro-cf-autotick-bot regro-cf-autotick-bot deleted the 5.0.4_ha51f19 branch July 30, 2024 15:40
@isuruf
Copy link
Member

isuruf commented Jul 30, 2024

In the conda world we disable new-dtags everywhere and add --disable-new-dtags in LDFLAGS because lots of users have LD_LIBRARY_PATH set in their bashrc because of someone on the internet told them to. We even patch distutils/setuptools to not use new drags.

@dalcinl
Copy link
Contributor

dalcinl commented Jul 30, 2024

@isuruf OK, so there we have the explanation... decisions made to favor the clueless and unexperienced over the professional. The problem is that in the particular case of RPATH/RUNPATH entries in ELF binaries, the --disable-new-dtags option renders LD_LIBRARY_PATH adjustments practically useless.

@isuruf
Copy link
Member

isuruf commented Jul 31, 2024

The problem is that in the particular case of RPATH/RUNPATH entries in ELF binaries, the --disable-new-dtags option renders LD_LIBRARY_PATH adjustments practically useless.

Yes, that is a problem. In that case, LD_PRELOAD is a good solution. You can also change the binary if you want. An experienced user will be able to figure this out, while an inexperienced user can easily trip over LD_LIBRARY_PATH issues.

In any case, I don't think individual packages like openmpi should override the default LDFLAGS in conda-forge and decisions should be made globally.

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.

Configure --with-mpi-moduledir='${includedir}'
4 participants