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.

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

oac_check_package: Adding library path to LDFLAGS #10

Open
rfeki opened this issue Feb 22, 2023 · 11 comments
Open

oac_check_package: Adding library path to LDFLAGS #10

rfeki opened this issue Feb 22, 2023 · 11 comments

Comments

@rfeki
Copy link

rfeki commented Feb 22, 2023

I am working on creating a new component in ompio to support nvidia GPUDirect storage. When creating the config file and using "OAC_CHECK_PACKAGE", the function checks the library path but it does not add -L${check_package_prefix}/lib64 to LDFLAGS as it does for -L${check_package_prefix}/lib in the function _OAC_CHECK_PACKAGE_GENERIC_PREFIX

File: oac_check_package.m4
line 624:$2_LDFLAGS=-L${check_package_prefix}/lib
line 628: libdir_prefix=${check_package_prefix}/lib64

I also checked if the variable libdir_prefix is used anywhere else but it's not. Should I make a pull request with this minor change or make a workaround in the new component conf file?

@jsquyres
Copy link
Member

Ah, are you proposing:

diff --git a/oac_check_package.m4 b/oac_check_package.m4
index 47d5dd5..6ab607b 100644
--- a/oac_check_package.m4
+++ b/oac_check_package.m4
@@ -625,7 +625,7 @@ disambiguate.])],
                          AC_MSG_RESULT([found -- lib])],
                         [test $check_package_generic_prefix_lib64 -eq 1],
                         [check_package_generic_prefix_happy=1
-                         libdir_prefix=${check_package_prefix}/lib64
+                         $2_LDFLAGS=-L${check_package_prefix}/lib64
                          AC_MSG_RESULT([found -- lib64])],
                         [AC_MSG_RESULT([not found])])])])
 

@bwbarrett Does this look right to you? Or is there a reason that the lib64 stanza is that way?

@bwbarrett
Copy link
Member

yeah, that's a bug. Sigh.

@jsquyres
Copy link
Member

Cool. @rfeki Can you file a PR?

@rfeki
Copy link
Author

rfeki commented Feb 22, 2023

Yes sure

@raafatfeki
Copy link
Contributor

PR is filed but I cannot assign reviewers for some reason.

@bwbarrett
Copy link
Member

Should be fixed now; we'll have to file PRs against OMPI, PMIx, and PRRTE, of course.

@jsquyres
Copy link
Member

Yeah, we'll have to check which branches in each repo will need this. E.g., in the ompi repo, I think we only need to update main and v5.0.x (i.e., we're not using the oac submodule on the older 4.x branches).

@raafatfeki Can you file submodule updates on each of these repos + branches? Thanks!

@rhc54
Copy link
Contributor

rhc54 commented Feb 23, 2023

FWIW: PMIx v4.2 branch now uses OAC, and so does PRRTE v3.0

@raafatfeki
Copy link
Contributor

Quick question here: So basically 3rd-party/openpmix/config/oac, 3rd-party/prrte/config/oac and ompi/config/oac are impacted. So if I want to update submodule of ompi, shouldn't I update first openpmix and prrte first, then update ompi.
That means pull each repo independently, update oac submodule there, submit a pull request for openpmix and prrte first. After being merged, I do the same for ompi.
What is the right approach?

@jsquyres
Copy link
Member

jsquyres commented Feb 23, 2023

That will work:

  1. Update PMIx oac submodule (on relevant branches)
  2. Update PRRTE oac submodule (on relevant branches)
  3. Update OMPI oac + pmix + prrte submodules (on relevant branches)

@bwbarrett
Copy link
Member

Across the three projects, post the PRs in whatever order makes you happy. There's no dependency that everyone gets the same version of OAC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants