Skip to contentSkip to conversation

c-icap: avoid host library search path

PATCH New Baruch Siach 3 participants 15 messages Apr 19, 2016 – Apr 21, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 c-icap: avoid host library search path

c-icap adds link time -rpath ${exec_prefix}/lib which defaults to /usr/lib. This adds host libraries to the search path which is bad for cross compilation. Direct make exec_prefix variable to staging instead.

Fixes:
http://autobuild.buildroot.net/results/077/077374329b2782dbb4b4a9ee7af264cdda311ccc/
http://autobuild.buildroot.net/results/057/057a88179150a4f02990d7b9b0d8f2de3c9da1a3/
http://autobuild.buildroot.net/results/da6/da62b80989df6b8f1b5557b4987d7a6999d30bb8/

---
 package/c-icap/c-icap.mk | 1 +
 1 file changed, 1 insertion(+)
package/c-icap/c-icap.mk +1 -0
index b4defb87379a..40926ea8e5d8 100644
@@ -17,6 +17,7 @@ C_ICAP_CONF_OPTS = \
 	--without-perl \
 	--enable-large-files \
 	--enable-ipv6
+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr
 
 ifeq ($(BR2_PACKAGE_BERKELEYDB),y)
 C_ICAP_CONF_OPTS += --with-berkeleydb
Signature · 1 line
2.8.0.rc3
#1 lore raw

PATCH 2/3 c-icap: bump to version 0.4.2

---
 package/c-icap/c-icap.hash | 2 +-
 package/c-icap/c-icap.mk   | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
package/c-icap/c-icap.hash +1 -1
index 89c70f423e00..2b083ef659ab 100644
@@ -1,2 +1,2 @@
 # Locally calculated
-sha256 46b97fab73dbb9e8def8e34c3f94048c92f50a5341bb85bec4f6f6349aec529a c_icap-0.3.5.tar.gz
+sha256 b138c7d7d9828d54c3307bcfe7b4917911266593832ffc26a60df9a0dfd2511e c_icap-0.4.2.tar.gz
package/c-icap/c-icap.mk +2 -1
index 40926ea8e5d8..a70993fa4490 100644
@@ -4,7 +4,7 @@
 #
 ################################################################################
 
-C_ICAP_VERSION = 0.3.5
+C_ICAP_VERSION = 0.4.2
 C_ICAP_SOURCE = c_icap-$(C_ICAP_VERSION).tar.gz
 C_ICAP_SITE = http://downloads.sourceforge.net/c-icap
 C_ICAP_LICENSE = LGPLv2.1+
@@ -17,6 +17,7 @@ C_ICAP_CONF_OPTS = \
 	--without-perl \
 	--enable-large-files \
 	--enable-ipv6
+C_ICAP_CONF_ENV = ac_cv_10031b_ipc_sem=yes ac_cv_fcntl=yes
 C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr
 
 ifeq ($(BR2_PACKAGE_BERKELEYDB),y)
Signature · 1 line
2.8.0.rc3
#2 lore raw

PATCH 2/3 c-icap: bump to version 0.4.2

Hello,

Quoted 1 line · Baruch Siach

+C_ICAP_CONF_ENV = ac_cv_10031b_ipc_sem=yes ac_cv_fcntl=yes

These look a bit special/weird, especially ac_cv_fcntl=yes. Normally for functions, the cache variable is named ac_cv_func_<function>, and the checks don't need to pre-seed cache variable values.

Can you add a comment on top of this line that explains why those two values need to be passed? Or at least, it should be explained in the commit log (but a comment in the code is better IMO).

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#3 lore raw

PATCH 2/3 c-icap: bump to version 0.4.2

Hi Thomas,

Quoted 7 lines · Thomas Petazzoni
Quoted 1 line · Baruch Siach

+C_ICAP_CONF_ENV = ac_cv_10031b_ipc_sem=yes ac_cv_fcntl=yes

These look a bit special/weird, especially ac_cv_fcntl=yes. Normally for functions, the cache variable is named ac_cv_func_<function>, and the checks don't need to pre-seed cache variable values.

configure.ac uses AC_TRY_RUN (inside AC_CACHE_CHECK) to test for sem_init/sem_post and fcntl. The fourth AC_TRY_RUN parameter (ACTION-IF-CROSS-COMPILING) of these tests gives this advice:

[AC_MSG_ERROR([cross-compiling,

        presetting ac_cv_10031b_ipc_sem=(yes|no) will help])]
        )

]

[AC_MSG_ERROR([cross-compiling,
presetting ac_cv_fcntl=(yes|no) will help])] )
]

Quoted 3 lines

