* [PATCH] libgfortran : Use the libtool macro to determine libm availability. @ 2021-08-19 20:09 Iain Sandoe 2021-08-20 7:34 ` Richard Biener 0 siblings, 1 reply; 7+ messages in thread From: Iain Sandoe @ 2021-08-19 20:09 UTC (permalink / raw) To: GCC Fortran; +Cc: GCC Patches Hi, A while ago had a report of build failure against a Darwin branch on the latest OS release. This was because (temporarily) the symlink from libm.dylib => libSystem.dylib had been removed/omitted. libm is not needed on Darwin, and should not be added unconditionally even if that is (mostly) harmless since it is a symlink to libc. There could be cases where the addition was not completely harmless because the presentation of the symlink would cause the symbols exposed in libSystem to be considered ahead of ones presented in convenience libraries. tested on x86_64, i686-darwin, x86_64-linux, OK for master? thanks Iain libgfortran/ChangeLog: * Makefile.am: Use configured libm availability. * Makefile.in: Regenerate. * configure: Regenerate. * configure.ac: Use libtool macro to find libm availability. * libgfortran.spec.in: Use configured libm availability. --- libgfortran/Makefile.am | 2 +- libgfortran/Makefile.in | 3 +- libgfortran/configure | 142 ++++++++++++++++++++++++++++++++ libgfortran/configure.ac | 1 + libgfortran/libgfortran.spec.in | 2 +- 5 files changed, 147 insertions(+), 3 deletions(-) diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am index 8d104321567..6fc4b465c6e 100644 --- a/libgfortran/Makefile.am +++ b/libgfortran/Makefile.am @@ -42,7 +42,7 @@ libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS) libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` \ $(LTLDFLAGS) $(LIBQUADLIB) ../libbacktrace/libbacktrace.la \ $(HWCAP_LDFLAGS) \ - -lm $(extra_ldflags_libgfortran) \ + $(LIBM) $(extra_ldflags_libgfortran) \ $(version_arg) -Wc,-shared-libgcc libgfortran_la_DEPENDENCIES = $(version_dep) libgfortran.spec $(LIBQUADLIB_DEP) diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index 523eb24bca1..513fd80b936 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -260,6 +260,7 @@ AC_PROG_INSTALL #AC_MSG_NOTICE([====== Starting libtool configuration]) AC_LIBTOOL_DLOPEN AM_PROG_LIBTOOL +AC_CHECK_LIBM ACX_LT_HOST_FLAGS AC_SUBST(enable_shared) AC_SUBST(enable_static) diff --git a/libgfortran/libgfortran.spec.in b/libgfortran/libgfortran.spec.in index 95aa3f842a3..b870e78c151 100644 --- a/libgfortran/libgfortran.spec.in +++ b/libgfortran/libgfortran.spec.in @@ -5,4 +5,4 @@ # %rename lib liborig -*lib: @LIBQUADSPEC@ -lm %(libgcc) %(liborig) +*lib: @LIBQUADSPEC@ @LIBM@ %(libgcc) %(liborig) -- 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libgfortran : Use the libtool macro to determine libm availability. 2021-08-19 20:09 [PATCH] libgfortran : Use the libtool macro to determine libm availability Iain Sandoe @ 2021-08-20 7:34 ` Richard Biener 2021-08-20 7:52 ` Tobias Burnus 0 siblings, 1 reply; 7+ messages in thread From: Richard Biener @ 2021-08-20 7:34 UTC (permalink / raw) To: Iain Sandoe; +Cc: GCC Fortran, GCC Patches On Thu, Aug 19, 2021 at 10:10 PM Iain Sandoe <iain@sandoe.co.uk> wrote: > > Hi, > > A while ago had a report of build failure against a Darwin branch on > the latest OS release. This was because (temporarily) the symlink > from libm.dylib => libSystem.dylib had been removed/omitted. > > libm is not needed on Darwin, and should not be added unconditionally > even if that is (mostly) harmless since it is a symlink to libc. > > There could be cases where the addition was not completely harmless > because the presentation of the symlink would cause the symbols exposed > in libSystem to be considered ahead of ones presented in convenience > libraries. > > tested on x86_64, i686-darwin, x86_64-linux, > OK for master? OK. Richard. > thanks > Iain > > libgfortran/ChangeLog: > > * Makefile.am: Use configured libm availability. > * Makefile.in: Regenerate. > * configure: Regenerate. > * configure.ac: Use libtool macro to find libm availability. > * libgfortran.spec.in: Use configured libm availability. > --- > libgfortran/Makefile.am | 2 +- > libgfortran/Makefile.in | 3 +- > libgfortran/configure | 142 ++++++++++++++++++++++++++++++++ > libgfortran/configure.ac | 1 + > libgfortran/libgfortran.spec.in | 2 +- > 5 files changed, 147 insertions(+), 3 deletions(-) > > diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am > index 8d104321567..6fc4b465c6e 100644 > --- a/libgfortran/Makefile.am > +++ b/libgfortran/Makefile.am > @@ -42,7 +42,7 @@ libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS) > libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` \ > $(LTLDFLAGS) $(LIBQUADLIB) ../libbacktrace/libbacktrace.la \ > $(HWCAP_LDFLAGS) \ > - -lm $(extra_ldflags_libgfortran) \ > + $(LIBM) $(extra_ldflags_libgfortran) \ > $(version_arg) -Wc,-shared-libgcc > libgfortran_la_DEPENDENCIES = $(version_dep) libgfortran.spec $(LIBQUADLIB_DEP) > > diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac > index 523eb24bca1..513fd80b936 100644 > --- a/libgfortran/configure.ac > +++ b/libgfortran/configure.ac > @@ -260,6 +260,7 @@ AC_PROG_INSTALL > #AC_MSG_NOTICE([====== Starting libtool configuration]) > AC_LIBTOOL_DLOPEN > AM_PROG_LIBTOOL > +AC_CHECK_LIBM > ACX_LT_HOST_FLAGS > AC_SUBST(enable_shared) > AC_SUBST(enable_static) > diff --git a/libgfortran/libgfortran.spec.in b/libgfortran/libgfortran.spec.in > index 95aa3f842a3..b870e78c151 100644 > --- a/libgfortran/libgfortran.spec.in > +++ b/libgfortran/libgfortran.spec.in > @@ -5,4 +5,4 @@ > # > > %rename lib liborig > -*lib: @LIBQUADSPEC@ -lm %(libgcc) %(liborig) > +*lib: @LIBQUADSPEC@ @LIBM@ %(libgcc) %(liborig) > -- > 2.24.3 (Apple Git-128) > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libgfortran : Use the libtool macro to determine libm availability. 2021-08-20 7:34 ` Richard Biener @ 2021-08-20 7:52 ` Tobias Burnus 2021-08-20 8:02 ` Iain Sandoe 2021-08-21 22:18 ` Eric Gallager 0 siblings, 2 replies; 7+ messages in thread From: Tobias Burnus @ 2021-08-20 7:52 UTC (permalink / raw) To: Richard Biener, Iain Sandoe; +Cc: GCC Patches, GCC Fortran On 20.08.21 09:34, Richard Biener via Fortran wrote: > On Thu, Aug 19, 2021 at 10:10 PM Iain Sandoe <iain@sandoe.co.uk> wrote: >> libm is not needed on Darwin, and should not be added unconditionally >> even if that is (mostly) harmless since it is a symlink to libc. >> >> tested on x86_64, i686-darwin, x86_64-linux, >> OK for master? > OK. > Richard. Looks also good to me – but for completeness and as background, I also want to remark the following. (At least as I understand it, I did not dig deeper.) > --- a/libgfortran/configure.ac > +++ b/libgfortran/configure.ac > ... > +AC_CHECK_LIBM This CHECK_LIBM has a hard-coded list of targets and for some (like Darwin) it simply does not set $LIBM. >> +++ b/libgfortran/Makefile.am >> @@ -42,7 +42,7 @@ libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS) >> libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` \ >> $(LTLDFLAGS) $(LIBQUADLIB) ../libbacktrace/libbacktrace.la \ >> $(HWCAP_LDFLAGS) \ >> - -lm $(extra_ldflags_libgfortran) \ >> + $(LIBM) $(extra_ldflags_libgfortran) \ I think usage like this one do not actually link '-lm' as gcc/config/darwin.h contains: #define LINK_SPEC \ ... %:remove-outfile(-lm) \ However, this spec change comes too early for: > --- a/libgfortran/libgfortran.spec.in > +++ b/libgfortran/libgfortran.spec.in > @@ -5,4 +5,4 @@ > # > > %rename lib liborig > -*lib: @LIBQUADSPEC@ -lm %(libgcc) %(liborig) > +*lib: @LIBQUADSPEC@ @LIBM@ %(libgcc) %(liborig) NIT: There are two spaces instead of one before @LIBM@. Thus, it makes sense to the unconditional '-lm' from the .spec file. Tobias ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libgfortran : Use the libtool macro to determine libm availability. 2021-08-20 7:52 ` Tobias Burnus @ 2021-08-20 8:02 ` Iain Sandoe 2021-08-21 22:18 ` Eric Gallager 1 sibling, 0 replies; 7+ messages in thread From: Iain Sandoe @ 2021-08-20 8:02 UTC (permalink / raw) To: Tobias Burnus; +Cc: Richard Biener, GCC Patches, GCC Fortran > On 20 Aug 2021, at 08:52, Tobias Burnus <tobias@codesourcery.com> wrote: > > On 20.08.21 09:34, Richard Biener via Fortran wrote: > >> On Thu, Aug 19, 2021 at 10:10 PM Iain Sandoe <iain@sandoe.co.uk> wrote: >>> libm is not needed on Darwin, and should not be added unconditionally >>> even if that is (mostly) harmless since it is a symlink to libc. >>> >>> tested on x86_64, i686-darwin, x86_64-linux, >>> OK for master? >> OK. >> Richard. > > Looks also good to me – but for completeness and as background, I also > want to remark the following. > > (At least as I understand it, I did not dig deeper.) > >> --- a/libgfortran/configure.ac >> +++ b/libgfortran/configure.ac >> ... >> +AC_CHECK_LIBM > > This CHECK_LIBM has a hard-coded list of targets and for some (like > Darwin) it simply does not set $LIBM. however, it’s one place to make changes (and, probably, I suppose a table is about the only way to do it, reliably)? >>> +++ b/libgfortran/Makefile.am >>> @@ -42,7 +42,7 @@ libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS) >>> libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` \ >>> $(LTLDFLAGS) $(LIBQUADLIB) ../libbacktrace/libbacktrace.la \ >>> $(HWCAP_LDFLAGS) \ >>> - -lm $(extra_ldflags_libgfortran) \ >>> + $(LIBM) $(extra_ldflags_libgfortran) \ > > I think usage like this one do not actually link '-lm' as > gcc/config/darwin.h contains: > > #define LINK_SPEC \ > ... > %:remove-outfile(-lm) \ right, I have tried to be defensive w.r.t unneeded libs in the Darwin specs, but as you note it doesn’t work for libraries added as infiles… It’s also consistent to use the configure-discovered value, right? > However, this spec change comes too early for: >> --- a/libgfortran/libgfortran.spec.in >> +++ b/libgfortran/libgfortran.spec.in >> @@ -5,4 +5,4 @@ >> # >> >> %rename lib liborig >> -*lib: @LIBQUADSPEC@ -lm %(libgcc) %(liborig) >> +*lib: @LIBQUADSPEC@ @LIBM@ %(libgcc) %(liborig) > NIT: There are two spaces instead of one before @LIBM@. … thanks. > > Thus, it makes sense to the unconditional '-lm' from the .spec file. I’ll hold off committing until this evening in case there are other comments. thanks Iain > > Tobias > > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libgfortran : Use the libtool macro to determine libm availability. 2021-08-20 7:52 ` Tobias Burnus 2021-08-20 8:02 ` Iain Sandoe @ 2021-08-21 22:18 ` Eric Gallager 2021-08-22 8:44 ` Iain Sandoe 1 sibling, 1 reply; 7+ messages in thread From: Eric Gallager @ 2021-08-21 22:18 UTC (permalink / raw) To: Tobias Burnus; +Cc: Richard Biener, Iain Sandoe, GCC Patches, GCC Fortran On Fri, Aug 20, 2021 at 3:53 AM Tobias Burnus <tobias@codesourcery.com> wrote: > > On 20.08.21 09:34, Richard Biener via Fortran wrote: > > > On Thu, Aug 19, 2021 at 10:10 PM Iain Sandoe <iain@sandoe.co.uk> wrote: > >> libm is not needed on Darwin, and should not be added unconditionally > >> even if that is (mostly) harmless since it is a symlink to libc. > >> > >> tested on x86_64, i686-darwin, x86_64-linux, > >> OK for master? > > OK. > > Richard. > > Looks also good to me – but for completeness and as background, I also > want to remark the following. > > (At least as I understand it, I did not dig deeper.) > > > --- a/libgfortran/configure.ac > > +++ b/libgfortran/configure.ac > > ... > > +AC_CHECK_LIBM > > This CHECK_LIBM has a hard-coded list of targets and for some (like > Darwin) it simply does not set $LIBM. I thought the proper macro to use was LT_LIB_M ? > > >> +++ b/libgfortran/Makefile.am > >> @@ -42,7 +42,7 @@ libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS) > >> libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` \ > >> $(LTLDFLAGS) $(LIBQUADLIB) ../libbacktrace/libbacktrace.la \ > >> $(HWCAP_LDFLAGS) \ > >> - -lm $(extra_ldflags_libgfortran) \ > >> + $(LIBM) $(extra_ldflags_libgfortran) \ > > I think usage like this one do not actually link '-lm' as > gcc/config/darwin.h contains: > > #define LINK_SPEC \ > ... > %:remove-outfile(-lm) \ > > However, this spec change comes too early for: > > --- a/libgfortran/libgfortran.spec.in > > +++ b/libgfortran/libgfortran.spec.in > > @@ -5,4 +5,4 @@ > > # > > > > %rename lib liborig > > -*lib: @LIBQUADSPEC@ -lm %(libgcc) %(liborig) > > +*lib: @LIBQUADSPEC@ @LIBM@ %(libgcc) %(liborig) > NIT: There are two spaces instead of one before @LIBM@. > > Thus, it makes sense to the unconditional '-lm' from the .spec file. > > Tobias > > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libgfortran : Use the libtool macro to determine libm availability. 2021-08-21 22:18 ` Eric Gallager @ 2021-08-22 8:44 ` Iain Sandoe 2021-08-27 15:53 ` Iain Sandoe 0 siblings, 1 reply; 7+ messages in thread From: Iain Sandoe @ 2021-08-22 8:44 UTC (permalink / raw) To: Eric Gallager; +Cc: Tobias Burnus, Richard Biener, GCC Patches, GCC Fortran > On 21 Aug 2021, at 23:18, Eric Gallager <egall@gwmail.gwu.edu> wrote: > > On Fri, Aug 20, 2021 at 3:53 AM Tobias Burnus <tobias@codesourcery.com> wrote: >> >> On 20.08.21 09:34, Richard Biener via Fortran wrote: >> >>> On Thu, Aug 19, 2021 at 10:10 PM Iain Sandoe <iain@sandoe.co.uk> wrote: >>>> libm is not needed on Darwin, and should not be added unconditionally >>>> even if that is (mostly) harmless since it is a symlink to libc. >>>> >>>> tested on x86_64, i686-darwin, x86_64-linux, >>>> OK for master? >>> OK. >>> Richard. >> >> Looks also good to me – but for completeness and as background, I also >> want to remark the following. >> >> (At least as I understand it, I did not dig deeper.) >> >>> --- a/libgfortran/configure.ac >>> +++ b/libgfortran/configure.ac >>> ... >>> +AC_CHECK_LIBM >> >> This CHECK_LIBM has a hard-coded list of targets and for some (like >> Darwin) it simply does not set $LIBM. > > I thought the proper macro to use was LT_LIB_M ? you could well be right, libtool.m4 contains: # Old name: AU_ALIAS([AC_CHECK_LIBM], [LT_LIB_M]) I guess the patch can be changed and then do another round of testing … … will add this to the TODO, and withdraw the current patch. Iain ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libgfortran : Use the libtool macro to determine libm availability. 2021-08-22 8:44 ` Iain Sandoe @ 2021-08-27 15:53 ` Iain Sandoe 0 siblings, 0 replies; 7+ messages in thread From: Iain Sandoe @ 2021-08-27 15:53 UTC (permalink / raw) To: GCC Fortran; +Cc: Eric Gallager, Tobias Burnus, GCC Patches > On 22 Aug 2021, at 09:44, Iain Sandoe <iain@sandoe.co.uk> wrote: > > > >> On 21 Aug 2021, at 23:18, Eric Gallager <egall@gwmail.gwu.edu> wrote: >> >> On Fri, Aug 20, 2021 at 3:53 AM Tobias Burnus <tobias@codesourcery.com> wrote: >>> >>> On 20.08.21 09:34, Richard Biener via Fortran wrote: >>> >>>> On Thu, Aug 19, 2021 at 10:10 PM Iain Sandoe <iain@sandoe.co.uk> wrote: >>>>> libm is not needed on Darwin, and should not be added unconditionally >>>>> even if that is (mostly) harmless since it is a symlink to libc. >>>>> >>>>> tested on x86_64, i686-darwin, x86_64-linux, >>>>> OK for master? >>>> OK. >>>> Richard. >>> >>> Looks also good to me – but for completeness and as background, I also >>> want to remark the following. >>> >>> (At least as I understand it, I did not dig deeper.) >>> >>>> --- a/libgfortran/configure.ac >>>> +++ b/libgfortran/configure.ac >>>> ... >>>> +AC_CHECK_LIBM >>> >>> This CHECK_LIBM has a hard-coded list of targets and for some (like >>> Darwin) it simply does not set $LIBM. >> >> I thought the proper macro to use was LT_LIB_M ? > > you could well be right, libtool.m4 contains: > > # Old name: > AU_ALIAS([AC_CHECK_LIBM], [LT_LIB_M]) > > I guess the patch can be changed and then do another round of testing … > … will add this to the TODO, and withdraw the current patch. this is what I pushed after retesting (with the extras space removed too): ====== libgfortran: Use the libtool macro to determine libm availability. We recently had a report of build failure against a Darwin branch on the latest OS release. This was because (temporarily) the symlink from libm.dylib => libSystem.dylib had been removed/omitted. libm is not needed on Darwin, and should not be added unconditionally even if that is (mostly) harmless since it is a symlink to libc. There could be cases where the addition was not completely harmless because the presentation of the symlink would cause the symbols exposed in libSystem to be considered ahead of ones presented in convenience libraries. libgfortran/ChangeLog: * Makefile.am: Use configured libm availability. * Makefile.in: Regenerate. * configure: Regenerate. * configure.ac: Use libtool macro to find libm availability. * libgfortran.spec.in: Use configured libm availability. --- libgfortran/Makefile.am | 2 +- libgfortran/Makefile.in | 3 +- libgfortran/configure | 146 +++++++++++++++++++++++++++++++- libgfortran/configure.ac | 1 + libgfortran/libgfortran.spec.in | 2 +- 5 files changed, 149 insertions(+), 5 deletions(-) diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am index 8d104321567..6fc4b465c6e 100644 --- a/libgfortran/Makefile.am +++ b/libgfortran/Makefile.am @@ -42,7 +42,7 @@ libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS) libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` \ $(LTLDFLAGS) $(LIBQUADLIB) ../libbacktrace/libbacktrace.la \ $(HWCAP_LDFLAGS) \ - -lm $(extra_ldflags_libgfortran) \ + $(LIBM) $(extra_ldflags_libgfortran) \ $(version_arg) -Wc,-shared-libgcc libgfortran_la_DEPENDENCIES = $(version_dep) libgfortran.spec $(LIBQUADLIB_DEP) diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index 523eb24bca1..a77509801e6 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -260,6 +260,7 @@ AC_PROG_INSTALL #AC_MSG_NOTICE([====== Starting libtool configuration]) AC_LIBTOOL_DLOPEN AM_PROG_LIBTOOL +LT_LIB_M ACX_LT_HOST_FLAGS AC_SUBST(enable_shared) AC_SUBST(enable_static) diff --git a/libgfortran/libgfortran.spec.in b/libgfortran/libgfortran.spec.in index 95aa3f842a3..367d485c230 100644 --- a/libgfortran/libgfortran.spec.in +++ b/libgfortran/libgfortran.spec.in @@ -5,4 +5,4 @@ # %rename lib liborig -*lib: @LIBQUADSPEC@ -lm %(libgcc) %(liborig) +*lib: @LIBQUADSPEC@ @LIBM@ %(libgcc) %(liborig) -- 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-27 15:53 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-19 20:09 [PATCH] libgfortran : Use the libtool macro to determine libm availability Iain Sandoe 2021-08-20 7:34 ` Richard Biener 2021-08-20 7:52 ` Tobias Burnus 2021-08-20 8:02 ` Iain Sandoe 2021-08-21 22:18 ` Eric Gallager 2021-08-22 8:44 ` Iain Sandoe 2021-08-27 15:53 ` Iain Sandoe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).