On Fri, 1 Dec 2023 16:36:18 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

>> You need to expand this logic to cover more instances. See e.g. lib-ffi.m4 
>> for inspiration.
>> 
>> Basic flow:
>> * if user has specified libsleef root with argument, check both lib/ and 
>> lib64/ under that root.
>> * if user has not specified libsleef root, and we have no SYSROOT, try 
>> PKG_CHECK
>> * Otherwise, look in well-known directories which is 
>> $SYSROOT/usr/[local/]lib[64].
>
> also, ideally, you will add the corresponding specific overrides like in ffi:
> 
>   AC_ARG_WITH(libffi-include, [AS_HELP_STRING([--with-libffi-include],
>       [specify directory for the libffi include files])])
>   AC_ARG_WITH(libffi-lib, [AS_HELP_STRING([--with-libffi-lib],
>       [specify directory for the libffi library])])

Thanks for the suggestion @magicus !

The check in current `lib-sleef.m4` is very common:

-  If user has specified libsleef root by '--with-libsleef', we assume it is 
the manually built sleef lib. So only `lib/` and `include/` is checked. And the 
flags are set based on that path.
- If user has not specified the libsleef root, and no `SYSROOT` is set, we try 
`PKG_CHECK` (like what you suggested)
- Otherwise, check `sleef.h`   
   - We assume the sleef module is installed under one of the valid system 
paths if the header can be found. So just linking with `-lsleef` will success.

It's an issue in current flow like what @theRealAph met. I will add the options 
like `--with-libsleef-lib` and `--with-libsleef-include` like ffi. Regarding to 
extending the check for`--with-libsleef`, I think we can just make it simple 
like what it is now. Or, we have to check all the potential valid lib paths 
like `lib/`, `lib64/`, or maybe `lib/aarch64-linux-gnu`. The same to the 
`include` part.  @theRealAph @magicus , WDYT?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1414980861

Reply via email to