Skip to contentSkip to conversation

Fix logrotate autobuild failure

PATCH Superseded Fabio Porcedda 3 participants 10 messages Mar 1, 2015 – Mar 5, 2015

lore
Share & tools
lore
#0 lore raw

v3 cover Fix logrotate autobuild failure

This patch set fix a logrotate autobuild failure:

    http://autobuild.buildroot.net/results/159bf5730414ca7f73dcdae95090177355193636
    http://autobuild.buildroot.net/results/ebe6ab7fc0f2cff98de06b3dc374730da9e9e4f2

v3:
- popt: instead of using sed add a patch and send it upstream
- logrotate: improve commit message
v2:
- logroate: instead of adding a patch just overwrite from the command line the LOADLIBES variable

Fabio Porcedda (2):
popt: add to the "popt.pc" file the libintl library
logrotate: use pkg-config for the opt library

 package/logrotate/logrotate.mk                     |  6 ++-
 ...-the-libintl-library-to-the-Libs.private-.patch | 47 ++++++++++++++++++++++
 package/popt/popt.mk                               |  4 ++
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 package/popt/0004-popt.pc-add-the-libintl-library-to-the-Libs.private-.patch
Signature · 1 line
2.3.1
#1 lore raw

PATCH v3 1/2 popt: add to the "popt.pc" file the libintl library

This change permits through pkg-config to retrive even the depenencies that is useful for a static linking.

---
 ...-the-libintl-library-to-the-Libs.private-.patch | 47 ++++++++++++++++++++++
 package/popt/popt.mk                               |  4 ++
 2 files changed, 51 insertions(+)
 create mode 100644 package/popt/0004-popt.pc-add-the-libintl-library-to-the-Libs.private-.patch
package/popt/0004-popt.pc-add-the-libintl-library-to-the-Libs.private-.patch new file +47 -0
new file mode 100644
index 0000000..9b0b07e
@@ -0,0 +1,47 @@
+From 470755ccc353b59107c957972d908cbde4942c8d Mon Sep 17 00:00:00 2001
+From: Fabio Porcedda <[email protected]>
+Date: Sun, 1 Mar 2015 21:13:48 +0100
+Subject: [PATCH] popt.pc: add the libintl library to the Libs.private field
+
+If static linking and libintl is used, the program that uses the popt
+library needs to link to the libintl library too so add the libintl
+library to the libs.private field because is useful for programs that call
+  pkg-config --static --libs popt
+they get the libintl library too:
+  -L<...> -lpopt -lintl
+
+This patch was already sent upstream:
+http://rpm5.org/community/popt-devel/0294.html
+
+Signed-off-by: Fabio Porcedda <[email protected]>
+---
+ configure.ac | 1 +
+ popt.pc.in   | 1 +
+ 2 files changed, 2 insertions(+)
+
+diff --git a/configure.ac b/configure.ac
+index e8eb238..816d272 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -90,6 +90,7 @@ popt_sysconfdir="${sysconfdir}"
+ eval "popt_sysconfdir=\"${popt_sysconfdir}\"" # expand contained ${prefix}
+ AC_DEFINE_UNQUOTED([POPT_SYSCONFDIR], ["$popt_sysconfdir"], [Full path to default POPT configuration directory])
+ 
++AC_SUBST([POPT_PKGCONFIG_LIBS_PRIVATE],"$LIBINTL")
+ 
+ # Define a (hope) portable Libs pkgconfig directive that 
+ # - Don't harm if the default library search path include ${libdir}
+diff --git a/popt.pc.in b/popt.pc.in
+index a86437c..0acfdb9 100644
+--- a/popt.pc.in
++++ b/popt.pc.in
+@@ -6,5 +6,6 @@ includedir=@includedir@
+ Name: popt
+ Version: @VERSION@
+ Description: popt library.
++Libs.private: @POPT_PKGCONFIG_LIBS_PRIVATE@
+ Libs: @POPT_PKGCONFIG_LIBS@
+ Cflags: -I${includedir}
+-- 
+2.3.1
+
package/popt/popt.mk +4 -0
index 8ce64d4..605649b 100644
@@ -13,6 +13,10 @@ POPT_LICENSE_FILES = COPYING
 POPT_AUTORECONF = YES
 POPT_GETTEXTIZE = YES
 
