Re: [XEN PATCH] xen: allow XSM_FLASK_POLICY only if checkpolicy binary is available

From: Juergen Gross <jgross@suse.com>
To: George Dunlap <George.Dunlap@citrix.com>,
	Anthony Perard <anthony.perard@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Jan Beulich <jbeulich@suse.com>,
	Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>
Subject: Re: [XEN PATCH] xen: allow XSM_FLASK_POLICY only if checkpolicy binary is available
Date: Fri, 16 Jul 2021 17:50:06 +0200	[thread overview]
Message-ID: <e072846b-5aa3-2403-5e0f-723577fe25e5@suse.com> (raw)
In-Reply-To: <40A28D22-91BC-438D-99C9-46DF6DA90A5E@citrix.com>

[-- Attachment #1.1.1: Type: text/plain, Size: 1680 bytes --]

On 16.07.21 17:26, George Dunlap wrote:
> 
> 
>> On Jul 14, 2021, at 5:17 PM, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>
>> This will help prevent the CI loop from having build failures when
>> `checkpolicy` isn't available, when doing "randconfig" jobs.
> 
> Hang on, just to clarify what’s going on here.
> 
> ‘randconfig’ is setting CONFIG_XSM_FLASK_POLICY in the .config file; and then when the build happens, we error out because one of the required components isn’t there.
> 
> What this patch does is to make it so that if someone explicitly sets CONFIG_XSM_FLASK_POLICY=y, but doesn’t have checkpolicy, the build system will silently disable the policy behind their backs without telling them?
> 
> Or does the randconfig test run kConfig one more time, at which point *then* the .config will be disabled?
> 
> The former I think is broken; the latter I think is fine.

I still think that Kconfig should not disable explicit settings due to
tools not being available.

I'd be fine with trimodal settings for such parameters:

- Off
- On
- On if tools available

And the last one could even be the default.

But anyone wanting to test a specific option or to enable an option
should not be catched by surprise because some internal modification is
requesting another tool which happens not to be available.

BTW, the same applies to ./configure options for tools. I really don't
like that some components are silently not built because e.g. a header
file is not available. I'd rather have the possibility to tell
./configure that I want everything built and let the build fail if that
is not possible.

Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]