Skip to contentSkip to conversation

core: ditch LD_LIBRARY_PATH (branch yem/no-ld-library-path)

PATCH New Yann E. MORIN 3 participants 21 messages Nov 13, 2015 – Nov 18, 2015

lore
Share & tools
lore
#0 lore raw

cover core: ditch LD_LIBRARY_PATH (branch yem/no-ld-library-path)

Hello All!

This series is an attempt to ditch LD_LIBRARY_PATH for host binaries.

Using LD_LIBRARY_PATH is causing build issues on certain distributions, like Fedore 23, see:

    https://bugs.busybox.net/show_bug.cgi?id=8456
    http://lists.busybox.net/pipermail/buildroot/2015-October/142898.html

Basically, on Fedora 23, ls (from coreutils) is now linked to libcap, which is also linked to libattr. However, outr own libcap is not linked to libattr, and ls is too. However, because of our LD_LIBRARY_PATH, the host ls is using our libcap, and thus fails to run.

This series:
- ?xes a few host packages, add RPATH to packages where it is missing
- fixes the host-mysql definition, allow it to be built always
- removes host-perl-file-util, it is not used anywhere and is broken
- cleanups libcurl about LD_LIBRARY_PATH
- adds a script to check that host binaries have appropriate RPATH
- finally removes LD_LIBRARY_PATH

Even though this is a relatively intrusive change, it in my opinion should go in master now, otherwise, we'd get a full release cycle broken on Fedora, which would really be a shame... :-/

Regards,
Yann E. MORIN.

The following changes since commit 2bc7c2e009ac08ae80cbf3ce736ade16a6cfcb26:

zxing-cpp: needs dynamic library (2015-11-13 16:39:29 +0100)

are available in the git repository at:

git://git.busybox.net/~ymorin/git/buildroot yem/no-ld-library-path

for you to fetch changes up to 0a14a9ff7f39c78d2a39f8ccde40a9f7a6d020b2:

core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment (2015-11-13 19:51:07 +0100)

----------------------------------------------------------------
Ben Boeckel (1):
core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment

Yann E. MORIN (5):
package/axfsutils: fix Makefile package/mysql: unconditionally define host variables package/perl-file-util: remove host variant package/libcurl: carefully override LD_LIBRARY_PATH
core: check host executables have appropriate RPATH

 package/Makefile.in                      |  2 -
 package/axfsutils/0001-fix-cflags.patch  | 19 +++++++++
 package/axfsutils/0002-use-ldflags.patch | 20 +++++++++
 package/axfsutils/axfsutils.mk           |  2 +-
 package/libcurl/libcurl.mk               |  2 +-
 package/mysql/mysql.mk                   | 39 +++++++++---------
 package/perl-file-util/perl-file-util.mk |  1 -
 package/pkg-generic.mk                   |  8 ++++
 support/scripts/check-host-rpath         | 71 ++++++++++++++++++++++++++++++++
 9 files changed, 140 insertions(+), 24 deletions(-)
 create mode 100644 package/axfsutils/0001-fix-cflags.patch
 create mode 100644 package/axfsutils/0002-use-ldflags.patch
 create mode 100755 support/scripts/check-host-rpath
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#1 lore raw

PATCH 1/6 package/axfsutils: fix Makefile

mkfs.axfs uses zlib, but does not have an rpath to our host dir.

That's because:
- we're not passing our host CFLAGS or LDFLAGS
- it is forcibly setting CFLAGS in the Makefile, overriding anything specified by the user
- it is not using LDFLAGS at all

Add two patches so that CFLAGS and LDFLAGS from the environment are used if present.

---
 package/axfsutils/0001-fix-cflags.patch  | 19 +++++++++++++++++++
 package/axfsutils/0002-use-ldflags.patch | 20 ++++++++++++++++++++
 package/axfsutils/axfsutils.mk           |  2 +-
 3 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 package/axfsutils/0001-fix-cflags.patch
 create mode 100644 package/axfsutils/0002-use-ldflags.patch
package/axfsutils/0001-fix-cflags.patch new file +19 -0
new file mode 100644
index 0000000..0415d82
@@ -0,0 +1,19 @@
+Makefile: complement CFLAGS provided by the user
+
+In some circumstances, the user may want to pass its own CFLAGS,
+like for when the zlib headers are not located in the standard gcc
+search paths.
+
+Signed-off-by: "Yann E. MORIN" <[email protected]>
+
+diff -durN a/mkfs.axfs-legacy/Makefile b/mkfs.axfs-legacy/Makefile
+--- a/mkfs.axfs-legacy/Makefile
++++ b/mkfs.axfs-legacy/Makefile
+@@ -1,5 +1,5 @@
+-INC = -I./
+-CFLAGS = -g $(INC) -O0
++INC += -I./
++CFLAGS += -g $(INC) -O0
+ 
+ MKFSOBJS = mkfs.axfs.o
+ 
package/axfsutils/0002-use-ldflags.patch new file +20 -0
new file mode 100644
index 0000000..20e03a9
@@ -0,0 +1,20 @@
+Makefile: use LDFLAGS as provided by the user
+
+In some circumstances, the user may want to pass some LDFLAGS, like
+-L flags to point to the zlib location if it was not installed in a
+standard location.
+
+Signed-off-by: "Yann E. MORIN" <[email protected]>
+
+diff -durN a/mkfs.axfs-legacy/Makefile b/mkfs.axfs-legacy/Makefile
+--- a/mkfs.axfs-legacy/Makefile
++++ b/mkfs.axfs-legacy/Makefile
+@@ -6,7 +6,7 @@
+ all:   mkfs.axfs
+ 
+ mkfs.axfs: $(MKFSOBJS)
+-	$(CC) $(CFLAGS) -o mkfs.axfs $(MKFSOBJS) -lz
++	$(CC) $(CFLAGS) $(LDFLAGS) -o mkfs.axfs $(MKFSOBJS) -lz
+ 
+ clean_mkfs.axfs:
+ 	rm -rf $(MKFSOBJS) mkfs.axfs
package/axfsutils/axfsutils.mk +1 -1
index 3e6ea52..4dd60ef 100644
@@ -13,7 +13,7 @@ AXFSUTILS_DEPENDENCIES = host-zlib
 # The 'new' mkfs.axfs version requires GNUstep which is not a buildroot
 # prerequisite. The 'legacy' one works just as well without that requirement.
 define HOST_AXFSUTILS_BUILD_CMDS
-	$(HOST_MAKE_ENV) $(MAKE) -C $(@D)/mkfs.axfs-legacy
+	$(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) -C $(@D)/mkfs.axfs-legacy
 endef
 
 define HOST_AXFSUTILS_INSTALL_CMDS
Signature · 1 line
1.9.1
#2 lore raw

PATCH 1/6 package/axfsutils: fix Makefile

Quoted 8 lines · Yann E. MORIN

