[apparmor] [patch 01/13] parser - build against in-tree libapparmor

[John Johansen]

On 10/15/2013 11:50 AM, Steve Beattie wrote:
> On Tue, Oct 15, 2013 at 09:12:26AM -0700, John Johansen wrote:
>>> There are two ways that I can see to link libapparmor in statically,
>>> while not linking the entire binary statically:
>>>
>>>   - link directly against ../library/libapparmor/src/.libs/libapparmor.a
>>>   - play some loader games with gnu ld to switch between static and
>>>     dynamic linking when specific dealing with libapparmor like so:
>>>     '-Wl,-Bstatic -lapparmor -Wl,-Bdynamic -lpthread'
>>>
>>> The latter has the advantage of falling back to the system libapparmor.a
>>> if the in-tree versus isn't or won't build for whatever reason, making
>>> it less likely to break your development style.
>>>
>> not just my development style. It also means someone can build an updated
>> parser against their system without rolling new libraries. This is something
>> that happens from time to time
> 
> Okay, I guess. Still, doing so seems fraught with the possibilities of
> ending up with a parser that makes assumptions about the capabilities of
> libapparmor that don't match with those of the system libapparmor.
> 

that shouldn't be the case if it uses the system's apparmor.h, I know we
aren't there yet but ..


>>> That said, I think I prefer the former approach as with the latter,
>>> it's also possible that confusion could occur during development where,
>>> in developing some combined change across the parser and libapparmor,
>>> the system library is used mistakenly, causing either tests to pass or
>>> fail mistakenly, and leading either to developer confusion in trying
>>> to figure out why the feature is not working *or* for the developer
>>> to believe it *is* working when it does not when used with the in-tree
>>> (and to be released in the future) version of libapparmor.
>>>
>> true, but it also ties the parser to the library building, which I don't
>> particularly like either. And I think we want the ability to test against
>> the system library not just the one that is built in tree.
> 
> I can kind of see that, though you're going to at a minimum need
> to relink the parser (since we're statically linking it), which
> invalidates the purity of your testing a little bit.
> 
hrmmm, not really. Its just only valid against that release library. The
issue is really one of what/how tightly components in the tree are
coupled.

I tend to favor a looser coupling (well as much as is reasonable, /me is
guilty of trying to stuff common code into libapparmor), and want/like/use
that where I selectively replace different components on different machines.

I guess my big issue with requiring the library build is the mess around
autotools, and swig, which another set of deps and setup to walk people
through just so they can test a new parser. Realistically since we are
staticly linking the library to the parser I should just live with the
inconvenience of having to build the intree version of the lib. A pita but
something I can live with


> Anyway. I don't have a terribly strong opinion about it, so here's two
> patches. The first patch switches over from using our hackish way to get
> a statically linked libstdc++ (which was based on the article at
> http://www.trilithium.com/johan/2005/06/static-libstdc/) with the
> -static-libstdc++ compiler option:
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>
> ---
>  parser/Makefile |    9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> Index: b/parser/Makefile
> ===================================================================
> --- a/parser/Makefile
> +++ b/parser/Makefile
> @@ -85,8 +85,8 @@ OBJECTS = $(SRCS:.c=.o)
>  
>  AAREDIR= libapparmor_re
>  AAREOBJECT = ${AAREDIR}/libapparmor_re.a
> -AAREOBJECTS = $(AAREOBJECT) libstdc++.a
> -AARE_LDFLAGS=-static-libgcc -L. -L$(LIBAPPARMOR_PATH)/.libs/
> +AAREOBJECTS = $(AAREOBJECT)
> +AARE_LDFLAGS=-static-libgcc -static-libstdc++ -L. -L$(LIBAPPARMOR_PATH)/.libs/
>  AALIB = -lapparmor
>  
>  LEX_C_FILES	= parser_lex.c
> @@ -149,11 +149,6 @@ indep: docs
>  all:	arch indep
>  
>  
> -.PHONY: libstdc++.a
> -libstdc++.a:
> -	rm -f ./libstdc++.a
> -	ln -s `$(CXX) -print-file-name=libstdc++.a`
> -
>  apparmor_parser: $(OBJECTS) $(AAREOBJECTS)
>  	$(CXX) $(LDFLAGS) $(EXTRA_CFLAGS) -o $@ $(OBJECTS) $(LIBS) \
>  	      ${LEXLIB}  $(AAREOBJECTS) $(AARE_LDFLAGS) $(AALIB)
> 
> The second patch then converts to statically linking libapparmor with
> whichever static libapparmor it can find on its library search path (and
> verified to choose the in-tree version over the system one if both are
> available):
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>
Acked-by: John Johansen <john.johansen at canonical.com>

> ---
>  parser/Makefile |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: b/parser/Makefile
> ===================================================================
> --- a/parser/Makefile
> +++ b/parser/Makefile
> @@ -87,7 +87,7 @@ AAREDIR= libapparmor_re
>  AAREOBJECT = ${AAREDIR}/libapparmor_re.a
>  AAREOBJECTS = $(AAREOBJECT)
>  AARE_LDFLAGS=-static-libgcc -static-libstdc++ -L. -L$(LIBAPPARMOR_PATH)/.libs/
> -AALIB = -lapparmor
> +AALIB = -Wl,-Bstatic -lapparmor -Wl,-Bdynamic -lpthread
>  
>  LEX_C_FILES	= parser_lex.c
>  YACC_C_FILES	= parser_yacc.c parser_yacc.h
> 
> Both patches are on top of the original patch to modify the search
> path to use the in-tree libapparmor if available, though that patch
> should be simplified to eliminate all the LD_LIBRARY_PATH shenanigans
> for the tests (yay!).
> 
> 
> 
Acked-by: John Johansen <john.johansen at canonical.com>