Can you add a comment on top of this line that explains why those two values need to be passed? Or at least, it should be explained in the commit log (but a comment in the code is better IMO).

I'll add a comment.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#4 lore raw

PATCH 2/3 c-icap: bump to version 0.4.2

Hello,

Quoted 4 lines · Baruch Siach

configure.ac uses AC_TRY_RUN (inside AC_CACHE_CHECK) to test for sem_init/sem_post and fcntl. The fourth AC_TRY_RUN parameter (ACTION-IF-CROSS-COMPILING) of these tests gives this advice:

Quoted 9 lines
   [AC_MSG_ERROR([cross-compiling,
        presetting ac_cv_10031b_ipc_sem=(yes|no) will help])]
        )
   ]

   [AC_MSG_ERROR([cross-compiling,
        presetting ac_cv_fcntl=(yes|no) will help])]
        )
   ]

OK. I don't know why those tests are using AC_TRY_RUN(), but assuming they really need to, then yes, pre-seeding the cache variables in the only way.

Quoted 5 lines
Quoted 3 lines

Can you add a comment on top of this line that explains why those two values need to be passed? Or at least, it should be explained in the commit log (but a comment in the code is better IMO).

I'll add a comment.

Yes, just something like:

# Pre-seed cache variables for tests done with AC_TRY_RUN that are not

# cross-compile friendly

Thanks a lot!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#5 lore raw

PATCH 3/3 c-icap-modules: bump to version 0.4.2

In reply to Baruch Siach
---
 package/c-icap-modules/c-icap-modules.hash | 6 +++---
 package/c-icap-modules/c-icap-modules.mk   | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
package/c-icap-modules/c-icap-modules.hash +3 -3
index cf325ecddd5e..5df14498711e 100644
@@ -1,3 +1,3 @@
-# From http://sourceforge.net/projects/c-icap/files/c-icap-modules/0.3.x/
-md5	25a904f98cddbcc7431b22720cd3996e	c_icap_modules-0.3.2.tar.gz
-sha1	98dd7b3bf40b8831e297ffa8c4e1eaf7cef57cd0	c_icap_modules-0.3.2.tar.gz
+# From http://sourceforge.net/projects/c-icap/files/c-icap-modules/0.4.x/
+md5	5acd9487679ed47fbee00270d436718d	c_icap_modules-0.4.2.tar.gz
+sha1	e5f4203980b732b9de165c1e83c4703734c3e0e2	c_icap_modules-0.4.2.tar.gz
package/c-icap-modules/c-icap-modules.mk +1 -1
index 5252a8106c1a..cf388aaabcc5 100644
@@ -4,7 +4,7 @@
 #
 ################################################################################
 
-C_ICAP_MODULES_VERSION = 0.3.2
+C_ICAP_MODULES_VERSION = 0.4.2
 C_ICAP_MODULES_SOURCE = c_icap_modules-$(C_ICAP_MODULES_VERSION).tar.gz
 C_ICAP_MODULES_SITE = http://downloads.sourceforge.net/c-icap
 C_ICAP_MODULES_LICENSE = GPLv2+
Signature · 1 line
2.8.0.rc3
#6 lore raw

PATCH 1/3 c-icap: avoid host library search path

In reply to Baruch Siach

Hello,

Quoted 1 line · Baruch Siach

+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr

This is not correct. Setting exec_prefix to $(STAGING_DIR)/usr is wrong. Instead, can you try to remove:

-rpath @libdir@

from the various Makefile.am ?

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#7 lore raw

PATCH 1/3 c-icap: avoid host library search path

Hi Thomas,

Quoted 6 lines · Thomas Petazzoni
Quoted 1 line · Baruch Siach

+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr

This is not correct. Setting exec_prefix to $(STAGING_DIR)/usr is wrong.

Quoting configure:

# Installation directory options.
# These are left unexpanded so users can "make install exec_prefix=/foo"

# and all the variables that are supposed to be based on exec_prefix
# by default will actually change.
Quoted 5 lines

Instead, can you try to remove:

-rpath @libdir@

from the various Makefile.am ?

I can do that, but I'm not sure upstream would like it.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#8 lore raw

PATCH 1/3 c-icap: avoid host library search path

In reply to Thomas Petazzoni

Hi Thomas,

Quoted 9 lines · Thomas Petazzoni
Quoted 1 line · Baruch Siach

+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr

This is not correct. Setting exec_prefix to $(STAGING_DIR)/usr is wrong. Instead, can you try to remove:

-rpath @libdir@

from the various Makefile.am ?

