[apparmor] Cache update broken

[Christian Boltz]

Hello,

(I've seen the updated patch and already submitted it to a test package 
in the OBS in home:cboltz - but I'll comment on this mail nevertheless. 
Most of the comments below should have a "SCNR" marker - don't take them 
too serious ;-)

Am Dienstag, 7. August 2012 schrieb John Johansen:
> On 08/07/2012 01:34 PM, Christian Boltz wrote:
> > John, thanks for honoring the golden rules of bad programming in
> > your
> > patch! I'm especially talking about rule 18 - "take great care in
> > setting bad defaults" ;-)
> 
> Hehe I did it on purpose to get a discussion of what it should be on
> list :)

Looks like it worked ;-)

> > Am Dienstag, 7. August 2012 schrieb Seth Arnold:
> >> I expect --clear-cache-if-needed to be the default set in the
> >> config
> >> file
> > 
> > Let me ask a simple question: Can you give me a good reason _not_ to
> > automatically clear the cache if .features differs, and to keep an
> > outdated cache? [1]
> 
> well I can think of a corner case where you are booting between
> different kernels, and want to keep one. But I don't really think
> that is a good reason, 

That's indeed a corner case - but OTOH people doing this will "waste" 
lots of time with booting - updating the cache every time they choose 
another kernel is only a small part of this and won't hurt much.

> and I think the proper solution for that is
> having separate caches for each kernel.

... and run a cleanup script whenever an old kernel is removed? 
That's of course possible, but I'd say it's not worth the effort. 
(Hey, it's just a cache, and worst thing that can happen is that we 
waste some seconds.)

> >> -- redundant for ubuntu but also a chance to bring both
> >> initscripts together again -- at least for this feature.
> > 
> > I don't know the history and background why there are separate
> > initscripts (pointers welcome), but I'm a big fan of avoiding
> > duplicate work (especially if I have to maintain the duplicate ;-))
> 
> Well the history is fairly simple, Ubuntu added caching support when
> it was in a very early iteration in the parser. [...]

Sounds like someone didn't like if blocks ;-)
Hmmm... Wasn't there something? Wait...

    Rule 14: Don't use small if blocks. Use cp instead.

*SCNR*

> > [1] oh, now I remember:
> >         rule 22 - "invent new ways to make your program slow"
> >     
> >     ;-)
> 
> Having broken cache management is a new way to make your program slow?
> :)

Yes - it makes booting slow because it keeps the useless cache. I'm not 
sure if someone used this method before to slow down booting ;-) so it 
might really be really new.
(I'm talking about the "load the AppArmor profiles" part of booting 
here, of course.)

> > [2] aa-enable is more important IMHO because it needs to
> >     a) delete a symlink
> >     b) load the profile
> 
> sure that make sense, but I'll way it against I am in the patch right
> now and can add it with just a few lines of code while its fresh in
> my mind.

Of course. My footnote was more about "at which position of your 
wishlist would you place these features?"  That's all ;-)
If we get a lower-priority feature nearly "for free", I won't object ;-)


Regards,

Christian Boltz
-- 
> [1] Schmerzen wg. einer Zerrung
> --
> Nicht alles, was hinkt, ist ein Vergleich.
In manchen Fällen ist es auch ein David Haller... *SCNR*
[> David Haller und Mario van der Linde in suse-linux]