Use compiler basename in ltmain_flang_darwin patch#13771
Use compiler basename in ltmain_flang_darwin patch#13771janjust merged 1 commit intoopen-mpi:mainfrom
Conversation
Checking $CC directly fails if the compiler is given with a full path (e.g., in a Spack-based build). This change fixes the check of the compiler to use the basename, as is done in a few other places. Signed-off-by: Bill Sacks <sacks@ucar.edu>
|
Credit goes to @mcmehrtens for pointing out the problem and suggested fix in https://groups.google.com/a/lists.open-mpi.org/g/users/c/4mIxq9op75k |
bosilca
left a comment
bosilca
left a comment
There was a problem hiding this comment.
The fix seems generic and should be applied to all platforms not only to OSX.
The fix in this PR is just a small modification to the fix added by @ggouaillardet in #13142 . I can't comment on why it was only applied to OSX. But I am open to any changes to this PR that you or others would like. |
| compile_deplibs="$new_inherited_linker_flags $compile_deplibs" | ||
| finalize_deplibs="$new_inherited_linker_flags $finalize_deplibs" | ||
| else | ||
| - compiler_flags="$compiler_flags "`$ECHO " $new_inherited_linker_flags" | $SED 's% \([^ $]*\).ltframework% -framework \1%g'` |
There was a problem hiding this comment.
Duplicating this line (to handle non flang compiler on darwin os and all compilers on non darwin os is error prone imho, so I prefer the current approach (always set compiler_flags and overwrite if and only if flang compiler on darwin) that makes it obvious flang on darwin is a special case..
Not a strong opinion though.
There was a problem hiding this comment.
The flags to be added being very specific OSX related, I guess it makes sense to target the change specifically for OSX. If one day we need to extend, will cross that bridge when needed.
|
is there a v6.0.x corresponding cherry pick? |
@janjust - so far I just made this into main and v5.0.x. Would you like me to open a PR into v6.0.x as well? (It looks like the relevant file is identical in main and v6.0.x, so the commit here should cherry-pick cleanly into v6.0.x. That said, I have the same concerns that I expressed above about whether this patch on main will apply cleanly to libtool's ltmain.sh.) |
|
Is there anything I can do to help move this PR forward? From the above discussions, it seems like the only open question is whether there should be a v6.0.x cherry pick. |
|
at this point while v6.0.x isn't officially released I think we should cherry-pick it @hppritcha @edgargabriel ? |
|
yes, I think this should go to v6.0.x branch as well |
|
Can we merge this? |
|
@bosilca approved the PR, so I would say we are good to go |
|
@billsacks please open up v6.0.x with cherry-pick message, and add cherry-pick message to v5.0.x |
Checking $CC directly fails if the compiler is given with a full path (e.g., in a Spack-based build). This change fixes the check of the compiler to use the basename, as is done in a few other places. backport-of: fc7fa11 (PR open-mpi#13771 in main) Signed-off-by: Bill Sacks <sacks@ucar.edu>
|
See #13804 for the equivalent PR to v6.0.x. |
6 participants
Checking $CC directly fails if the compiler is given with a full path (e.g., in a Spack-based build). This change fixes the check of the compiler to use the basename, as is done in a few other places.
Resolves #13770 for the main branch.
I haven't tested this with the main branch; I have tested it in the context of 5.0.10 (I'll open a separate PR for that shortly). At least in the version of libtool that I have, the ltmain_flang_darwin patch on main does not apply cleanly (before or after the changes in this PR): the versions of libtool that I see use a direct
case $CCin hunk 2 rather than usingfunc_cc_basenamethere, and hunk 3 also doesn't apply cleanly. (The patch on the v5.0.x branch does apply cleanly.)