It turns out that forcing AUTORECONF is enough to fix the problem. I'm not sure why. I have only noticed that after AUTORECONF, -lz appears explicitly in the link command line, whereas before libz was linked in implicitly via libicapapi.so NEEDED tag. Manually running the failed link command with -lz added, fixes the link as well. I guess that -lz makes the linker search in sysroot before rpath, but I could not find an explanation to this behaviour in the ld documentation.

Any suggestion for the comment text to explain the AUTORECONF?

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#9 lore raw

PATCH 1/3 c-icap: avoid host library search path

Hello,

Quoted 11 lines · Baruch Siach
Quoted 9 lines · Thomas Petazzoni
Quoted 1 line · Baruch Siach

+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr

This is not correct. Setting exec_prefix to $(STAGING_DIR)/usr is wrong. Instead, can you try to remove:

-rpath @libdir@

from the various Makefile.am ?

Quoted 7 lines

It turns out that forcing AUTORECONF is enough to fix the problem. I'm not sure why. I have only noticed that after AUTORECONF, -lz appears explicitly in the link command line, whereas before libz was linked in implicitly via libicapapi.so NEEDED tag. Manually running the failed link command with -lz added, fixes the link as well. I guess that -lz makes the linker search in sysroot before rpath, but I could not find an explanation to this behaviour in the ld documentation.

Are you talking about linking of programs against libicapapi.so, or the linking of libicapapi.so itself?

If you're talking about linking the programs against libicapapi.so, then adding -lz should not be needed, unless the programs use zlib function calls directly. But if only libicapapi.so is the zlib user, then there should normally be no need to link the programs with -lz (except in static linking scenarios, of course).

Did you verify that the paranoid path checking was not complaining?

Quoted 1 line

Any suggestion for the comment text to explain the AUTORECONF?

Just explains that it fixes stuff and which stuff :)

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#10 lore raw

PATCH 1/3 c-icap: avoid host library search path

Hi Thomas,

Quoted 20 lines · Thomas Petazzoni
Quoted 11 lines · Baruch Siach
Quoted 9 lines · Thomas Petazzoni
Quoted 1 line · Baruch Siach

+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr

This is not correct. Setting exec_prefix to $(STAGING_DIR)/usr is wrong. Instead, can you try to remove:

-rpath @libdir@

from the various Makefile.am ?

Quoted 7 lines

It turns out that forcing AUTORECONF is enough to fix the problem. I'm not sure why. I have only noticed that after AUTORECONF, -lz appears explicitly in the link command line, whereas before libz was linked in implicitly via libicapapi.so NEEDED tag. Manually running the failed link command with -lz added, fixes the link as well. I guess that -lz makes the linker search in sysroot before rpath, but I could not find an explanation to this behaviour in the ld documentation.

Quoted 2 lines

Are you talking about linking of programs against libicapapi.so, or the linking of libicapapi.so itself?

I meant the former. The failure is when linking the c-icap executable.

Quoted 5 lines

If you're talking about linking the programs against libicapapi.so, then adding -lz should not be needed, unless the programs use zlib function calls directly. But if only libicapapi.so is the zlib user, then there should normally be no need to link the programs with -lz (except in static linking scenarios, of course).

But that's exactly what happens after AUTORECONF.

Quoted 1 line

Did you verify that the paranoid path checking was not complaining?

BR2_COMPILER_PARANOID_UNSAFE_PATH is enabled but there was no complain. That's apparently because libtool breaks '-rpath /usr/lib' into '-Wl,-rpath -Wl,/usr/lib'.

Quoted 3 lines
Quoted 1 line

Any suggestion for the comment text to explain the AUTORECONF?

Just explains that it fixes stuff and which stuff :)

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#11 lore raw

PATCH 1/3 c-icap: avoid host library search path

In reply to Baruch Siach
Quoted 13 lines · Baruch Siach

Hi Thomas,

Quoted 9 lines · Thomas Petazzoni
Quoted 1 line · Baruch Siach

+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr

This is not correct. Setting exec_prefix to $(STAGING_DIR)/usr is wrong. Instead, can you try to remove:

-rpath @libdir@

from the various Makefile.am ?

Quoted 7 lines

It turns out that forcing AUTORECONF is enough to fix the problem. I'm not sure why. I have only noticed that after AUTORECONF, -lz appears explicitly in the link command line, whereas before libz was linked in implicitly via libicapapi.so NEEDED tag. Manually running the failed link command with -lz added, fixes the link as well. I guess that -lz makes the linker search in sysroot before rpath, but I could not find an explanation to this behaviour in the ld documentation.

As I wrote yesterday: -rpath is used for searching for NEEDED entries, not for
searching for -l argument. And vice versa: -L is used to search for -l arguments, but not for NEEDED entries.

Regards,
Arnout

Quoted 5 lines

Any suggestion for the comment text to explain the AUTORECONF?

