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

[Steve Beattie]

On Mon, Oct 14, 2013 at 06:35:39PM -0700, John Johansen wrote:
> On 10/14/2013 05:46 PM, John Johansen wrote:
> > On 10/10/2013 01:46 PM, Steve Beattie wrote:
> >> With trunk commit 2205 "use libapparmor's find mountpoint fn",
> >> the parser now builds against and uses libapparmor at runtime. However,
> >> it currently builds against the system installed libapparmor library and
> >> header files, which fails if either aren't installed, and is thus
> >> painful for bootstrapping in a new environment.
> >>
> >> Instead, the parser, like pam_apparmor and mod_apparmor, should build
> >> against the in-tree libapparmor header and library. This patch does
> >> that and adjusts the tests to point LD_LIBRARY_PATH at the location
> >> of the built library as well.
> >>
> >> Signed-off-by: Steve Beattie <steve at nxnw.org>
> > 
> > So with my previous objection dropped as a manage my own patch queue
> > better issue.
> > 
> > Acked-by: John Johansen <john.johansen at canonical.com>
> > 
> > But my other question still stands, do we not want libapparmor and
> > libstdc++ being statically linked in. And yes I realize that I uh
> > messed both those up and should be on the hook for the patch.

We have had libstdc++ statically linked with the parser from the
very early days of the libapparmor_re being a c++ library (though,
-static-libstdc++ looks to be the correct solution to the wonky
workaround that we have had to link libstdc++ statically).

> And that patch is below, sadly it ballons the parser a bit, from a
> size of
>   3696580 bytes  (803792 bytes stripped)
> to
>   4666902 bytes (1672648 bytes stripped)
> 
> ---
> 
> --- Makefile.bak	2013-10-14 17:48:36.128747740 -0700
> +++ Makefile	2013-10-14 18:28:10.524651367 -0700
> @@ -86,8 +86,8 @@
>  AAREDIR= libapparmor_re
>  AAREOBJECT = ${AAREDIR}/libapparmor_re.a
>  AAREOBJECTS = $(AAREOBJECT) libstdc++.a
> -AARE_LDFLAGS=-static-libgcc -L. -L$(LIBAPPARMOR_PATH)/.libs/
> -AALIB = -lapparmor
> +AARE_LDFLAGS=-static-libgcc -static-libstdc++ -static -L. -L$(LIBAPPARMOR_PATH)/.libs/
> +AALIB = -lapparmor -lpthread
>  
>  LEX_C_FILES	= parser_lex.c
>  YACC_C_FILES	= parser_yacc.c parser_yacc.h

NACK. This turns the parser completely static, which is why the
resulting binary nearly doubled in size. We still want to dynamically
link against libc, I believe.

(Also, why does the patch add linking against libpthread?)

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.

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.

I'll draw up a patch.

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20131015/eb3643a9/attachment.pgp>