[Buildroot] [PATCH v11 1/4] skeleton: move LIB_SYMLINK definition from Makefile

[Buildroot] [PATCH v11 1/4] skeleton: move LIB_SYMLINK definition from Makefile

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

Commit 7a6b83a211612ff95a1f5d35b2861ad5655ac8b1 introduced the skeleton
package, which took over the lib32/lib64 -> lib symlink creation from the
main Makefile.
However, the definition of the LIB_SYMLINK variable did not move along, for
no real reason.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
v11: place LIB_SYMLINK in skeleton iso toolchain/helpers.mk (ThomasP)
v10: new patch

 Makefile                     |  8 --------
 package/skeleton/skeleton.mk | 16 ++++++++++++----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 6bf9864..2e09611 100644
--- a/Makefile
+++ b/Makefile
@@ -501,14 +501,6 @@ world: target-post-image
 $(BUILD_DIR) $(TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
 	@mkdir -p $@
 
-# We make a symlink lib32->lib or lib64->lib as appropriate
-# MIPS64/n32 requires lib32 even though it's a 64-bit arch.
-ifeq ($(BR2_ARCH_IS_64)$(BR2_MIPS_NABI32),y)
-LIB_SYMLINK = lib64
-else
-LIB_SYMLINK = lib32
-endif
-
 # Populating the staging with the base directories is handled by the skeleton package
 $(STAGING_DIR):
 	@mkdir -p $(STAGING_DIR)
diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
index 6b5ab79..7618835 100644
--- a/package/skeleton/skeleton.mk
+++ b/package/skeleton/skeleton.mk
@@ -77,13 +77,21 @@ define SKELETON_USR_SYMLINKS_OR_DIRS
 endef
 endif
 
+# We make a symlink lib32->lib or lib64->lib as appropriate
+# MIPS64/n32 requires lib32 even though it's a 64-bit arch.
+ifeq ($(BR2_ARCH_IS_64)$(BR2_MIPS_NABI32),y)
+SKELETON_LIB_SYMLINK = lib64
+else
+SKELETON_LIB_SYMLINK = lib32
+endif
+
 define SKELETON_INSTALL_TARGET_CMDS
 	rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
 		--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
 		$(SKELETON_PATH)/ $(TARGET_DIR)/
 	$(call SKELETON_USR_SYMLINKS_OR_DIRS,$(TARGET_DIR))
-	ln -snf lib $(TARGET_DIR)/$(LIB_SYMLINK)
-	ln -snf lib $(TARGET_DIR)/usr/$(LIB_SYMLINK)
+	ln -snf lib $(TARGET_DIR)/$(SKELETON_LIB_SYMLINK)
+	ln -snf lib $(TARGET_DIR)/usr/$(SKELETON_LIB_SYMLINK)
 	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
 		$(TARGET_DIR_WARNING_FILE)
 endef
@@ -99,8 +107,8 @@ define SKELETON_INSTALL_STAGING_CMDS
 	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/sbin
 	$(INSTALL) -d -m 0755 $(STAGING_DIR)/usr/include
 	$(call SKELETON_USR_SYMLINKS_OR_DIRS,$(STAGING_DIR))
-	ln -snf lib $(STAGING_DIR)/$(LIB_SYMLINK)
-	ln -snf lib $(STAGING_DIR)/usr/$(LIB_SYMLINK)
+	ln -snf lib $(STAGING_DIR)/$(SKELETON_LIB_SYMLINK)
+	ln -snf lib $(STAGING_DIR)/usr/$(SKELETON_LIB_SYMLINK)
 endef
 
 SKELETON_TARGET_GENERIC_HOSTNAME = $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
-- 
2.4.10

[Buildroot] [PATCH v11 2/4] toolchain-external: don't exclude too much lib in sysroot rsync

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

The copy_toolchain_sysroot helper in toolchain/helpers.mk performs an
rsync of various directories from the extracted external toolchain to the
corresponding directory in staging.

The relevant (simplified) snippet is:
    for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
            rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
                    --exclude lib --exclude lib32 --exclude lib64 \
                    $${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
    done ; \

The exclusion logic of lib/lib32/lib64 has been added by commit
5628776c4a4d29d0715633ea463b64cc19e19c5a with the purpose of only copying
the relevant usr/lib* directory from the toolchain to staging, instead of
all. For example, if ARCH_LIB_DIR is 'lib64', then only usr/lib64 would be
copied and usr/lib and usr/lib32 are ignored. It works by ignoring any
lib/lib32/lib64 subdirectory on the rsync of 'usr' and then separately
copying usr/{lib,lib32,lib64} as appropriate. (The exclusion rules only have
impact on the files beneath the main source directory.)

However, on the rsync of 'usr', ANY of the following directories AND files
would be excluded:
    lib/
    lib
    lib32/
    foobar/something/lib/
    something-else/lib64/

while it is only the intention to skip directories directly under usr.

Therefore, add a leading (to restrict the scope to first-level) and trailing
(to restrict to directories) slash to the exclude pattern. From 'man rsync':

    - if the pattern starts with a / then it is anchored to [..] the root of
      the transfer.
    - if the pattern ends with a / then it will only match a directory, not
      a regular file, symlink, or device.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
v11: unchanged
v10: new patch

 toolchain/helpers.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index 70695ee..906993d 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -154,7 +154,8 @@ copy_toolchain_sysroot = \
 	for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
 		if [ -d $${ARCH_SYSROOT_DIR}/$$i ] ; then \
 			rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
-				--exclude lib --exclude lib32 --exclude lib64 \
+				--exclude '/lib/' --exclude '/lib32/' \
+				--exclude '/lib64/' \
 				$${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
 		fi ; \
 	done ; \
-- 
2.4.10

[Buildroot] [PATCH v11 3/4] toolchain-external: improve sysroot rsync if ARCH_LIB_DIR != lib/lib32/lib64

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

The copy_toolchain_sysroot helper in toolchain/helpers.mk performs an
rsync of various directories from the extracted external toolchain to the
corresponding directory in staging.

The relevant (simplified) snippet is:
	for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
		rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
			--exclude '/lib/' --exclude '/lib32/' \
			--exclude '/lib64/' \
			$${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
	done ; \

The exclusion logic of lib/lib32/lib64 has originally been added by commit
5628776c4a4d29d0715633ea463b64cc19e19c5a with the purpose of only copying
the relevant usr/lib* directory from the toolchain to staging, instead of
all. For example, if ARCH_LIB_DIR is 'lib64', then only usr/lib64 would be
copied and usr/lib and usr/lib32 are ignored. It works by ignoring any
lib/lib32/lib64 subdirectory on the rsync of 'usr' and then separately
copying usr/{lib,lib32,lib64} as appropriate. (The exclusion rules only have
impact on the files beneath the main source directory.)

However, ARCH_LIB_DIR can take other values than (lib, lib32, lib64), for
example lib32-fp or lib64-fp (Octeon III toolchain with -march=octeon3). In
the existing code, the rsync for 'usr' would then already copy these lib
directories, and the next rsync for 'usr/$${ARCH_LIB_DIR}' does nothing.

By itself, this is not a very big problem: the staging directory simply has
some extra directories. However, a subsequent patch will create a staging
symlink from $${ARCH_LIB_DIR} to lib. The first rsync would then overwrite
that symlink with the real directory usr/$${ARCH_LIB_DIR} from the
toolchain, which is not correct.

Assuming the patch that creates the symlink ARCH_LIB_DIR->lib is applied,
the original situation after 'make clean toolchain' with an
ARCH_LIB_DIR=lib32-fp is:

$ ls -ld output/staging/{,usr/}lib* output/target/{usr/,}lib*
drwxr-xr-x 2  4096 May 26  2015 output/staging/lib
lrwxrwxrwx 1     3 Jan 20 13:47 output/staging/lib32 -> lib
lrwxrwxrwx 1     3 Jan 20 13:47 output/staging/lib32-fp -> lib
drwxr-xr-x 2  4096 Jan 20 13:47 output/staging/usr/lib
lrwxrwxrwx 1     3 Jan 20 13:47 output/staging/usr/lib32 -> lib
drwxr-xr-x 4  4096 May 26  2015 output/staging/usr/lib32-fp
drwxr-xr-x 4  4096 May 26  2015 output/staging/usr/lib64-fp
drwxr-xr-x 4  4096 May 26  2015 output/staging/usr/libexec
drwxr-xr-x 3  4096 May 26  2015 output/staging/usr/libexec32
drwxr-xr-x 3  4096 May 26  2015 output/staging/usr/libexec32-fp
drwxr-xr-x 3  4096 May 26  2015 output/staging/usr/libexec64-fp
drwxr-xr-x 2  4096 Jan 20 13:48 output/target/lib
lrwxrwxrwx 1     3 Jan 20 13:47 output/target/lib32 -> lib
lrwxrwxrwx 1     3 Jan 20 13:47 output/target/lib32-fp -> lib
drwxr-xr-x 2  4096 Jan 20 13:48 output/target/usr/lib
lrwxrwxrwx 1     3 Jan 20 13:47 output/target/usr/lib32 -> lib
lrwxrwxrwx 1     3 Jan 20 13:47 output/target/usr/lib32-fp -> lib

Notice how usr/lib32-fp is not a symlink but a directory, and the presence
of an unnecessary directory usr/lib64-fp.

This patch improves the rsync exclusion rules by excluding any lib*
directory on the first rsync. As this would also exclude any
libexec/libexec32/... directory, explicitly include them first (first match
takes precedence). This (as is already the case today) results in more
usr/libexec* directories than needed, but it is not touched by this patch.

With the fix applied, the situation becomes:

drwxr-xr-x 2 4096 May 26  2015 output/staging/lib
lrwxrwxrwx 1    3 Jan 20 14:27 output/staging/lib32 -> lib
lrwxrwxrwx 1    3 Jan 20 14:27 output/staging/lib32-fp -> lib
drwxr-xr-x 4 4096 May 26  2015 output/staging/usr/lib
lrwxrwxrwx 1    3 Jan 20 14:27 output/staging/usr/lib32 -> lib
lrwxrwxrwx 1    3 Jan 20 14:27 output/staging/usr/lib32-fp -> lib
drwxr-xr-x 4 4096 May 26  2015 output/staging/usr/libexec
drwxr-xr-x 3 4096 May 26  2015 output/staging/usr/libexec32
drwxr-xr-x 3 4096 May 26  2015 output/staging/usr/libexec32-fp
drwxr-xr-x 3 4096 May 26  2015 output/staging/usr/libexec64-fp
drwxr-xr-x 2 4096 Jan 20 14:27 output/target/lib
lrwxrwxrwx 1    3 Jan 20 14:27 output/target/lib32 -> lib
lrwxrwxrwx 1    3 Jan 20 14:27 output/target/lib32-fp -> lib
drwxr-xr-x 2 4096 Jan 20 14:27 output/target/usr/lib
lrwxrwxrwx 1    3 Jan 20 14:27 output/target/usr/lib32 -> lib
lrwxrwxrwx 1    3 Jan 20 14:27 output/target/usr/lib32-fp -> lib

For cases where ARCH_LIB_DIR is one of lib, lib32 or lib64 this fix
makes no difference, and likewise for internal toolchains.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
v11: unchanged
v10: new patch

 toolchain/helpers.mk | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index 906993d..02cc0bb 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -154,8 +154,7 @@ copy_toolchain_sysroot = \
 	for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
 		if [ -d $${ARCH_SYSROOT_DIR}/$$i ] ; then \
 			rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
-				--exclude '/lib/' --exclude '/lib32/' \
-				--exclude '/lib64/' \
+				--include '/libexec*/' --exclude '/lib*/' \
 				$${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
 		fi ; \
 	done ; \
-- 
2.4.10

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

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de.schampheleire@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@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: "Yann E. Morin" <yann.morin.1998@free.fr>
Cc: Romain Naour <romain.naour@gmail.com>
Cc: Peter Korsgaard <peter@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 \
+               ln -snf lib "$${DESTDIR}/$${ARCH_LIB_DIR}" ; \
+               ln -snf lib "$${DESTDIR}/usr/$${ARCH_LIB_DIR}" ; \
+       fi
+
+define TOOLCHAIN_EXTERNAL_CREATE_STAGING_LIB_SYMLINK
+       $(call create_lib_symlinks,$(STAGING_DIR))
+endef
+
+define TOOLCHAIN_EXTERNAL_CREATE_TARGET_LIB_SYMLINK
+       $(call create_lib_symlinks,$(TARGET_DIR))
+endef
+
 # Integration of the toolchain into Buildroot: find the main sysroot
 # and the variant-specific sysroot, then copy the needed libraries to
 # the $(TARGET_DIR) and copy the whole sysroot (libraries and headers)
@@ -732,6 +753,7 @@ endef
 TOOLCHAIN_EXTERNAL_BUILD_CMDS = $(TOOLCHAIN_BUILD_WRAPPER)
 
 define TOOLCHAIN_EXTERNAL_INSTALL_STAGING_CMDS
+	$(TOOLCHAIN_EXTERNAL_CREATE_STAGING_LIB_SYMLINK)
 	$(TOOLCHAIN_EXTERNAL_INSTALL_SYSROOT_LIBS)
 	$(TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER)
 	$(TOOLCHAIN_EXTERNAL_INSTALL_GDBINIT)
@@ -741,6 +763,7 @@ endef
 # and the target directory, we do everything within the
 # install-staging step, arbitrarily.
 define TOOLCHAIN_EXTERNAL_INSTALL_TARGET_CMDS
+	$(TOOLCHAIN_EXTERNAL_CREATE_TARGET_LIB_SYMLINK)
 	$(TOOLCHAIN_EXTERNAL_INSTALL_TARGET_LIBS)
 	$(TOOLCHAIN_EXTERNAL_INSTALL_BFIN_FDPIC)
 	$(TOOLCHAIN_EXTERNAL_INSTALL_BFIN_FLAT)
-- 
2.4.10

[Buildroot] [PATCH v11 1/4] skeleton: move LIB_SYMLINK definition from Makefile

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Wed, 20 Jan 2016 20:11:28 +0100, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> 
> Commit 7a6b83a211612ff95a1f5d35b2861ad5655ac8b1 introduced the skeleton
> package, which took over the lib32/lib64 -> lib symlink creation from the
> main Makefile.
> However, the definition of the LIB_SYMLINK variable did not move along, for
> no real reason.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> v11: place LIB_SYMLINK in skeleton iso toolchain/helpers.mk (ThomasP)
> v10: new patch

Applied, thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v11 2/4] toolchain-external: don't exclude too much lib in sysroot rsync

[Romain Naour]

Hi Thomas, All,

Le 20/01/2016 20:11, Thomas De Schampheleire a ?crit :
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> 
> The copy_toolchain_sysroot helper in toolchain/helpers.mk performs an
> rsync of various directories from the extracted external toolchain to the
> corresponding directory in staging.
> 
> The relevant (simplified) snippet is:
>     for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
>             rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
>                     --exclude lib --exclude lib32 --exclude lib64 \
>                     $${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
>     done ; \
> 
> The exclusion logic of lib/lib32/lib64 has been added by commit
> 5628776c4a4d29d0715633ea463b64cc19e19c5a with the purpose of only copying
> the relevant usr/lib* directory from the toolchain to staging, instead of
> all. For example, if ARCH_LIB_DIR is 'lib64', then only usr/lib64 would be
> copied and usr/lib and usr/lib32 are ignored. It works by ignoring any
> lib/lib32/lib64 subdirectory on the rsync of 'usr' and then separately
> copying usr/{lib,lib32,lib64} as appropriate. (The exclusion rules only have
> impact on the files beneath the main source directory.)
> 
> However, on the rsync of 'usr', ANY of the following directories AND files
> would be excluded:
>     lib/
>     lib
>     lib32/
>     foobar/something/lib/
>     something-else/lib64/
> 
> while it is only the intention to skip directories directly under usr.
> 
> Therefore, add a leading (to restrict the scope to first-level) and trailing
> (to restrict to directories) slash to the exclude pattern. From 'man rsync':
> 
>     - if the pattern starts with a / then it is anchored to [..] the root of
>       the transfer.
>     - if the pattern ends with a / then it will only match a directory, not
>       a regular file, symlink, or device.

Good catch !
Reviewed-by: Romain Naour <romain.naour@gmail.com>

> 
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> v11: unchanged
> v10: new patch
> 
>  toolchain/helpers.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index 70695ee..906993d 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -154,7 +154,8 @@ copy_toolchain_sysroot = \
>  	for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
>  		if [ -d $${ARCH_SYSROOT_DIR}/$$i ] ; then \
>  			rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
> -				--exclude lib --exclude lib32 --exclude lib64 \
> +				--exclude '/lib/' --exclude '/lib32/' \
> +				--exclude '/lib64/' \
>  				$${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
>  		fi ; \
>  	done ; \
> 

[Buildroot] [PATCH v11 3/4] toolchain-external: improve sysroot rsync if ARCH_LIB_DIR != lib/lib32/lib64

[Romain Naour]

Hi Thomas, All,

Le 20/01/2016 20:11, Thomas De Schampheleire a ?crit :
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> 
> The copy_toolchain_sysroot helper in toolchain/helpers.mk performs an
> rsync of various directories from the extracted external toolchain to the
> corresponding directory in staging.
> 
> The relevant (simplified) snippet is:
> 	for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
> 		rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
> 			--exclude '/lib/' --exclude '/lib32/' \
> 			--exclude '/lib64/' \
> 			$${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
> 	done ; \
> 
> The exclusion logic of lib/lib32/lib64 has originally been added by commit
> 5628776c4a4d29d0715633ea463b64cc19e19c5a with the purpose of only copying
> the relevant usr/lib* directory from the toolchain to staging, instead of
> all. For example, if ARCH_LIB_DIR is 'lib64', then only usr/lib64 would be
> copied and usr/lib and usr/lib32 are ignored. It works by ignoring any
> lib/lib32/lib64 subdirectory on the rsync of 'usr' and then separately
> copying usr/{lib,lib32,lib64} as appropriate. (The exclusion rules only have
> impact on the files beneath the main source directory.)
> 
> However, ARCH_LIB_DIR can take other values than (lib, lib32, lib64), for
> example lib32-fp or lib64-fp (Octeon III toolchain with -march=octeon3). In
> the existing code, the rsync for 'usr' would then already copy these lib
> directories, and the next rsync for 'usr/$${ARCH_LIB_DIR}' does nothing.
> 
> By itself, this is not a very big problem: the staging directory simply has
> some extra directories. However, a subsequent patch will create a staging
> symlink from $${ARCH_LIB_DIR} to lib. The first rsync would then overwrite
> that symlink with the real directory usr/$${ARCH_LIB_DIR} from the
> toolchain, which is not correct.
> 
> Assuming the patch that creates the symlink ARCH_LIB_DIR->lib is applied,
> the original situation after 'make clean toolchain' with an
> ARCH_LIB_DIR=lib32-fp is:
> 
> $ ls -ld output/staging/{,usr/}lib* output/target/{usr/,}lib*
> drwxr-xr-x 2  4096 May 26  2015 output/staging/lib
> lrwxrwxrwx 1     3 Jan 20 13:47 output/staging/lib32 -> lib
> lrwxrwxrwx 1     3 Jan 20 13:47 output/staging/lib32-fp -> lib
> drwxr-xr-x 2  4096 Jan 20 13:47 output/staging/usr/lib
> lrwxrwxrwx 1     3 Jan 20 13:47 output/staging/usr/lib32 -> lib
> drwxr-xr-x 4  4096 May 26  2015 output/staging/usr/lib32-fp
> drwxr-xr-x 4  4096 May 26  2015 output/staging/usr/lib64-fp
> drwxr-xr-x 4  4096 May 26  2015 output/staging/usr/libexec
> drwxr-xr-x 3  4096 May 26  2015 output/staging/usr/libexec32
> drwxr-xr-x 3  4096 May 26  2015 output/staging/usr/libexec32-fp
> drwxr-xr-x 3  4096 May 26  2015 output/staging/usr/libexec64-fp
> drwxr-xr-x 2  4096 Jan 20 13:48 output/target/lib
> lrwxrwxrwx 1     3 Jan 20 13:47 output/target/lib32 -> lib
> lrwxrwxrwx 1     3 Jan 20 13:47 output/target/lib32-fp -> lib
> drwxr-xr-x 2  4096 Jan 20 13:48 output/target/usr/lib
> lrwxrwxrwx 1     3 Jan 20 13:47 output/target/usr/lib32 -> lib
> lrwxrwxrwx 1     3 Jan 20 13:47 output/target/usr/lib32-fp -> lib
> 
> Notice how usr/lib32-fp is not a symlink but a directory, and the presence
> of an unnecessary directory usr/lib64-fp.
> 
> This patch improves the rsync exclusion rules by excluding any lib*
> directory on the first rsync. As this would also exclude any
> libexec/libexec32/... directory, explicitly include them first (first match
> takes precedence). This (as is already the case today) results in more
> usr/libexec* directories than needed, but it is not touched by this patch.
> 
> With the fix applied, the situation becomes:
> 
> drwxr-xr-x 2 4096 May 26  2015 output/staging/lib
> lrwxrwxrwx 1    3 Jan 20 14:27 output/staging/lib32 -> lib
> lrwxrwxrwx 1    3 Jan 20 14:27 output/staging/lib32-fp -> lib
> drwxr-xr-x 4 4096 May 26  2015 output/staging/usr/lib
> lrwxrwxrwx 1    3 Jan 20 14:27 output/staging/usr/lib32 -> lib
> lrwxrwxrwx 1    3 Jan 20 14:27 output/staging/usr/lib32-fp -> lib
> drwxr-xr-x 4 4096 May 26  2015 output/staging/usr/libexec
> drwxr-xr-x 3 4096 May 26  2015 output/staging/usr/libexec32
> drwxr-xr-x 3 4096 May 26  2015 output/staging/usr/libexec32-fp
> drwxr-xr-x 3 4096 May 26  2015 output/staging/usr/libexec64-fp
> drwxr-xr-x 2 4096 Jan 20 14:27 output/target/lib
> lrwxrwxrwx 1    3 Jan 20 14:27 output/target/lib32 -> lib
> lrwxrwxrwx 1    3 Jan 20 14:27 output/target/lib32-fp -> lib
> drwxr-xr-x 2 4096 Jan 20 14:27 output/target/usr/lib
> lrwxrwxrwx 1    3 Jan 20 14:27 output/target/usr/lib32 -> lib
> lrwxrwxrwx 1    3 Jan 20 14:27 output/target/usr/lib32-fp -> lib
> 
> For cases where ARCH_LIB_DIR is one of lib, lib32 or lib64 this fix
> makes no difference, and likewise for internal toolchains.

Reviewed-by: Romain Naour <romain.naour@gmail.com>

> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> v11: unchanged
> v10: new patch
> 
>  toolchain/helpers.mk | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index 906993d..02cc0bb 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -154,8 +154,7 @@ copy_toolchain_sysroot = \
>  	for i in etc $${ARCH_LIB_DIR} sbin usr usr/$${ARCH_LIB_DIR}; do \
>  		if [ -d $${ARCH_SYSROOT_DIR}/$$i ] ; then \
>  			rsync -au --chmod=u=rwX,go=rX --exclude 'usr/lib/locale' \
> -				--exclude '/lib/' --exclude '/lib32/' \
> -				--exclude '/lib64/' \
> +				--include '/libexec*/' --exclude '/lib*/' \
>  				$${ARCH_SYSROOT_DIR}/$$i/ $(STAGING_DIR)/$$i/ ; \
>  		fi ; \
>  	done ; \
> 

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

[Romain Naour]

Hi Thomas, All,

Le 20/01/2016 20:11, Thomas De Schampheleire a ?crit :
> From: Thomas De Schampheleire <thomas.de.schampheleire@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@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: "Yann E. Morin" <yann.morin.1998@free.fr>
> Cc: Romain Naour <romain.naour@gmail.com>
> Cc: Peter Korsgaard <peter@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...

With that fixed, you can add my:
	Reviewed-by: Romain Naour <romain.naour@gmail.com>

Best regards,
Romain

> +               ln -snf lib "$${DESTDIR}/$${ARCH_LIB_DIR}" ; \
> +               ln -snf lib "$${DESTDIR}/usr/$${ARCH_LIB_DIR}" ; \
> +       fi
> +
> +define TOOLCHAIN_EXTERNAL_CREATE_STAGING_LIB_SYMLINK
> +       $(call create_lib_symlinks,$(STAGING_DIR))
> +endef
> +
> +define TOOLCHAIN_EXTERNAL_CREATE_TARGET_LIB_SYMLINK
> +       $(call create_lib_symlinks,$(TARGET_DIR))
> +endef
> +
>  # Integration of the toolchain into Buildroot: find the main sysroot
>  # and the variant-specific sysroot, then copy the needed libraries to
>  # the $(TARGET_DIR) and copy the whole sysroot (libraries and headers)
> @@ -732,6 +753,7 @@ endef
>  TOOLCHAIN_EXTERNAL_BUILD_CMDS = $(TOOLCHAIN_BUILD_WRAPPER)
>  
>  define TOOLCHAIN_EXTERNAL_INSTALL_STAGING_CMDS
> +	$(TOOLCHAIN_EXTERNAL_CREATE_STAGING_LIB_SYMLINK)
>  	$(TOOLCHAIN_EXTERNAL_INSTALL_SYSROOT_LIBS)
>  	$(TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER)
>  	$(TOOLCHAIN_EXTERNAL_INSTALL_GDBINIT)
> @@ -741,6 +763,7 @@ endef
>  # and the target directory, we do everything within the
>  # install-staging step, arbitrarily.
>  define TOOLCHAIN_EXTERNAL_INSTALL_TARGET_CMDS
> +	$(TOOLCHAIN_EXTERNAL_CREATE_TARGET_LIB_SYMLINK)
>  	$(TOOLCHAIN_EXTERNAL_INSTALL_TARGET_LIBS)
>  	$(TOOLCHAIN_EXTERNAL_INSTALL_BFIN_FDPIC)
>  	$(TOOLCHAIN_EXTERNAL_INSTALL_BFIN_FLAT)
> 

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

[Thomas De Schampheleire]

Hi Romain,

Thanks a lot for reviewing/testing!

On Sun, Jan 24, 2016 at 8:59 PM, Romain Naour <romain.naour@gmail.com> wrote:
> Hi Thomas, All,
>
> Le 20/01/2016 20:11, Thomas De Schampheleire a ?crit :
>> From: Thomas De Schampheleire <thomas.de.schampheleire@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@gmail.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Cc: Arnout Vandecappelle <arnout@mind.be>
>> Cc: "Yann E. Morin" <yann.morin.1998@free.fr>
>> Cc: Romain Naour <romain.naour@gmail.com>
>> Cc: Peter Korsgaard <peter@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

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

[Romain Naour]

Thomas, All,

Le 24/01/2016 21:23, Thomas De Schampheleire a ?crit :
> Hi Romain,
> 
> Thanks a lot for reviewing/testing!

You're welcome :)

> 
> On Sun, Jan 24, 2016 at 8:59 PM, Romain Naour <romain.naour@gmail.com> wrote:
>> Hi Thomas, All,
>>
>> Le 20/01/2016 20:11, Thomas De Schampheleire a ?crit :
>>> From: Thomas De Schampheleire <thomas.de.schampheleire@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@gmail.com>
>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Cc: Arnout Vandecappelle <arnout@mind.be>
>>> Cc: "Yann E. Morin" <yann.morin.1998@free.fr>
>>> Cc: Romain Naour <romain.naour@gmail.com>
>>> Cc: Peter Korsgaard <peter@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' ?

Indeed I'm in the case where ARCH_LIB_DIR is 'lib'.
I'm using the CS ARM 2014.05 as example.

> 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?

Seems good to me (tested locally).

> I'd need to test tomorrow and resend, thanks a lot for noticing!

Best regards,
Romain

> 
> Thanks,
> Thomas
>