mkfs.axfs uses zlib, but does not have an rpath to our host dir.

That's because:
- we're not passing our host CFLAGS or LDFLAGS
- it is forcibly setting CFLAGS in the Makefile, overriding anything specified by the user
- it is not using LDFLAGS at all

Quoted 4 lines

Add two patches so that CFLAGS and LDFLAGS from the environment are used if present.

(verified with ldd)

Probably difficult to upstream this.

Regards,
Arnout

Quoted 7 lines
---
 package/axfsutils/0001-fix-cflags.patch  | 19 +++++++++++++++++++
 package/axfsutils/0002-use-ldflags.patch | 20 ++++++++++++++++++++
 package/axfsutils/axfsutils.mk           |  2 +-
 3 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 package/axfsutils/0001-fix-cflags.patch
 create mode 100644 package/axfsutils/0002-use-ldflags.patch

[snip]

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#3 lore raw

PATCH 1/6 package/axfsutils: fix Makefile

In reply to Yann E. MORIN
Quoted 9 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

mkfs.axfs uses zlib, but does not have an rpath to our host dir. That's because:
- we're not passing our host CFLAGS or LDFLAGS
- it is forcibly setting CFLAGS in the Makefile, overriding anything specified by the user
- it is not using LDFLAGS at all

Quoted 4 lines

Add two patches so that CFLAGS and LDFLAGS from the environment are used if present.

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#4 lore raw

PATCH 2/6 package/mysql: unconditionally define host variables

In reply to Yann E. MORIN

To build the host-mysql, we only build parts of the source, just the strictly minimum required to then cross-compile it.

However, the host variables (conf opts, build and install cmds) are only defined when the mysql server is enabled in the configuration.

So, this breaks:
make defconfig; make host-mysql

Even though it is not much use to have that partial host-mysql on its own, it is still very interesting to be able to build it, if at least for testing changes in the core package infrastructures (like new step hooks or the likes...)

Move the definitions of the host variant out of the server conditional block.

Cc Thomas Petazzoni <[email protected]>
Cc Marcelo Guti?rrez(UTN/FRH) <[email protected]>
---
 package/mysql/mysql.mk | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)
package/mysql/mysql.mk +20 -19
index c6d9f3a..00fab85 100644
@@ -33,6 +33,26 @@ MYSQL_CONF_OPTS = \
 	--enable-thread-safe-client \
 	--disable-mysql-maintainer-mode
 
+HOST_MYSQL_DEPENDENCIES = host-zlib host-ncurses
+
+HOST_MYSQL_CONF_OPTS = \
+	--with-embedded-server \
+	--disable-mysql-maintainer-mode
+
+define HOST_MYSQL_BUILD_CMDS
+	$(MAKE) -C $(@D)/include my_config.h
+	$(MAKE) -C $(@D)/mysys libmysys.a
+	$(MAKE) -C $(@D)/strings libmystrings.a
+	$(MAKE) -C $(@D)/vio libvio.a
+	$(MAKE) -C $(@D)/dbug libdbug.a
+	$(MAKE) -C $(@D)/regex libregex.a
+	$(MAKE) -C $(@D)/sql gen_lex_hash
+endef
+
+define HOST_MYSQL_INSTALL_CMDS
+	$(INSTALL) -m 0755  $(@D)/sql/gen_lex_hash $(HOST_DIR)/usr/bin/
+endef
+
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 MYSQL_DEPENDENCIES += openssl
 endif
@@ -43,11 +63,6 @@ endif
 
 ifeq ($(BR2_PACKAGE_MYSQL_SERVER),y)
 MYSQL_DEPENDENCIES += host-mysql host-bison
-HOST_MYSQL_DEPENDENCIES = host-zlib host-ncurses
-
-HOST_MYSQL_CONF_OPTS = \
-	--with-embedded-server \
-	--disable-mysql-maintainer-mode
 
 MYSQL_CONF_OPTS += \
 	--localstatedir=/var/mysql \
@@ -74,20 +89,6 @@ else
 MYSQL_CONF_OPTS += --without-debug
 endif
 
-define HOST_MYSQL_BUILD_CMDS
-	$(MAKE) -C $(@D)/include my_config.h
-	$(MAKE) -C $(@D)/mysys libmysys.a
-	$(MAKE) -C $(@D)/strings libmystrings.a
-	$(MAKE) -C $(@D)/vio libvio.a
-	$(MAKE) -C $(@D)/dbug libdbug.a
-	$(MAKE) -C $(@D)/regex libregex.a
-	$(MAKE) -C $(@D)/sql gen_lex_hash
-endef
-
-define HOST_MYSQL_INSTALL_CMDS
-	$(INSTALL) -m 0755  $(@D)/sql/gen_lex_hash  $(HOST_DIR)/usr/bin/
-endef
-
 define MYSQL_USERS
 	mysql -1 nogroup -1 * /var/mysql - - MySQL daemon
 endef
Signature · 1 line
1.9.1
#5 lore raw

PATCH 2/6 package/mysql: unconditionally define host variables

Quoted 14 lines · Yann E. MORIN

To build the host-mysql, we only build parts of the source, just the strictly minimum required to then cross-compile it.

However, the host variables (conf opts, build and install cmds) are only defined when the mysql server is enabled in the configuration.

So, this breaks:
make defconfig; make host-mysql

Even though it is not much use to have that partial host-mysql on its own, it is still very interesting to be able to build it, if at least for testing changes in the core package infrastructures (like new step hooks or the likes...)

Quoted 6 lines

Move the definitions of the host variant out of the server conditional block.

Cc Thomas Petazzoni <[email protected]>
Cc Marcelo Guti?rrez(UTN/FRH) <[email protected]>

Perhaps it would be a good opportunity to add a comment:

# host-mysql only installs what is needed to build mysql, i.e. the gen_lex_hash
# tool, and it only builds the parts that are needed to create this tool.

Regards,
Arnout

Quoted 69 lines
---
 package/mysql/mysql.mk | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)
package/mysql/mysql.mk +20 -19
index c6d9f3a..00fab85 100644
@@ -33,6 +33,26 @@ MYSQL_CONF_OPTS = \
 	--enable-thread-safe-client \
 	--disable-mysql-maintainer-mode
 
+HOST_MYSQL_DEPENDENCIES = host-zlib host-ncurses
+
+HOST_MYSQL_CONF_OPTS = \
+	--with-embedded-server \
+	--disable-mysql-maintainer-mode
+
+define HOST_MYSQL_BUILD_CMDS
+	$(MAKE) -C $(@D)/include my_config.h
+	$(MAKE) -C $(@D)/mysys libmysys.a
+	$(MAKE) -C $(@D)/strings libmystrings.a
+	$(MAKE) -C $(@D)/vio libvio.a
+	$(MAKE) -C $(@D)/dbug libdbug.a
+	$(MAKE) -C $(@D)/regex libregex.a
+	$(MAKE) -C $(@D)/sql gen_lex_hash
+endef
+
+define HOST_MYSQL_INSTALL_CMDS
+	$(INSTALL) -m 0755  $(@D)/sql/gen_lex_hash $(HOST_DIR)/usr/bin/
+endef
+
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 MYSQL_DEPENDENCIES += openssl
 endif
