resolv: Do not build libanl.so for ABIs starting at 2.35

Message ID 20211229163951.3517051-1-adhemerval.zanella@linaro.org
State Superseded
Headers
Series resolv: Do not build libanl.so for ABIs starting at 2.35 |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Adhemerval Zanella Dec. 29, 2021, 4:39 p.m. UTC
  ---
 resolv/Makefile | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
  

Comments

Florian Weimer Dec. 29, 2021, 6:30 p.m. UTC | #1
* Adhemerval Zanella:

> ---
>  resolv/Makefile | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/resolv/Makefile b/resolv/Makefile
> index 59e599535c..5606eab1fd 100644
> --- a/resolv/Makefile
> +++ b/resolv/Makefile
> @@ -78,8 +78,12 @@ generate := mtrace-tst-leaks.out tst-leaks.mtrace tst-leaks2.mtrace
>  
>  extra-libs := libresolv libnss_dns
>  ifeq ($(have-thread-library),yes)
> -extra-libs += libanl
>  routines += gai_sigqueue
> +endif
> +
> +ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
> +# Empty compatibility library for old binaries.
> +extra-libs += libanl
>  
>  tests += \
>    tst-bug18665 \
> @@ -176,6 +180,11 @@ $(libanl-routines-var) += \
>  libanl-routines += libanl-compat
>  libanl-shared-only-routines += libanl-compat
>  
> +# Pretend that libanl.so is a linker script, so that the symbolic link
> +# is not installed.
> +install-lib-ldscripts = libanl.so
> +$(inst_libdir)/libanl.so:
> +

Looks okay.  But the second hunk should be in a separate commit; I think
we should backport it.

Thanks,
Florian
  
Adhemerval Zanella Dec. 30, 2021, 2:06 p.m. UTC | #2
On 29/12/2021 15:30, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> ---
>>  resolv/Makefile | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/resolv/Makefile b/resolv/Makefile
>> index 59e599535c..5606eab1fd 100644
>> --- a/resolv/Makefile
>> +++ b/resolv/Makefile
>> @@ -78,8 +78,12 @@ generate := mtrace-tst-leaks.out tst-leaks.mtrace tst-leaks2.mtrace
>>  
>>  extra-libs := libresolv libnss_dns
>>  ifeq ($(have-thread-library),yes)
>> -extra-libs += libanl
>>  routines += gai_sigqueue
>> +endif
>> +
>> +ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
>> +# Empty compatibility library for old binaries.
>> +extra-libs += libanl
>>  
>>  tests += \
>>    tst-bug18665 \
>> @@ -176,6 +180,11 @@ $(libanl-routines-var) += \
>>  libanl-routines += libanl-compat
>>  libanl-shared-only-routines += libanl-compat
>>  
>> +# Pretend that libanl.so is a linker script, so that the symbolic link
>> +# is not installed.
>> +install-lib-ldscripts = libanl.so
>> +$(inst_libdir)/libanl.so:
>> +
> 
> Looks okay.  But the second hunk should be in a separate commit; I think
> we should backport it.

Done.
  
Stafford Horne Jan. 3, 2022, 4:53 a.m. UTC | #3
On Thu, Dec 30, 2021 at 11:06:31AM -0300, Adhemerval Zanella via Libc-alpha wrote:
> 
> 
> On 29/12/2021 15:30, Florian Weimer wrote:
> > * Adhemerval Zanella:
> > 
> >> ---
> >>  resolv/Makefile | 11 ++++++++++-
> >>  1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/resolv/Makefile b/resolv/Makefile
> >> index 59e599535c..5606eab1fd 100644
> >> --- a/resolv/Makefile
> >> +++ b/resolv/Makefile
> >> @@ -78,8 +78,12 @@ generate := mtrace-tst-leaks.out tst-leaks.mtrace tst-leaks2.mtrace
> >>  
> >>  extra-libs := libresolv libnss_dns
> >>  ifeq ($(have-thread-library),yes)
> >> -extra-libs += libanl
> >>  routines += gai_sigqueue
> >> +endif
> >> +
> >> +ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
> >> +# Empty compatibility library for old binaries.
> >> +extra-libs += libanl
> >>  
> >>  tests += \
> >>    tst-bug18665 \
> >> @@ -176,6 +180,11 @@ $(libanl-routines-var) += \
> >>  libanl-routines += libanl-compat
> >>  libanl-shared-only-routines += libanl-compat
> >>  
> >> +# Pretend that libanl.so is a linker script, so that the symbolic link
> >> +# is not installed.
> >> +install-lib-ldscripts = libanl.so
> >> +$(inst_libdir)/libanl.so:
> >> +
> > 
> > Looks okay.  But the second hunk should be in a separate commit; I think
> > we should backport it.
> 
> Done.

