[apparmor] [PATCH] parser: Add make variable to build against local or system libapparmor

[Steve Beattie]

On Fri, Dec 20, 2013 at 11:40:13PM -0800, Seth Arnold wrote:
> On Fri, Dec 20, 2013 at 11:06:26PM -0800, Steve Beattie wrote:
> > Ah, that's a very good point. I don't object to this approach for
> > the parser. Another solution would be to move the public facing
> > headers outside of the src/ tree (include/ ? include/sys/ ?) and
> > add that location to the search path when building against in-tree
> > libapparmor. That would also serve us should libapparmor grow
> > additional external headers.
> 
> I like the idea of clearly separating internal from external headers. Off
> the cuff it feels like it'd be a fair amount of churn up front but I think
> the long-term benefits of keeping the ABI documented seperately from the
> internal implementation details is worth the hassle.

Yeah, in the end, I think it's the more correct solution. I'll try
to put together a patch to do that (and the associated automakery to
get things right in the libapparmor tree).

> > I chose a different variable name "USE_SYSTEM" because I'd like to also
> > decide to use which parser in the tests based on that flag (the in-tree
> > version versus the system parser), with the ability to override with a
> > user specified parser as well.
> 
> This muddles my head. It feels like the tests ought to be compiled using
> whichever libraries and headers were used for the parser that's going
> to be tested...

In the general case, I think it's true that we want the library used
by the tests to be the same as the library used by the parser. IMO,
the common default use case is to build the whole tree and use the
regression tests to ensure that the result is consistent.

However, other use cases would be the "just the parser" style of
feature development (I'm guilty of doing this too, so it's not just
supporting jj), as well as using the regression tests to validate
that apparmor as installed in a system is functioning as expected
(in part to support binary distributions, where build environments
are not the same as eventual installed environments), and so we should
make sure that those use cases are easily supported.

The complicating factor is that the tests (on trunk) will want
to stay current with where trunk development goes. As additional
functionality gets added to libapparmor, if the tests are to exercise
that functionality, then building against older library versions
becomes more difficult (especially if we don't want horrid messes of
#ifdefs in the test code). So it becomes more likely that the tests
need to compile and link against the in-tree libapparmor, even when
the parser is using the system libapparmor.

(OTOH, in Ubuntu, we use the regression test as shipped in the tarball
apparmor release that our packages are based off of, not trunk's tip,
which mostly avoids the "tests requiring additional functionality
that the system libapparmor does not provide" problem.)

> so when run from top-level Makefile, SYSTEM_LIBAPPARMOR
> would need to be passed through as USE_SYSTEM.

Sadly, the toplevel Makefile is currently focused on distribution, not
building the tree. I expect that to be addressed when we commit to using
something like autotools fully, though I fear that it will complicate
supporting the use cases outlined above.

> Of course we might want to expose different libraries for the tests than
> the parser, just to test that condition.
> 
> We may need more variables, perhaps:
> 
> For the tools, when building them, "system" or "source":
> TOOLS_USE_LIBAPPARMOR
> TOOLS_USE_PARSER
> 
> For the tests, when building them, "system" or "source":
> TESTS_USE_LIBAPPRMOR
> TESTS_USE_PARSER
> 
> Is this insanity? Or clean separation of concerns? They sound the same.

Uhhh, yes? I get where you're going, but want clear use situations
for adding the complexity.

-- 
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/20131223/c3f4a396/attachment.pgp>