@@ -43,11 +63,6 @@ endif
 
 ifeq ($(BR2_PACKAGE_MYSQL_SERVER),y)
 MYSQL_DEPENDENCIES += host-mysql host-bison
-HOST_MYSQL_DEPENDENCIES = host-zlib host-ncurses
-
-HOST_MYSQL_CONF_OPTS = \
-	--with-embedded-server \
-	--disable-mysql-maintainer-mode
 
 MYSQL_CONF_OPTS += \
 	--localstatedir=/var/mysql \
@@ -74,20 +89,6 @@ else
 MYSQL_CONF_OPTS += --without-debug
 endif
 
-define HOST_MYSQL_BUILD_CMDS
-	$(MAKE) -C $(@D)/include my_config.h
-	$(MAKE) -C $(@D)/mysys libmysys.a
-	$(MAKE) -C $(@D)/strings libmystrings.a
-	$(MAKE) -C $(@D)/vio libvio.a
-	$(MAKE) -C $(@D)/dbug libdbug.a
-	$(MAKE) -C $(@D)/regex libregex.a
-	$(MAKE) -C $(@D)/sql gen_lex_hash
-endef
-
-define HOST_MYSQL_INSTALL_CMDS
-	$(INSTALL) -m 0755  $(@D)/sql/gen_lex_hash  $(HOST_DIR)/usr/bin/
-endef
-
 define MYSQL_USERS
 	mysql -1 nogroup -1 * /var/mysql - - MySQL daemon
 endef
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#6 lore raw

PATCH 2/6 package/mysql: unconditionally define host variables

In reply to Yann E. MORIN
Quoted 16 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

To build the host-mysql, we only build parts of the source, just the strictly minimum required to then cross-compile it.

However, the host variables (conf opts, build and install cmds) are only defined when the mysql server is enabled in the configuration.

So, this breaks:
make defconfig; make host-mysql

Even though it is not much use to have that partial host-mysql on its own, it is still very interesting to be able to build it, if at least for testing changes in the core package infrastructures (like new step hooks or the likes...)

Quoted 6 lines

Move the definitions of the host variant out of the server conditional block.

Cc Thomas Petazzoni <[email protected]>
Cc Marcelo Guti?rrez(UTN/FRH) <[email protected]>

Committed with the comment suggested by Arnout added, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#7 lore raw

PATCH 3/6 package/perl-file-util: remove host variant

In reply to Yann E. MORIN

It does not build, and no one depends on it.

---
 package/perl-file-util/perl-file-util.mk | 1 -
 1 file changed, 1 deletion(-)
package/perl-file-util/perl-file-util.mk +0 -1
index 2712aa4..fe83585 100644
@@ -12,4 +12,3 @@ PERL_FILE_UTIL_LICENSE = Artistic or GPLv1+
 PERL_FILE_UTIL_LICENSE_FILES = COPYING LICENSE
 
 $(eval $(perl-package))
-$(eval $(host-perl-package))
Signature · 1 line
1.9.1
#8 lore raw

PATCH 3/6 package/perl-file-util: remove host variant

Quoted 1 line · Yann E. MORIN

It does not build, and no one depends on it.

Confirmed that it doesn't build:
Can't locate Module/Build.pm

Regards,
Arnout

Quoted 17 lines
---
 package/perl-file-util/perl-file-util.mk | 1 -
 1 file changed, 1 deletion(-)
package/perl-file-util/perl-file-util.mk +0 -1
index 2712aa4..fe83585 100644
@@ -12,4 +12,3 @@ PERL_FILE_UTIL_LICENSE = Artistic or GPLv1+
 PERL_FILE_UTIL_LICENSE_FILES = COPYING LICENSE
 
 $(eval $(perl-package))
-$(eval $(host-perl-package))
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#10 lore raw

PATCH 4/6 package/libcurl: carefully override LD_LIBRARY_PATH

In reply to Yann E. MORIN

To build libcurl, we need to override LD_LIBRARY and force it to a sane value, otherwise libcurl is confused when target == host (see a51ce319,
libcurl: fix configure with openssl when target == host).

That is currently OK, since we always set LD_LIBRARY_PATH to a non-empty value.

However, we're soon to stop setting it at all.

So, if the user has an empty (or no) LD_LIBRARY_PATH in his envirnment, we'd end up adding the current working directory to LD_LIBRARY_PATH (as an empty entry in a colon-separated list is most probably interpreted as meaning the currentworking directory, which we do know can cause issue, and which we expfressely check against in support/dependencies/dependencies.sh

Fix that by only using an existing LD_LIBRARY_PATH if it is not empty. Also use a Makefile construct as it is easier to read than a shell one (we can do that, as all variables from the environment as available as make variables).

Cc Peter Korsgaard <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 package/libcurl/libcurl.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/libcurl/libcurl.mk +1 -1
index 8587baa..7c259b2 100644
@@ -30,7 +30,7 @@ LIBCURL_CONF_ENV += ac_cv_lib_crypto_CRYPTO_lock=yes
 # native stuff during the rest of configure when target == host.
 # Fix it by setting LD_LIBRARY_PATH to something sensible so those libs
 # are found first.
-LIBCURL_CONF_ENV += LD_LIBRARY_PATH=$$LD_LIBRARY_PATH:/lib:/usr/lib
+LIBCURL_CONF_ENV += LD_LIBRARY_PATH=$(if $(LD_LIBRARY_PATH),$(LD_LIBRARY_PATH):)/lib:/usr/lib
 LIBCURL_CONF_OPTS += --with-ssl=$(STAGING_DIR)/usr \
 	--with-ca-path=/etc/ssl/certs
 else ifeq ($(BR2_PACKAGE_GNUTLS),y)
Signature · 1 line
1.9.1
#11 lore raw

PATCH 4/6 package/libcurl: carefully override LD_LIBRARY_PATH

Quoted 15 lines · Yann E. MORIN

To build libcurl, we need to override LD_LIBRARY and force it to a sane value, otherwise libcurl is confused when target == host (see a51ce319,
libcurl: fix configure with openssl when target == host).

That is currently OK, since we always set LD_LIBRARY_PATH to a non-empty value.

However, we're soon to stop setting it at all.

So, if the user has an empty (or no) LD_LIBRARY_PATH in his envirnment, we'd end up adding the current working directory to LD_LIBRARY_PATH (as an empty entry in a colon-separated list is most probably interpreted as meaning the currentworking directory, which we do know can cause issue, and which we expfressely check against in support/dependencies/dependencies.sh

Quoted 8 lines

