[Please CC me, I'm not subscribed to the mailinglist] Hello, regarding the initial patch in this thread: The patch looks good and should go upstream IMHO. (Maybe except creating the dummy local/* files for AppArmor 3.x - see below for details.) A note about what you mentioned in the patch comment: If someone uses aa-logprof to update a profile, it will modify the profile, _not_ the local/ file. (Changing that is on the TODO list, but so far nobody did it.) Therefore I'm not sure if switching from %config(noreplace) to %config is a good idea. Am Montag, 26. Juni 2023, 18:29:11 CEST schrieb Andrea Bolognani: > On Mon, Jun 26, 2023 at 09:42:32AM -0600, Jim Fehlig wrote: [...] > > Specifying which copy to use via a build time option is also an > > option :-). Does your idea include preserving commit 9b743ee19053 > > and adjusting the 'include if exists' to 'include'? > > The modern (3.x) version would install the profile as it is > currently, while the legacy (2.x) version would have the "include if > exists" replaced with "include". Sounds like a good idea. Note that you'll have to install dummy files for "include", while they don't have to exist when using "include if exists". > We could also decide to leave the include out altogether for the > legacy version, and simply keep it as the less-featureful version it > has been until now. In fact, I wonder if we should install > placeholder files for the profiles at all? For AppArmor 3.x, I'd tend to "no". And I say that despite knowing that upstream still creates all the local/ dummy files. (Which reminds me that this should maybe stop doig this in the upcoming 4.0 release. If you want to follow the discussion: https://gitlab.com/apparmor/apparmor/-/issues/337 ) These local sniplets are mostly "noise" (empty/comment-only) files, and it's harder than needed to find files that were actually modified. (Besides that, I'm currently testing a set of ~1000 AppArmor profiles, and having 1000 empty local/ files would make things much harder.) > For abstractions on 3.x, the abstractions/foo.d approach means that > such placeholders do not exist. On the other hand, you could argue > that the empty abstractions/foo.d directory and the empty local/foo > file both serve the same purpose of providing a hint for the user > that they can customize things using that mechanism... If all abstractions would create empty *.d directories by default, that would be quite some noise and useless directories. So please don't do that ;-) > FWIW, Debian seems to consistently create placeholders for profiles. > I think this is done automatically by the dh-apparmor utility that's > used as standard for packaging. But that feels like a decision better > left to each downstream... Maybe we should look into what upstream > AppArmor does for its own profiles and abstractions. See above - IMHO the current upstream behaviour is not perfect, and will hopefully change to not creating the local/ files by default in 4.0. Regards, Christian Boltz -- Social Media News: Instagram is down Science News: Scientists baffled as average human IQ increases over 25% in less than an hour [https://twitter.com/PaulRigbywrites/status/1146505629491781632]
Attachment:
signature.asc
Description: This is a digitally signed message part.