After this make check fails on the OpenRISC port.

tst-linkall-static fails with:

    make[3]: *** No rule to make target 'build/glibcs/or1k-linux-gnu-soft/glibc/resolv/libanl.a', needed by 'build/glibcs/or1k-linux-gnu-soft/glibc/elf/tst-linkall-static'.  Stop.

We may want to keep libanl.a around as an empty binary for build compatibility
reasons like we did with libutil.  There may be builds wheck hard code -lanl,
maybe not as much as -lutil though.

Never the less, this patch seems to fix it:

diff --git a/elf/Makefile b/elf/Makefile
index 06bfa1642f..71bc2987df 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -1683,7 +1683,6 @@ $(objpfx)tst-linkall-static: \
   $(common-objpfx)resolv/libresolv.a \
   $(common-objpfx)login/libutil.a \
   $(common-objpfx)rt/librt.a \
-  $(common-objpfx)resolv/libanl.a \
   $(static-thread-library)
 
 ifeq ($(build-crypt),yes)
@@ -1704,6 +1703,12 @@ $(objpfx)tst-linkall-static: \
 endif
 endif
 
+# Libanl is only available up to GLIBC_2.34
+ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
+$(objpfx)tst-linkall-static: \
+  $(common-objpfx)resolv/libanl.a
+endif
+
 # The application depends on the DSO, and the DSO loads the plugin.
 # The plugin also depends on the DSO. This creates the circular
 # dependency via dlopen that we're testing to make sure works.
  
Adhemerval Zanella Jan. 3, 2022, 1:58 p.m. UTC | #4
On 03/01/2022 01:53, Stafford Horne wrote:
> On Thu, Dec 30, 2021 at 11:06:31AM -0300, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 29/12/2021 15:30, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> ---
>>>>  resolv/Makefile | 11 ++++++++++-
>>>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/resolv/Makefile b/resolv/Makefile
>>>> index 59e599535c..5606eab1fd 100644
>>>> --- a/resolv/Makefile
>>>> +++ b/resolv/Makefile
>>>> @@ -78,8 +78,12 @@ generate := mtrace-tst-leaks.out tst-leaks.mtrace tst-leaks2.mtrace
>>>>  
>>>>  extra-libs := libresolv libnss_dns
>>>>  ifeq ($(have-thread-library),yes)
>>>> -extra-libs += libanl
>>>>  routines += gai_sigqueue
>>>> +endif
>>>> +
>>>> +ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
>>>> +# Empty compatibility library for old binaries.
>>>> +extra-libs += libanl
>>>>  
>>>>  tests += \
>>>>    tst-bug18665 \
>>>> @@ -176,6 +180,11 @@ $(libanl-routines-var) += \
>>>>  libanl-routines += libanl-compat
>>>>  libanl-shared-only-routines += libanl-compat
>>>>  
>>>> +# Pretend that libanl.so is a linker script, so that the symbolic link
>>>> +# is not installed.
>>>> +install-lib-ldscripts = libanl.so
>>>> +$(inst_libdir)/libanl.so:
>>>> +
>>>
>>> Looks okay.  But the second hunk should be in a separate commit; I think
>>> we should backport it.
>>
>> Done.
> 
> After this make check fails on the OpenRISC port.
> 
> tst-linkall-static fails with:
> 
>     make[3]: *** No rule to make target 'build/glibcs/or1k-linux-gnu-soft/glibc/resolv/libanl.a', needed by 'build/glibcs/or1k-linux-gnu-soft/glibc/elf/tst-linkall-static'.  Stop.
> 
> We may want to keep libanl.a around as an empty binary for build compatibility
> reasons like we did with libutil.  There may be builds wheck hard code -lanl,
> maybe not as much as -lutil though.
> 
> Never the less, this patch seems to fix it:

