Message ID | 20220328084543.GA5967@delia.home |
---|---|
State | New |
Headers | show |
Series | [libgomp,testsuite] Fix hardcoded libexec in plugin/configfrag.ac | expand |
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
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)"