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) > <dvh...@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) <dvh...@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)" <dvh...@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) <dvh...@infradead.org> --- configure.ac | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index ca51350..4b4d807 100644 --- a/configure.ac +++ b/configure.ac @@ -487,9 +487,7 @@ AM_CONDITIONAL(HAVE_LIBDW_STRTAB,[test "$HAVE_LIBDW_STRTAB" = yes]) AC_ARG_ENABLE([bdb], [AS_HELP_STRING([--disable-bdb],[build without bdb rpm database format support])]) AS_IF([test "x$enable_bdb" != "xno"], - [AC_DEFINE(ENABLE_BDB, 1, [Build with bdb rpm database format support?])], - # If BDB is disabled, force enable NDB - [enable_ndb=yes]) + [AC_DEFINE(ENABLE_BDB, 1, [Build with bdb rpm database format support?])]) AM_CONDITIONAL(BDB,[test "x$enable_bdb" != "xno"]) #================= @@ -569,6 +567,20 @@ AM_GNU_GETTEXT_VERSION([0.16.1]) AM_GNU_GETTEXT([external]) AM_ICONV +#================= +# Ensure at least one of bdb or ndb is configured +AC_MSG_CHECKING([database backends enabled]) +if test "x$enable_bdb" != "xno" && test "x$enable_ndb" != "xyes"; then + AC_MSG_RESULT([bdb]) +elif test "x$enable_bdb" != "xno" && test "x$enable_ndb" = "xyes"; then + AC_MSG_RESULT([bdb ndb]) +elif test "x$enable_bdb" = "xno" && test "x$enable_ndb" = "xyes"; then + AC_MSG_RESULT([ndb]) +else + AC_MSG_RESULT([none]) + AC_MSG_ERROR([at least one of bdb or ndb must be enabled]) +fi + dnl Checks for header files we can live without. AC_HEADER_STDC AC_HEADER_MAJOR -- Darren Hart VMware Open Source Technology Center _______________________________________________ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