I think should just remove the libanl.a reference, as per 699361795f6af8.
I will fix it.

> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 06bfa1642f..71bc2987df 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1683,7 +1683,6 @@ $(objpfx)tst-linkall-static: \
>    $(common-objpfx)resolv/libresolv.a \
>    $(common-objpfx)login/libutil.a \
>    $(common-objpfx)rt/librt.a \
> -  $(common-objpfx)resolv/libanl.a \
>    $(static-thread-library)
>  
>  ifeq ($(build-crypt),yes)
> @@ -1704,6 +1703,12 @@ $(objpfx)tst-linkall-static: \
>  endif
>  endif
>  
> +# Libanl is only available up to GLIBC_2.34
> +ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
> +$(objpfx)tst-linkall-static: \
> +  $(common-objpfx)resolv/libanl.a
> +endif
> +
>  # The application depends on the DSO, and the DSO loads the plugin.
>  # The plugin also depends on the DSO. This creates the circular
>  # dependency via dlopen that we're testing to make sure works.
>
  
Stafford Horne Feb. 11, 2022, 11:34 a.m. UTC | #5
On Mon, Jan 03, 2022 at 10:58:53AM -0300, Adhemerval Zanella wrote:
> 
> 
> On 03/01/2022 01:53, Stafford Horne wrote:
> > On Thu, Dec 30, 2021 at 11:06:31AM -0300, Adhemerval Zanella via Libc-alpha wrote:
> >>
> >>
> >> On 29/12/2021 15:30, Florian Weimer wrote:
> >>> * Adhemerval Zanella:
> >>>
> >>>> ---
> >>>>  resolv/Makefile | 11 ++++++++++-
> >>>>  1 file changed, 10 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/resolv/Makefile b/resolv/Makefile
> >>>> index 59e599535c..5606eab1fd 100644
> >>>> --- a/resolv/Makefile
> >>>> +++ b/resolv/Makefile
> >>>> @@ -78,8 +78,12 @@ generate := mtrace-tst-leaks.out tst-leaks.mtrace tst-leaks2.mtrace
> >>>>  
> >>>>  extra-libs := libresolv libnss_dns
> >>>>  ifeq ($(have-thread-library),yes)
> >>>> -extra-libs += libanl
> >>>>  routines += gai_sigqueue
> >>>> +endif
> >>>> +
> >>>> +ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
> >>>> +# Empty compatibility library for old binaries.
> >>>> +extra-libs += libanl
> >>>>  
> >>>>  tests += \
> >>>>    tst-bug18665 \
> >>>> @@ -176,6 +180,11 @@ $(libanl-routines-var) += \
> >>>>  libanl-routines += libanl-compat
> >>>>  libanl-shared-only-routines += libanl-compat
> >>>>  
> >>>> +# Pretend that libanl.so is a linker script, so that the symbolic link
> >>>> +# is not installed.
> >>>> +install-lib-ldscripts = libanl.so
> >>>> +$(inst_libdir)/libanl.so:
> >>>> +
> >>>
> >>> Looks okay.  But the second hunk should be in a separate commit; I think
> >>> we should backport it.
> >>
> >> Done.
> > 
> > After this make check fails on the OpenRISC port.
> > 
> > tst-linkall-static fails with:
> > 
> >     make[3]: *** No rule to make target 'build/glibcs/or1k-linux-gnu-soft/glibc/resolv/libanl.a', needed by 'build/glibcs/or1k-linux-gnu-soft/glibc/elf/tst-linkall-static'.  Stop.
> > 
> > We may want to keep libanl.a around as an empty binary for build compatibility
> > reasons like we did with libutil.  There may be builds wheck hard code -lanl,
> > maybe not as much as -lutil though.
> > 
> > Never the less, this patch seems to fix it:
> 
> I think should just remove the libanl.a reference, as per 699361795f6af8.
> I will fix it.

