| Message ID | 20180307122647.GI8100@australia |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | toolchain-external: ld.so* vs ld.so.* | expand |
Hello, On Wed, 7 Mar 2018 13:26:47 +0100, Thomas De Schampheleire wrote: > I have a question on following commit: You like the difficult questions, pointing out a tiny detail (just a dot!) in an old patch :-) > The question is: did you intentionally remove the . before the final asterisk? > I.e. why is it not: > > TOOLCHAIN_EXTERNAL_LIBS += ld*.so.* > > as was the case before, even for the glibc+eabihf case? > I could not find a reference to why that specific change was made. > > Background is that I now notice (after upgrading to 2018.02 coming from > 2017.02.x) that an extra file is copied on my target system: the system used to > have just '/lib/ld.so.1' which is also what is encoded in the ELF files as > dynamic loader, but now there is also '/lib/ld-2.20.so' which is not actually > used and is non-stripped (due to an exception in target-finalize). > This adds about 150K on the root filesystem, which is quite a lot for an unused > file. > > So I wonder what would be wrong with following patch: > > diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk > --- a/toolchain/toolchain-external/pkg-toolchain-external.mk > +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk > @@ -108,7 +108,7 @@ endif > # Definitions of the list of libraries that should be copied to the target. > # > > -TOOLCHAIN_EXTERNAL_LIBS += ld*.so* libgcc_s.so.* libatomic.so.* > +TOOLCHAIN_EXTERNAL_LIBS += ld*.so.* libgcc_s.so.* libatomic.so.* > > ifeq ($(BR2_TOOLCHAIN_EXTERNAL_GLIBC)$(BR2_TOOLCHAIN_EXTERNAL_UCLIBC),y) > TOOLCHAIN_EXTERNAL_LIBS += libc.so.* libcrypt.so.* libdl.so.* libm.so.* libnsl.so.* libresolv.so.* librt.so.* libutil.so.* I looked at the commit and its commit message, and I can't remember why ld*.so.* was changed to ld*.so*, so I'd say that your patch is probably correct. Is there a way to improve our runtime tests to catch problems like this ? Best regards, Thomas
On Wed, Mar 07, 2018 at 01:58:53PM +0100, Thomas Petazzoni wrote: > Hello, > > On Wed, 7 Mar 2018 13:26:47 +0100, Thomas De Schampheleire wrote: > > > I have a question on following commit: > > You like the difficult questions, pointing out a tiny detail (just a > dot!) in an old patch :-) :-P > > > The question is: did you intentionally remove the . before the final asterisk? > > I.e. why is it not: > > > > TOOLCHAIN_EXTERNAL_LIBS += ld*.so.* > > > > as was the case before, even for the glibc+eabihf case? > > I could not find a reference to why that specific change was made. > > > > Background is that I now notice (after upgrading to 2018.02 coming from > > 2017.02.x) that an extra file is copied on my target system: the system used to > > have just '/lib/ld.so.1' which is also what is encoded in the ELF files as > > dynamic loader, but now there is also '/lib/ld-2.20.so' which is not actually > > used and is non-stripped (due to an exception in target-finalize). > > This adds about 150K on the root filesystem, which is quite a lot for an unused > > file. > > > > So I wonder what would be wrong with following patch: > > > > diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk > > --- a/toolchain/toolchain-external/pkg-toolchain-external.mk > > +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk > > @@ -108,7 +108,7 @@ endif > > # Definitions of the list of libraries that should be copied to the target. > > # > > > > -TOOLCHAIN_EXTERNAL_LIBS += ld*.so* libgcc_s.so.* libatomic.so.* > > +TOOLCHAIN_EXTERNAL_LIBS += ld*.so.* libgcc_s.so.* libatomic.so.* > > > > ifeq ($(BR2_TOOLCHAIN_EXTERNAL_GLIBC)$(BR2_TOOLCHAIN_EXTERNAL_UCLIBC),y) > > TOOLCHAIN_EXTERNAL_LIBS += libc.so.* libcrypt.so.* libdl.so.* libm.so.* libnsl.so.* libresolv.so.* librt.so.* libutil.so.* > > I looked at the commit and its commit message, and I can't remember why > ld*.so.* was changed to ld*.so*, so I'd say that your patch is probably > correct. Thanks, I'll let it run for a while on our system to see if any issue pops up, and then I will contribute a patch. > > Is there a way to improve our runtime tests to catch problems like > this ? I'm not really sure: we can of course add a hardcoded check for different toolchains about which files are present or should not be present, but it would not be a generic check to catch this type of issue. /Thomas
diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk --- a/toolchain/toolchain-external/pkg-toolchain-external.mk +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk @@ -108,7 +108,7 @@ endif # Definitions of the list of libraries that should be copied to the target. # -TOOLCHAIN_EXTERNAL_LIBS += ld*.so* libgcc_s.so.* libatomic.so.* +TOOLCHAIN_EXTERNAL_LIBS += ld*.so.* libgcc_s.so.* libatomic.so.* ifeq ($(BR2_TOOLCHAIN_EXTERNAL_GLIBC)$(BR2_TOOLCHAIN_EXTERNAL_UCLIBC),y) TOOLCHAIN_EXTERNAL_LIBS += libc.so.* libcrypt.so.* libdl.so.* libm.so.* libnsl.so.* libresolv.so.* librt.so.* libutil.so.*
Hi Thomas, I have a question on following commit: commit 32bec8ee2fb00c6750fa842bbb0eb79b0c081fa2 Author: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Date: Sun Jul 2 15:14:17 2017 +0200 toolchain-external: copy ld*.so* for all C libraries in which the base value of TOOLCHAIN_EXTERNAL_LIBS changed: (snippet) + +TOOLCHAIN_EXTERNAL_LIBS += ld*.so* + [..] -ifeq ($(BR2_TOOLCHAIN_EXTERNAL_GLIBC)$(BR2_ARM_EABIHF),yy) -TOOLCHAIN_EXTERNAL_LIBS += ld-linux-armhf.so.* -else -TOOLCHAIN_EXTERNAL_LIBS += ld*.so.* -endif The question is: did you intentionally remove the . before the final asterisk? I.e. why is it not: TOOLCHAIN_EXTERNAL_LIBS += ld*.so.* as was the case before, even for the glibc+eabihf case? I could not find a reference to why that specific change was made. Background is that I now notice (after upgrading to 2018.02 coming from 2017.02.x) that an extra file is copied on my target system: the system used to have just '/lib/ld.so.1' which is also what is encoded in the ELF files as dynamic loader, but now there is also '/lib/ld-2.20.so' which is not actually used and is non-stripped (due to an exception in target-finalize). This adds about 150K on the root filesystem, which is quite a lot for an unused file. So I wonder what would be wrong with following patch: Thanks, Thomas