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

Build NEST with Python, OpenMP, MPI, Boost and GSL for macOS on CI #2762

Merged
merged 13 commits into from
Jun 2, 2023
Merged

Build NEST with Python, OpenMP, MPI, Boost and GSL for macOS on CI #2762

merged 13 commits into from
Jun 2, 2023

Conversation

heplesser
Copy link
Contributor

This PR ensures NEST is build with all components on macOS on the CI (except sionlib, libneurosim and music). It also removes some no longer needed parts of the build_macos stage. It incorporates changes from #2660. It fixes #2756.

@heplesser heplesser added T: Enhancement New functionality, model or documentation S: High Should be handled next I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels May 2, 2023
terhorstd
terhorstd requested changes May 3, 2023
View reviewed changes
Copy link
Contributor

@terhorstd terhorstd left a comment

Choose a reason for hiding this comment

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

Thanks for starting to clean this up! There are a couple of smaller things that might need another look before merging.

.github/workflows/nestbuildmatrix.yml Outdated Show resolved Hide resolved
.github/workflows/nestbuildmatrix.yml Show resolved Hide resolved
.github/workflows/nestbuildmatrix.yml Outdated Show resolved Hide resolved
.github/workflows/nestbuildmatrix.yml Show resolved Hide resolved
heplesser and others added 3 commits May 3, 2023 11:25
@heplesser @terhorstd
Replace backticks with $()
bc16023 
Co-authored-by: Dennis Terhorst <terhorstd@users.noreply.github.com>
@heplesser @terhorstd
Remove superfluous variable.
117cd36 
Co-authored-by: Dennis Terhorst <terhorstd@users.noreply.github.com>
@heplesser
Remove empty env: section
760fce5
@heplesser heplesser requested a review from terhorstd May 8, 2023 07:16
@heplesser heplesser mentioned this pull request May 8, 2023
Open
@heplesser
Copy link
Contributor Author

@terhorstd @lekshmideepu Ping!

@lekshmideepu
Copy link
Contributor

@terhorstd @lekshmideepu Ping!

Am working on this.

lekshmideepu
lekshmideepu reviewed May 22, 2023
View reviewed changes
Copy link
Contributor

@lekshmideepu lekshmideepu left a comment

Choose a reason for hiding this comment

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

Thanks @heplesser for this nice work for the macOS builds. Few of my comments/ questions were asked by Dennis and got it answered. It looks good to me 👍

nicolossus
nicolossus suggested changes May 23, 2023
View reviewed changes
Copy link
Member

@nicolossus nicolossus left a comment

Choose a reason for hiding this comment

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

I am not sure why there is no conflict, but it looks to me that this PR will remove the build with SONATA support on macOS if it is merged now. Perhaps a merge with master will retrieve all the SONATA related changes to the build matrix.

@heplesser
Copy link
Contributor Author

I am not sure why there is no conflict, but it looks to me that this PR will remove the build with SONATA support on macOS if it is merged now. Perhaps a merge with master will retrieve all the SONATA related changes to the build matrix.

I have merged master into this branch.

nicolossus
nicolossus suggested changes May 23, 2023
View reviewed changes
Copy link
Member

@nicolossus nicolossus left a comment

Choose a reason for hiding this comment

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

@heplesser Great! As far as I can see there is only one addition missing, see suggestion.

.github/workflows/nestbuildmatrix.yml Outdated Show resolved Hide resolved
@heplesser heplesser requested review from terhorstd and removed request for terhorstd May 24, 2023 06:22
@heplesser
Copy link
Contributor Author

@terhorstd Are you happy as well?

@heplesser heplesser requested a review from nicolossus May 31, 2023 12:58
@heplesser
Copy link
Contributor Author

@nicolossus Could you re-approve?
@terhorstd Ping! Since you commented, you need to approve ;).

terhorstd
terhorstd approved these changes Jun 2, 2023
View reviewed changes
Copy link
Contributor

@terhorstd terhorstd left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@heplesser heplesser merged commit 256e435 into nest:master Jun 2, 2023
20 checks passed
@heplesser heplesser deleted the reactivate_full_macos_build branch September 15, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lekshmideepu lekshmideepu lekshmideepu approved these changes

@terhorstd terhorstd terhorstd approved these changes

@nicolossus nicolossus nicolossus approved these changes

Assignees
No one assigned
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Enhancement New functionality, model or documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New test setup runs only minimal tests on macOS
4 participants
@heplesser @lekshmideepu @terhorstd @nicolossus