Fix that by only using an existing LD_LIBRARY_PATH if it is not empty. Also use a Makefile construct as it is easier to read than a shell one (we can do that, as all variables from the environment as available as make variables).

Cc Peter Korsgaard <[email protected]>
Cc Thomas Petazzoni <[email protected]>

Regards,
Arnout

Quoted 18 lines
---
 package/libcurl/libcurl.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/libcurl/libcurl.mk +1 -1
index 8587baa..7c259b2 100644
@@ -30,7 +30,7 @@ LIBCURL_CONF_ENV += ac_cv_lib_crypto_CRYPTO_lock=yes
 # native stuff during the rest of configure when target == host.
 # Fix it by setting LD_LIBRARY_PATH to something sensible so those libs
 # are found first.
-LIBCURL_CONF_ENV += LD_LIBRARY_PATH=$$LD_LIBRARY_PATH:/lib:/usr/lib
+LIBCURL_CONF_ENV += LD_LIBRARY_PATH=$(if $(LD_LIBRARY_PATH),$(LD_LIBRARY_PATH):)/lib:/usr/lib
 LIBCURL_CONF_OPTS += --with-ssl=$(STAGING_DIR)/usr \
 	--with-ca-path=/etc/ssl/certs
 else ifeq ($(BR2_PACKAGE_GNUTLS),y)
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#12 lore raw

PATCH 4/6 package/libcurl: carefully override LD_LIBRARY_PATH

In reply to Yann E. MORIN
Quoted 17 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

To build libcurl, we need to override LD_LIBRARY and force it to a sane value, otherwise libcurl is confused when target == host (see a51ce319,
libcurl: fix configure with openssl when target == host).

That is currently OK, since we always set LD_LIBRARY_PATH to a non-empty value.

However, we're soon to stop setting it at all.

So, if the user has an empty (or no) LD_LIBRARY_PATH in his envirnment, we'd end up adding the current working directory to LD_LIBRARY_PATH (as an empty entry in a colon-separated list is most probably interpreted as meaning the currentworking directory, which we do know can cause issue, and which we expfressely check against in support/dependencies/dependencies.sh

Quoted 8 lines

Fix that by only using an existing LD_LIBRARY_PATH if it is not empty. Also use a Makefile construct as it is easier to read than a shell one (we can do that, as all variables from the environment as available as make variables).

Cc Peter Korsgaard <[email protected]>
Cc Thomas Petazzoni <[email protected]>

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#13 lore raw

PATCH 5/6 core: check host executables have appropriate RPATH

In reply to Yann E. MORIN

When we build our host programs, and they depend on a host library we also build, we want to ensure that program actually uses that library at runtime, and not the one from the system.

We currently ensure that in two ways:
- we add a RPATH tag that points to our host library directory,
- we export LD_LIBRARY_PATH to point to that same directory.

With thse two in place, we're pretty much confident that our host libraries will be used by our host programs.

However, it turns our that not all the host programs we build end up with an RPATH tag:
- some packages do not use our $(HOST_LDFLAGS)
- some packages' build system are oblivious to those LDFLAGS

In this case, there are two situation:
- the program is not linked to one of our host libraries: it in fact does not need an RPATH tag [0]
- the program actually uses one of our host libraries: in that case it should have had an RPATH tag pointing to the host directory.

As for libraries, it is unclear whether they should or should not have an RPATH pointing to our host directory. as for programs, it is only important they have such an RPATH if they have a dependency on another host lbrary we build. But even though, in practice this is not an issue, because the program that loads such a libray does have an RPATH (it did find that library!), so the RPATH from the program is also used to search for second-level (and third-level...) dependencies, as well as for libraries loaded via dlopen().

We add a new support script that checks that all ELF executables have a proper DT_RPATH (or DT_RUNPATH) tag when they link to our host libraries, and reports those file that are missing an RPATH. If a file missing an RPATH is an executable, the script aborts; if only libraries are are missing an RPATH, the script does not abort.

[0] Except if it were to dlopen() it, of course, but the only program I'm aware of that does that is openssl, and it has a correct RPATH tag.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
Cc Peter Korsgaard <[email protected]>
---
 package/pkg-generic.mk           |  8 +++++
 support/scripts/check-host-rpath | 71 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)
 create mode 100755 support/scripts/check-host-rpath
package/pkg-generic.mk +8 -0
index a5d0e57..ccb0d26 100644
@@ -87,6 +87,14 @@ define step_pkg_size
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
 
+# This hook checks that host packages that need libraries that we build
+# have a proper DT_RPATH or DT_RUNPATH tag
+define check_host_rpath
+	$(if $(filter install-host,$(2)),\
+		$(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR)))
+endef
+GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath
+
 # User-supplied script
 ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
 define step_user
support/scripts/check-host-rpath new file +71 -0

71 lines not on this page · raw message

Signature · 1 line
1.9.1
#14 lore raw

PATCH 5/6 core: check host executables have appropriate RPATH

All,

On 2015-11-13 22:48 +0100, Yann E. MORIN spake thusly:

Quoted 31 lines

When we build our host programs, and they depend on a host library we also build, we want to ensure that program actually uses that library at runtime, and not the one from the system.

We currently ensure that in two ways:
- we add a RPATH tag that points to our host library directory,
- we export LD_LIBRARY_PATH to point to that same directory.

With thse two in place, we're pretty much confident that our host libraries will be used by our host programs.

However, it turns our that not all the host programs we build end up with an RPATH tag:
- some packages do not use our $(HOST_LDFLAGS)
- some packages' build system are oblivious to those LDFLAGS

In this case, there are two situation:
- the program is not linked to one of our host libraries: it in fact does not need an RPATH tag [0]
- the program actually uses one of our host libraries: in that case it should have had an RPATH tag pointing to the host directory.

As for libraries, it is unclear whether they should or should not have an RPATH pointing to our host directory. as for programs, it is only important they have such an RPATH if they have a dependency on another host lbrary we build. But even though, in practice this is not an issue, because the program that loads such a libray does have an RPATH (it did find that library!), so the RPATH from the program is also used to search for second-level (and third-level...) dependencies, as well as for libraries loaded via dlopen().

Quoted 5 lines

We add a new support script that checks that all ELF executables have a proper DT_RPATH (or DT_RUNPATH) tag when they link to our host libraries, and reports those file that are missing an RPATH. If a file missing an RPATH is an executable, the script aborts; if only libraries are are missing an RPATH, the script does not abort.

I forgot to remove that last part of the sentence about libraries. At first, the script was also looking for shared libraries, but it no longer does.

Regards,
Yann E. MORIN.

Quoted 112 lines

[0] Except if it were to dlopen() it, of course, but the only program I'm aware of that does that is openssl, and it has a correct RPATH tag.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
Cc Peter Korsgaard <[email protected]>
---
 package/pkg-generic.mk           |  8 +++++
 support/scripts/check-host-rpath | 71 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)
 create mode 100755 support/scripts/check-host-rpath