baruch

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 1/3 c-icap: avoid host library search path

Hi Arnout,

Quoted 20 lines · Arnout Vandecappelle
Quoted 11 lines · Baruch Siach
Quoted 9 lines · Thomas Petazzoni
Quoted 1 line · Baruch Siach

+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr

This is not correct. Setting exec_prefix to $(STAGING_DIR)/usr is wrong. Instead, can you try to remove:

-rpath @libdir@

from the various Makefile.am ?

Quoted 7 lines

It turns out that forcing AUTORECONF is enough to fix the problem. I'm not sure why. I have only noticed that after AUTORECONF, -lz appears explicitly in the link command line, whereas before libz was linked in implicitly via libicapapi.so NEEDED tag. Manually running the failed link command with -lz added, fixes the link as well. I guess that -lz makes the linker search in sysroot before rpath, but I could not find an explanation to this behaviour in the ld documentation.

Quoted 3 lines

As I wrote yesterday: -rpath is used for searching for NEEDED entries, not
for searching for -l argument. And vice versa: -L is used to search for -l arguments, but not for NEEDED entries.

That explains the behaviour I see, thanks. The twist here is that -rpath does not search for libraries that were found already as a result of -l. This apparently means that -L would indirectly affect the results of -rpath search.

Do you have any reference to gcc/binutils documentation on that?

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#13 lore raw

PATCH 1/3 c-icap: avoid host library search path

Quoted 4 lines · Baruch Siach

Hi Arnout,

Quoted 1 line · Arnout Vandecappelle

On 04/20/16 20:04, Baruch Siach wrote:

[snip]

Quoted 12 lines
Quoted 11 lines
Quoted 7 lines

It turns out that forcing AUTORECONF is enough to fix the problem. I'm not sure why. I have only noticed that after AUTORECONF, -lz appears explicitly in the link command line, whereas before libz was linked in implicitly via libicapapi.so NEEDED tag. Manually running the failed link command with -lz added, fixes the link as well. I guess that -lz makes the linker search in sysroot before rpath, but I could not find an explanation to this behaviour in the ld documentation.

As I wrote yesterday: -rpath is used for searching for NEEDED entries, not
for searching for -l argument. And vice versa: -L is used to search for -l arguments, but not for NEEDED entries.

Quoted 5 lines

That explains the behaviour I see, thanks. The twist here is that -rpath does not search for libraries that were found already as a result of -l. This apparently means that -L would indirectly affect the results of -rpath search.

Do you have any reference to gcc/binutils documentation on that?

I just did 'man ld'.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . 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
#14 lore raw

PATCH 1/3 c-icap: avoid host library search path

In reply to Baruch Siach
Quoted 2 lines · Baruch Siach

c-icap adds link time -rpath ${exec_prefix}/lib which defaults to /usr/lib. This adds host libraries to the search path

I think this is actually uncovering a bigger problem.

-rpath doesn't add the specified library to the compile-time search path. So
it should be OK in this case.

However, -rpath _is_ used when ld links against that .so file and finds a
NEEDED entry there that it doesn't have yet.

For /usr/lib it's no problem, it can be safely removed. The problem is for
rpath entries that _are_ necessary: they should still stay, and of course they shouldn't refer to the build directory. For example, libdw.so has an RPATH of /usr/lib/elfutils, which is used at runtime to dlopen() libebl. Now the problem
is: if /usr/lib/elfutils exists on the host, we may try to link a target library or executable agains a host library that is there. For elfutils there isn't much risk, but there may be others that are more problematic.

But of course, all that bears no relation to this patch :-)

Regards,
Arnout

Quoted 26 lines

which is bad for cross
compilation. Direct make exec_prefix variable to staging instead.

Fixes:
http://autobuild.buildroot.net/results/077/077374329b2782dbb4b4a9ee7af264cdda311ccc/
http://autobuild.buildroot.net/results/057/057a88179150a4f02990d7b9b0d8f2de3c9da1a3/
http://autobuild.buildroot.net/results/da6/da62b80989df6b8f1b5557b4987d7a6999d30bb8/

---
  package/c-icap/c-icap.mk | 1 +
  1 file changed, 1 insertion(+)
package/c-icap/c-icap.mk +1 -0
index b4defb87379a..40926ea8e5d8 100644
@@ -17,6 +17,7 @@ C_ICAP_CONF_OPTS = \
  	--without-perl \
  	--enable-large-files \
  	--enable-ipv6
+C_ICAP_MAKE_OPTS = exec_prefix=$(STAGING_DIR)/usr
 
  ifeq ($(BR2_PACKAGE_BERKELEYDB),y)
  C_ICAP_CONF_OPTS += --with-berkeleydb
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