One more thing, sorry I just picked this up now.  After the fixes we are now
getting:

    make: Entering directory '/home/shorne/work/gnu-toolchain/glibc/resolv'
    make: *** No rule to make target '/home/shorne/work/gnu-toolchain/build-glibc/resolv/tst-resolv-res_ninit.out', needed by '/home/shorne/work/gnu-toolchain/build-glibc/resolv/mtrace-tst-resolv-res_ninit.out'.  Stop.
    make: Leaving directory '/home/shorne/work/gnu-toolchain/glibc/resolv'

The "if" condition change blocked many of the tst-resolv* tests from being
built or run for 2.35 only ports.  Is that expected?  There are now a few
things like above that are missing their dependencies. Do we need:

diff --git a/resolv/Makefile b/resolv/Makefile
index c465479e8b..11e3b203a3 100644
--- a/resolv/Makefile
+++ b/resolv/Makefile
@@ -84,7 +84,9 @@ endif
 ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
 # Empty compatibility library for old binaries.
 extra-libs += libanl
+endif
 
+ifeq ($(have-thread-library),yes)
 tests += \
   tst-bug18665 \
   tst-bug18665-tcp \
  
Florian Weimer Feb. 11, 2022, 11:42 a.m. UTC | #6
* Stafford Horne:

> One more thing, sorry I just picked this up now.  After the fixes we are now
> getting:
>
>     make: Entering directory '/home/shorne/work/gnu-toolchain/glibc/resolv'
>     make: *** No rule to make target '/home/shorne/work/gnu-toolchain/build-glibc/resolv/tst-resolv-res_ninit.out', needed by '/home/shorne/work/gnu-toolchain/build-glibc/resolv/mtrace-tst-resolv-res_ninit.out'.  Stop.
>     make: Leaving directory '/home/shorne/work/gnu-toolchain/glibc/resolv'
>
> The "if" condition change blocked many of the tst-resolv* tests from being
> built or run for 2.35 only ports.  Is that expected?  There are now a few
> things like above that are missing their dependencies. Do we need:
>
> diff --git a/resolv/Makefile b/resolv/Makefile
> index c465479e8b..11e3b203a3 100644
> --- a/resolv/Makefile
> +++ b/resolv/Makefile
> @@ -84,7 +84,9 @@ endif
>  ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
>  # Empty compatibility library for old binaries.
>  extra-libs += libanl
> +endif
>  
> +ifeq ($(have-thread-library),yes)
>  tests += \
>    tst-bug18665 \
>    tst-bug18665-tcp \

Sorry about that, I should have reviewed this more closely.

Can you move the libanl settings closer together?
$(libanl-routines-var) should be outside the $(have-GLIBC_2.34)
condition, libanl-routines += libanl-compat should be inside it.

Thanks,
Florian
  

Patch

diff --git a/resolv/Makefile b/resolv/Makefile
index 59e599535c..5606eab1fd 100644
--- a/resolv/Makefile
+++ b/resolv/Makefile
@@ -78,8 +78,12 @@  generate := mtrace-tst-leaks.out tst-leaks.mtrace tst-leaks2.mtrace
 
 extra-libs := libresolv libnss_dns
 ifeq ($(have-thread-library),yes)
-extra-libs += libanl
 routines += gai_sigqueue
+endif
+
+ifeq ($(have-GLIBC_2.34)$(have-thread-library),yesyes)
+# Empty compatibility library for old binaries.
+extra-libs += libanl
 
 tests += \
   tst-bug18665 \
@@ -176,6 +180,11 @@  $(libanl-routines-var) += \
 libanl-routines += libanl-compat
 libanl-shared-only-routines += libanl-compat
 
+# Pretend that libanl.so is a linker script, so that the symbolic link
+# is not installed.
+install-lib-ldscripts = libanl.so
+$(inst_libdir)/libanl.so:
+
 subdir-dirs = nss_dns
 vpath %.c nss_dns