package/pkg-generic.mk +8 -0
index a5d0e57..ccb0d26 100644
@@ -87,6 +87,14 @@ define step_pkg_size
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
 
+# This hook checks that host packages that need libraries that we build
+# have a proper DT_RPATH or DT_RUNPATH tag
+define check_host_rpath
+	$(if $(filter install-host,$(2)),\
+		$(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR)))
+endef
+GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath
+
 # User-supplied script
 ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
 define step_user
support/scripts/check-host-rpath new file +71 -0
new file mode 100755
index 0000000..b140974
@@ -0,0 +1,71 @@
+#!/usr/bin/env bash
+
+# This script scans $(HOST_DIR)/{bin,sbin} for all ELF files, and checks
+# they have an RPATH to $(HOT_DIR)/usr/lib if they need libraries from
+# there.
+
+# Override the user's locale so we are sure we can parse the output of
+# readelf(1) and file(1)
+export LC_ALL=C
+
+main() {
+    local pkg="${1}"
+    local hostdir="${2}"
+    local file ret
+
+    # Remove duplicate and trailing '/' for proper match
+    hostdir="$( sed -r -e 's:/+:/:g;' <<<"${hostdir}" )"
+
+    ret=0
+    while read file; do
+        elf_needs_rpath "${file}" "${hostdir}" || continue
+        check_elf_has_rpath "${file}" "${hostdir}" && continue
+        if [ ${ret} -eq 0 ]; then
+            ret=1
+            printf "***\n"
+            printf "*** ERROR: package %s installs executables without proper RPATH:\n" "${pkg}"
+        fi
+        printf "***   %s\n" "${file}"
+    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -exec file {} + 2>/dev/null \
+              |sed -r -e '/^([^:]+):.*\<ELF\>.*\<executable\>.*/!d'                \
+                      -e 's//\1/'                                                  \
+            )
+
+    return ${ret}
+}
+
+elf_needs_rpath() {
+    local file="${1}"
+    local hostdir="${2}"
+    local lib
+
+    while read lib; do
+        [ -e "${hostdir}/usr/lib/${lib}" ] && return 0
+    done < <( readelf -d "${file}"                                         \
+              |sed -r -e '/^.* \(NEEDED\) .*Shared library: \[(.+)\]$/!d;' \
+                     -e 's//\1/;'                                          \
+            )
+
+    return 1
+}
+
+check_elf_has_rpath() {
+    local file="${1}"
+    local hostdir="${2}"
+    local rpath dir
+
+    while read rpath; do
+        for dir in ${rpath//:/ }; do
+            # Remove duplicate and trailing '/' for proper match
+            dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
+            [ "${dir}" = "${hostdir}/usr/lib" ] && return 0
+        done
+    done < <( readelf -d "${file}"                                              \
+              |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \
+                      -e 's//\3/;'                                              \
+            )
+
+    return 1
+}
+
+main "${@}"
Signature · 1 line
1.9.1
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#15 lore raw

PATCH 5/6 core: check host executables have appropriate RPATH

In reply to Yann E. MORIN

Hi Yann,

Some comments on this one (as could be expected :-P )

Quoted 9 lines · Yann E. MORIN

When we build our host programs, and they depend on a host library we also build, we want to ensure that program actually uses that library at runtime, and not the one from the system.

We currently ensure that in two ways:
- we add a RPATH tag that points to our host library directory,
- we export LD_LIBRARY_PATH to point to that same directory.

With thse two in place, we're pretty much confident that our host

these

Quoted 8 lines

libraries will be used by our host programs.

However, it turns our that not all the host programs we build end up with an RPATH tag:
- some packages do not use our $(HOST_LDFLAGS)
- some packages' build system are oblivious to those LDFLAGS

In this case, there are two situation:

situations

Quoted 5 lines

- the program is not linked to one of our host libraries: it in fact does not need an RPATH tag [0]
- the program actually uses one of our host libraries: in that case it should have had an RPATH tag pointing to the host directory.

Quoted 8 lines

As for libraries, it is unclear whether they should or should not have an RPATH pointing to our host directory. as for programs, it is only important they have such an RPATH if they have a dependency on another host lbrary we build. But even though, in practice this is not an issue, because the program that loads such a libray does have an RPATH (it did find that library!), so the RPATH from the program is also used to search for second-level (and third-level...) dependencies, as well as for libraries loaded via dlopen().

This paragraph isn't clear enough. How about:

For libraries, they only need an RPATH if they depend on another library that is not installed in the standard library path. However, any system library will already be in the standard library path, and any library we install ourselves is in $(HOST_DIR)/usr/lib so already in RPATH.

Also, I think it would be good to repeat this explanation in the script itself.

Quoted 8 lines

We add a new support script that checks that all ELF executables have a proper DT_RPATH (or DT_RUNPATH) tag when they link to our host libraries, and reports those file that are missing an RPATH. If a file missing an RPATH is an executable, the script aborts; if only libraries are are missing an RPATH, the script does not abort.

[0] Except if it were to dlopen() it, of course, but the only program I'm aware of that does that is openssl, and it has a correct RPATH tag.

cmake and debugfs link with dlopen() as well, so possibly they will dlopen
libraries. Therefore, I'd check for dlopen as well.

Quoted 16 lines
Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
Cc Peter Korsgaard <[email protected]>
---
 package/pkg-generic.mk           |  8 +++++
 support/scripts/check-host-rpath | 71 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)
 create mode 100755 support/scripts/check-host-rpath
package/pkg-generic.mk +0 -0
index a5d0e57..ccb0d26 100644

package/pkg-generic.mk define step_pkg_size · line 96

Quoted 10 lines
package/pkg-generic.mk +7 -0
@@ -87,6 +87,14 @@ define step_pkg_size
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
 
+# This hook checks that host packages that need libraries that we build
+# have a proper DT_RPATH or DT_RUNPATH tag
+define check_host_rpath
+	$(if $(filter install-host,$(2)),\
+		$(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR)))
+endef
+GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath

As usual, I prefer it to be part of the .stamp_host_installed commands directly
instead of as a hook, because it is IMHO much more readable and simpler to understand. Not to mention that it is 6 lines shorter.

More importantly, though, there are also some packages that install stuff in
host during target or staging install, e.g. cppcms. Also, it is usually fairly clear where the executable comes from, and you should only really see this error while adding a package. So it seems to me that it would be sufficient to do this in the finailization step instead of after each host package install.

Quoted 11 lines
package/pkg-generic.mk +1 -0
+
 # User-supplied script
 ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
 define step_user
support/scripts/check-host-rpath new file +1 -0
new file mode 100755
index 0000000..b140974
@@ -0,0 +1,71 @@
+#!/usr/bin/env bash

support/scripts/check-host-rpath line 11

