On Thu, Mar 11, 2021 at 02:00:55PM +0100, Andrea Bolognani wrote: > On Thu, 2021-03-11 at 12:33 +0000, Daniel P. Berrangé wrote: > > On Thu, Mar 11, 2021 at 07:29:03AM -0500, Neal Gompa wrote: > > > On Thu, Mar 11, 2021 at 6:35 AM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote: > > > > On Wed, Mar 10, 2021 at 01:50:43PM -0500, Neal Gompa wrote: > > > > > On Wed, Mar 10, 2021 at 7:43 AM Nikola Knazekova <nknazeko@xxxxxxxxxx> wrote: > > > > > > Compile the policy using a shell script executed by meson. > > > > > > > > > > This smells like a bad idea, because we're not relying on the > > > > > framework that SELinux policies are supposed to be built with. I don't > > > > > think we should do this. > > > > > > > > The important part is the use of tools for compiling the policy. The way > > > > you glue them into a build system is a app specific, and it makes no sense > > > > to use SELinux provided Makefiles, when our build system is meson. > > > > > > Let's say I buy that argument (I don't). Even with that argument, this > > > patch is wrong because it makes assumptions about how SELinux policies > > > are structured on-disk. For example, the install directory is wrong, > > > since it should be share/selinux/packages, not > > > share/selinux/packages/targeted. If I were to accept that I might be > > > wrong about the directory in the previous statement, that means that > > > we're still wrong, because we don't have builds for mls and minimal > > > policy targets. Finally, we're missing the policy interface file. > > > > Well if there are bugs like these, that's what this review is intended > > to catch, and they'll need to be addressed before this can merge. > > > > > This sounds like you need to work with Meson and selinux-policy > > > upstream to add support for natively building policy modules with > > > Meson itself. > > > > Sure it would be nice if there was a meson extension that dealt > > with SELinux, but we need to implement something that works with > > the meson releases that exist today. If meson gains selinux > > support in future may be we can consider it then. > > We still need make for syntax-check though, so rewriting the SELinux > bits to Meson doesn't allow us to drop the dependency. And, > considering how complex and widely used the syntax-check logic is, I > don't see that being reimplemented anytime soon. That's only part of the test suite, and we don't even include syntax-check if not running from git, because it is only targetted at upstream maintainers. So that's quite different from including use of make in the primary build process. That is just not ok IMHO. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|