openmpi: 4.1.6 -> 5.0.3 by doronbehar · Pull Request #327438 · NixOS/nixpkgs

doronbehar · 2024-07-15T17:47:16Z

Description of changes

Things done

Add a 👍 reaction to pull requests you find important.

markuskowa · 2024-07-15T19:15:56Z

Duplicate of #275796

doronbehar · 2024-07-15T21:01:39Z

The GitHub search has disappointed me...

https://github.com/NixOS/nixpkgs/pulls?q=openmpi%205.0

@markuskowa perhaps you'd be interested in incorporating the changes here into #275796 ? The declarative approach in the reformat commit in this PR is a significant improvement IMO.

markuskowa · 2024-07-15T21:16:38Z

The GitHub search has disappointed me...

https://github.com/NixOS/nixpkgs/pulls?q=openmpi%205.0

Yep, finding things in this big list of PRs becomes increasingly difficult.

@markuskowa perhaps you'd be interested in incorporating the changes here into #275796 ? The declarative approach in the reformat commit in this PR is a significant improvement IMO.

Yes, although the update PR is already quite big (mostly due to the split output/.dev feature). I would like to get the update merged. This has been sitting around for way too long. I could easily integrate the finalAttrs and the withFeatureAs/enableFeature. More changes could go in a follow-up PR?

doronbehar · 2024-07-15T23:13:03Z

You did great job in #275796 but I managed to include the details I missed in this PR, and it came out nicer IMO - both in terms of the commit log, and the diff and the implementation details (also no merge conflict as well).

I also disentangled the plumed blas64 commit to a separate as that has no effect or isn't affected by the openmpi so it seems:

#327503

As for the mpi4py issue, I'm not sure, I'm still investigating whethere there are packages that actually, really, use this package - I suspect that many of the packages that depend upon it don't really use any functionality from there.

markuskowa · 2024-07-16T08:50:48Z

@GrahamcOfBorg build python3Packages.meep
@GrahamcOfBorg test slurm

markuskowa · 2024-07-16T08:58:13Z

@doronbehar thanks for integrating my changes in this PR. This looks pretty good now. Let me run nixpkgs-review on it to make sure we have everything covered.

On request: please put the reformatting in its own commit (i.e. put "and more" in a separate commit). That makes the changes easier to read.

doronbehar · 2024-07-16T09:58:29Z

On request: please put the reformatting in its own commit (i.e. put "and more" in a separate commit). That makes the changes easier to read.

Working on it now.

doronbehar · 2024-07-16T10:23:31Z

On request: please put the reformatting in its own commit (i.e. put "and more" in a separate commit). That makes the changes easier to read.

Working on it now.

Done.

doronbehar · 2024-07-16T10:23:46Z

@ofborg build cp2k python3Packages.meep

doronbehar · 2024-07-16T13:22:51Z

CI was green for aarch64-linux before this last force push. I noticed a small mistake in the commit log that is fixed now.

doronbehar · 2024-07-16T15:26:17Z

I'd suggest to handle #327444 before merging this.

SuperSandro2000

can't say much about most other changes

markuskowa · 2024-07-16T19:02:25Z

mpi4py requires a test to be disabled: c2e58ed
btw: mpi4py is the reason the openmpi update was blocked for so long. Otherwise, we could have merged the other PR 6 months ago.

doronbehar · 2024-07-16T20:24:12Z

mpi4py requires a test to be disabled: c2e58ed btw: mpi4py is the reason the openmpi update was blocked for so long. Otherwise, we could have merged the other PR 6 months ago.

@markuskowa thanks for explaining. Indeed mpi4py was is in a problematic state. Let's deal with that in #327444 .

doronbehar · 2024-07-22T14:24:05Z

@markuskowa I rebased this on top of latest master (with #327444 merged) and tested that mpi4py builds fine, and I also checked python311Packages.h5py-mpi (which runs tests that use python312Packages.mpi4py and python312Packages.pytest-mpi), and it built fine as well. Waiting for your review.

Less confusing when to use a list and a single argument.

…outputs

Using substituteInPlace (with `--replace-fail`) is much safer.

https://docs.open-mpi.org/en/v5.0.x/release-notes/changelog/v5.0.x.html Make build reproducible in a different, nicer manner Co-authored-by: Markus Kowalewski <markus.kowalewski@gmail.com>

Co-authored-by: Markus Kowalewski <markus.kowalewski@gmail.com>

doronbehar · 2024-08-06T16:55:57Z

Fixed a merge conflict after 63d05d9 .

@doronbehar I'll take a look at it next week. Sorry for the delay.

Still waiting :)

