[apparmor] Cache update broken

[John Johansen]

On 08/07/2012 01:34 PM, Christian Boltz wrote:
> Hello,
> 
> 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 :)

Basically I was hoping for more input than just our little discussion,
and the change is really easy to make so a 2nd iteration won't take
very long.

> 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, and I think the proper solution for that is having separate caches
for each kernel.

> IMHO most people want their cache updated automatically, and it doesn't 
> make much sense to force everybody to add --clear-cache-if-needed to the 
> initscript or the config file.
> 
> Can we please make it the default _without_ the need for an additional 
> parameter or config option?
> 
> 
Well I suspect that is what we will do

> If you really want, feel free to introduce a 
>     --never-clear-cache-automatically
> parrameter / config file option - but I doubt many people will use it 
> ;-)
> 
>> -- 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. I can't remember whether
there was even time stamp checks in the first iteration that shipped
(you could do that by dumping the profile out using -S and using a
separate utility to do the actual load).

But I do know that timestamp changes within include files where not taken
into account and cache clearing was being done by a script.

The goal has always been to merge back changes where appropriate or
replace the code as thing in the upstream project progressed. So keeping
the old script behavior isn't necessarily a goal. Using the --write-cache
with cache-clearing will have to be brought up and discussed.

I specifically mentioned that suse was going to use this for its cache
in my reply to Seth because I misunderstood what he was saying about a
debug option. Of course he is right, a clear cache option that isn't
default does feel like an admin debug option.


>> A direct --clear-cache would just be a debugging tool for admins, and
>> rarely used (hopefully) at that.
> 
> Indeed. It might be a nice feature, but I'd give it a low priority [2].
> The avarage admin most probably knows how to delete all files in a 
> directory ;-)
> 
yeah well thankfully that debug option is really easy to do now.

> 
> Regards,
> 
> Christian Boltz
> 
> [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?

:)

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