Skip to contentSkip to conversation

logrotate: fix missing acl and libselinux dependencies

PATCH In review Andre McCurdy 2 participants 4 messages Jan 13, 2015 – Jan 14, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 logrotate: fix missing acl and libselinux dependencies

---
 meta/recipes-extended/logrotate/logrotate_3.8.8.bb | 6 ++++++
 1 file changed, 6 insertions(+)
meta/recipes-extended/logrotate/logrotate_3.8.8.bb +6 -0
index 133f6d7..00432de 100644
@@ -3,6 +3,9 @@ SECTION = "console/utils"
 HOMEPAGE = "https://fedorahosted.org/logrotate/"
 LICENSE = "GPLv2"
 
+# TODO: logrotate 3.8.8 adds autotools/automake support, update recipe to use it.
+# TODO: Document coreutils dependency. Why not RDEPENDS? Why not busybox?
+
 DEPENDS="coreutils popt"
 
 LIC_FILES_CHKSUM = "file://COPYING;md5=18810669f13b87348459e611d31ab760"
@@ -21,6 +24,9 @@ PACKAGECONFIG ?= "\
     ${@base_contains('DISTRO_FEATURES', 'selinux', 'selinux', '', d)} \
 "
 
+PACKAGECONFIG[acl] = ",,acl"
+PACKAGECONFIG[selinux] = ",,libselinux"
+
 # If RPM_OPT_FLAGS is unset, it adds -g itself rather than obeying our
 # optimization variables, so use it rather than EXTRA_CFLAGS.
 EXTRA_OEMAKE = "\
Signature · 1 line
1.9.1
#1 lore raw

meta/recipes-extended/logrotate/logrotate_3.8.8.bb SECTION = "console/utils" · line 7

Quoted 2 lines · Andre McCurdy
meta/recipes-extended/logrotate/logrotate_3.8.8.bb +2 -0
+# TODO: logrotate 3.8.8 adds autotools/automake support, update recipe to use it.
+# TODO: Document coreutils dependency. Why not RDEPENDS? Why not busybox?

It might well be worth adding autotools support right now.

meta/recipes-extended/logrotate/logrotate_3.8.8.bb PACKAGECONFIG ?= "\ · line 28

Quoted 9 lines

DEPENDS="coreutils popt"

LIC_FILES_CHKSUM = "file://COPYING;md5=18810669f13b87348459e611d31ab760"

meta/recipes-extended/logrotate/logrotate_3.8.8.bb +2 -0
@@ -21,6 +24,9 @@ PACKAGECONFIG ?= "\
     ${@base_contains('DISTRO_FEATURES', 'selinux', 'selinux', '', d)} \
 "
 
+PACKAGECONFIG[acl] = ",,acl"
+PACKAGECONFIG[selinux] = ",,libselinux"

The problem with these two lines, are that you're not explicitly enabling and disabling those two features; thus our builds won't be deterministic.

Depending on how the actual tests are performed in the logrotate sources, it might happen that you're enabling the features just by making sure that they're going to be built prior to logrotate. Though, there's no guarantee that this will work (without looking at the source code).

However, if that enabling works, then we have the problem that the features will always be activated regardless of whether we add acl and selinux to PACKAGECONFIG, if these exists in the sysroot. Thus, the build is undeterministic.

Could you please look at enabling / disabling those features explicitly?

Cheers,
Anders

Signature · 2 lines
Anders Darander
ChargeStorm AB / eStorm AB
#2 lore raw
Quoted 13 lines · Anders Darander

meta/recipes-extended/logrotate/logrotate_3.8.8.bb SECTION = "console/utils" · line 7

Quoted 2 lines · Andre McCurdy
meta/recipes-extended/logrotate/logrotate_3.8.8.bb +2 -0
+# TODO: logrotate 3.8.8 adds autotools/automake support, update recipe to use it.
+# TODO: Document coreutils dependency. Why not RDEPENDS? Why not busybox?

It might well be worth adding autotools support right now.

Quoted 6 lines

DEPENDS="coreutils popt"

LIC_FILES_CHKSUM = "file://COPYING;md5=18810669f13b87348459e611d31ab760"

meta/recipes-extended/logrotate/logrotate_3.8.8.bb +0 -0
@@ -21,6 +24,9 @@ PACKAGECONFIG ?= "\
     ${@base_contains('DISTRO_FEATURES', 'selinux', 'selinux', '', d)} \
 "
Quoted 6 lines

meta/recipes-extended/logrotate/logrotate_3.8.8.bb PACKAGECONFIG ?= "\ · line 28

Quoted 2 lines
meta/recipes-extended/logrotate/logrotate_3.8.8.bb +2 -0
+PACKAGECONFIG[acl] = ",,acl"
+PACKAGECONFIG[selinux] = ",,libselinux"

The problem with these two lines, are that you're not explicitly enabling and disabling those two features; thus our builds won't be deterministic.

Currently the configure task for logrotate is a no-op, so the normal way of constructing a configure command line via PACKAGECONFIG doesn't work.

We do however need to include acl and libselinux in DEPENDS if enabled
- which wasn't happening before and is what the patch attempts to fix,

There are no floating dependencies since acl and selinux are enabled explicitly during compile, by options passed on the make command line via EXTRA_OEMAKE.

Quoted 18 lines

Depending on how the actual tests are performed in the logrotate sources, it might happen that you're enabling the features just by making sure that they're going to be built prior to logrotate. Though, there's no guarantee that this will work (without looking at the source code).

However, if that enabling works, then we have the problem that the features will always be activated regardless of whether we add acl and selinux to PACKAGECONFIG, if these exists in the sysroot. Thus, the build is undeterministic.

Could you please look at enabling / disabling those features explicitly?

Cheers,
Anders
--
Anders Darander
ChargeStorm AB / eStorm AB

#3 lore raw
Quoted 21 lines · Andre McCurdy
Quoted 13 lines · Anders Darander

meta/recipes-extended/logrotate/logrotate_3.8.8.bb SECTION = "console/utils" · line 7

Quoted 2 lines · Andre McCurdy
meta/recipes-extended/logrotate/logrotate_3.8.8.bb +2 -0
+# TODO: logrotate 3.8.8 adds autotools/automake support, update recipe to use it.
+# TODO: Document coreutils dependency. Why not RDEPENDS? Why not busybox?

It might well be worth adding autotools support right now.

Quoted 6 lines

DEPENDS="coreutils popt"

LIC_FILES_CHKSUM = "file://COPYING;md5=18810669f13b87348459e611d31ab760"

meta/recipes-extended/logrotate/logrotate_3.8.8.bb +0 -0
@@ -21,6 +24,9 @@ PACKAGECONFIG ?= "\
     ${@base_contains('DISTRO_FEATURES', 'selinux', 'selinux', '', d)} \
 "
Quoted 6 lines

meta/recipes-extended/logrotate/logrotate_3.8.8.bb PACKAGECONFIG ?= "\ · line 28

Quoted 2 lines
meta/recipes-extended/logrotate/logrotate_3.8.8.bb +2 -0
+PACKAGECONFIG[acl] = ",,acl"
+PACKAGECONFIG[selinux] = ",,libselinux"

The problem with these two lines, are that you're not explicitly enabling and disabling those two features; thus our builds won't be deterministic.

Quoted 3 lines

Currently the configure task for logrotate is a no-op, so the normal way of constructing a configure command line via PACKAGECONFIG doesn't work.

Yep, that's why I suggested moving to autotools etc...

Quoted 6 lines

We do however need to include acl and libselinux in DEPENDS if enabled
- which wasn't happening before and is what the patch attempts to fix,

There are no floating dependencies since acl and selinux are enabled explicitly during compile, by options passed on the make command line via EXTRA_OEMAKE.

Ok, I missed that one as it was outside of the patch.

Then I presume that there's no need to disable those features when they're not enabled? Right? If so, it should be fine for the moment.

Cheers,
Anders

Signature · 2 lines
Anders Darander
ChargeStorm AB / eStorm AB