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

[Christian Boltz]

Hello,

Am Dienstag, 11. März 2014 schrieb John Johansen:
> On 03/09/2014 09:36 AM, Christian Boltz wrote:
> > Am Freitag, 7. März 2014 schrieb John Johansen:
> >> 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 understand why you like this behaviour, but I don't like it too
> > much because it introduces some "surprising" behaviour. (And I
> > doubt someone reads the warning if the test succeeds ;-)
> 
> I'm not sure why its a surprising behavior, but okay. 

Maybe I should reprase "surprising" to "behaviour depends on other parts 
of the source tree _and_ on system-wide installed binaries". 

> Question is who
> do you see the consumer as?

Basically everybody who builds AppArmor - which can mean a developer or 
packager (who does or does not know about USE_SYSTEM), and also any 
build service (which for sure does not know about USE_SYSTEM).

I'm OK if the result varies between "works" and "fails with an error 
message" - that's a very obvious difference, and everybody (including 
OBS etc.) will notice it. However I don't like the idea of "somehow 
works, but in a totally different way" because that might give 
interesting[tm] results.

> > Thinking about it - can you implement an additional condition like
> > "file common/enable-auto-fallback-to-USE_SYSTEM exists" (or an
> > environment variable you set in .bashrc, but a file sounds better)?
> > 
> > That would mean people have to actively opt-in to the automatical
> > fallback by touch'ing the file once in their bzr checkout. This
> > would
> > fix the "surprise" part.
> > 
> > With the opt-in file added, the patch looks good to me.
> 
> using a file is possible, but how about opt-out instead? It would be
> easy to add USE_SYSTEM=0, to opt-out. So if USE_SYSTEM is defined
> (either 0 or 1) you get one or the other behavior other wise it tries
> to build against one then the other

That would mean everybody who builds AppArmor packages needs to add 
USE_SYSTEM=0 in the spec/whatever to be on the safe side, and we also 
need to explain it in README.

Using an opt-in file is better - it gives packagers etc. a guaranteed 
consistent behaviour, and allows you to enable the automatic fallback by 
creating a file [1]. (If I had to guess, I'd say only AppArmor 
developers are interested in the fallback - everybody else most probably 
prefers the consistent behaviour without automatic surprises.)

BTW: There's only an "ifdef USE_SYSTEM" check, but no check for the 
actual value. Did you test if USE_SYSTEM=0 does what you expect? ;-) 
(I seriously doubt after giving it a short test...)


Regards,

Christian Boltz

[1] You could even add a "make random" *eg* target to the Makefile to 
    create the opt-in file, and I'd happily ACK that ;-)

-- 
> [Ergebnis von validator.w3.org] 409 Fehler in 282 Zeilen.
> Erinnert mich ein bisserl an meine Deutsch-Diktate in der Schule ;-)
Ehrlich??? Ortographisches Genie dieser Größenordnung hätte ich eher
Anderen hier auf der Liste zugetraut ;-)
[ > Manfred Tremmel und Thorsten Körner in suse-linux]