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.

Sign up for GitHub

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

Jump to bottom

MPI fixes (macOS build/link flags), oneAPI ifort->ifx switch; turn off MSMPI CI #976

Merged
merged 5 commits into from
Dec 28, 2023
Merged

MPI fixes (macOS build/link flags), oneAPI ifort->ifx switch; turn off MSMPI CI #976

merged 5 commits into from
Dec 28, 2023

Conversation

perazz
Copy link
Contributor

@perazz perazz commented Dec 13, 2023 •
edited
Loading

@fortran-lang/admins

@perazz
Copy link
Contributor Author

perazz commented Dec 13, 2023

It seems like something is now failing in the oneAPI CI (classic intel compilers deprecated?). I see that no fpm actions have been run for the past few months, so that might be the case. I will do some more tests

@perazz perazz changed the title Sanitize MPI build/link flags; fix MSMPI CI MPI fixes (macOS build/link flags), oneAPI ifort->ifx switch; fix MSMPI CI Dec 13, 2023
@perazz perazz mentioned this pull request Dec 21, 2023
Merged
henilp105
henilp105 approved these changes Dec 21, 2023
View reviewed changes
Copy link
Member

@henilp105 henilp105 left a comment

Choose a reason for hiding this comment

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

Thanks @perazz , Looks Good to me.

gnikit
gnikit reviewed Dec 21, 2023
View reviewed changes
Copy link
Member

@gnikit gnikit left a comment

Choose a reason for hiding this comment

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

Looks good, there are quite a few changes though. I left some comments inline mostly with clean-up, underlying code seems good.

To make review faster, when possible, could we try and stick with 1 fix/feature per PR. One of the reasons why I delayed reviewing this is because fixes kept being added in this PR.

src/fpm_environment.f90 Outdated Show resolved Hide resolved
src/fpm_meta.f90 Outdated Show resolved Hide resolved
src/fpm_meta.f90 Outdated Show resolved Hide resolved
@perazz
Copy link
Contributor Author

perazz commented Dec 21, 2023

One of the reasons why I delayed reviewing this is because fixes kept being added in this PR.

I agree, committed to it. Unfortunately new issues kept arising in the CI as I was fixing the macOS flags

gnikit
gnikit approved these changes Dec 22, 2023
View reviewed changes
Copy link
Member

@gnikit gnikit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@perazz
Copy link
Contributor Author

perazz commented Dec 24, 2023

Thank you @gnikit @henilp105 @ivan-pi for the reviews, let's merge in a few days if there are no more comments.

@gnikit
Copy link
Member

gnikit commented Dec 25, 2023

I will have a look in the windows metapackages CI to see if we can get it to work more reliably. If the Action is not robust yet, it might be worth disabling whilst troubleshooting to merge in main.

@henilp105 henilp105 mentioned this pull request Dec 26, 2023
Merged
@perazz perazz changed the title MPI fixes (macOS build/link flags), oneAPI ifort->ifx switch; fix MSMPI CI MPI fixes (macOS build/link flags), oneAPI ifort->ifx switch; turn off MSMPI CI Dec 28, 2023
@perazz
Copy link
Contributor Author

perazz commented Dec 28, 2023

There are two approvals, so I will merge this one.

@perazz perazz merged commit c6ef3c4 into fortran-lang:main Dec 28, 2023
14 checks passed
@certik certik mentioned this pull request Feb 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivan-pi ivan-pi ivan-pi left review comments

@henilp105 henilp105 henilp105 approved these changes

@gnikit gnikit gnikit approved these changes

@certik certik Awaiting requested review from certik

@milancurcic milancurcic Awaiting requested review from milancurcic

@LKedward LKedward Awaiting requested review from LKedward

@awvwgk awvwgk Awaiting requested review from awvwgk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Latest Xcode update has broken MPI builds on macOS
4 participants
@perazz @gnikit @ivan-pi @henilp105