[libgomp,testsuite] Fix hardcoded libexec in plugin/configfrag.ac
Commit Message
Hi,
When building an nvptx offloading configuration on openSUSE Leap 15.3, the
site script /usr/share/site/x86_64-unknown-linux-gnu is activated, setting
libexecdir to ${exec_prefix}/lib rather than ${exec_prefix}/libexec:
...
| # If user did not specify libexecdir, set the correct target:
| # Nor FHS nor openSUSE allow prefix/libexec. Let's default to prefix/lib.
|
| if test "$libexecdir" = '${exec_prefix}/libexec' ; then
| libexecdir='${exec_prefix}/lib'
| fi
...
However, in libgomp libgomp/plugin/configfrag.ac we hardcode libexec:
...
# Configure additional search paths.
if test x"$tgt_dir" != x; then
offload_additional_options="$offload_additional_options \
-B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) \
-B$tgt_dir/bin"
...
Fix this by using /$(libexecdir:\$(exec_prefix)/%=%)/ instead of /libexec/.
Tested on x86_64-linux with nvptx accelerator.
OK for trunk?
Thanks,
- Tom
[libgomp, testsuite] Fix hardcoded libexec in plugin/configfrag.ac
libgomp/ChangeLog:
2022-03-28 Tom de Vries <tdevries@suse.de>
* plugin/configfrag.ac: Use /$(libexecdir:\$(exec_prefix)/%=%)/
instead of /libexec/.
* configure: Regenerate.
---
libgomp/configure | 2 +-
libgomp/plugin/configfrag.ac | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Comments
On Mon, 28 Mar 2022, Tom de Vries wrote:
> Hi,
>
> When building an nvptx offloading configuration on openSUSE Leap 15.3, the
> site script /usr/share/site/x86_64-unknown-linux-gnu is activated, setting
> libexecdir to ${exec_prefix}/lib rather than ${exec_prefix}/libexec:
> ...
> | # If user did not specify libexecdir, set the correct target:
> | # Nor FHS nor openSUSE allow prefix/libexec. Let's default to prefix/lib.
> |
> | if test "$libexecdir" = '${exec_prefix}/libexec' ; then
> | libexecdir='${exec_prefix}/lib'
> | fi
> ...
>
> However, in libgomp libgomp/plugin/configfrag.ac we hardcode libexec:
> ...
> # Configure additional search paths.
> if test x"$tgt_dir" != x; then
> offload_additional_options="$offload_additional_options \
> -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) \
> -B$tgt_dir/bin"
> ...
>
> Fix this by using /$(libexecdir:\$(exec_prefix)/%=%)/ instead of /libexec/.
>
> Tested on x86_64-linux with nvptx accelerator.
>
> OK for trunk?
OK in principle, but I have no idea on how portable
$(libexecdir:\$(exec_prefix)/%=%)
is going to be? We should aim for POSIX shell compatibility here,
whatever that exactly is.
Richard.
> Thanks,
> - Tom
>
> [libgomp, testsuite] Fix hardcoded libexec in plugin/configfrag.ac
>
> libgomp/ChangeLog:
>
> 2022-03-28 Tom de Vries <tdevries@suse.de>
>
> * plugin/configfrag.ac: Use /$(libexecdir:\$(exec_prefix)/%=%)/
> instead of /libexec/.
> * configure: Regenerate.
>
> ---
> libgomp/configure | 2 +-
> libgomp/plugin/configfrag.ac | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libgomp/configure b/libgomp/configure
> index a73a6d44003..bdbe3d142d1 100755
> --- a/libgomp/configure
> +++ b/libgomp/configure
> @@ -15419,7 +15419,7 @@ rm -f core conftest.err conftest.$ac_objext \
> fi
> # Configure additional search paths.
> if test x"$tgt_dir" != x; then
> - offload_additional_options="$offload_additional_options -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
> + offload_additional_options="$offload_additional_options -B$tgt_dir/\$(libexecdir:\$(exec_prefix)/%=%)/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
> offload_additional_lib_paths="$offload_additional_lib_paths:$tgt_dir/lib64:$tgt_dir/lib:$tgt_dir/lib32"
> else
> offload_additional_options="$offload_additional_options -B\$(libexecdir)/gcc/\$(target_alias)/\$(gcc_version) -B\$(bindir)"
> diff --git a/libgomp/plugin/configfrag.ac b/libgomp/plugin/configfrag.ac
> index da573bd8387..9f9d0a7f08c 100644
> --- a/libgomp/plugin/configfrag.ac
> +++ b/libgomp/plugin/configfrag.ac
> @@ -254,7 +254,7 @@ if test x"$enable_offload_targets" != x; then
> fi
> # Configure additional search paths.
> if test x"$tgt_dir" != x; then
> - offload_additional_options="$offload_additional_options -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
> + offload_additional_options="$offload_additional_options -B$tgt_dir/\$(libexecdir:\$(exec_prefix)/%=%)/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
> offload_additional_lib_paths="$offload_additional_lib_paths:$tgt_dir/lib64:$tgt_dir/lib:$tgt_dir/lib32"
> else
> offload_additional_options="$offload_additional_options -B\$(libexecdir)/gcc/\$(target_alias)/\$(gcc_version) -B\$(bindir)"
>
On 3/28/22 10:49, Richard Biener wrote:
> On Mon, 28 Mar 2022, Tom de Vries wrote:
>
>> Hi,
>>
>> When building an nvptx offloading configuration on openSUSE Leap 15.3, the
>> site script /usr/share/site/x86_64-unknown-linux-gnu is activated, setting
>> libexecdir to ${exec_prefix}/lib rather than ${exec_prefix}/libexec:
>> ...
>> | # If user did not specify libexecdir, set the correct target:
>> | # Nor FHS nor openSUSE allow prefix/libexec. Let's default to prefix/lib.
>> |
>> | if test "$libexecdir" = '${exec_prefix}/libexec' ; then
>> | libexecdir='${exec_prefix}/lib'
>> | fi
>> ...
>>
>> However, in libgomp libgomp/plugin/configfrag.ac we hardcode libexec:
>> ...
>> # Configure additional search paths.
>> if test x"$tgt_dir" != x; then
>> offload_additional_options="$offload_additional_options \
>> -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) \
>> -B$tgt_dir/bin"
>> ...
>>
>> Fix this by using /$(libexecdir:\$(exec_prefix)/%=%)/ instead of /libexec/.
>>
>> Tested on x86_64-linux with nvptx accelerator.
>>
>> OK for trunk?
>
> OK in principle, but I have no idea on how portable
>
> $(libexecdir:\$(exec_prefix)/%=%)
>
> is going to be? We should aim for POSIX shell compatibility here,
> whatever that exactly is.
I tried to avoid this construct by using shell variable substitution,
but then I end up using $(shell ...) instead, I'm not sure if that is
any better.
Thanks,
- Tom
On Mär 28 2022, Richard Biener via Gcc-patches wrote:
> OK in principle, but I have no idea on how portable
>
> $(libexecdir:\$(exec_prefix)/%=%)
>
> is going to be?
We already require GNU make, don't we?
> We should aim for POSIX shell compatibility here, whatever that
> exactly is.
It's not a shell construct.
On Mon, 28 Mar 2022, Andreas Schwab wrote:
> On Mär 28 2022, Richard Biener via Gcc-patches wrote:
>
> > OK in principle, but I have no idea on how portable
> >
> > $(libexecdir:\$(exec_prefix)/%=%)
> >
> > is going to be?
>
> We already require GNU make, don't we?
>
> > We should aim for POSIX shell compatibility here, whatever that
> > exactly is.
>
> It's not a shell construct.
Ah, it's only substituted into Makefile.in - in that case yes,
we already require GNU make. If it's evaluated by that and not
a subshell of it then it should be indeed fine.
Richard.
On 3/28/22 14:04, Richard Biener wrote:
> On Mon, 28 Mar 2022, Andreas Schwab wrote:
>
>> On Mär 28 2022, Richard Biener via Gcc-patches wrote:
>>
>>> OK in principle, but I have no idea on how portable
>>>
>>> $(libexecdir:\$(exec_prefix)/%=%)
>>>
>>> is going to be?
>>
>> We already require GNU make, don't we?
>>
>>> We should aim for POSIX shell compatibility here, whatever that
>>> exactly is.
>>
>> It's not a shell construct.
>
> Ah, it's only substituted into Makefile.in - in that case yes,
> we already require GNU make. If it's evaluated by that and not
> a subshell of it then it should be indeed fine.
Ack, then committed the first version.
Thanks,
- Tom
@@ -15419,7 +15419,7 @@ rm -f core conftest.err conftest.$ac_objext \
fi
# Configure additional search paths.
if test x"$tgt_dir" != x; then
- offload_additional_options="$offload_additional_options -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
+ offload_additional_options="$offload_additional_options -B$tgt_dir/\$(libexecdir:\$(exec_prefix)/%=%)/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
offload_additional_lib_paths="$offload_additional_lib_paths:$tgt_dir/lib64:$tgt_dir/lib:$tgt_dir/lib32"
else
offload_additional_options="$offload_additional_options -B\$(libexecdir)/gcc/\$(target_alias)/\$(gcc_version) -B\$(bindir)"
@@ -254,7 +254,7 @@ if test x"$enable_offload_targets" != x; then
fi
# Configure additional search paths.
if test x"$tgt_dir" != x; then
- offload_additional_options="$offload_additional_options -B$tgt_dir/libexec/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
+ offload_additional_options="$offload_additional_options -B$tgt_dir/\$(libexecdir:\$(exec_prefix)/%=%)/gcc/\$(target_alias)/\$(gcc_version) -B$tgt_dir/bin"
offload_additional_lib_paths="$offload_additional_lib_paths:$tgt_dir/lib64:$tgt_dir/lib:$tgt_dir/lib32"
else
offload_additional_options="$offload_additional_options -B\$(libexecdir)/gcc/\$(target_alias)/\$(gcc_version) -B\$(bindir)"