Quoted 10 lines
support/scripts/check-host-rpath new file +10 -0
+
+# This script scans $(HOST_DIR)/{bin,sbin} for all ELF files, and checks
+# they have an RPATH to $(HOT_DIR)/usr/lib if they need libraries from
+# there.
+
+# Override the user's locale so we are sure we can parse the output of
+# readelf(1) and file(1)
+export LC_ALL=C
+
+main() {

Not sure if I like this approach of a main() function, but OK.

support/scripts/check-host-rpath line 20

Quoted 9 lines
support/scripts/check-host-rpath new file +9 -0
+    local pkg="${1}"
+    local hostdir="${2}"
+    local file ret
+
+    # Remove duplicate and trailing '/' for proper match
+    hostdir="$( sed -r -e 's:/+:/:g;' <<<"${hostdir}" )"
+
+    ret=0
+    while read file; do

I definitely don't like this while read ... <( ... ) approach, because it is
IMHO much harder to understand (like a German sentence where all the verbs are at the end :-). So I would prefer a much simpler:

   for file in $(find "${hostdir}"/usr/{bin,sbin} -type f); do
       if file $file | grep -q -E '^([^:]+):.*\<ELF\>.*\<executable\>.*'; then
           ...

If you're worried about spaces in filenames, just add at the top of the file:

IFS=$(printf '\n')

support/scripts/check-host-rpath line 31

Quoted 11 lines
support/scripts/check-host-rpath new file +11 -0
+        elf_needs_rpath "${file}" "${hostdir}" || continue
+        check_elf_has_rpath "${file}" "${hostdir}" && continue
+        if [ ${ret} -eq 0 ]; then
+            ret=1
+            printf "***\n"
+            printf "*** ERROR: package %s installs executables without proper RPATH:\n" "${pkg}"
+        fi
+        printf "***   %s\n" "${file}"
+    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -exec file {} + 2>/dev/null \
+              |sed -r -e '/^([^:]+):.*\<ELF\>.*\<executable\>.*/!d'                \
+                      -e 's//\1/'                                                  \

As shown above, I prefer a simple grep over this complicated sed expression.

In fact I also don't really like extended regexps (because less people are
familiar with them) but in this case it really makes it simpler.

support/scripts/check-host-rpath line 42

Quoted 11 lines
support/scripts/check-host-rpath new file +11 -0
+            )
+
+    return ${ret}
+}
+
+elf_needs_rpath() {
+    local file="${1}"
+    local hostdir="${2}"
+    local lib
+
+    while read lib; do

Same while story here.

support/scripts/check-host-rpath line 43

Quoted 1 line
support/scripts/check-host-rpath new file +1 -0
+        [ -e "${hostdir}/usr/lib/${lib}" ] && return 0

Nite: I would only use [] inside if constructs, and test if you use it like here.

support/scripts/check-host-rpath line 47

Quoted 4 lines
support/scripts/check-host-rpath new file +4 -0
+    done < <( readelf -d "${file}"                                         \
+              |sed -r -e '/^.* \(NEEDED\) .*Shared library: \[(.+)\]$/!d;' \
+                     -e 's//\1/;'                                          \
+            )

This is also where the check for dlopen should be added:

    if readelf -s "${file}" | grep -q 'UND dlopen'; then
        return 0
    else
        return 1
    fi

Well, actually it would be enough to put

     readelf -s "${file}" | grep -q 'UND dlopen'

(because the return value of a function is the return value of the last pipeline) but that's in fact harder to understand so I don't like it.

Quoted 9 lines
support/scripts/check-host-rpath new file +9 -0
+
+    return 1
+}
+
+check_elf_has_rpath() {
+    local file="${1}"
+    local hostdir="${2}"
+    local rpath dir
+

support/scripts/check-host-rpath line 66

Quoted 10 lines
support/scripts/check-host-rpath new file +10 -0
+    while read rpath; do
+        for dir in ${rpath//:/ }; do
+            # Remove duplicate and trailing '/' for proper match
+            dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
+            [ "${dir}" = "${hostdir}/usr/lib" ] && return 0
+        done
+    done < <( readelf -d "${file}"                                              \
+              |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \
+                      -e 's//\3/;'                                              \
+            )

I stopped trying to parse this :-)

Regards,
Arnout

Quoted 6 lines
support/scripts/check-host-rpath new file +5 -0
+
+    return 1
+}
+
+main "${@}"
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#16 lore raw

PATCH 5/6 core: check host executables have appropriate RPATH

Arnout, All,

On 2015-11-15 22:49 +0100, Arnout Vandecappelle spake thusly:

Quoted 1 line

Some comments on this one (as could be expected :-P )

Eh! ;-)

Quoted 1 line · Yann E. MORIN

[--SNIP--]

Quoted 9 lines
Quoted 8 lines

As for libraries, it is unclear whether they should or should not have an RPATH pointing to our host directory. as for programs, it is only important they have such an RPATH if they have a dependency on another host lbrary we build. But even though, in practice this is not an issue, because the program that loads such a libray does have an RPATH (it did find that library!), so the RPATH from the program is also used to search for second-level (and third-level...) dependencies, as well as for libraries loaded via dlopen().

Quoted 6 lines

This paragraph isn't clear enough. How about:

For libraries, they only need an RPATH if they depend on another library that is not installed in the standard library path. However, any system library will already be in the standard library path, and any library we install ourselves is in $(HOST_DIR)/usr/lib so already in RPATH.

Further upgrade:

s/already in RPATH/already in the RPATH of the executable/

Quoted 1 line

Also, I think it would be good to repeat this explanation in the script itself.

ACK

Quoted 10 lines
Quoted 8 lines

We add a new support script that checks that all ELF executables have a proper DT_RPATH (or DT_RUNPATH) tag when they link to our host libraries, and reports those file that are missing an RPATH. If a file missing an RPATH is an executable, the script aborts; if only libraries are are missing an RPATH, the script does not abort.

[0] Except if it were to dlopen() it, of course, but the only program I'm aware of that does that is openssl, and it has a correct RPATH tag.

cmake and debugfs link with dlopen() as well, so possibly they will dlopen

'debugfs'? We have no such package...

Quoted 1 line

libraries. Therefore, I'd check for dlopen as well.

Hmm... I am not confident enough about that. What if it uses dlopen() on a system library?

[--SNIP--]

Quoted 9 lines

package/pkg-generic.mk define step_pkg_size · line 96

Quoted 5 lines
package/pkg-generic.mk +5 -0
+define check_host_rpath
+	$(if $(filter install-host,$(2)),\
+		$(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR)))
+endef
+GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath

As usual, I prefer it to be part of the .stamp_host_installed commands directly
instead of as a hook, because it is IMHO much more readable and simpler to understand. Not to mention that it is 6 lines shorter.

Except this is clearly instrumentation. We're not tweaking anything here.

Quoted 2 lines

More importantly, though, there are also some packages that install stuff in
host during target or staging install, e.g. cppcms.

