You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jonathan Nieder <jr...@gmail.com> on 2011/10/29 13:45:50 UTC
[PATCH] Make SQLite compatibility check less picky
Hi,
This patch is taken from Debian's subversion packaging. It avoids
having to rebuild Subversion each time an older or newer patchlevel of
SQLite gets installed (i.e., the x in 3.7.x changes). I'd appreciate
any thoughts you have.
Thanks,
Jonathan
[[[
The actual ABI compatibility of sqlite3 doesn't depend on the patchlevel
(the x in 3.7.x), so stop being picky about the patchlevel when checking
the version number at runtime. This avoids spurious errors of the form
"svn: Couldn't perform atomic initialization" / "svn: SQLite compiled
for 3.7.4, but running with 3.7.3" when sqlite gets a minor update
without Subversion being rebuilt to match.
* subversion/libsvn_subr/sqlite.c
(init_sqlite): Omit the patchlevel in version number sanity check.
(svn_sqlite__open): Include compatibility code for running against
libsqlite 3.7.7 even when built against a later version.
Patch by: peters
(Tweaked by me to be cautious about SQLITE_VERSION_AT_LEAST checks.)
Found by: Joao Palhoto Matos <jo...@gmail.com>
http://bugs.debian.org/608925
]]]
Index: subversion/libsvn_subr/sqlite.c
===================================================================
--- subversion/libsvn_subr/sqlite.c (revision 1194866)
+++ subversion/libsvn_subr/sqlite.c (working copy)
@@ -606,7 +606,7 @@ static volatile svn_atomic_t sqlite_init_state = 0
static svn_error_t *
init_sqlite(void *baton, apr_pool_t *pool)
{
- if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER)
+ if (sqlite3_libversion_number()/1000 < SQLITE_VERSION_NUMBER/1000)
{
return svn_error_createf(
SVN_ERR_SQLITE_ERROR, NULL,
@@ -772,7 +772,7 @@ svn_sqlite__open(svn_sqlite__db_t **db, const char
*/
{
int ignored_err = SQLITE_OK;
-#if !SQLITE_VERSION_AT_LEAST(3,7,8) && defined(SQLITE_SCHEMA)
+#if defined(SQLITE_SCHEMA)
if (!strcmp(sqlite3_libversion(), "3.7.7"))
ignored_err = SQLITE_SCHEMA;
#endif
Re: [PATCH] Make SQLite compatibility check less picky
Posted by Jonathan Nieder <jr...@gmail.com>.
Stefan Sperling wrote:
> On Sat, Oct 29, 2011 at 06:45:50AM -0500, Jonathan Nieder wrote:
>> [[[
>> The actual ABI compatibility of sqlite3 doesn't depend on the patchlevel
>> (the x in 3.7.x), so stop being picky about the patchlevel when checking
>> the version number at runtime. This avoids spurious errors of the form
>> "svn: Couldn't perform atomic initialization" / "svn: SQLite compiled
>> for 3.7.4, but running with 3.7.3" when sqlite gets a minor update
>> without Subversion being rebuilt to match.
>
> Shouldn't the version numbers in your example be the other way around?
No, my description was just bogus. This is not about when sqlite gets
a minor update, but rather when either
A. sqlite gets downgraded in a minor way, or, more likely,
B. subversion is installed using pre-built binaries that were built
against a slightly newer version of sqlite.
>> --- subversion/libsvn_subr/sqlite.c (revision 1194866)
>> +++ subversion/libsvn_subr/sqlite.c (working copy)
>> @@ -606,7 +606,7 @@ static volatile svn_atomic_t sqlite_init_state = 0
>> static svn_error_t *
>> init_sqlite(void *baton, apr_pool_t *pool)
>> {
>> - if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER)
>> + if (sqlite3_libversion_number()/1000 < SQLITE_VERSION_NUMBER/1000)
This test was already a '<' rather than '!=' so minor sqlite updates
were already handled fine.
Thanks for the sanity check. I'll sleep on this for now, planning to
resend with a new log message and some other tweaks tomorrow.
Re: [PATCH] Make SQLite compatibility check less picky
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Oct 29, 2011 at 06:45:50AM -0500, Jonathan Nieder wrote:
> Hi,
>
> This patch is taken from Debian's subversion packaging. It avoids
> having to rebuild Subversion each time an older or newer patchlevel of
> SQLite gets installed (i.e., the x in 3.7.x changes). I'd appreciate
> any thoughts you have.
>
> Thanks,
> Jonathan
>
> [[[
> The actual ABI compatibility of sqlite3 doesn't depend on the patchlevel
> (the x in 3.7.x), so stop being picky about the patchlevel when checking
> the version number at runtime. This avoids spurious errors of the form
> "svn: Couldn't perform atomic initialization" / "svn: SQLite compiled
> for 3.7.4, but running with 3.7.3" when sqlite gets a minor update
> without Subversion being rebuilt to match.
Shouldn't the version numbers in your example be the other way around?
This idea sounds sane to me.
>
> * subversion/libsvn_subr/sqlite.c
> (init_sqlite): Omit the patchlevel in version number sanity check.
> (svn_sqlite__open): Include compatibility code for running against
> libsqlite 3.7.7 even when built against a later version.
>
> Patch by: peters
> (Tweaked by me to be cautious about SQLITE_VERSION_AT_LEAST checks.)
>
> Found by: Joao Palhoto Matos <jo...@gmail.com>
> http://bugs.debian.org/608925
> ]]]
>
> Index: subversion/libsvn_subr/sqlite.c
> ===================================================================
> --- subversion/libsvn_subr/sqlite.c (revision 1194866)
> +++ subversion/libsvn_subr/sqlite.c (working copy)
> @@ -606,7 +606,7 @@ static volatile svn_atomic_t sqlite_init_state = 0
> static svn_error_t *
> init_sqlite(void *baton, apr_pool_t *pool)
> {
> - if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER)
> + if (sqlite3_libversion_number()/1000 < SQLITE_VERSION_NUMBER/1000)
> {
> return svn_error_createf(
> SVN_ERR_SQLITE_ERROR, NULL,
> @@ -772,7 +772,7 @@ svn_sqlite__open(svn_sqlite__db_t **db, const char
> */
> {
> int ignored_err = SQLITE_OK;
> -#if !SQLITE_VERSION_AT_LEAST(3,7,8) && defined(SQLITE_SCHEMA)
> +#if defined(SQLITE_SCHEMA)
> if (!strcmp(sqlite3_libversion(), "3.7.7"))
> ignored_err = SQLITE_SCHEMA;
> #endif
Re: [PATCH/RFC v3] Optionally allow binaries to run against older
SQLite
Posted by Jonathan Nieder <jr...@gmail.com>.
Daniel Shahaf wrote:
> On Sunday, November 06, 2011 9:28 AM, "Daniel Shahaf" <d....@daniel.shahaf.name> wrote:
>> I've committed the sqlite3.m4 part in r1198169. The rest looks good
>> too, but I'm going to wait a day or two before committing the remainder,
>> to let people join the conversation if they have feedback.
>
> That took more than "a day or two" :-(. Anyway, committed this in
> r1201421, after some minor tweaks.
Thank you. The tweaks were worth the wait.
Re: [PATCH/RFC v3] Optionally allow binaries to run against older
SQLite
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Sunday, November 06, 2011 9:28 AM, "Daniel Shahaf" <d....@daniel.shahaf.name> wrote:
> On Friday, November 04, 2011 1:24 AM, "Jonathan Nieder" <jr...@gmail.com> wrote:
> > [[[
> > Introduce a --enable-sqlite-compatibility-version=X.Y.Z option for
> > ./configure to allow people building Subversion to specify how old the
...
> > +++ subversion/include/private/svn_dep_compat.h (working copy)
> > @@ -107,6 +107,32 @@
> > #endif /* SERF_VERSION_AT_LEAST */
> >
> > +/**
> > + * By default, if libsvn is built against one version of SQLite
> > + * and then run using an older version, svn will error out:
> ...
> > +#ifndef SVN_SQLITE_MIN_VERSION_NUMBER
> > +#define SVN_SQLITE_MIN_VERSION_NUMBER SQLITE_VERSION_NUMBER
> > +#define SVN_SQLITE_MIN_VERSION SQLITE_VERSION
> > +#endif /* SVN_SQLITE_MIN_VERSION_NUMBER */
> > +
>
> Hmm. I wonder if we should be more paranoid here when checking/defining
> the macros. Probably I'm just being overly careful.
>
I put myself to rest by documenting this in notes/knobs.
> > +++ configure.ac (working copy)
> > @@ -172,6 +172,18 @@
> > +AC_ARG_ENABLE(sqlite-compatibility-version,
> > + AS_HELP_STRING([--enable-sqlite-compatibility-version=X.Y.Z],
> > + [Allow binary to run against older SQLite]),
>
> Perhaps have the help string mention how ARG is used?
> "Allow binary to run against SQLite as old as ARG"
>
Done.
> Also, should you forbid passing this arg when linking against a static SQLite?
>
I still think that's a good idea: if svn isn't built to support shared
libraries, then the new configure switch should cause an error. (Or, at least,
should cause an error if the static SQLite library is older than ARG.)
> > + [sqlite_compat_ver=$enableval],[sqlite_compat_ver=no])
> > +
> > +if test -n "$sqlite_compat_ver" && test "$sqlite_compat_ver" != no; then
> > + SVN_SQLITE_VERNUM_PARSE([$sqlite_compat_ver],
> > + [sqlite_compat_ver_num])
> > + CFLAGS="-DSVN_SQLITE_MIN_VERSION='\"$sqlite_compat_ver\"' $CFLAGS"
> > + CFLAGS="-DSVN_SQLITE_MIN_VERSION_NUMBER=$sqlite_compat_ver_num $CFLAGS"
> > +fi
> > +
> > dnl Set up a number of directories ---------------------
> >
> > dnl Create SVN_BINDIR for proper substitution
>
> I've committed the sqlite3.m4 part in r1198169. The rest looks good
> too, but I'm going to wait a day or two before committing the remainder,
> to let people join the conversation if they have feedback.
That took more than "a day or two" :-(. Anyway, committed this in
r1201421, after some minor tweaks.
Thanks!
Daniel
Re: [PATCH/RFC v3] Optionally allow binaries to run against older
SQLite
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Friday, November 04, 2011 1:24 AM, "Jonathan Nieder" <jr...@gmail.com> wrote:
> [[[
> Introduce a --enable-sqlite-compatibility-version=X.Y.Z option for
> ./configure to allow people building Subversion to specify how old the
> system being deployed on might be. Setting the compatibility version
> to an older version turns on code in subversion that works around
> infelicities in older versions and relaxes the version check ("SQLite
> compiled for 3.7.4, but running with 3.7.3") at initialization time.
>
> If the compat version is set to a version before the minimum supported
> version (3.6.18), the build will fail early so the person building can
> adjust her expectations.
>
> The default for the compat version is the currently installed version,
> so there should be no change in behavior for users not passing this
> option to the configure script.
>
> * subversion/include/private/svn_dep_compat.h
> (SVN_SQLITE_MIN_VERSION_NUMBER, SVN_SQLITE_MIN_VERSION): Set to
> SQLITE_VERSION_NUMBER, SQLITE_VERSION if undefined.
> (SQLITE_VERSION_AT_LEAST): Check SVN_SQLITE_MIN_VERSION_NUMBER instead
> of SQLITE_VERSION_NUMBER.
>
> * subversion/libsvn_subr/sqlite.c (init_sqlite): Check sqlite version
> against SVN_SQLITE_MIN_VERSION_NUMBER instead of SQLITE_VERSION_NUMBER.
>
> * configure.ac: Provide a --enable-sqlite-compatibility-version switch
> that sets SVN_SQLITE_MIN_VERSION_NUMBER and SVN_SQLITE_MIN_VERSION.
>
> * build/ac-macros/sqlite.m4
> (SVN_SQLITE_VERNUM_PARSE): Make it reusable (in particular for
> configure.ac), by taking a version string and a variable to store the
> corresponding version number as arguments.
> (SVN_SQLITE_MIN_VERNUM_PARSE): Simplify by calling
> SVN_SQLITE_VERNUM_PARSE.
> (SVN_SQLITE_PKG_CONFIG): Adapt SVN_SQLITE_VERNUM_PARSE call to the new
> calling convention.
> ]]]
>
> +++ subversion/include/private/svn_dep_compat.h (working copy)
> @@ -107,6 +107,32 @@
> #endif /* SERF_VERSION_AT_LEAST */
>
> +/**
> + * By default, if libsvn is built against one version of SQLite
> + * and then run using an older version, svn will error out:
...
> +#ifndef SVN_SQLITE_MIN_VERSION_NUMBER
> +#define SVN_SQLITE_MIN_VERSION_NUMBER SQLITE_VERSION_NUMBER
> +#define SVN_SQLITE_MIN_VERSION SQLITE_VERSION
> +#endif /* SVN_SQLITE_MIN_VERSION_NUMBER */
> +
Hmm. I wonder if we should be more paranoid here when checking/defining
the macros. Probably I'm just being overly careful.
> #ifdef __cplusplus
> Index: configure.ac
> ===================================================================
> --- configure.ac (revision 1197399)
> +++ configure.ac (working copy)
> @@ -172,6 +172,18 @@
> SVN_LIB_SQLITE(${SQLITE_MINIMUM_VER}, ${SQLITE_RECOMMENDED_VER},
> ${SQLITE_URL})
>
> +AC_ARG_ENABLE(sqlite-compatibility-version,
> + AS_HELP_STRING([--enable-sqlite-compatibility-version=X.Y.Z],
> + [Allow binary to run against older SQLite]),
Perhaps have the help string mention how ARG is used?
"Allow binary to run against SQLite as old as ARG"
Also, should you forbid passing this arg when linking against a static SQLite?
> + [sqlite_compat_ver=$enableval],[sqlite_compat_ver=no])
> +
> +if test -n "$sqlite_compat_ver" && test "$sqlite_compat_ver" != no; then
> + SVN_SQLITE_VERNUM_PARSE([$sqlite_compat_ver],
> + [sqlite_compat_ver_num])
> + CFLAGS="-DSVN_SQLITE_MIN_VERSION='\"$sqlite_compat_ver\"' $CFLAGS"
> + CFLAGS="-DSVN_SQLITE_MIN_VERSION_NUMBER=$sqlite_compat_ver_num $CFLAGS"
> +fi
> +
> dnl Set up a number of directories ---------------------
>
> dnl Create SVN_BINDIR for proper substitution
I've committed the sqlite3.m4 part in r1198169. The rest looks good
too, but I'm going to wait a day or two before committing the remainder,
to let people join the conversation if they have feedback.
Thanks,
Daniel
Re: [PATCH/RFC v3] Optionally allow binaries to run against older
SQLite
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Friday, November 04, 2011 1:24 AM, "Jonathan Nieder" <jr...@gmail.com> wrote:
> Daniel Shahaf wrote:
> > On Wednesday, November 02, 2011 4:25 PM, "Jonathan Nieder" <jr...@gmail.com> wrote:
>
> >> I'm not very happy about putting -DSVN_SQLITE_COMPAT_VERSION in CFLAGS
> >> --- does subversion have a config.h somewhere?
> >
> > http://s.apache.org/xy-problem --- Why do you think you need config.h?
>
> Sorry, that was cryptic of me. The -DSVN_SQLITE_COMPAT_VERSION looked
> lonely on the command line not surrounded by settings like -DHAVE_INT
> and -DHAVE_PRINTF --- I was afraid there might be some other idiom I
> was missing.
>
> I'm happy keeping it in CFLAGS.
>
There is the svn_config_private.h idiom. There is also the
${target}_CFLAGS Makefile variable, generated by the build/*/*.py
infrastructure.
I don't feel that throwing it in CFLAGS is intuitively wrong, in our
build system.
> Your other comments looked reasonable, too. Details:
>
> - there were no unpatched instances of SQLITE_VERSION_NUMBER, but
> there was one unpatched instance of SQLITE_VERSION. So now I do
>
> -DSVN_SQLITE_MIN_VERSION_NUMBER=1002003
> -DSVN_SQLITE_MIN_VERSION="1.2.3"
>
Okay.
> - it would be simple to make the configure script take care of the
> default for SVN_SQLITE_MIN_VERSION_NUMBER instead of the header
> doing so, but I prefer the latter since it means you can use
> "gcc -c" directly to build without remembering the -D
> option and still have a chance of the result working. So I've
> left that alone for now.
>
+1
> - assignment to $2 in sqlite.m4 could be replaced by
>
> ver_num=`expr ...`
> $2=$ver_num
>
> or:
>
> result_var="$2"
> ...
> eval "$result_var=\$ver_num"
>
> Left untouched for now.
>
What I actually had in mind was:
f() {
echo foo
}
bar=`f`
That's primarily because I'm not used to the $1=* idiom (in its various
forms as you suggest above). If it's a standard idiom for configure
script writing I wouldn't feel strongly to change it.
> [[[
> Introduce a --enable-sqlite-compatibility-version=X.Y.Z option for
> ./configure to allow people building Subversion to specify how old the
> system being deployed on might be. Setting the compatibility version
> to an older version turns on code in subversion that works around
> infelicities in older versions and relaxes the version check ("SQLite
> compiled for 3.7.4, but running with 3.7.3") at initialization time.
>
> If the compat version is set to a version before the minimum supported
> version (3.6.18), the build will fail early so the person building can
> adjust her expectations.
>
Looks good.
> The default for the compat version is the currently installed version,
> so there should be no change in behavior for users not passing this
> option to the configure script.
>
I'll read/review the rest later.
[PATCH/RFC v3] Optionally allow binaries to run against older SQLite
Posted by Jonathan Nieder <jr...@gmail.com>.
Daniel Shahaf wrote:
> On Wednesday, November 02, 2011 4:25 PM, "Jonathan Nieder" <jr...@gmail.com> wrote:
>> I'm not very happy about putting -DSVN_SQLITE_COMPAT_VERSION in CFLAGS
>> --- does subversion have a config.h somewhere?
>
> http://s.apache.org/xy-problem --- Why do you think you need config.h?
Sorry, that was cryptic of me. The -DSVN_SQLITE_COMPAT_VERSION looked
lonely on the command line not surrounded by settings like -DHAVE_INT
and -DHAVE_PRINTF --- I was afraid there might be some other idiom I
was missing.
I'm happy keeping it in CFLAGS.
Your other comments looked reasonable, too. Details:
- best to define SVN_SQLITE_MIN_VERSION_NUMBER in the header
that uses it, instead of relying on sqlite.c being the only source
file that does version checks. Fixed.
- there were no unpatched instances of SQLITE_VERSION_NUMBER, but
there was one unpatched instance of SQLITE_VERSION. So now I do
-DSVN_SQLITE_MIN_VERSION_NUMBER=1002003
-DSVN_SQLITE_MIN_VERSION="1.2.3"
- it would be simple to make the configure script take care of the
default for SVN_SQLITE_MIN_VERSION_NUMBER instead of the header
doing so, but I prefer the latter since it means you can use
"gcc -c" directly to build without remembering the -D
option and still have a chance of the result working. So I've
left that alone for now.
- assignment to $2 in sqlite.m4 could be replaced by
ver_num=`expr ...`
$2=$ver_num
or:
result_var="$2"
...
eval "$result_var=\$ver_num"
Left untouched for now.
[[[
Introduce a --enable-sqlite-compatibility-version=X.Y.Z option for
./configure to allow people building Subversion to specify how old the
system being deployed on might be. Setting the compatibility version
to an older version turns on code in subversion that works around
infelicities in older versions and relaxes the version check ("SQLite
compiled for 3.7.4, but running with 3.7.3") at initialization time.
If the compat version is set to a version before the minimum supported
version (3.6.18), the build will fail early so the person building can
adjust her expectations.
The default for the compat version is the currently installed version,
so there should be no change in behavior for users not passing this
option to the configure script.
* subversion/include/private/svn_dep_compat.h
(SVN_SQLITE_MIN_VERSION_NUMBER, SVN_SQLITE_MIN_VERSION): Set to
SQLITE_VERSION_NUMBER, SQLITE_VERSION if undefined.
(SQLITE_VERSION_AT_LEAST): Check SVN_SQLITE_MIN_VERSION_NUMBER instead
of SQLITE_VERSION_NUMBER.
* subversion/libsvn_subr/sqlite.c (init_sqlite): Check sqlite version
against SVN_SQLITE_MIN_VERSION_NUMBER instead of SQLITE_VERSION_NUMBER.
* configure.ac: Provide a --enable-sqlite-compatibility-version switch
that sets SVN_SQLITE_MIN_VERSION_NUMBER and SVN_SQLITE_MIN_VERSION.
* build/ac-macros/sqlite.m4
(SVN_SQLITE_VERNUM_PARSE): Make it reusable (in particular for
configure.ac), by taking a version string and a variable to store the
corresponding version number as arguments.
(SVN_SQLITE_MIN_VERNUM_PARSE): Simplify by calling
SVN_SQLITE_VERNUM_PARSE.
(SVN_SQLITE_PKG_CONFIG): Adapt SVN_SQLITE_VERNUM_PARSE call to the new
calling convention.
]]]
Index: subversion/include/private/svn_dep_compat.h
===================================================================
--- subversion/include/private/svn_dep_compat.h (revision 1197399)
+++ subversion/include/private/svn_dep_compat.h (working copy)
@@ -107,6 +107,32 @@
#endif /* SERF_VERSION_AT_LEAST */
+/**
+ * By default, if libsvn is built against one version of SQLite
+ * and then run using an older version, svn will error out:
+ *
+ * svn: Couldn't perform atomic initialization
+ * svn: SQLite compiled for 3.7.4, but running with 3.7.3
+ *
+ * That can be annoying when building on a modern system in order
+ * to deploy on a less modern one. So these constants allow one
+ * to specify how old the system being deployed on might be.
+ * For example,
+ *
+ * EXTRA_CFLAGS += -DSVN_SQLITE_MIN_VERSION_NUMBER=3007003
+ * EXTRA_CFLAGS += '-DSVN_SQLITE_MIN_VERSION="3.7.3"'
+ *
+ * turns on code that works around infelicities in older versions
+ * as far back as 3.7.3 and relaxes the check at initialization time
+ * to permit them.
+ *
+ * @since New in 1.8.
+ */
+#ifndef SVN_SQLITE_MIN_VERSION_NUMBER
+#define SVN_SQLITE_MIN_VERSION_NUMBER SQLITE_VERSION_NUMBER
+#define SVN_SQLITE_MIN_VERSION SQLITE_VERSION
+#endif /* SVN_SQLITE_MIN_VERSION_NUMBER */
+
/**
* Check at compile time if the SQLite version is at least a certain
* level.
* @param major The major version component of the version checked
@@ -120,7 +146,7 @@
*/
#ifndef SQLITE_VERSION_AT_LEAST
#define SQLITE_VERSION_AT_LEAST(major,minor,patch) \
-((major*1000000 + minor*1000 + patch) <= SQLITE_VERSION_NUMBER)
+((major*1000000 + minor*1000 + patch) <= SVN_SQLITE_MIN_VERSION_NUMBER)
#endif /* SQLITE_VERSION_AT_LEAST */
#ifdef __cplusplus
Index: subversion/libsvn_subr/sqlite.c
===================================================================
--- subversion/libsvn_subr/sqlite.c (revision 1197399)
+++ subversion/libsvn_subr/sqlite.c (working copy)
@@ -606,12 +606,12 @@
static svn_error_t *
init_sqlite(void *baton, apr_pool_t *pool)
{
- if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER)
+ if (sqlite3_libversion_number() < SVN_SQLITE_MIN_VERSION_NUMBER)
{
return svn_error_createf(
SVN_ERR_SQLITE_ERROR, NULL,
_("SQLite compiled for %s, but running with %s"),
- SQLITE_VERSION, sqlite3_libversion());
+ SVN_SQLITE_MIN_VERSION, sqlite3_libversion());
}
#if APR_HAS_THREADS
Index: configure.ac
===================================================================
--- configure.ac (revision 1197399)
+++ configure.ac (working copy)
@@ -172,6 +172,18 @@
SVN_LIB_SQLITE(${SQLITE_MINIMUM_VER}, ${SQLITE_RECOMMENDED_VER},
${SQLITE_URL})
+AC_ARG_ENABLE(sqlite-compatibility-version,
+ AS_HELP_STRING([--enable-sqlite-compatibility-version=X.Y.Z],
+ [Allow binary to run against older SQLite]),
+ [sqlite_compat_ver=$enableval],[sqlite_compat_ver=no])
+
+if test -n "$sqlite_compat_ver" && test "$sqlite_compat_ver" != no; then
+ SVN_SQLITE_VERNUM_PARSE([$sqlite_compat_ver],
+ [sqlite_compat_ver_num])
+ CFLAGS="-DSVN_SQLITE_MIN_VERSION='\"$sqlite_compat_ver\"' $CFLAGS"
+ CFLAGS="-DSVN_SQLITE_MIN_VERSION_NUMBER=$sqlite_compat_ver_num $CFLAGS"
+fi
+
dnl Set up a number of directories ---------------------
dnl Create SVN_BINDIR for proper substitution
Index: build/ac-macros/sqlite.m4
===================================================================
--- build/ac-macros/sqlite.m4 (revision 1197399)
+++ build/ac-macros/sqlite.m4 (working copy)
@@ -106,7 +106,7 @@
sqlite_version=`$PKG_CONFIG $SQLITE_PKGNAME --modversion --silence-errors`
if test -n "$sqlite_version"; then
- SVN_SQLITE_VERNUM_PARSE
+ SVN_SQLITE_VERNUM_PARSE([$sqlite_version], [sqlite_ver_num])
if test "$sqlite_ver_num" -ge "$sqlite_min_ver_num"; then
AC_MSG_RESULT([$sqlite_version])
@@ -198,20 +198,22 @@
fi
])
-dnl SVN_SQLITE_VERNUM_PARSE()
+dnl SVN_SQLITE_VERNUM_PARSE(version_string, result_var)
dnl
-dnl Parse a x.y[.z] version string sqlite_version into a number sqlite_ver_num.
+dnl Parse a x.y[.z] version string version_string into a number result_var.
AC_DEFUN(SVN_SQLITE_VERNUM_PARSE,
[
- sqlite_major=`expr $sqlite_version : '\([[0-9]]*\)'`
- sqlite_minor=`expr $sqlite_version : '[[0-9]]*\.\([[0-9]]*\)'`
- sqlite_micro=`expr $sqlite_version : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
- if test -z "$sqlite_micro"; then
- sqlite_micro=0
+ version_string="$1"
+
+ major=`expr $version_string : '\([[0-9]]*\)'`
+ minor=`expr $version_string : '[[0-9]]*\.\([[0-9]]*\)'`
+ micro=`expr $version_string : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
+ if test -z "$micro"; then
+ micro=0
fi
- sqlite_ver_num=`expr $sqlite_major \* 1000000 \
- \+ $sqlite_minor \* 1000 \
- \+ $sqlite_micro`
+ $2=`expr $major \* 1000000 \
+ \+ $minor \* 1000 \
+ \+ $micro`
])
dnl SVN_SQLITE_MIN_VERNUM_PARSE()
@@ -220,12 +222,7 @@
dnl sqlite_min_ver_num.
AC_DEFUN(SVN_SQLITE_MIN_VERNUM_PARSE,
[
- sqlite_min_major=`expr $SQLITE_MINIMUM_VER : '\([[0-9]]*\)'`
- sqlite_min_minor=`expr $SQLITE_MINIMUM_VER : '[[0-9]]*\.\([[0-9]]*\)'`
- sqlite_min_micro=`expr $SQLITE_MINIMUM_VER : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
- sqlite_min_ver_num=`expr $sqlite_min_major \* 1000000 \
- \+ $sqlite_min_minor \* 1000 \
- \+ $sqlite_min_micro`
+ SVN_SQLITE_VERNUM_PARSE([$SQLITE_MINIMUM_VER], [sqlite_min_ver_num])
])
dnl SVN_DOWNLOAD_SQLITE()
Re: [PATCH/RFC v2] Optionally allow binaries to run against older
SQLite
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Wednesday, November 02, 2011 4:25 PM, "Jonathan Nieder" <jr...@gmail.com> wrote:
> Hi again,
>
> Here's a hopefully saner patch. Thanks much for the quick feedback on
> the previous incarnation.
>
> I'm not very happy about putting -DSVN_SQLITE_COMPAT_VERSION in CFLAGS
> --- does subversion have a config.h somewhere?
>
http://s.apache.org/xy-problem --- Why do you think you need config.h?
Anyway: have a look at the other build/ac-macros/ files for precedents.
The makefile supports EXTRA_CFLAGS for the user to add his own CFLAGS at
'make' time.
> I can split this into three patches (one to expose
> SVN_SQLITE_VERNUM_PARSE from build/ac-macros/sqlite.m4, one
> introducing the SVN_SQLITE_MIN_VERSION_NUMBER handling in code, and
> another to wire it up into the configure script) if that's wanted.
> Let me know.
>
Will do. Thanks.
> Thanks a lot,
> Jonathan
>
> [[[
> If libsvn is built against one version of sqlite3 and then run using
> an older version, currently svn will error out:
>
> svn: Couldn't perform atomic initialization
> svn: SQLite compiled for 3.7.4, but running with 3.7.3
>
> Not all sqlite3 updates include ABI changes that are relevant to
> Subversion, though. This can be annoying when building on a modern
> system in order to deploy on a less modern one.
>
> This patch introduces a new --enable-sqlite-compatibility-version=X.Y.Z
The above line should be the first line of the log message: describe
the change first and the historical reasons for it either later or in
code comments.
> option for ./configure to allow people building Subversion to
> specify how old the system being deployed on might be, to address
> that. Setting the compatibility version to an older version turns
> on code in subversion that works around infelicities in those
> older versions and relaxes the version check at initialization
> time.
>
> If the compat version is set to a version before the minimum supported
> version (3.6.18), the build will fail early so the person building can
> adjust her expectations.
>
> The default for the compat version is the currently installed version,
> so there should be no change in behavior for users not passing this
> option to the configure script.
>
Sounds sane.
> * subversion/include/private/svn_dep_compat.h (SQLITE_VERSION_AT_LEAST):
> Check SVN_SQLITE_MIN_VERSION_NUMBER instead of SQLITE_VERSION_NUMBER.
>
> * subversion/libsvn_subr/sqlite.c
> (SVN_SQLITE_MIN_VERSION_NUMBER): Set to SQLITE_VERSION_NUMBER if
> undefined.
> (init_sqlite): Check sqlite version against SVN_SQLITE_MIN_VERSION_NUMBER
> instead of SQLITE_VERSION_NUMBER.
>
> * configure.ac: Provide a --enable-sqlite-compatibility-version switch
> that sets SVN_SQLITE_MIN_VERSION_NUMBER.
>
> * build/ac-macros/sqlite.m4
> (SVN_SQLITE_VERNUM_PARSE): Make it reusable (in particular for
> configure.ac), by taking a version string and a variable to store the
> corresponding version number as arguments.
> (SVN_SQLITE_MIN_VERNUM_PARSE): Simplify by calling
> SVN_SQLITE_VERNUM_PARSE.
> (SVN_SQLITE_PKG_CONFIG): Adapt SVN_SQLITE_VERNUM_PARSE call to the new
> calling convention.
>
> Found by: Joao Palhoto Matos <jo...@gmail.com>
> http://bugs.debian.org/608925
> ]]]
>
> Index: subversion/include/private/svn_dep_compat.h
> ===================================================================
> --- subversion/include/private/svn_dep_compat.h (revision 1196775)
> +++ subversion/include/private/svn_dep_compat.h (working copy)
> @@ -120,7 +120,7 @@ typedef apr_uint32_t apr_uintptr_t;
> */
> #ifndef SQLITE_VERSION_AT_LEAST
> #define SQLITE_VERSION_AT_LEAST(major,minor,patch) \
> -((major*1000000 + minor*1000 + patch) <= SQLITE_VERSION_NUMBER)
> +((major*1000000 + minor*1000 + patch) <= SVN_SQLITE_MIN_VERSION_NUMBER)
> #endif /* SQLITE_VERSION_AT_LEAST */
>
What if SVN_SQLITE_MIN_VERSION_NUMBER isn't #define'd yet? Below you
have an #ifndef guard, so presumably you need one here too.
> #ifdef __cplusplus
> Index: subversion/libsvn_subr/sqlite.c
> ===================================================================
> --- subversion/libsvn_subr/sqlite.c (revision 1196775)
> +++ subversion/libsvn_subr/sqlite.c (working copy)
> @@ -50,6 +50,10 @@
> #include <sqlite3.h>
> #endif
>
> +#ifndef SVN_SQLITE_MIN_VERSION_NUMBER
> + #define SVN_SQLITE_MIN_VERSION_NUMBER SQLITE_VERSION_NUMBER
> +#endif
> +
> #if !SQLITE_VERSION_AT_LEAST(3,6,18)
> #error SQLite is too old -- version 3.6.18 is the minimum required version
> #endif
> @@ -606,7 +610,7 @@ static volatile svn_atomic_t sqlite_init_state = 0
> static svn_error_t *
> init_sqlite(void *baton, apr_pool_t *pool)
> {
> - if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER)
> + if (sqlite3_libversion_number() < SVN_SQLITE_MIN_VERSION_NUMBER)
I don't have a working copy handy. Are there instance of SQLITE_VERSION_NUMBER
that _didn't_ get changed to SVN_SQLITE_MIN_VERSION_NUMBER?
> {
> return svn_error_createf(
> SVN_ERR_SQLITE_ERROR, NULL,
> Index: configure.ac
> ===================================================================
> --- configure.ac (revision 1196775)
> +++ configure.ac (working copy)
> @@ -172,6 +172,17 @@ SQLITE_URL="http://www.sqlite.org/sqlite-amalgamat
> SVN_LIB_SQLITE(${SQLITE_MINIMUM_VER}, ${SQLITE_RECOMMENDED_VER},
> ${SQLITE_URL})
>
> +AC_ARG_ENABLE(sqlite-compatibility-version,
> + AS_HELP_STRING([--enable-sqlite-compatibility-version=X.Y.Z],
> + [Allow binary to run against older SQLite]),
> + [sqlite_compat_ver=$enableval],[sqlite_compat_ver=no])
> +
> +if test -n "$sqlite_compat_ver" && test "$sqlite_compat_ver" != no; then
> + SVN_SQLITE_VERNUM_PARSE([$sqlite_compat_ver],
> + [sqlite_compat_ver_num])
> + CFLAGS="-DSVN_SQLITE_MIN_VERSION_NUMBER=$sqlite_compat_ver_num $CFLAGS"
> +fi
> +
Don't you have the SQLite version number here too? (The one we're building
against or linking to.) If so, you could simplify the semantics of
SVN_SQLITE_MIN_VERSION_NUMBER by declaring that configure will _always_
define it (to the installed version if --enable-sqlite-compatibility-version was supplied).
(I'm assuming this should indeed be --enable rather than --with, but not
sure enough about this to not mention it here for people to disagree if
I'm wrong.)
> dnl Set up a number of directories ---------------------
>
> dnl Create SVN_BINDIR for proper substitution
> Index: build/ac-macros/sqlite.m4
> ===================================================================
> --- build/ac-macros/sqlite.m4 (revision 1196775)
> +++ build/ac-macros/sqlite.m4 (working copy)
> @@ -106,7 +106,7 @@ AC_DEFUN(SVN_SQLITE_PKG_CONFIG,
> sqlite_version=`$PKG_CONFIG $SQLITE_PKGNAME --modversion --silence-errors`
>
> if test -n "$sqlite_version"; then
> - SVN_SQLITE_VERNUM_PARSE
> + SVN_SQLITE_VERNUM_PARSE([$sqlite_version], [sqlite_ver_num])
>
> if test "$sqlite_ver_num" -ge "$sqlite_min_ver_num"; then
> AC_MSG_RESULT([$sqlite_version])
> @@ -198,20 +198,22 @@ SQLITE_VERSION_OKAY
> fi
> ])
>
> -dnl SVN_SQLITE_VERNUM_PARSE()
> +dnl SVN_SQLITE_VERNUM_PARSE(version_string, result_var)
> dnl
> -dnl Parse a x.y[.z] version string sqlite_version into a number sqlite_ver_num.
> +dnl Parse a x.y[.z] version string version_string into a number result_var.
> AC_DEFUN(SVN_SQLITE_VERNUM_PARSE,
> [
> - sqlite_major=`expr $sqlite_version : '\([[0-9]]*\)'`
> - sqlite_minor=`expr $sqlite_version : '[[0-9]]*\.\([[0-9]]*\)'`
> - sqlite_micro=`expr $sqlite_version : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
> - if test -z "$sqlite_micro"; then
> - sqlite_micro=0
> + version_string="$1"
> +
> + major=`expr $version_string : '\([[0-9]]*\)'`
> + minor=`expr $version_string : '[[0-9]]*\.\([[0-9]]*\)'`
> + micro=`expr $version_string : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
> + if test -z "$micro"; then
> + micro=0
> fi
> - sqlite_ver_num=`expr $sqlite_major \* 1000000 \
> - \+ $sqlite_minor \* 1000 \
> - \+ $sqlite_micro`
> + $2=`expr $major \* 1000000 \
> + \+ $minor \* 1000 \
> + \+ $micro`
Assignment to $2 directly strikes me as bad style, can we avoid it?
> ])
>
> dnl SVN_SQLITE_MIN_VERNUM_PARSE()
> @@ -220,12 +222,7 @@ dnl Parse a x.y.z version string SQLITE_MINIMUM_VE
> dnl sqlite_min_ver_num.
> AC_DEFUN(SVN_SQLITE_MIN_VERNUM_PARSE,
> [
> - sqlite_min_major=`expr $SQLITE_MINIMUM_VER : '\([[0-9]]*\)'`
> - sqlite_min_minor=`expr $SQLITE_MINIMUM_VER : '[[0-9]]*\.\([[0-9]]*\)'`
> - sqlite_min_micro=`expr $SQLITE_MINIMUM_VER : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
> - sqlite_min_ver_num=`expr $sqlite_min_major \* 1000000 \
> - \+ $sqlite_min_minor \* 1000 \
> - \+ $sqlite_min_micro`
> + SVN_SQLITE_VERNUM_PARSE([$SQLITE_MINIMUM_VER], [sqlite_min_ver_num])
> ])
>
> dnl SVN_DOWNLOAD_SQLITE()
>
Overall looks good, though I'd want to audit the uses of
SQLITE_VERSION_NUMBER in our code prior to applying it.
[PATCH/RFC v2] Optionally allow binaries to run against older SQLite
Posted by Jonathan Nieder <jr...@gmail.com>.
Hi again,
Here's a hopefully saner patch. Thanks much for the quick feedback on
the previous incarnation.
I'm not very happy about putting -DSVN_SQLITE_COMPAT_VERSION in CFLAGS
--- does subversion have a config.h somewhere?
I can split this into three patches (one to expose
SVN_SQLITE_VERNUM_PARSE from build/ac-macros/sqlite.m4, one
introducing the SVN_SQLITE_MIN_VERSION_NUMBER handling in code, and
another to wire it up into the configure script) if that's wanted.
Let me know.
Thanks a lot,
Jonathan
[[[
If libsvn is built against one version of sqlite3 and then run using
an older version, currently svn will error out:
svn: Couldn't perform atomic initialization
svn: SQLite compiled for 3.7.4, but running with 3.7.3
Not all sqlite3 updates include ABI changes that are relevant to
Subversion, though. This can be annoying when building on a modern
system in order to deploy on a less modern one.
This patch introduces a new --enable-sqlite-compatibility-version=X.Y.Z
option for ./configure to allow people building Subversion to
specify how old the system being deployed on might be, to address
that. Setting the compatibility version to an older version turns
on code in subversion that works around infelicities in those
older versions and relaxes the version check at initialization
time.
If the compat version is set to a version before the minimum supported
version (3.6.18), the build will fail early so the person building can
adjust her expectations.
The default for the compat version is the currently installed version,
so there should be no change in behavior for users not passing this
option to the configure script.
* subversion/include/private/svn_dep_compat.h (SQLITE_VERSION_AT_LEAST):
Check SVN_SQLITE_MIN_VERSION_NUMBER instead of SQLITE_VERSION_NUMBER.
* subversion/libsvn_subr/sqlite.c
(SVN_SQLITE_MIN_VERSION_NUMBER): Set to SQLITE_VERSION_NUMBER if
undefined.
(init_sqlite): Check sqlite version against SVN_SQLITE_MIN_VERSION_NUMBER
instead of SQLITE_VERSION_NUMBER.
* configure.ac: Provide a --enable-sqlite-compatibility-version switch
that sets SVN_SQLITE_MIN_VERSION_NUMBER.
* build/ac-macros/sqlite.m4
(SVN_SQLITE_VERNUM_PARSE): Make it reusable (in particular for
configure.ac), by taking a version string and a variable to store the
corresponding version number as arguments.
(SVN_SQLITE_MIN_VERNUM_PARSE): Simplify by calling
SVN_SQLITE_VERNUM_PARSE.
(SVN_SQLITE_PKG_CONFIG): Adapt SVN_SQLITE_VERNUM_PARSE call to the new
calling convention.
Found by: Joao Palhoto Matos <jo...@gmail.com>
http://bugs.debian.org/608925
]]]
Index: subversion/include/private/svn_dep_compat.h
===================================================================
--- subversion/include/private/svn_dep_compat.h (revision 1196775)
+++ subversion/include/private/svn_dep_compat.h (working copy)
@@ -120,7 +120,7 @@ typedef apr_uint32_t apr_uintptr_t;
*/
#ifndef SQLITE_VERSION_AT_LEAST
#define SQLITE_VERSION_AT_LEAST(major,minor,patch) \
-((major*1000000 + minor*1000 + patch) <= SQLITE_VERSION_NUMBER)
+((major*1000000 + minor*1000 + patch) <= SVN_SQLITE_MIN_VERSION_NUMBER)
#endif /* SQLITE_VERSION_AT_LEAST */
#ifdef __cplusplus
Index: subversion/libsvn_subr/sqlite.c
===================================================================
--- subversion/libsvn_subr/sqlite.c (revision 1196775)
+++ subversion/libsvn_subr/sqlite.c (working copy)
@@ -50,6 +50,10 @@
#include <sqlite3.h>
#endif
+#ifndef SVN_SQLITE_MIN_VERSION_NUMBER
+ #define SVN_SQLITE_MIN_VERSION_NUMBER SQLITE_VERSION_NUMBER
+#endif
+
#if !SQLITE_VERSION_AT_LEAST(3,6,18)
#error SQLite is too old -- version 3.6.18 is the minimum required version
#endif
@@ -606,7 +610,7 @@ static volatile svn_atomic_t sqlite_init_state = 0
static svn_error_t *
init_sqlite(void *baton, apr_pool_t *pool)
{
- if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER)
+ if (sqlite3_libversion_number() < SVN_SQLITE_MIN_VERSION_NUMBER)
{
return svn_error_createf(
SVN_ERR_SQLITE_ERROR, NULL,
Index: configure.ac
===================================================================
--- configure.ac (revision 1196775)
+++ configure.ac (working copy)
@@ -172,6 +172,17 @@ SQLITE_URL="http://www.sqlite.org/sqlite-amalgamat
SVN_LIB_SQLITE(${SQLITE_MINIMUM_VER}, ${SQLITE_RECOMMENDED_VER},
${SQLITE_URL})
+AC_ARG_ENABLE(sqlite-compatibility-version,
+ AS_HELP_STRING([--enable-sqlite-compatibility-version=X.Y.Z],
+ [Allow binary to run against older SQLite]),
+ [sqlite_compat_ver=$enableval],[sqlite_compat_ver=no])
+
+if test -n "$sqlite_compat_ver" && test "$sqlite_compat_ver" != no; then
+ SVN_SQLITE_VERNUM_PARSE([$sqlite_compat_ver],
+ [sqlite_compat_ver_num])
+ CFLAGS="-DSVN_SQLITE_MIN_VERSION_NUMBER=$sqlite_compat_ver_num $CFLAGS"
+fi
+
dnl Set up a number of directories ---------------------
dnl Create SVN_BINDIR for proper substitution
Index: build/ac-macros/sqlite.m4
===================================================================
--- build/ac-macros/sqlite.m4 (revision 1196775)
+++ build/ac-macros/sqlite.m4 (working copy)
@@ -106,7 +106,7 @@ AC_DEFUN(SVN_SQLITE_PKG_CONFIG,
sqlite_version=`$PKG_CONFIG $SQLITE_PKGNAME --modversion --silence-errors`
if test -n "$sqlite_version"; then
- SVN_SQLITE_VERNUM_PARSE
+ SVN_SQLITE_VERNUM_PARSE([$sqlite_version], [sqlite_ver_num])
if test "$sqlite_ver_num" -ge "$sqlite_min_ver_num"; then
AC_MSG_RESULT([$sqlite_version])
@@ -198,20 +198,22 @@ SQLITE_VERSION_OKAY
fi
])
-dnl SVN_SQLITE_VERNUM_PARSE()
+dnl SVN_SQLITE_VERNUM_PARSE(version_string, result_var)
dnl
-dnl Parse a x.y[.z] version string sqlite_version into a number sqlite_ver_num.
+dnl Parse a x.y[.z] version string version_string into a number result_var.
AC_DEFUN(SVN_SQLITE_VERNUM_PARSE,
[
- sqlite_major=`expr $sqlite_version : '\([[0-9]]*\)'`
- sqlite_minor=`expr $sqlite_version : '[[0-9]]*\.\([[0-9]]*\)'`
- sqlite_micro=`expr $sqlite_version : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
- if test -z "$sqlite_micro"; then
- sqlite_micro=0
+ version_string="$1"
+
+ major=`expr $version_string : '\([[0-9]]*\)'`
+ minor=`expr $version_string : '[[0-9]]*\.\([[0-9]]*\)'`
+ micro=`expr $version_string : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
+ if test -z "$micro"; then
+ micro=0
fi
- sqlite_ver_num=`expr $sqlite_major \* 1000000 \
- \+ $sqlite_minor \* 1000 \
- \+ $sqlite_micro`
+ $2=`expr $major \* 1000000 \
+ \+ $minor \* 1000 \
+ \+ $micro`
])
dnl SVN_SQLITE_MIN_VERNUM_PARSE()
@@ -220,12 +222,7 @@ dnl Parse a x.y.z version string SQLITE_MINIMUM_VE
dnl sqlite_min_ver_num.
AC_DEFUN(SVN_SQLITE_MIN_VERNUM_PARSE,
[
- sqlite_min_major=`expr $SQLITE_MINIMUM_VER : '\([[0-9]]*\)'`
- sqlite_min_minor=`expr $SQLITE_MINIMUM_VER : '[[0-9]]*\.\([[0-9]]*\)'`
- sqlite_min_micro=`expr $SQLITE_MINIMUM_VER : '[[0-9]]*\.[[0-9]]*\.\([[0-9]]*\)'`
- sqlite_min_ver_num=`expr $sqlite_min_major \* 1000000 \
- \+ $sqlite_min_minor \* 1000 \
- \+ $sqlite_min_micro`
+ SVN_SQLITE_VERNUM_PARSE([$SQLITE_MINIMUM_VER], [sqlite_min_ver_num])
])
dnl SVN_DOWNLOAD_SQLITE()
Re: [PATCH] Make SQLite compatibility check less picky
Posted by Jonathan Nieder <jr...@gmail.com>.
Stefan Sperling wrote:
> OK, so in this case I would prefer if distros patched Subversion accordingly.
If packagers for other binary-based distros (e.g., OpenSUSE, Fedora,
Cygwin) do not also want a change like this, then it seems sane enough
to keep it a private patch.
On the other hand, if there is interest, I would be happy to provide a
./configure option for affected distros to use to avoid the version
checks in a non-invasive way.
Re: [PATCH] Make SQLite compatibility check less picky
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Oct 29, 2011 at 07:28:07AM -0500, Jonathan Nieder wrote:
> Daniel Shahaf wrote:
> > On Saturday, October 29, 2011 6:45 AM, "Jonathan Nieder" <jr...@gmail.com> wrote:
>
> >> The actual ABI compatibility of sqlite3 doesn't depend on the patchlevel
> >> (the x in 3.7.x),
> >
> > [citation needed]
> >
> > (I already searched sqlite.org for "ABI" and "binary compatibility" and so on; zero matches)
>
> Sloppy me. In fact, if I had only looked at the changelog, I would
> have found many cases of introduction of new ABI in 3.7.x versions.
>
> In Debian, that is already handled by checking at build time which
> version introduced each function that is used and setting appropriate
> dependencies through the package manager, fortunately, so as a
> distro-specific patch it was not doing much harm. A more useful and
> less misleading patch would involve doing something like the
> following. Sorry for the nonsense, and thanks very much for catching
> it quickly.
OK, so in this case I would prefer if distros patched Subversion accordingly.
The Subversion project already recommends appropriate sqlite versions.
If distros like Debian want to play by different rules then let's have
them take the burden.
(I seem to recall that Debian is using rather relaxed rules for shared
library versioning, since they cannot afford to recompile every package
all the time.)
>
> Index: subversion/libsvn_subr/sqlite.c
> ===================================================================
> --- subversion/libsvn_subr/sqlite.c (revision 1194866)
> +++ subversion/libsvn_subr/sqlite.c (working copy)
> @@ -606,6 +606,7 @@
> static svn_error_t *
> init_sqlite(void *baton, apr_pool_t *pool)
> {
> +#if !defined(PACKAGE_MANAGER_CHECKED_THE_VERSION_NUMBER_ALREADY)
> if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER)
> {
> return svn_error_createf(
> @@ -613,6 +614,7 @@
> _("SQLite compiled for %s, but running with %s"),
> SQLITE_VERSION, sqlite3_libversion());
> }
> +#endif
>
> #if APR_HAS_THREADS
>
Re: [PATCH] Make SQLite compatibility check less picky
Posted by Jonathan Nieder <jr...@gmail.com>.
Daniel Shahaf wrote:
> On Saturday, October 29, 2011 6:45 AM, "Jonathan Nieder" <jr...@gmail.com> wrote:
>> The actual ABI compatibility of sqlite3 doesn't depend on the patchlevel
>> (the x in 3.7.x),
>
> [citation needed]
>
> (I already searched sqlite.org for "ABI" and "binary compatibility" and so on; zero matches)
Sloppy me. In fact, if I had only looked at the changelog, I would
have found many cases of introduction of new ABI in 3.7.x versions.
In Debian, that is already handled by checking at build time which
version introduced each function that is used and setting appropriate
dependencies through the package manager, fortunately, so as a
distro-specific patch it was not doing much harm. A more useful and
less misleading patch would involve doing something like the
following. Sorry for the nonsense, and thanks very much for catching
it quickly.
Index: subversion/libsvn_subr/sqlite.c
===================================================================
--- subversion/libsvn_subr/sqlite.c (revision 1194866)
+++ subversion/libsvn_subr/sqlite.c (working copy)
@@ -606,6 +606,7 @@
static svn_error_t *
init_sqlite(void *baton, apr_pool_t *pool)
{
+#if !defined(PACKAGE_MANAGER_CHECKED_THE_VERSION_NUMBER_ALREADY)
if (sqlite3_libversion_number() < SQLITE_VERSION_NUMBER)
{
return svn_error_createf(
@@ -613,6 +614,7 @@
_("SQLite compiled for %s, but running with %s"),
SQLITE_VERSION, sqlite3_libversion());
}
+#endif
#if APR_HAS_THREADS
Re: [PATCH] Make SQLite compatibility check less picky
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
On Saturday, October 29, 2011 6:45 AM, "Jonathan Nieder" <jr...@gmail.com> wrote:
> Hi,
>
> This patch is taken from Debian's subversion packaging. It avoids
> having to rebuild Subversion each time an older or newer patchlevel of
> SQLite gets installed (i.e., the x in 3.7.x changes). I'd appreciate
> any thoughts you have.
>
> Thanks,
> Jonathan
>
> [[[
> The actual ABI compatibility of sqlite3 doesn't depend on the patchlevel
> (the x in 3.7.x),
[citation needed]
(I already searched sqlite.org for "ABI" and "binary compatibility" and so on; zero matches)
> so stop being picky about the patchlevel when checking
> the version number at runtime. This avoids spurious errors of the form
> "svn: Couldn't perform atomic initialization" / "svn: SQLite compiled
> for 3.7.4, but running with 3.7.3" when sqlite gets a minor update
> without Subversion being rebuilt to match.
>
> @@ -772,7 +772,7 @@ svn_sqlite__open(svn_sqlite__db_t **db, const char
> */
> {
> int ignored_err = SQLITE_OK;
> -#if !SQLITE_VERSION_AT_LEAST(3,7,8) && defined(SQLITE_SCHEMA)
> +#if defined(SQLITE_SCHEMA)
> if (!strcmp(sqlite3_libversion(), "3.7.7"))
This seems to be an independent change. I assume you're guarding against a downgrade of libsqlite3.so?
> ignored_err = SQLITE_SCHEMA;
> #endif
>