Re: [PATCH] Support Berkeley DB 4.7

Arfrever Frehtes Taifersar Arahesis wrote:
> [[[
> Support Berkeley DB 4.7.
>
> * subversion/libsvn_fs_base/fs.c
> (check_env_flags, get_db_pagesize, base_hotcopy):
> Use '#if DB_VERSION_MINOR >= 2' instead of '#ifdef DB_LOG_AUTOREMOVE'
> to check for Berkeley DB >= 4.2.
> (check_env_flags):
> Call DB_ENV->log_get_config() instead of DB_ENV->get_flags() for
> Berkeley DB >= 4.7.
> (base_hotcopy):
> Call check_env_flags() with DB_LOG_AUTO_REMOVE instead of DB_LOG_AUTOREMOVE
> for Berkeley DB >= 4.7.
>
> * build/generator/gen_win.py
> (GeneratorBase._find_bdb): Allow Berkeley DB 4.7.
> ]]]
>
>
> Index: subversion/libsvn_fs_base/fs.c
> ===================================================================
> --- subversion/libsvn_fs_base/fs.c (revision 31974)
> +++ subversion/libsvn_fs_base/fs.c (working copy)
> @@ -917,11 +917,9 @@ svn_fs_base__clean_logs(const char *live_path,
> }
>
>
> -/* ### There -must- be a more elegant way to do a compile-time check
> - for BDB 4.2 or later. We're doing this because apparently
> - env->get_flags() and DB->get_pagesize() don't exist in earlier
> - versions of BDB. */
> -#ifdef DB_LOG_AUTOREMOVE
> +/* env->get_flags() and DB->get_pagesize() don't exist prior to
> + Berkeley DB 4.2. */
> +#if DB_VERSION_MINOR >= 2

What happens when Berkeley DB 5.0 comes along? Rather than check
DB_VERSION_MINOR explicitly, consider instead using:

    #if SVN_BDB_VERSION_AT_LEAST(major,minor)

By the way, were you aware of libsvn_fs_base/bdb/bdb_compat.[ch]? Seems we
could avoid some of the preprocessor noise fs.c with stuff like this:

/* In BDB 4.7, the DB_LOG_AUTOREMOVE flag was renamed to
    DB_LOG_AUTO_REMOVE. */
#if SVN_BDB_VERSION_AT_LEAST(4,7)
#define SVN_BDB_LOG_AUTO_REMOVE DB_LOG_AUTO_REMOVE
#elif SVN_BDB_VERSION_AT_LEAST(4,2)
#define SVN_BDB_LOG_AUTO_REMOVE DB_LOG_AUTOREMOVE
#endif

>
> /* Open the BDB environment at PATH and compare its configuration
> flags with FLAGS. If every flag in FLAGS is set in the
> @@ -933,16 +931,28 @@ check_env_flags(svn_boolean_t *match,
> apr_pool_t *pool)
> {
> bdb_env_baton_t *bdb;
> +#if DB_VERSION_MINOR >= 7
> + int flag_state;
> +#else
> u_int32_t envflags;
> +#endif
>
> SVN_ERR(svn_fs_bdb__open(&bdb, path,
> SVN_BDB_STANDARD_ENV_FLAGS,
> 0666, pool));
> +#if DB_VERSION_MINOR >= 7
> + SVN_BDB_ERR(bdb, bdb->env->log_get_config(bdb->env, flags, &flag_state));
> +#else
> + SVN_BDB_ERR(bdb, bdb->env->get_flags(bdb->env, &envflags));
> +#endif
>
> - SVN_BDB_ERR(bdb, bdb->env->get_flags(bdb->env, &envflags));
> SVN_ERR(svn_fs_bdb__close(bdb));
>
> +#if DB_VERSION_MINOR >= 7
> + if (flag_state == 0)
> +#else
> if (flags & envflags)
> +#endif
> *match = TRUE;
> else
> *match = FALSE;
> @@ -977,7 +987,7 @@ get_db_pagesize(u_int32_t *pagesize,
>
> return svn_fs_bdb__close(bdb);
> }
> -#endif /* DB_LOG_AUTOREMOVE */
> +#endif /* DB_VERSION_MINOR >= 2 */

Should we add a helper function that calls the right DB_ENV function
depending on the BDB version to bdb_compat.c?

Also, did the DB_CONFIG file flag DB_LOG_AUTOREMOVE get renamed, too? If
so, how do we compatibly deal with that, given that it is a configuration
option that's under our users' control?

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand