On 03/11/2014 11:52 AM, Steve Beattie wrote: > On Fri, Mar 07, 2014 at 08:19:45AM -0800, John Johansen wrote: >> So this is a patch I have been using for a while to get around having >> to specify USE_SYSTEM=1 every time I run the tests. Is it worth >> upstreaming and applying to the other Makefiles that currently can take >> USE_SYSTEM as a parameter >> >> If USE_SYSTEM=1 is NOT specified >> it tries to find a local build and if it can't find one, it sets >> USE_SYSTEM=1 and issues a warning message >> If USE_SYSTEM=1 is specified >> it only tries building against the system libraries > > I'm still mulling/trying to remember the arguments from when we added > USE_SYSTEM so I don't have a yay or nay on this patch, except for two > things. The first is that make clean should work even if neither the > system libapparmor or the in-tree libapparmor exists. The second is:
I don't remember it either, but I am finding the current behavior a major annoyance. This or something like it doesn't have to make it upstream but I will continue to use it, which is why the RFC. > >> === modified file 'tests/regression/apparmor/Makefile' >> --- tests/regression/apparmor/Makefile 2014-01-24 19:03:22 +0000 >> +++ tests/regression/apparmor/Makefile 2014-03-07 00:39:38 +0000 >> @@ -6,7 +6,33 @@ >> # published by the Free Software Foundation, version 2 of the >> # License. >> >> -ifdef USE_SYSTEM >> +define nl >> + >> + >> +endef > > The nl definition is useful generally, so I'd like to see it go into > common/Make.rules and then used in whatever make warning/error messages > we have. Here's a prototype patch that does that (though I didn't touch > the parser handling of USE_SYSTEM): > sure that works for me, the patch looks good even without the updating the parser's handling of USE_SYSTEM which can come as a separate patch > Signed-off-by: Steve Beattie <[email protected]> Acked-by: John Johansen <[email protected]> > --- > changehat/mod_apparmor/Makefile | 26 ++++++++++++++------------ > changehat/pam_apparmor/Makefile | 26 ++++++++++++++------------ > common/Make.rules | 5 +++++ > tests/regression/apparmor/Makefile | 30 ++++++++++++++++++------------ > 4 files changed, 51 insertions(+), 36 deletions(-) > > Index: b/common/Make.rules > =================================================================== > --- a/common/Make.rules > +++ b/common/Make.rules > @@ -36,6 +36,11 @@ ifndef AWK > $(error awk utility required for build but not available) > endif > > +define nl > + > + > +endef > + > # OVERRIDABLE variables > # Set these variables before including Make.rules to change its behavior > # SPECFILE - for packages that have a non-standard specfile name > Index: b/tests/regression/apparmor/Makefile > =================================================================== > --- a/tests/regression/apparmor/Makefile > +++ b/tests/regression/apparmor/Makefile > @@ -6,6 +6,10 @@ > # published by the Free Software Foundation, version 2 of the > # License. > > +all: > +COMMONDIR=../../../common/ > +include $(COMMONDIR)/Make.rules > + > ifdef USE_SYSTEM > # use the system libapparmor headers and library > LIBAPPARMOR = $(shell if pkg-config --exists libapparmor ; then \ > @@ -14,9 +18,12 @@ ifdef USE_SYSTEM > echo -lapparmor ; \ > fi ) > ifeq ($(strip $(LIBAPPARMOR)),) > - ERROR_MESSAGE = Unable to find libapparmor installed on this system; > either \ > - install libapparmor devel packages, set the LIBAPPARMOR variable \ > - manually, or build against in-tree libapparmor) > + ERROR_MESSAGE = $(error ${nl}\ > +************************************************************************${nl}\ > +Unable to find libapparmor installed on this system; either${nl}\ > +install libapparmor devel packages, set the LIBAPPARMOR variable${nl}\ > +manually, or build against in-tree libapparmor.${nl}\ > +************************************************************************${nl}) > endif # LIBAPPARMOR not set > LDLIBS += $(LIBAPPARMOR) > > @@ -26,10 +33,13 @@ else # !USE_SYSTEM > LIBAPPARMOR_INCLUDE = $(LIBAPPARMOR_SRC)/include > LIBAPPARMOR_PATH := $(LIBAPPARMOR_SRC)/src/.libs/ > ifeq ($(realpath $(LIBAPPARMOR_PATH)/libapparmor.a),) > - ERROR_MESSAGE = $(LIBAPPARMOR_PATH)/libapparmor.a is missing; either > build against \ > - the in-tree libapparmor by building it first and then trying again \ > - (see the top-level README for help) or build against the system \ > - libapparmor by adding USE_SYSTEM=1 to your make command.) > + ERROR_MESSAGE = $(error ${nl}\ > +************************************************************************${nl}\ > +$(LIBAPPARMOR_PATH)/libapparmor.a is missing; either build against${nl}\ > +the in-tree libapparmor by building it first and then trying again${nl}\ > +(see the top-level README for help) or build against the system${nl}\ > +libapparmor by adding USE_SYSTEM=1 to your make command.${nl}\ > +************************************************************************${nl}) > endif > > CFLAGS += -L$(LIBAPPARMOR_PATH) -I$(LIBAPPARMOR_INCLUDE) > @@ -180,11 +190,7 @@ RISKY_TESTS= > > .PHONY: libapparmor_check > .SILENT: libapparmor_check > -libapparmor_check: > - @if [ -n "$(ERROR_MESSAGE)" ] ; then \ > - echo "$(ERROR_MESSAGE)" 1>&2 ; \ > - return 1 ; \ > - fi > +libapparmor_check: ; $(ERROR_MESSAGE) > > all: libapparmor_check $(EXEC) changehat.h uservars.inc > > Index: b/changehat/mod_apparmor/Makefile > =================================================================== > --- a/changehat/mod_apparmor/Makefile > +++ b/changehat/mod_apparmor/Makefile > @@ -48,9 +48,12 @@ ifdef USE_SYSTEM > echo -lapparmor ; \ > fi ) > ifeq ($(strip $(LIBAPPARMOR)),) > - ERROR_MESSAGE = Unable to find libapparmor installed on this system; > either \ > - install libapparmor devel packages, set the LIBAPPARMOR variable > \ > - manually, or build against in-tree libapparmor) > + ERROR_MESSAGE = $(error ${nl}\ > +************************************************************************${nl}\ > +Unable to find libapparmor installed on this system; either${nl}\ > +install libapparmor devel packages, set the LIBAPPARMOR variable${nl}\ > +manually, or build against in-tree libapparmor.${nl}\ > +************************************************************************${nl}) > endif # LIBAPPARMOR not set > LDLIBS += $(LIBAPPARMOR) > else > @@ -58,10 +61,13 @@ else > LIBAPPARMOR_INCLUDE = $(LIBAPPARMOR_SRC)/include > LIBAPPARMOR_PATH := $(LIBAPPARMOR_SRC)/src/.libs/ > ifeq ($(realpath $(LIBAPPARMOR_PATH)/libapparmor.a),) > - ERROR_MESSAGE = $(LIBAPPARMOR_PATH)/libapparmor.a is missing; either > build against \ > - the in-tree libapparmor by building it first and then trying > again \ > - (see the top-level README for help) or build against the > system \ > - libapparmor by adding USE_SYSTEM=1 to your make command.) > + ERROR_MESSAGE = $(error ${nl}\ > +************************************************************************${nl}\ > +$(LIBAPPARMOR_PATH)/libapparmor.a is missing; either build against${nl}\ > +the in-tree libapparmor by building it first and then trying again${nl}\ > +(see the top-level README for help) or build against the system${nl}\ > +libapparmor by adding USE_SYSTEM=1 to your make command.${nl}\ > +************************************************************************${nl}) > endif > # Need to pass -Wl twice here to get past both apxs2 and libtool, as > # libtool will add the path to the RPATH of the library if passed > -L/some/path > @@ -71,11 +77,7 @@ endif > > .PHONY: libapparmor_check > .SILENT: libapparmor_check > -libapparmor_check: > - @if [ -n "$(ERROR_MESSAGE)" ] ; then \ > - echo "$(ERROR_MESSAGE)" 1>&2 ; \ > - return 1 ; \ > - fi > +libapparmor_check: ; $(ERROR_MESSAGE) > > all: libapparmor_check $(TARGET) ${MANPAGES} ${HTMLMANPAGES} > > Index: b/changehat/pam_apparmor/Makefile > =================================================================== > --- a/changehat/pam_apparmor/Makefile > +++ b/changehat/pam_apparmor/Makefile > @@ -33,9 +33,12 @@ ifdef USE_SYSTEM > echo -lapparmor ; \ > fi ) > ifeq ($(strip $(LIBAPPARMOR)),) > - ERROR_MESSAGE = Unable to find libapparmor installed on this system; > either \ > - install libapparmor devel packages, set the LIBAPPARMOR variable > \ > - manually, or build against in-tree libapparmor) > + ERROR_MESSAGE = $(error ${nl}\ > +************************************************************************${nl}\ > +Unable to find libapparmor installed on this system; either${nl}\ > +install libapparmor devel packages, set the LIBAPPARMOR variable${nl}\ > +manually, or build against in-tree libapparmor.${nl}\ > +************************************************************************${nl}) > endif > LIBAPPARMOR_INCLUDE = > AA_LDLIBS = $(LIBAPPARMOR) > @@ -45,10 +48,13 @@ else > LIBAPPARMOR_INCLUDE_PATH = $(LIBAPPARMOR_SRC)/include > LIBAPPARMOR_PATH := $(LIBAPPARMOR_SRC)/src/.libs/ > ifeq ($(realpath $(LIBAPPARMOR_PATH)/libapparmor.a),) > - ERROR_MESSAGE = $(LIBAPPARMOR_PATH)/libapparmor.a is missing; either > build against \ > - the in-tree libapparmor by building it first and then trying > again \ > - (see the top-level README for help) or build against the > system \ > - libapparmor by adding USE_SYSTEM=1 to your make command.) > + ERROR_MESSAGE = $(error ${nl}\ > +************************************************************************${nl}\ > +$(LIBAPPARMOR_PATH)/libapparmor.a is missing; either build against${nl}\ > +the in-tree libapparmor by building it first and then trying again${nl}\ > +(see the top-level README for help) or build against the system${nl}\ > +libapparmor by adding USE_SYSTEM=1 to your make command.${nl}\ > +************************************************************************${nl}) > endif > LIBAPPARMOR_INCLUDE = -I$(LIBAPPARMOR_INCLUDE_PATH) > AA_LINK_FLAGS = -L$(LIBAPPARMOR_PATH) > @@ -61,11 +67,7 @@ OBJECTS=${NAME}.o get_options.o > > .PHONY: libapparmor_check > .SILENT: libapparmor_check > -libapparmor_check: > - @if [ -n "$(ERROR_MESSAGE)" ] ; then \ > - echo "$(ERROR_MESSAGE)" 1>&2 ; \ > - return 1 ; \ > - fi > +libapparmor_check: ; $(ERROR_MESSAGE) > > all: libapparmor_check $(NAME).so > > > > -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
