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

[John Johansen]

On 10/15/2013 08:20 AM, Steve Beattie wrote:
> 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.

likely

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

libapparmor is linking against it now

> 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

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