Gah... Dirty... :-(

Quoted 4 lines

Also, it is usually fairly
clear where the executable comes from, and you should only really see this error while adding a package. So it seems to me that it would be sufficient to do this in the finailization step instead of after each host package install.

No, I really do want the build to break early, as soon as a package does not have a proper RPATH, so we can catch it. Scanning the host/usr/bin and host/usr/sbin directories takes roughly two seconds on my not-so-fast-but-not-so-slow-either machine (core-i5).

Quoted 14 lines
Quoted 14 lines

# User-supplied script
ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
define step_user

support/scripts/check-host-rpath new file +5 -0
new file mode 100755
index 0000000..b140974
@@ -0,0 +1,71 @@
+#!/usr/bin/env bash
+
+# This script scans $(HOST_DIR)/{bin,sbin} for all ELF files, and checks
+# they have an RPATH to $(HOT_DIR)/usr/lib if they need libraries from
+# there.
Quoted 8 lines

support/scripts/check-host-rpath line 11

Quoted 6 lines
support/scripts/check-host-rpath new file +6 -0
+
+# Override the user's locale so we are sure we can parse the output of
+# readelf(1) and file(1)
+export LC_ALL=C
+
+main() {

Not sure if I like this approach of a main() function, but OK.

I have come to always use a main() function even in shell scripts. It is much cleaner, and since this is bash, we can use local variables.

And with the experience, it has proven to be much more maintenable over time.

Quoted 4 lines
Quoted 4 lines
support/scripts/check-host-rpath new file +4 -0
+    local pkg="${1}"
+    local hostdir="${2}"
+    local file ret
+
Quoted 8 lines

support/scripts/check-host-rpath line 20

Quoted 5 lines
support/scripts/check-host-rpath new file +5 -0
+    # Remove duplicate and trailing '/' for proper match
+    hostdir="$( sed -r -e 's:/+:/:g;' <<<"${hostdir}" )"
+
+    ret=0
+    while read file; do

I definitely don't like this while read ... <( ... ) approach, because it is
IMHO much harder to understand

I do not like Python because it is IMHO much harder to understand... ;-]

Really, this is a little bit advanced bash scripting, but we would not refrain ourselves from using advanced constructs in other languages (the yield stuff in Python took me a while to understand, it is not trivial, and yet we use it, and that's good because the program is more efficient and smaller). Let's use the constructs the language offers, when they make our lives easier (yes, I believe that <() construct makes it easier).

Quoted 2 lines

(like a German sentence where all the verbs are
at the end :-).

Eh! I love German (kidding, it sounds weird in my ear, and I am not able to write any meaningfull sentence).

Quoted 5 lines

So I would prefer a much simpler:

   for file in $(find "${hostdir}"/usr/{bin,sbin} -type f); do
       if file $file | grep -q -E '^([^:]+):.*\<ELF\>.*\<executable\>.*'; then
           ...

The big advantage of using "find ... -exec file {} +' is that it will not spwan 'file' for each instance of the file it found, as it iwll pass it as many arguments as possible on the current system (which is a *lot* on Linux, so basically a single call to 'file'), which is much faster than spawning 'file' as many time as there are results.

Quoted 3 lines

If you're worried about spaces in filenames, just add at the top of the file:

IFS=$(printf '\n')

That does not work:

    $ cat ess
    #!/bin/bash
    IFS=$(printf '\n')
    for i in $( printf "a b\nc d e\n" ); do
        printf "'%s'\n" "${i}"
    done
    $ ./ess
    'a b
    c d e'

We can do it with IFS="${// /}" but that leaves tabs. Grantd, files with a tab in their name would be just insane...

But no, spaces in filenames are not my main concern...

Quoted 12 lines

support/scripts/check-host-rpath line 31

Quoted 11 lines
support/scripts/check-host-rpath new file +11 -0
+        elf_needs_rpath "${file}" "${hostdir}" || continue
+        check_elf_has_rpath "${file}" "${hostdir}" && continue
+        if [ ${ret} -eq 0 ]; then
+            ret=1
+            printf "***\n"
+            printf "*** ERROR: package %s installs executables without proper RPATH:\n" "${pkg}"
+        fi
+        printf "***   %s\n" "${file}"
+    done < <( find "${hostdir}"/usr/{bin,sbin} -type f -exec file {} + 2>/dev/null \
+              |sed -r -e '/^([^:]+):.*\<ELF\>.*\<executable\>.*/!d'                \
+                      -e 's//\1/'                                                  \
Quoted 1 line

As shown above, I prefer a simple grep over this complicated sed expression.

When I conpare your grep expression and my sed expression, I can;t see much difference, save for the pattern-matching in mine... :-/

Quoted 2 lines

In fact I also don't really like extended regexps (because less people are
familiar with them) but in this case it really makes it simpler.

Extended regular expressions were formally defined in POSIX.2, i.e. in 1992, 23 years ago... Get up-to-speed! ;-)

[--SNIP--]

Quoted 3 lines

support/scripts/check-host-rpath line 43

Quoted 1 line
support/scripts/check-host-rpath new file +1 -0
+        [ -e "${hostdir}/usr/lib/${lib}" ] && return 0

Nite: I would only use [] inside if constructs, and test if you use it like here.

Using [] make the test actually well delimited, that why I like it over test(1).

Quoted 13 lines

support/scripts/check-host-rpath line 47

Quoted 4 lines
support/scripts/check-host-rpath new file +4 -0
+    done < <( readelf -d "${file}"                                         \
+              |sed -r -e '/^.* \(NEEDED\) .*Shared library: \[(.+)\]$/!d;' \
+                     -e 's//\1/;'                                          \
+            )

This is also where the check for dlopen should be added:

    if readelf -s "${file}" | grep -q 'UND dlopen'; then
        return 0
    else
        return 1
    fi
Quoted 6 lines

Well, actually it would be enough to put

     readelf -s "${file}" | grep -q 'UND dlopen'

(because the return value of a function is the return value of the last pipeline) but that's in fact harder to understand so I don't like it.

Yes, the status of a pipeline is confusing, and I don't like relying on it either...

[--SNIP--]

Quoted 6 lines

support/scripts/check-host-rpath line 66

Quoted 4 lines
support/scripts/check-host-rpath new file +4 -0
+    done < <( readelf -d "${file}"                                              \
+              |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \
+                      -e 's//\3/;'                                              \
+            )

I stopped trying to parse this :-)

Eh! ;-)

ELF tag names are printed between parenthesis. Whatever comes before is of no interest.

Then, there are two rpath tags [*]:
- DT_RPATH, the old tag, displayed as 'RPATH',
- DT_RUNPATH, the new tag, displayed as 'RUNPATH'.

So we match both in a single pattern, by making the 'UN' (and 'un') parts optional.

Then, all we care about the is the actual value which is in square brackets.

And we delete lines that do not match the full pattern.

[*] They behave slightly differently, mostly in the order they are searched for, compared to other locations like LD_LIBRARY_PATH and the standard search paths.

