Re: [PATCH] Support Berkeley DB 4.7

2008-07-02 20:33:50 C. Michael Pilato napisał(a):
> 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)

OK.

> 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

DB_LOG_AUTO_REMOVE / DB_LOG_AUTOREMOVE is used only in one place (in a call
to check_env_flags()), so defining SVN_BDB_LOG_AUTO_REMOVE is rather unnecessary.

> >
> > /* 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?

bdb->env->log_get_config(bdb->env, flags, &flag_state) (or its older equivalent) is called
only in one place in libsvn_fs_base, so a helper function is unneeded.

>
> Also, did the DB_CONFIG file flag DB_LOG_AUTOREMOVE get renamed, too?

It wasn't renamed.

I'm attaching the updated patch.

[[[
Support Berkeley DB 4.7.

* subversion/libsvn_fs_base/fs.c
  (check_env_flags, get_db_pagesize, base_hotcopy):
   Use '#if SVN_BDB_VERSION_AT_LEAST(4, 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.
]]]

-- 
Arfrever Frehtes Taifersar Arahesis