| Message ID | 1425244013-2509-3-git-send-email-fabio.porcedda@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
Dear Fabio Porcedda, In the title: s/opt/popt/ On Sun, 1 Mar 2015 22:06:53 +0100, Fabio Porcedda wrote: > 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. > + $(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
On Wed, Mar 4, 2015 at 11:29 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Fabio Porcedda, > > In the title: s/opt/popt/ > > On Sun, 1 Mar 2015 22:06:53 +0100, Fabio Porcedda wrote: >> 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. > > >> + $(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. > But ok, this can be handled when/if someone adds ACL and/or SELinux > support to logrotate. Good idea. Thanks
Dear Fabio Porcedda, On Thu, 5 Mar 2015 00:38:25 +0100, Fabio Porcedda wrote: > > 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@skate:/tmp$ make -lbaz thomas@skate:/tmp$ make FOO=-lpouet -lpouet thomas@skate:/tmp$ make FOO=-lpouet WITH_SELINUX=yes -lpouet thomas@skate:/tmp$ make WITH_SELINUX=yes -lbaz -lselinux thomas@skate:/tmp$ FOO=-lpouet make -lbaz thomas@skate:/tmp$ FOO=-lpouet make WITH_SELINUX=yes -lbaz -lselinux Best regards, Thomas
On Thu, Mar 5, 2015 at 9:28 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Fabio Porcedda, > > On Thu, 5 Mar 2015 00:38:25 +0100, Fabio Porcedda wrote: > >> > 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@skate:/tmp$ make > -lbaz > thomas@skate:/tmp$ make FOO=-lpouet > -lpouet > thomas@skate:/tmp$ make FOO=-lpouet WITH_SELINUX=yes > -lpouet > thomas@skate:/tmp$ make WITH_SELINUX=yes > -lbaz -lselinux > thomas@skate:/tmp$ FOO=-lpouet make > -lbaz > thomas@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
diff --git a/package/logrotate/logrotate.mk b/package/logrotate/logrotate.mk index dc26d85..c1bca48 100644 --- a/package/logrotate/logrotate.mk +++ b/package/logrotate/logrotate.mk @@ -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
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 Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com> --- package/logrotate/logrotate.mk | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)