Thank you! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#17 lore raw

PATCH 5/6 core: check host executables have appropriate RPATH

In reply to Yann E. MORIN
Quoted 39 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

When we build our host programs, and they depend on a host library we also build, we want to ensure that program actually uses that library at runtime, and not the one from the system.

We currently ensure that in two ways:
- we add a RPATH tag that points to our host library directory,
- we export LD_LIBRARY_PATH to point to that same directory.

With thse two in place, we're pretty much confident that our host libraries will be used by our host programs.

However, it turns our that not all the host programs we build end up with an RPATH tag:
- some packages do not use our $(HOST_LDFLAGS)
- some packages' build system are oblivious to those LDFLAGS

In this case, there are two situation:
- the program is not linked to one of our host libraries: it in fact does not need an RPATH tag [0]
- the program actually uses one of our host libraries: in that case it should have had an RPATH tag pointing to the host directory.

As for libraries, it is unclear whether they should or should not have an RPATH pointing to our host directory. as for programs, it is only important they have such an RPATH if they have a dependency on another host lbrary we build. But even though, in practice this is not an issue, because the program that loads such a libray does have an RPATH (it did find that library!), so the RPATH from the program is also used to search for second-level (and third-level...) dependencies, as well as for libraries loaded via dlopen().

We add a new support script that checks that all ELF executables have a proper DT_RPATH (or DT_RUNPATH) tag when they link to our host libraries, and reports those file that are missing an RPATH. If a file missing an RPATH is an executable, the script aborts; if only libraries are are missing an RPATH, the script does not abort.

Quoted 7 lines

[0] Except if it were to dlopen() it, of course, but the only program I'm aware of that does that is openssl, and it has a correct RPATH tag.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
Cc Peter Korsgaard <[email protected]>

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#18 lore raw

PATCH 6/6 core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment

In reply to Yann E. MORIN

From: Ben Boeckel <[email protected]>

If system tools are selected, the host's lib/ directory may shadow libraries from the system which are configured differently and do not have all of the symbols required by the system tool.

Since buildroot now uses rpath everywhere, LD_LIBRARY_PATH should not be necessary anyways.

---
 package/Makefile.in | 2 --
 1 file changed, 2 deletions(-)
package/Makefile.in +0 -2
index ca34660..85008bb 100644
@@ -296,12 +296,10 @@ HOST_CONFIGURE_OPTS = \
 	PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
 	PKG_CONFIG_SYSROOT_DIR="/" \
 	PKG_CONFIG_LIBDIR="$(HOST_DIR)/usr/lib/pkgconfig:$(HOST_DIR)/usr/share/pkgconfig" \
-	LD_LIBRARY_PATH="$(HOST_DIR)/usr/lib$(if $(LD_LIBRARY_PATH),:$(LD_LIBRARY_PATH))" \
 	INTLTOOL_PERL=$(PERL)
 
 HOST_MAKE_ENV = \
 	PATH=$(BR_PATH) \
-	LD_LIBRARY_PATH="$(HOST_DIR)/usr/lib$(if $(LD_LIBRARY_PATH),:$(LD_LIBRARY_PATH))" \
 	PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
 	PKG_CONFIG_SYSROOT_DIR="/" \
 	PKG_CONFIG_LIBDIR="$(HOST_DIR)/usr/lib/pkgconfig"
Signature · 1 line
1.9.1
#19 lore raw

PATCH 6/6 core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment

Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

From: Ben Boeckel <[email protected]>
If system tools are selected, the host's lib/ directory may shadow libraries from the system which are configured differently and do not have all of the symbols required by the system tool.

Quoted 7 lines

Since buildroot now uses rpath everywhere, LD_LIBRARY_PATH should not be necessary anyways.

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#20 lore raw

cover core: ditch LD_LIBRARY_PATH (branch yem/no-ld-library-path)

In reply to Yann E. MORIN

Hi Peter,

Quoted 22 lines · Yann E. MORIN

Hello All!

This series is an attempt to ditch LD_LIBRARY_PATH for host binaries.

Using LD_LIBRARY_PATH is causing build issues on certain distributions, like Fedore 23, see:

    https://bugs.busybox.net/show_bug.cgi?id=8456
    http://lists.busybox.net/pipermail/buildroot/2015-October/142898.html

Basically, on Fedora 23, ls (from coreutils) is now linked to libcap, which is also linked to libattr. However, outr own libcap is not linked to libattr, and ls is too. However, because of our LD_LIBRARY_PATH, the host ls is using our libcap, and thus fails to run.

This series:
- ?xes a few host packages, add RPATH to packages where it is missing
- fixes the host-mysql definition, allow it to be built always
- removes host-perl-file-util, it is not used anywhere and is broken
- cleanups libcurl about LD_LIBRARY_PATH
- adds a script to check that host binaries have appropriate RPATH
- finally removes LD_LIBRARY_PATH

Quoted 6 lines

Even though this is a relatively intrusive change, it in my opinion should go in master now, otherwise, we'd get a full release cycle broken on Fedora, which would really be a shame... :-/

Regards,
Yann E. MORIN.

Please commit this series ASAP so it can still get some autobuilder exposure.
Except for patch 5/6 which could use some work, but that's anyway not essential.

Regards,
Arnout

Quoted 39 lines

The following changes since commit 2bc7c2e009ac08ae80cbf3ce736ade16a6cfcb26:

zxing-cpp: needs dynamic library (2015-11-13 16:39:29 +0100)

are available in the git repository at:

git://git.busybox.net/~ymorin/git/buildroot yem/no-ld-library-path

for you to fetch changes up to 0a14a9ff7f39c78d2a39f8ccde40a9f7a6d020b2:

core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment (2015-11-13 19:51:07 +0100)

----------------------------------------------------------------
Ben Boeckel (1):
core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment

Yann E. MORIN (5):
package/axfsutils: fix Makefile package/mysql: unconditionally define host variables package/perl-file-util: remove host variant package/libcurl: carefully override LD_LIBRARY_PATH
core: check host executables have appropriate RPATH

 package/Makefile.in                      |  2 -
 package/axfsutils/0001-fix-cflags.patch  | 19 +++++++++
 package/axfsutils/0002-use-ldflags.patch | 20 +++++++++
 package/axfsutils/axfsutils.mk           |  2 +-
 package/libcurl/libcurl.mk               |  2 +-
 package/mysql/mysql.mk                   | 39 +++++++++---------
 package/perl-file-util/perl-file-util.mk |  1 -
 package/pkg-generic.mk                   |  8 ++++
 support/scripts/check-host-rpath         | 71 ++++++++++++++++++++++++++++++++
 9 files changed, 140 insertions(+), 24 deletions(-)
 create mode 100644 package/axfsutils/0001-fix-cflags.patch
 create mode 100644 package/axfsutils/0002-use-ldflags.patch
 create mode 100755 support/scripts/check-host-rpath
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF