Re: [PATCH] add version queries and version checks (issue 1861)

Edmund Horner wrote:

> Hi, sorry about disappearing for a while. Blame my tendency to
> procrastinate...
>
> I've "generalised" the version checking from libsvn_client/version.c
> and clients/cmdline/main.c into these libraries:
>
> svn_delta

[...]

> svn_wc
>
> Several second-level libraries (svn_fs_base, svn_fs_fs, svn_ra_dav,
> svn_ra_local, svn_ra_svn) don't have public include/svn_libname.h
> files so I've changed their private header files instead, although I
> expect this version checking mechanism needs to be altered for those
> cases, if it's warranted at all. Those changes might well be excluded
> from the patch.

If those headers don't declare public APIs, then we should make those
functions "private exported"; that is, instead of "svn _fs_fs_version",
you'd have "svn _fs_fs__version", and similariy for RA implementations,
unless they have a public API (AFAIK only ra_svn has one).

Then, of course, the RA and FS vtables need to be extended with a
version-check entry. I think that's another issue, though.

(Maybe the RA and FS loader should check the version number instead of
each having its own ABI version?)

> Altered "clients" include:
>
> svnlook
> svndumpfilter
> svnadmin
> svnserve
> svnversion
> svn-push (because it was there)

You also need to add the missing checks to the svn client.

> * subversion/libsvn_delta/version.c
> * subversion/libsvn_diff/version.c
> * subversion/libsvn_fs/version.c
> * subversion/libsvn_fs_base/version.c
> * subversion/libsvn_fs_fs/version.c
> * subversion/libsvn_ra/version.c
> * subversion/libsvn_ra_dav/version.c
> * subversion/libsvn_ra_local/version.c
> * subversion/libsvn_ra_svn/version.c
> * subversion/libsvn_repos/version.c
> * subversion/libsvn_wc/version.c:
> New files.

If the library already has an appropriate file (e.g., util.c) then the
version check can be defined there. There's no need for new files all
over the place.

>
> * subversion/include/svn_delta.h (svn_delta_version)
> * subversion/include/svn_diff.h (svn_diff_version)
> * subversion/include/svn_fs.h (svn_fs_version)
> * subversion/libsvn_fs_base/fs.h (svn_fs_base_version)

Should be svn_fs_base__version

> * subversion/libsvn_fs_fs/fs.h (svn_fs_fs_version)

Should be svn_fs_fs__version

> * subversion/include/svn_ra.h (svn_ra_version)
> * subversion/libsvn_ra_dav/ra_dav.h (svn_ra_dav_version)

Should be svn_ra_dav__version

> * subversion/libsvn_ra_local/ra_local.h (svn_ra_local_version)

Should be svn_ra_local__version

> * subversion/include/svn_ra_svn.h (svn_ra_svn_version)
> * subversion/include/svn_repos.h (svn_repos_version)
> * subversion/include/svn_wc.h (svn_wc_version):
> New function symbols.

(by the way, the canonical GNU-ish way to list several files in a single
log entry is:

    * file1 (symbol2),
      file2 (symbol2): log

That way, the asterisk in column 1 signals the start of a log message).

>Property changes on: subversion/libsvn_fs_base/version.c
>___________________________________________________________________
>Name: svn:keywords
> + Id
>
>
We don't add keyword expansion to source files.

The rest looks fine. Thanks for working on this!

-- 
Brane Čibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org