[apparmor] [RFC] [patch] Makefile automatically fallback to USE_SYSTEM=1

Tue Mar 11 19:22:19 UTC 2014

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 <steve at nxnw.org>
Acked-by: John Johansen <john.johansen at canonical.com>

> ---
>  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
>  
> 
> 
> 