[Rpm-maint] [PATCH] RFC: Allow for disabling Berkeley DB support

[Neal Gompa]

On Fri, Jun 30, 2017 at 12:38 AM, Darren Hart <dvhart at infradead.org> wrote:
> On Thu, Jun 29, 2017 at 08:33:46AM +0100, Neal Gompa wrote:
>> On Thu, Jun 22, 2017 at 10:20 PM, Darren Hart (VMware)
>> <dvhart at infradead.org> wrote:
>> > Introduce a --disable-bdb configuration option which disables the use of
>> > Berkeley DB entirely. Update the various autotools to ensure that at
>> > least one of BDB or NDB is enabled. Existing configuration options
>> > continue as before. Minor updates to dbi.h and dbi.c to handle bdb being
>> > optional. Add a little extra paranoia to dbi.c which will error out of
>> > the build if neither BDB nor NDB are enabled (which should not be
>> > possible to configure).
>> >
>> > Signed-off-by: Darren Hart (VMware) <dvhart at infradead.org>
>>
>> So, I tested this patch against rpm git master on MacOS, and while the
>> --enable/--disable for ndb and bdb works when one of them are enabled,
>> the case when both are disabled is surprising. Since your patch
>> doesn't validate the acceptable conditions at the configure.ac level,
>> it accepts it and attempts to build ndb... which fails because missing
>> things on macOS, but that's a different issue.
>>
>> The patch is basically mostly okay, but I would really like to see the
>> configure.ac have logic that prevents creating the makefiles and
>> whatnot if both available rpmdb backends are disabled.
>
> Thanks for testing the patch.
>
> The intent behind the patch is to allow for the following configurations:
>
> bdb
> ndb
> bdb+ndb
>
> If you disable bdb, it forces the enabling of ndb - similar to how if you do
> nothing, bdb is automatically selected.
>
> I did not consider the following case:
>
> --disable-bdb --disable-ndb
>
> Which I believe the is the case you are describing. The behavior there would be
> as you describe, since --disable-bdb forces --enable-bdb.
>
> What would you consider to be the correct behavior in this case? Error out of
> configure with a message like "Error: At least one of bdb or ndb must be
> configured" ?
>
> Consider the following, where y=--enable, n=--disable, and - = no argument
> specified. This represents the intention of the patch:
>
> CURRENT PATCH BEHAVIOR
> bdb     ndb     result
> -----------------------------------
> y       y       build bdb and ndb
> y       n       build bdb
> y       -       build bdb
> n       y       build ndb
> n       n       build ndb
> n       -       build ndb
> -       y       build ndb
> -       n       build bdb
> -       -       build bdb
>
> How would you suggest I update the behavior? Something like this?:
>
> PROPOSED PATCH BEHAVIOR
> bdb     ndb     result
> -----------------------------------
> y       y       build bdb and ndb
> y       n       build bdb
> y       -       build bdb
> n       y       build ndb
> n       n       configure: error: at least one of bdb or ndb must be enabled
> n       -       configure: error: at least one of bdb or ndb must be enabled
> -       y       build bdb and ndb
> -       n       build bdb
> -       -       build bdb
>
> The above will do no automatic configuration of ndb based on the state of bdb.
>
> The following patch, applied on top of this RFC patch, implements this change,
> and configure behaves as follows under the following conditions:
>
> ./configure --disable-bdb
> ...
> checking database backends enabled... none
> configure: error: at least one of bdb or ndb must be enabled
> ...
>
> ./configure --disable-bdb --disable-ndb
> ...
> checking database backends enabled... none
> configure: error: at least one of bdb or ndb must be enabled
> ...
>
> ./configure
> ...
> checking database backends enabled... bdb
> ...
>
> ./configure --enable-ndb
> ...
> checking database backends enabled... bdb ndb
> ...
>
> ./configure --disable-bdb --enable-ndb
> ...
> checking database backends enabled... ndb
> ...
>
> Is this preferable? If so, I'll roll them together and resubmit as a v2.o
>
>
> Since this configuration appears to never have been tested, ss there an
> automated battery of tests we can run a "--disable-bdb --enable-ndb" build
> against?
>
>
> From 74ff641eee0c372396e8610dcda26c4a4e0a9896 Mon Sep 17 00:00:00 2001
> From: "Darren Hart (VMware)" <dvhart at infradead.org>
> Date: Thu, 29 Jun 2017 16:32:38 -0700
> Subject: [PATCH] RFC: Enforce at least one of bdb or ndb in configure.ac
>
> Rather than automatically configuring ndb if bdb is disabled, exit with
> a configure error if at least one of bdb or ndb is not enabled.
>
> Add an AC_MSG_CHECKING block after bdb and ndb have been processed, and
> report to the user which database backends have been enabled. Abort with
> AC_MSG_ERROR if neither are enabled.
>
> Signed-off-by: Darren Hart (VMware) <dvhart at infradead.org>

My suggestion is to assume enabled (unless dependencies can't be
satisfied) if not specified to be disabled. If specified to be enabled
and dependencies not satisfied, throw an error.


-- 
真実はいつも一つ！/ Always, there's only one truth!