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

openmpi: migrate to Conan v2 #18980

Merged
merged 52 commits into from
Sep 5, 2024
Merged

openmpi: migrate to Conan v2 #18980

merged 52 commits into from
Sep 5, 2024
+265 −101

Conversation

valgur
Copy link
Contributor

@valgur valgur commented Jul 31, 2023 •
edited
Loading

In addition to typical migration changes also:

test_package builds and runs, but displaying of help messages currently fails due to OpenMPI not being able to locate its res folder.

@conan-center-bot

This comment has been minimized.

Sign in to view
@valgur valgur mentioned this pull request Aug 1, 2023
Open
@conan-center-bot

This comment has been minimized.

Sign in to view
@stale
Copy link

stale bot commented Oct 15, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 15, 2023
This was referenced Oct 18, 2023
Closed
Closed
@AbrilRBS AbrilRBS self-assigned this Oct 23, 2023
@stale stale bot removed the stale label Oct 23, 2023
@AbrilRBS
Copy link
Member

@valgur any way we can help this one moving along? (Also, could you please set the zlib requirement to the [>=1.2.11 <2] version range? Thanks!)

@valgur
Copy link
Contributor Author

valgur commented Oct 23, 2023 •
edited
Loading

@valgur any way we can help this one moving along? (Also, could you please set the zlib requirement to the [>=1.2.11 <2] version range? Thanks!)

A review of the rdma-core recipe, which is a dependency of this one, would be of great help. I hope the build for that one succeeds now as well. Thanks!

@valgur valgur mentioned this pull request Nov 8, 2023
Open
@conan-center-bot

This comment has been minimized.

Sign in to view
@valgur valgur marked this pull request as ready for review December 13, 2023 22:59
@conan-center-bot

This comment has been minimized.

Sign in to view
@prince-chrismc prince-chrismc mentioned this pull request Dec 13, 2023
Open
@conan-center-bot

This comment has been minimized.

Sign in to view
uilianries
uilianries previously approved these changes Sep 2, 2024
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@valgur
Copy link
Contributor Author

valgur commented Sep 2, 2024

@jwillikers Maybe you can give this one a look as well.

@uilianries uilianries added the blocked Affected by an external issue and waiting until it is solved label Sep 3, 2024
jcar87
jcar87 previously requested changes Sep 3, 2024
View reviewed changes
recipes/openmpi/all/conanfile.py Outdated Show resolved Hide resolved
@uilianries
Copy link
Member

I'll still don't have the root cause, but so far:

  • It does not depends on Conan 1.x. The build fails for Clang on Linux, using the Docker image conanio/clang13-ubuntu16.04:<conan_version>. It fails for both Conan 1.65.0 and 2.7.0.
  • When hwloc is not listed as requirement, it does not fail.

I'll continue to investigate.

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@uilianries
Simplify test package to avoid networking usage
2b728b5 
The current openmpi test package tries to use networking,
and in OS like Mac, the GUI asks about permission to
execute the test package. It's too much for a simple
package validation. It would be better keep a version
check for library linkage validation.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
@uilianries
Revert "openmpi: macOS armv8 fails when shared=True"
35520e7 
Enable again dynamic library build for Mac. With the simplified
test package, I see no errors locally for Mac M1.

This reverts commit d79c757.
@valgur
Copy link
Contributor Author

valgur commented Sep 4, 2024

Thanks, @uilianries! LGTM.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 35 (35520e79ec92158492af78c3c7c0aba896bf6dae):

  • openmpi/4.1.6:
    All packages built successfully! (All logs)

  • openmpi/4.1.0:
    All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 35 (35520e79ec92158492af78c3c7c0aba896bf6dae):

  • openmpi/4.1.6:
    All packages built successfully! (All logs)

  • openmpi/4.1.0:
    All packages built successfully! (All logs)

@uilianries
Copy link
Member

@valgur I applied few changes related to the test package, I replaced the previous code by a simpler one to avoid networking usage. The CI is more restricted, and probably caused the previous error. In my machine, Mac displayed some pop-ups asking for network permission, which is bad for users.

uilianries
uilianries approved these changes Sep 4, 2024
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM

@uilianries uilianries removed the blocked Affected by an external issue and waiting until it is solved label Sep 4, 2024
@conan-center-bot conan-center-bot merged commit 30a5ab7 into conan-io:master Sep 5, 2024
16 checks passed
@valgur valgur mentioned this pull request Sep 6, 2024
Open
3 tasks
@valgur valgur mentioned this pull request Sep 26, 2024
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maksim-petukhov maksim-petukhov maksim-petukhov left review comments

@uilianries uilianries uilianries approved these changes

@jwillikers jwillikers jwillikers approved these changes

@jcar87 jcar87 Awaiting requested review from jcar87

Assignees

@uilianries uilianries

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@valgur @conan-center-bot @AbrilRBS @uilianries @jcar87 @maksim-petukhov @jwillikers