+ifeq ($(BR2_PACKAGE_GETTEXT),y)
+POPT_DEPENDENCIES += gettext
+endif
+
 POPT_CONF_ENV = ac_cv_va_copy=yes
 
 ifeq ($(BR2_PACKAGE_LIBICONV),y)
Signature · 1 line
2.3.1
#2 lore raw

PATCH v3 1/2 popt: add to the "popt.pc" file the libintl library

Dear Fabio Porcedda,

Quoted 4 lines · Fabio Porcedda

This change permits through pkg-config to retrive even the depenencies that is useful for a static linking.

Applied, thanks!

Thomas

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

PATCH v3 2/2 logrotate: use pkg-config for the opt library

In reply to Fabio Porcedda

Without using the pkg-config the dependencies are not included for a static linking so it fails to build.
These failures are fixed by linking the libintl library that is a dependency of the opt library.

This commit to be useful requires the previous commit 'popt: add to the "popt.pc" file the libintl library'.

Fixes:
http://autobuild.buildroot.net/results/159bf5730414ca7f73dcdae95090177355193636
http://autobuild.buildroot.net/results/ebe6ab7fc0f2cff98de06b3dc374730da9e9e4f2

Minimal defconfig to reproduce the build failure:
BR2_STATIC_LIBS=y
BR2_TOOLCHAIN_BUILDROOT_LOCALE=y
BR2_PACKAGE_GETTEXT=y
BR2_PACKAGE_LOGROTATE=y

---
 package/logrotate/logrotate.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
package/logrotate/logrotate.mk +4 -2
index dc26d85..c1bca48 100644
@@ -9,10 +9,12 @@ LOGROTATE_SITE = https://www.fedorahosted.org/releases/l/o/logrotate
 LOGROTATE_LICENSE = GPLv2+
 LOGROTATE_LICENSE_FILES = COPYING
 
-LOGROTATE_DEPENDENCIES = popt
+LOGROTATE_DEPENDENCIES = popt host-pkgconf
 
 define LOGROTATE_BUILD_CMDS
-	$(MAKE) CC="$(TARGET_CC) $(TARGET_CFLAGS)" LDFLAGS="$(LDFLAGS)" -C $(@D)
+	$(MAKE) CC="$(TARGET_CC) $(TARGET_CFLAGS)" LDFLAGS="$(LDFLAGS)" \
+		LOADLIBES="$(shell $(PKG_CONFIG_HOST_BINARY) --libs popt)" \
+		-C $(@D)
 endef
 
 define LOGROTATE_INSTALL_TARGET_CMDS
Signature · 1 line
2.3.1
#4 lore raw

PATCH v3 2/2 logrotate: use pkg-config for the opt library

Dear Fabio Porcedda,

In the title: s/opt/popt/

Quoted 4 lines · Fabio Porcedda

Without using the pkg-config the dependencies are not included for a static linking so it fails to build.
These failures are fixed by linking the libintl library that is a dependency of the opt library.

s/opt/popt/

I've fixed and committed. Though I have one concern below.

package/logrotate/logrotate.mk LOGROTATE_SITE = https://www.fedorahosted.org/releases/l/o/logrotate · line 16

Quoted 2 lines
package/logrotate/logrotate.mk +2 -0
+	$(MAKE) CC="$(TARGET_CC) $(TARGET_CFLAGS)" LDFLAGS="$(LDFLAGS)" \
+		LOADLIBES="$(shell $(PKG_CONFIG_HOST_BINARY) --libs popt)" \

It's not currently supported in Buildroot, but LOADLIBES is extended by the logrotate Makefile with -lacl or -lselinux when WITH_ACL or WITH_SELINUX are used. I'm not sure this will play well with your proposed solution.

But ok, this can be handled when/if someone adds ACL and/or SELinux support to logrotate.

Thanks!

Thomas

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

PATCH v3 2/2 logrotate: use pkg-config for the opt library

Quoted 15 lines · Thomas Petazzoni

Dear Fabio Porcedda,

In the title: s/opt/popt/

Quoted 4 lines · Fabio Porcedda

