Skip to content

oshmem/shmem/c: include missing headers #12360

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

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

alex--m
Copy link
Contributor

@alex--m alex--m commented Feb 22, 2024

Not sure why no one has caught this before... these files simply fail to build without the additional include statements: oshmem/mca/spml/spml.h makes MCA_SPML_CALL available, and oshmem/proc/proc.h is required for ompi_rte_abort.

Signed-off-by: Alex Margolin <alex.margolin@mail.huji.ac.il>
@jsquyres
Copy link
Member

jsquyres commented Mar 5, 2024

@alex--m Can you get someone to review this so that it can be merged?

@alex--m
Copy link
Contributor Author

alex--m commented Mar 5, 2024

@alex--m Can you get someone to review this so that it can be merged?

@jsquyres I'm not sure whom to ask... any recommendations? from the logs I couldn't see if there's a clear owner to this area of the code (plus no other contributors from my university AFAIK)

@jsquyres
Copy link
Member

jsquyres commented Mar 5, 2024

@alex--m Sorry, I thought you were Mellanox/Nvidia.

@janjust Can you find a reviewer? This is OSHMEM, so it's Nivida.

@jsquyres jsquyres requested a review from janjust March 5, 2024 18:18
@alex--m
Copy link
Contributor Author

alex--m commented Mar 5, 2024

BTW, since I submitted this issue I figured out how nobody came across this: these missing include statements only break the build when using Clang, not GCC.

@janjust
Copy link
Contributor

janjust commented Mar 21, 2024

@alex--m can you open up v5.0.x as well?

@janjust janjust merged commit 97eac28 into open-mpi:main Mar 21, 2024
@alex--m alex--m deleted the topic/oshmem_c_fix branch March 21, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants