Re: [Rpm-maint] [PATCH] RFC: Allow for disabling Berkeley DB support
On Fri, Jun 30, 2017 at 10:57:05AM +0100, Neal Gompa wrote: ... > 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. > I did not want to change the default behavior of ndb support, which is currently disabled by default. That accounts for the bulk of the inverted configure.ac logic in the patch(es). So this series assumes --enabled-bdb and --disable-ndb, unless otherwise specified. It will fail configure if bdb is enabled and the dependencies (internal or external) are not satisfied, or if no database backend is enabled. I think I should leave it to the ndb developers to determine when ndb is ready to be enabled by default. -- Darren Hart VMware Open Source Technology Center ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [PATCH] RFC: Allow for disabling Berkeley DB support
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],[bu
[Rpm-maint] [PATCH] RFC: Allow for disabling Berkeley DB support
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>
---
configure.ac | 22 --
lib/Makefile.am | 7 ++-
lib/backend/dbi.c | 10 +-
lib/backend/dbi.h | 2 ++
4 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/configure.ac b/configure.ac
index cc657ec..ca51350 100644
--- a/configure.ac
+++ b/configure.ac
@@ -483,7 +483,18 @@ AM_CONDITIONAL(LIBDW,[test "$WITH_LIBDW" = yes])
AM_CONDITIONAL(HAVE_LIBDW_STRTAB,[test "$HAVE_LIBDW_STRTAB" = yes])
#=
-# Process --with/without-external-db
+# Process --disbale-bdb
+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])
+AM_CONDITIONAL(BDB,[test "x$enable_bdb" != "xno"])
+
+#=
+# Process --with/without-external-db if bdb is not disabled
+AS_IF([test "x$enable_bdb" != "xno"],[
AC_ARG_WITH(external_db, [AS_HELP_STRING([--with-external-db],[build against
an external Berkeley db])],
[case "$with_external_db" in
yes|no) ;;
@@ -538,7 +549,7 @@ no|maybe )
fi
;;
esac
-
+])
AC_SUBST([WITH_DB_LIB])
#=
@@ -548,9 +559,8 @@ AC_ARG_ENABLE([ndb], [AS_HELP_STRING([--enable-ndb],[enable
the new rpm database
yes|no) ;;
*) AC_MSG_ERROR([invalid argument to --enable-ndb])
;;
-esac],
-[enable_ndb=no])
-AS_IF([test "$enable_ndb" = yes],[
+esac])
+AS_IF([test "$enable_ndb" = yes ],[
AC_DEFINE(ENABLE_NDB, 1, [Enable new rpm database format?])
])
AM_CONDITIONAL([NDB], [test "$enable_ndb" = yes])
@@ -1014,7 +1024,7 @@ if test "$with_external_db" = no; then
AC_CONFIG_SUBDIRS(db3)
fi
-AM_CONDITIONAL([WITH_INTERNAL_DB],[test "$with_external_db" = no])
+AM_CONDITIONAL([WITH_INTERNAL_DB],[test "$enable-bdb" = yes &&
"$with_external_db" = no])
AM_CONDITIONAL([DOXYGEN],[test "$DOXYGEN" != no])
AM_CONDITIONAL([HACKINGDOCS],[test "$with_hackingdocs" = yes])
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 3bb5af9..1eede2e 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -23,7 +23,7 @@ EXTRA_PROGRAMS =
usrlib_LTLIBRARIES = librpm.la
librpm_la_SOURCES = \
- backend/db3.c backend/dbi.c backend/dbi.h \
+ backend/dbi.c backend/dbi.h \
backend/dbiset.c backend/dbiset.h \
headerutil.c header.c headerfmt.c header_internal.h \
rpmdb.c rpmdb_internal.h \
@@ -58,11 +58,14 @@ librpm_la_LIBADD += @LUA_LIBS@
librpm_la_SOURCES += rpmliblua.c rpmliblua.h
endif
+if BDB
+librpm_la_SOURCES += backend/db3.c
if WITH_INTERNAL_DB
librpm_la_LIBADD += $(libdb_la)
else
librpm_la_LIBADD += @WITH_DB_LIB@
endif
+endif
if NDB
librpm_la_SOURCES += \
@@ -81,6 +84,7 @@ tagtbl.C: Makefile.am $(srcdir)/rpmtag.h gentagtbl.sh
mv -f $@.new $@
BUILT_SOURCES = tagtbl.C
+if BDB
if WITH_INTERNAL_DB
# XXX watchout, $(top_builddir)/db3/libdb.la created by this Makefile may
surprise
libdb_la = $(top_builddir)/db3/libdb.la
@@ -145,5 +149,6 @@ rpmdb_verify_LDADD = \
$(top_builddir)/db3/util_sig.o \
librpm.la
endif
+endif
CLEANFILES += $(BUILT_SOURCES)
diff --git a/lib/backend/dbi.c b/lib/backend/dbi.c
index beba49f..7c0c21c 100644
--- a/lib/backend/dbi.c
+++ b/lib/backend/dbi.c
@@ -35,6 +35,7 @@ static void
dbDetectBackend(rpmdb rdb)
{
#ifdef ENABLE_NDB
+#ifdef ENABLE_BDB
const char *dbhome = rpmdbHome(rdb);
char *db_backend = rpmExpand("%{?_db_backend}");
if (!strcmp(db_backend, "ndb")) {
@@ -52,8 +53,15 @@ dbDetectBackend(rpmdb rdb)
if (access(path, F_OK) == 0)
rdb->db_ops = _dbops;
free(path);
-#else
+#else /* No BDB backend support */
+rdb->db_ops = _dbops;
+#endif
+#else /* No NDB backend support */
+#ifdef ENABLE_BDB
rdb->db_ops = _dbops;
+#else /* ERROR: No backend enabled, this should not be a configurable state */
+#error No backend enabled
+#endif
#endif
}
diff --git a/lib/backend/dbi.h b/lib/backend/dbi.h
index a575d3c..21e3fbf 100644
--- a/lib/backend/dbi.h
+++ b/lib/backend/dbi.h
@@ -249,8 +249,10 @@ struct rpmdbOps_s {
const void * (*idxdbKey)(dbiIndex dbi, dbiCursor dbc, unsigned int
*keylen);
};
+#ifdef ENABLE_BDB
RPM_GNUC_INTERNAL
extern
