[Buildroot] [PATCH v11 4/4] toolchain-external: create symlink ARCH_LIB_DIR->lib

Sun Jan 24 20:23:13 UTC 2016

Hi Romain,

Thanks a lot for reviewing/testing!

On Sun, Jan 24, 2016 at 8:59 PM, Romain Naour <romain.naour at gmail.com> wrote:
> Hi Thomas, All,
>
> Le 20/01/2016 20:11, Thomas De Schampheleire a écrit :
>> From: Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>>
>> Currently, following symbolic links are created in both target and
>> staging directories:
>> - lib(32|64) --> lib
>> - usr/lib(32|64) --> lib
>>
>> The decision for lib32 or lib64 is based on the target architecture
>> configuration in buildroot (BR2_ARCH_IS_64).
>>
>> In at least one case this is not correct: when building for a Cavium Octeon
>> III processor using the toolchain from the Cavium Networks SDK, and
>> specifying -march=octeon3 in BR2_TARGET_OPTIMIZATION, libraries are expected
>> in directory 'lib32-fp' rather than 'lib32' (ABI=n32; likewise for
>> lib64-fp in case of ABI=n64)
>>
>> More generally the correct symbolic link is from (usr/)${ARCH_LIB_DIR}->lib.
>> However, feedback from Arnout Vandecappelle is that there are packages that
>> do depend on the lib32/lib64 symlink, even if ARCH_LIB_DIR is different.
>> Hence, these links must be kept.
>>
>> Fix the problem as follows:
>> - For internal toolchains: no change
>> - For external toolchains: create a symlink ARCH_LIB_DIR->lib if
>>   (usr/)ARCH_LIB_DIR does not exist yet.
>>
>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni at free-electrons.com>
>> Cc: Arnout Vandecappelle <arnout at mind.be>
>> Cc: "Yann E. Morin" <yann.morin.1998 at free.fr>
>> Cc: Romain Naour <romain.naour at gmail.com>
>> Cc: Peter Korsgaard <peter at korsgaard.com>
>>
>> ---
>> v11:
>> - check for existence of destination instead of explicitly checking on the
>>   known values lib/lib32/lib64. (ThomasP)
>> v10:
>> - simplify after realization that skeleton symlink creation can be kept
>>   (thanks Thomas Petazzoni for noticing this)
>> v9:
>> - remove redundant mkdir's (handled by skeleton) (Yann)
>> v8:
>> - use helper only for external toolchain and incorporate ARCH_LIB_DIR
>>   definition (Arnout)
>> - keep lib32/lib64->lib symlink anyway
>> v7: rebase
>> v6: rebase only
>> v5:
>> - move internal toolchain logic into gcc-initial.mk
>> - also silence the internal toolchain link steps with $(Q)
>> v4:
>> - merge both helpers into one
>> - remove the separate target for the internal toolchain and hook into
>>   gcc-initial
>> - re-add deleted comment about MIPS64/n32
>> v3:
>> - update commit message wrapping
>> - change dependency on $(BUILD_DIR) to a order-only dependency
>> v2:
>> - fix 'lib32-fp' leftover in toolchain-buildroot
>> - silence commands creating symlink with $(Q)
>> - fix case where ARCH_LIB_DIR is 'lib'
>>
>> Note: in output/staging/usr/ there would still be more directories than I
>> think are really necessary. This behavior is not changed by this patch, it
>> was already present before.
>> For example, with the mentioned Octeon III toolchain, output/staging/usr/
>> contains:
>>     bin      bin32      bin32-fp      bin64-fp,
>>     lib      lib32      lib32-fp      lib64-fp
>>     libexec  libexec32  libexec32-fp  libexec64-fp
>>     sbin     sbin32     sbin32-fp     sbin64-fp
>>
>> where bin/lib/libexec/sbin seem to be the 64-bit equivalents of
>> bin32/lib32/libexec32/sbin32.
>> This is related to the behavior of copy_toolchain_sysroot in
>> toolchain/helpers.mk. It already attempts to filter out the unnecessary lib*
>> directories, but does not care about any bin/sbin/libexec directories.
>> As this poses no known problem and is not impacted by this patch, I make no
>> attempt to change it.
>>
>>  toolchain/toolchain-external/toolchain-external.mk | 23 ++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
>> index ddefd01..6383f8b 100644
>> --- a/toolchain/toolchain-external/toolchain-external.mk
>> +++ b/toolchain/toolchain-external/toolchain-external.mk
>> @@ -517,6 +517,27 @@ endef
>>  TOOLCHAIN_EXTERNAL_POST_INSTALL_STAGING_HOOKS += TOOLCHAIN_EXTERNAL_MUSL_LD_LINK
>>  endif
>>
>> +# Create a symlink from (usr/)$(ARCH_LIB_DIR) to lib.
>> +# Note: the skeleton package additionally creates lib32->lib or lib64->lib
>> +# (as appropriate)
>> +#
>> +# $1: destination directory (TARGET_DIR / STAGING_DIR)
>> +create_lib_symlinks = \
>> +       $(Q)DESTDIR="$(strip $1)" ; \
>> +       ARCH_LIB_DIR="$(call toolchain_find_libdir,$(TOOLCHAIN_EXTERNAL_CC) $(TOOLCHAIN_EXTERNAL_CFLAGS))" ; \
>> +       if [ ! -f "$${DESTDIR}/$${ARCH_LIB_DIR}" -a ! -f "$${DESTDIR}/usr/$${ARCH_LIB_DIR}" ]; then \
>
> -f (regular file) ?
> I expected -d (directory) instead.
>
> With -f the test doesn't work and create a dead simlink (lib -> lib) in
> staging/lib and staging/usr/lib/ etc...
>

Is it correct that you see this in a case where ARCH_LIB_DIR is simply 'lib' ?
Seems I did not test that case then, and I will fix it.

For the case of ARCH_LIB_DIR=lib32-fp, ARCH_LIB_DIR does not exist at
all in DESTDIR/ and DESTDIR/usr. Links need to be created.
For the case of ARCH_LIB_DIR=lib64, DESTDIR/ARCH_LIB_DIR and
DESTDIR/usr/ARCH_LIB_DIR are symlinks already. No links need to be
created. This was the case I was testing, really.
For the case of ARCH_LIB_DIR=lib, DESTDIR/ARCH_LIB_DIR and
DESTDIR/usr/ARCH_LIB_DIR will be directories, as you noticed, and then
the test -f indeed fails. No links need to be created.

I think that a better test would then be 'test -e' (exists). Do you agree?
I'd need to test tomorrow and resend, thanks a lot for noticing!

Thanks,
Thomas