markuskowa

I checked and it does look good now. Let's get this merged.

Only one note: I am not a big fan of the "Handle all compiler wrappers consistently & declaratively". It is more complicated than the old shell code version, which is easier to read.

doronbehar · 2024-08-07T06:13:50Z

I checked and it does look good now. Let's get this merged.

Only one note: I am not a big fan of the "Handle all compiler wrappers consistently & declaratively". It is more complicated than the old shell code version, which is easier to read.

Hmmm that is the highlight of this PR haha :). I agree it is less readable, but I think it is much harder to maintain a manually written list of 7x3 commands that look almost the same. Also it was annoying for me when I tried to debug this part in the old way, that I had to fully build the package every time I wanted to slightly change something there, due to small typos etc.. No wonder so many files weren't handled in that phase before this commit. With the new code, there should be no typos.

You can also now debug the resulted hook with:

nix eval --json .\#openmpi.postInstall | jq --raw-output .

I'll also note before I'll merge this, that I tried to run a nixpkgs-review a few days ago, and most packages built fine, besides the tensorflow related packages which my machine just didn't manage to finish building, even after a few days.

cc me for any breakages :).

Artturin · 2024-09-11T23:42:27Z

+        // lib.optionalAttrs fortranSupport {
+          "fort" = [
+            "gfortran"
+            "${targetPackages.gfortran}/bin/${targetPackages.gfortran.targetPrefix}gfortran"


EDIT: disregard the below, it should be for when openmpi goes in nativeBuildInputs right?

~~What's the reason for using targetPackages.gfortran here? It'll only work on non-cross where targetPackages.gfortran is the same as gfortran? Asking for #341269~~

~~If you were just following the targetPackages.stdenv above then targetPackages.stdenv is the same offset as gfortran~~

nix-repl> pkgsCross.aarch64-multiplatform.gfortran.cc.stdenv.cc «derivation /nix/store/cy0iv1sk4c60df358j3ff8xb2x0db1qh-aarch64-unknown-linux-gnu-gcc-wrapper-13.3.0.drv» nix-repl> pkgsCross.aarch64-multiplatform.targetPackages.stdenv.cc «derivation /nix/store/bh6d4yc7dyx1694gz3bx65l0flv1blc2-gcc-wrapper-13.3.0.drv» nix-repl> pkgsCross.aarch64-multiplatform.targetPackages.stdenv.cc.stdenv.hostPlatform.system "aarch64-linux" nix-repl> pkgsCross.aarch64-multiplatform.targetPackages.stdenv.cc.stdenv.targetPlatform.system "aarch64-linux" nix-repl> pkgsCross.aarch64-multiplatform.gfortran.cc.stdenv.hostPlatform.system "aarch64-linux" nix-repl> pkgsCross.aarch64-multiplatform.gfortran.cc.stdenv.targetPlatform.system "aarch64-linux"

EDIT: disregard the below, it should be for when openmpi goes in nativeBuildInputs right?

Correct. I'm not sure how to test that with nix-repl like tests as you tried to demonstrate, but yes.

our current openmpi package does not include orte.pc any since NixOS#327438 got merged

doronbehar requested a review from markuskowa July 15, 2024 17:52

markuskowa marked this as a duplicate of #275796 Jul 15, 2024

ofborg Bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. labels Jul 15, 2024

doronbehar force-pushed the pkg/openmpi branch from 1957181 to 63b8bc9 Compare July 15, 2024 23:14

ofborg Bot added the 8.has: package (new) This PR adds a new package label Jul 16, 2024

markuskowa reviewed Jul 16, 2024

View reviewed changes

Comment thread pkgs/development/libraries/openmpi/default.nix Outdated

doronbehar force-pushed the pkg/openmpi branch from 63b8bc9 to a417747 Compare July 16, 2024 09:34

ofborg Bot requested a review from markuskowa July 16, 2024 10:12

doronbehar force-pushed the pkg/openmpi branch from a417747 to 0a4b979 Compare July 16, 2024 10:23

doronbehar force-pushed the pkg/openmpi branch from 0a4b979 to ab88eae Compare July 16, 2024 13:21

SuperSandro2000 reviewed Jul 16, 2024

View reviewed changes

Comment thread pkgs/by-name/pr/prrte/package.nix Outdated

SuperSandro2000 reviewed Jul 16, 2024

View reviewed changes

Comment thread pkgs/development/libraries/openmpi/default.nix Outdated

doronbehar force-pushed the pkg/openmpi branch from ab88eae to 86a7903 Compare July 16, 2024 20:21

doronbehar force-pushed the pkg/openmpi branch from 86a7903 to 34ab312 Compare July 22, 2024 14:21

doronbehar mentioned this pull request Jul 28, 2024

Show the references of a failing build when disallowedReferences are referenced NixOS/nix#11201

Open

doronbehar and others added 17 commits August 6, 2024 19:54

openmpi: don't overuse 'with lib;' in meta

5ee3a6e

openmpi: use a finalAttrs function to mkDerivation

2613455

openmpi: use more elegent lib.versions.majorMinor in src

51f5d82

openmpi: put all common buildInputs in the same place

494efeb

openmpi: always use lib.optionals and not lib.optional

6f348b8

Less confusing when to use a list and a single argument.

openmpi: use lib.*Feature functions to create configureFlags

0d81346

openmpi: always use ${!outputDev} to allow easily disabling multiple …

2a4636b

…outputs

openmpi: separate opal_wrapper moveToOutput a bit

d07b887

openmpi: separate ortecc related moveToOutput commands

eaf6367

openmpi: replace sed cross compilation fixes with substituteInPlace

5809830

Using substituteInPlace (with `--replace-fail`) is much safer.

openmpi: fix gfortran targetPackages Nix attribute

13e4a57

openmpi: Handle all compiler wrappers consistently & declaratively

36494fe

openmpi: another nixfmt-rfc-style

7589791

openmpi: 4.1.6 -> 5.0.3

bf35ff5

https://docs.open-mpi.org/en/v5.0.x/release-notes/changelog/v5.0.x.html Make build reproducible in a different, nicer manner Co-authored-by: Markus Kowalewski <markus.kowalewski@gmail.com>

openmpi: support setting avxOptions

bae9114

Co-authored-by: Markus Kowalewski <markus.kowalewski@gmail.com>

openmpi: fix build on non x86_64-linux platforms

3d70c20

Co-authored-by: Markus Kowalewski <markus.kowalewski@gmail.com>

openmpi: don't use finalAttrs.pname in src.url

da423c7

doronbehar force-pushed the pkg/openmpi branch from 34ab312 to da423c7 Compare August 6, 2024 16:55

markuskowa approved these changes Aug 6, 2024

View reviewed changes

doronbehar merged commit 9d67f43 into NixOS:master Aug 7, 2024

doronbehar deleted the pkg/openmpi branch August 7, 2024 06:13

nim65s mentioned this pull request Aug 7, 2024

casadi: init at 3.6.6 #331343

Merged

13 tasks

This was referenced Aug 7, 2024

openmpi: fix build on darwin #332983

Closed

mpiCheckPhaseHook: update to use openmpi-5 compatible parameters #333444

Merged

FRidh reviewed Aug 14, 2024

View reviewed changes

Comment thread pkgs/development/libraries/openmpi/default.nix

Artturin reviewed Sep 11, 2024

View reviewed changes

jopejoe1 added a commit to jopejoe1/nixpkgs that referenced this pull request May 4, 2025

defaultPkgConfigPackages.orte: drop

be4ef83

our current openmpi package does not include orte.pc any since NixOS#327438 got merged

jopejoe1 mentioned this pull request May 4, 2025

defaultPkgConfigPackages.orte: drop #404127

Merged

13 tasks

Uh oh!

Uh oh!

Conversation

doronbehar commented Jul 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

markuskowa commented Jul 15, 2024

Uh oh!

doronbehar commented Jul 15, 2024

Uh oh!

markuskowa commented Jul 15, 2024

Uh oh!

doronbehar commented Jul 15, 2024

Uh oh!

markuskowa commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markuskowa commented Jul 16, 2024

Uh oh!

Uh oh!

doronbehar commented Jul 16, 2024

Uh oh!

doronbehar commented Jul 16, 2024

Uh oh!

doronbehar commented Jul 16, 2024

Uh oh!

doronbehar commented Jul 16, 2024

Uh oh!

doronbehar commented Jul 16, 2024

Uh oh!

Uh oh!

SuperSandro2000 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

markuskowa commented Jul 16, 2024

Uh oh!

doronbehar commented Jul 16, 2024

Uh oh!

doronbehar commented Jul 22, 2024

Uh oh!

doronbehar commented Aug 6, 2024

Uh oh!

markuskowa left a comment

Choose a reason for hiding this comment

Uh oh!

doronbehar commented Aug 7, 2024

Uh oh!

Uh oh!

Artturin Sep 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

doronbehar Sep 12, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

doronbehar commented Jul 15, 2024 •

edited

Loading

markuskowa commented Jul 16, 2024 •

edited

Loading

Artturin Sep 11, 2024 •

edited

Loading