Without using the pkg-config the dependencies are not included for a static linking so it fails to build.
These failures are fixed by linking the libintl library that is a dependency of the opt library.

s/opt/popt/

I've fixed and committed. Though I have one concern below.

Quoted 7 lines

package/logrotate/logrotate.mk LOGROTATE_SITE = https://www.fedorahosted.org/releases/l/o/logrotate · line 16

Quoted 2 lines
package/logrotate/logrotate.mk +2 -0
+     $(MAKE) CC="$(TARGET_CC) $(TARGET_CFLAGS)" LDFLAGS="$(LDFLAGS)" \
+             LOADLIBES="$(shell $(PKG_CONFIG_HOST_BINARY) --libs popt)" \

It's not currently supported in Buildroot, but LOADLIBES is extended by the logrotate Makefile with -lacl or -lselinux when WITH_ACL or WITH_SELINUX are used. I'm not sure this will play well with your proposed solution.

Thanks for noticing it.
I didn't know that a variable forced from the command line cannot be extended. Now I'm aware of it.

Quoted 2 lines

But ok, this can be handled when/if someone adds ACL and/or SELinux support to logrotate.

Good idea.

Thanks

Signature · 1 line
Fabio Porcedda
#6 lore raw

PATCH v3 2/2 logrotate: use pkg-config for the opt library

Dear Fabio Porcedda,

Quoted 8 lines · Fabio Porcedda
Quoted 4 lines

It's not currently supported in Buildroot, but LOADLIBES is extended by the logrotate Makefile with -lacl or -lselinux when WITH_ACL or WITH_SELINUX are used. I'm not sure this will play well with your proposed solution.

Thanks for noticing it.
I didn't know that a variable forced from the command line cannot be extended. Now I'm aware of it.

Depends on whether you pass variables in the environment (on the left hand side of make) or as make options (on the right hand side of make). Demonstration with a simple Makefile:

===
FOO = -lbaz

ifeq ($(WITH_SELINUX),yes)
FOO += -lselinux
endif

all:

	@echo $(FOO)

===

And now, the different cases:

thomas at skate:/tmp$ make
-lbaz
thomas at skate:/tmp$ make FOO=-lpouet
-lpouet
thomas at skate:/tmp$ make FOO=-lpouet WITH_SELINUX=yes -lpouet
thomas at skate:/tmp$ make WITH_SELINUX=yes -lbaz -lselinux
thomas at skate:/tmp$ FOO=-lpouet make
-lbaz
thomas at skate:/tmp$ FOO=-lpouet make WITH_SELINUX=yes -lbaz -lselinux

Best regards,

Thomas

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

PATCH v3 2/2 logrotate: use pkg-config for the opt library

Quoted 30 lines · Thomas Petazzoni

Dear Fabio Porcedda,

Quoted 8 lines · Fabio Porcedda
Quoted 4 lines

It's not currently supported in Buildroot, but LOADLIBES is extended by the logrotate Makefile with -lacl or -lselinux when WITH_ACL or WITH_SELINUX are used. I'm not sure this will play well with your proposed solution.

Thanks for noticing it.
I didn't know that a variable forced from the command line cannot be extended. Now I'm aware of it.

Depends on whether you pass variables in the environment (on the left hand side of make) or as make options (on the right hand side of make). Demonstration with a simple Makefile:

===
FOO = -lbaz

ifeq ($(WITH_SELINUX),yes)
FOO += -lselinux
endif

all:

        @echo $(FOO)

===

And now, the different cases:

Quoted 12 lines

thomas at skate:/tmp$ make
-lbaz
thomas at skate:/tmp$ make FOO=-lpouet
-lpouet
thomas at skate:/tmp$ make FOO=-lpouet WITH_SELINUX=yes -lpouet
thomas at skate:/tmp$ make WITH_SELINUX=yes -lbaz -lselinux
thomas at skate:/tmp$ FOO=-lpouet make
-lbaz
thomas at skate:/tmp$ FOO=-lpouet make WITH_SELINUX=yes -lbaz -lselinux

Even the += behavior is different if the variable is passed on the left side or on the right side.

===
FOO ?= -lbaz

ifeq ($(WITH_SELINUX),yes)
FOO += -lselinux
endif

all:
@echo $(FOO)
===

$ make FOO=-lpouet WITH_SELINUX=yes
-lpouet
$ FOO=-lpouet make WITH_SELINUX=yes
-lpouet -lselinux

So if we want that the default option can be changed from the command line and the makefile will still be able to add options to the same variable we need to use ?= and pass the variable on the left side.

$ FOO=-lpouet make WITH_SELINUX=yes
-lpouet -lselinux

Best regards

Signature · 1 line
Fabio Porcedda
#8 lore raw

v3 cover Fix logrotate autobuild failure

In reply to Fabio Porcedda
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Fabio" == Fabio Porcedda <[email protected]> writes:

This patch set fix a logrotate autobuild failure:

    http://autobuild.buildroot.net/results/159bf5730414ca7f73dcdae95090177355193636
    http://autobuild.buildroot.net/results/ebe6ab7fc0f2cff98de06b3dc374730da9e9e4f2
Quoted 6 lines

v3:
- popt: instead of using sed add a patch and send it upstream
- logrotate: improve commit message
v2:
- logroate: instead of adding a patch just overwrite from the command line the LOADLIBES variable

Hmm, while this is an improvement I still see failures.

I could only get popt to pull in the gettext/libintl dependency if I added libiconv, but as we don't add libiconv to Libs.private we then end up with linker issues for it in logrotate:

/home/peko/source/buildroot/output/host/usr/bin/arm-linux-gcc -Os -static logrotate.o log.o config.o basenames.o -L/home/peko/source/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib -lpopt -o logrotate
logrotate.o: In function `compressLogFile':
/home/peko/source/buildroot/output/build/logrotate-3.8.7/logrotate.c:537: warning: the use of OBSOLESCENT utime' is discouraged, use utimes' /home/peko/source/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib/libpopt.a(poptint.o): In function `POPT_fprintf':
poptint.c:(.text+0x534): undefined reference to `libiconv_open'
poptint.c:(.text+0x58c): undefined reference to `libiconv'
poptint.c:(.text+0x5b8): undefined reference to `libiconv'
poptint.c:(.text+0x62c): undefined reference to `libiconv_close'
collect2: ld returned 1 exit status
<builtin>: recipe for target 'logrotate' failed

So I have left out this series for 2015.02.

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

v3 cover Fix logrotate autobuild failure

Quoted 19 lines · Peter Korsgaard
Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Fabio" == Fabio Porcedda <[email protected]> writes:

This patch set fix a logrotate autobuild failure:

    http://autobuild.buildroot.net/results/159bf5730414ca7f73dcdae95090177355193636
    http://autobuild.buildroot.net/results/ebe6ab7fc0f2cff98de06b3dc374730da9e9e4f2
Quoted 6 lines

v3:
- popt: instead of using sed add a patch and send it upstream
- logrotate: improve commit message
v2:
- logroate: instead of adding a patch just overwrite from the command line the LOADLIBES variable

Hmm, while this is an improvement I still see failures.

I could only get popt to pull in the gettext/libintl dependency if I added libiconv, but as we don't add libiconv to Libs.private we then end up with linker issues for it in logrotate:

Quoted 10 lines

/home/peko/source/buildroot/output/host/usr/bin/arm-linux-gcc -Os -static logrotate.o log.o config.o basenames.o -L/home/peko/source/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib -lpopt -o logrotate
logrotate.o: In function `compressLogFile':
/home/peko/source/buildroot/output/build/logrotate-3.8.7/logrotate.c:537: warning: the use of OBSOLESCENT utime' is discouraged, use utimes' /home/peko/source/buildroot/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/lib/libpopt.a(poptint.o): In function `POPT_fprintf':
poptint.c:(.text+0x534): undefined reference to `libiconv_open'
poptint.c:(.text+0x58c): undefined reference to `libiconv'
poptint.c:(.text+0x5b8): undefined reference to `libiconv'
poptint.c:(.text+0x62c): undefined reference to `libiconv_close'
collect2: ld returned 1 exit status
<builtin>: recipe for target 'logrotate' failed

Thanks, for testing it, i forgot about the libiconv library, I will send an updated patch that fix it.

Quoted 1 line

So I have left out this series for 2015.02.

BR

Signature · 1 line
Fabio Porcedda