You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Michael W Thelen <th...@cs.utah.edu> on 2004/07/02 07:00:35 UTC

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

* Edmund Horner <ch...@chrysophylax.cjb.net> [2004-06-14 05:40]:
> In summary: thanks for the hints, I'll go ahead and redo the patch then.
>  You should hear from me again tomorrow or so.

It's been a little while... Edmund, have you been able to work on this
patch?  If you haven't been able to find time, I'm willing to help out
if you want it.

-- Mike

-- 
Michael W. Thelen
A satirist is a man who discovers unpleasant things about himself and then
says them about other people.
                -- Peter McArthur

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

Posted by Branko Čibej <br...@xbc.nu>.
Edmund Horner wrote:

> Greg Hudson wrote:
>
>> On Mon, 2004-07-05 at 02:01, Edmund Horner wrote:
>>
>>> However I have added svn_libname__version() functions for the plugin 
>>> libraries.
>>
>>
>> Pointless and messy.  Until such a time as the vtable work is done to
>> make it possible to query an FS or RA module's version after loading it,
>> don't add any symbols like this.
>>
>> (There's no urgency to doing this work, since the RA/FS ABI version will
>> serve in the meantime.)
>
>
> Righty ho.
>
> I have removed the svn_libname__version() functions from svn_fs_base, 
> svn_fs_fs, svn_ra_local, svn_ra_dav.
>
> The function svn_ra_svn_version() function remains, since it is 
> checked by svnserve/main.c.  Does that sound ok?
>
> Regards, etc.
> Edmund.
>
>
>
> Issue 1861: Implement library version functions, and check library
> versions in command-line clients.  Each library features a
> "svn_libname_version" function that returns information on the
> Subversion version the library is compiled as.  Clients call their own
> "check_lib_versions" (or a similarly named) function that uses
> svn_ver_check_list (from libsvn_subr/version.c) to call the version
> functions of a list of libraries, which ensures there are no version
> mismatches.
>
> This change is a generalisation of r9697 to other libraries and
> clients.
>
> * subversion/libsvn_delta/version.c (svn_delta_version),
>   subversion/libsvn_ra_svn/version.c (svn_ra_svn_version):
>     New files, with functions for returning library versions.
>
> * subversion/libsvn_diff/util.c (svn_diff_version),
>   subversion/libsvn_fs/fs-loader.c (svn_fs_version),
>   subversion/libsvn_ra/ra_loader.c (svn_ra_version),
>   subversion/libsvn_repos/repos.c (svn_repos_version),
>   subversion/libsvn_wc/util.c (svn_wc_version):
>     New functions for returning library versions.
>
> * 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/include/svn_ra.h (svn_ra_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.
>
> * subversion/svnadmin/main.c,
>   subversion/svnlook/main.c,
>   subversion/svndumpfilter/main.c,
>   subversion/svnserve/main.c,
>   subversion/svnversion/main.c,
>   contrib/client-side/svn-push/svn-push.c:
>     (check_lib_versions): New function that checks the compiled
>         versions of all svn libraries needed by this client.
>     (main): Call check_lib_versions() and exit with failure if library
>         versions are not adequate.
>
> * subversion/clients/cmdline/main.c:
>     (check_lib_versions): Add remaining library version checks.

Committed in r10167, with minor tweaks (just a few docstrings). Thanks!

Off to the vtables now...

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2004-07-06 at 08:03, Branko Čibej wrote:
> Edmund Horner wrote:
> > The function svn_ra_svn_version() function remains, since it is 
> > checked by svnserve/main.c.  Does that sound ok?

That's fine.

> Um. I said in my previous post that I disagree. I think these functions 
> should go in, and so should the version check changes in the vtables. 
> And there's no need to change the vtables first.

There's certainly no reason they should be library-exported symbols,
outside of ra_svn.  They can be defined as static functions in the same
place as the vtable is defined.

Ergo, it's pretty silly to put them in without also extending the
vtable.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


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

Posted by Edmund Horner <ch...@chrysophylax.cjb.net>.
Greg Hudson wrote:

 > On Wed, 2004-07-07 at 01:09, Edmund Horner wrote:
 >
 >>OK.  So shall I go ahead and add the private svn_pluginname__version()
 >>functions, and also append them to the vtable?  (I take it the nicest
 >>place to add them is at the end.)
 >
 >
 > Slightly more architecture required.  But I think Branko volunteered to
 > work on this part.

Yup, I'll leave it all with him then.

 > In general, static functions do not need to have any special prefix, so
 > it makes me nervous to see you continuing to refer to the functions as
 > having svn_pluginname__ prefixes.

Ah, right.  Sorry 'bout that.




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Greg Hudson <gh...@MIT.EDU>.
On Wed, 2004-07-07 at 01:09, Edmund Horner wrote:
> OK.  So shall I go ahead and add the private svn_pluginname__version()
> functions, and also append them to the vtable?  (I take it the nicest
> place to add them is at the end.)

Slightly more architecture required.  But I think Branko volunteered to
work on this part.

In general, static functions do not need to have any special prefix, so
it makes me nervous to see you continuing to refer to the functions as
having svn_pluginname__ prefixes.

> Also: can the public svn_ra_svn_version() function -- checked by
> svnserve -- be used in the vtable for ra_svn?

If the vtable entry can have the same function signature, then sure.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Edmund Horner <ch...@chrysophylax.cjb.net>.
Greg Hudson wrote:
 > On Tue, 2004-07-06 at 17:35, Branko Čibej wrote:
 >
 >>Mm, looks like we have a bit of a misunderstanding there. I thought you
 >>were against putting version queries in the vtables, when you're
 >>actually just objecting to exporting them from the libraries. Is that
 >>correct? If so, I can definitely go along with that.
 >
 >
 > Right.  Adding them to the vtables is fine.  Adding them to the module
 > libraries without doing the vtable work is confusing and pointless;
 > exporting them unnecessarily is also pointless.

OK.  So shall I go ahead and add the private svn_pluginname__version()
functions, and also append them to the vtable?  (I take it the nicest
place to add them is at the end.)

Also: can the public svn_ra_svn_version() function -- checked by
svnserve -- be used in the vtable for ra_svn?

I did consider adding some code for checking the versions of plugins to
lisvn_fs and libsvn_ra, but TBH I'm not confident to do that at this stage.

Edmund.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2004-07-06 at 17:35, Branko Čibej wrote:
> Mm, looks like we have a bit of a misunderstanding there. I thought you 
> were against putting version queries in the vtables, when you're 
> actually just objecting to exporting them from the libraries. Is that 
> correct? If so, I can definitely go along with that.

Right.  Adding them to the vtables is fine.  Adding them to the module
libraries without doing the vtable work is confusing and pointless;
exporting them unnecessarily is also pointless.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


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

Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:

>On Tue, 2004-07-06 at 09:58, Branko Čibej wrote:
>  
>
>>It was for me. Greg Hudson and I disagree, as usual -- you simply got 
>>caught in the crossfire. :-)
>>If he doesn't actually veto your second patch, I'll commit it as is. 
>>Then I'll start work on the vtables.
>>    
>>
>
>I am, in fact, -1 on pointlessly exporting version queries from RA and
>FS modules.
>
>(It's certainly pointful to export it from libsvn_ra_svn, so that's
>fine.  And if there were some reason to define the version function in a
>different place from the vtable, then it would make sense.  But as far
>as I know, the most sensible implementation of these functions is in a
>static function alongside the vtable definition.)
>
Mm, looks like we have a bit of a misunderstanding there. I thought you 
were against putting version queries in the vtables, when you're 
actually just objecting to exporting them from the libraries. Is that 
correct? If so, I can definitely go along with that.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2004-07-06 at 09:58, Branko Čibej wrote:
> It was for me. Greg Hudson and I disagree, as usual -- you simply got 
> caught in the crossfire. :-)
> If he doesn't actually veto your second patch, I'll commit it as is. 
> Then I'll start work on the vtables.

I am, in fact, -1 on pointlessly exporting version queries from RA and
FS modules.

(It's certainly pointful to export it from libsvn_ra_svn, so that's
fine.  And if there were some reason to define the version function in a
different place from the vtable, then it would make sense.  But as far
as I know, the most sensible implementation of these functions is in a
static function alongside the vtable definition.)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


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

Posted by Branko Čibej <br...@xbc.nu>.
Edmund Horner wrote:

> Branko Čibej wrote:
> > Edmund Horner wrote:
> >
> >> Greg Hudson wrote:
> >>
> >>> On Mon, 2004-07-05 at 02:01, Edmund Horner wrote:
> >>>
> >>>> However I have added svn_libname__version() functions for the plugin
> >>>> libraries.
> >>>
> >>> Pointless and messy.  Until such a time as the vtable work is done to
> >>> make it possible to query an FS or RA module's version after 
> loading it,
> >>> don't add any symbols like this.
> >>>
> >>> (There's no urgency to doing this work, since the RA/FS ABI 
> version will
> >>> serve in the meantime.)
> >>
> >> Righty ho.
> >>
> >> I have removed the svn_libname__version() functions from svn_fs_base,
> >> svn_fs_fs, svn_ra_local, svn_ra_dav.
> >>
> >> The function svn_ra_svn_version() function remains, since it is
> >> checked by svnserve/main.c.  Does that sound ok?
> >
> >
> > Um. I said in my previous post that I disagree. I think these functions
> > should go in, and so should the version check changes in the vtables.
> > And there's no need to change the vtables first.
> >
> > -- Brane
>
> Okay, I'm a little confused about what needs to be done.  Does this mean
> the second patch was (relatively) acceptable, even with the plugin 
> version functions not in the vtables?

It was for me. Greg Hudson and I disagree, as usual -- you simply got 
caught in the crossfire. :-)
If he doesn't actually veto your second patch, I'll commit it as is. 
Then I'll start work on the vtables.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Edmund Horner <ch...@chrysophylax.cjb.net>.
Branko Čibej wrote:
 > Edmund Horner wrote:
 >
 >> Greg Hudson wrote:
 >>
 >>> On Mon, 2004-07-05 at 02:01, Edmund Horner wrote:
 >>>
 >>>> However I have added svn_libname__version() functions for the plugin
 >>>> libraries.
 >>>
 >>> Pointless and messy.  Until such a time as the vtable work is done to
 >>> make it possible to query an FS or RA module's version after 
loading it,
 >>> don't add any symbols like this.
 >>>
 >>> (There's no urgency to doing this work, since the RA/FS ABI version 
will
 >>> serve in the meantime.)
 >>
 >> Righty ho.
 >>
 >> I have removed the svn_libname__version() functions from svn_fs_base,
 >> svn_fs_fs, svn_ra_local, svn_ra_dav.
 >>
 >> The function svn_ra_svn_version() function remains, since it is
 >> checked by svnserve/main.c.  Does that sound ok?
 >
 >
 > Um. I said in my previous post that I disagree. I think these functions
 > should go in, and so should the version check changes in the vtables.
 > And there's no need to change the vtables first.
 >
 > -- Brane

Okay, I'm a little confused about what needs to be done.  Does this mean
the second patch was (relatively) acceptable, even with the plugin 
version functions not in the vtables?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Branko Čibej <br...@xbc.nu>.
Edmund Horner wrote:

> Greg Hudson wrote:
>
>> On Mon, 2004-07-05 at 02:01, Edmund Horner wrote:
>>
>>> However I have added svn_libname__version() functions for the plugin 
>>> libraries.
>>
>>
>> Pointless and messy.  Until such a time as the vtable work is done to
>> make it possible to query an FS or RA module's version after loading it,
>> don't add any symbols like this.
>>
>> (There's no urgency to doing this work, since the RA/FS ABI version will
>> serve in the meantime.)
>
>
> Righty ho.
>
> I have removed the svn_libname__version() functions from svn_fs_base, 
> svn_fs_fs, svn_ra_local, svn_ra_dav.
>
> The function svn_ra_svn_version() function remains, since it is 
> checked by svnserve/main.c.  Does that sound ok?

Um. I said in my previous post that I disagree. I think these functions 
should go in, and so should the version check changes in the vtables. 
And there's no need to change the vtables first.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Edmund Horner <ch...@chrysophylax.cjb.net>.
Greg Hudson wrote:
> On Mon, 2004-07-05 at 02:01, Edmund Horner wrote:
> 
>>However I have added svn_libname__version() functions 
>>for the plugin libraries.
> 
> Pointless and messy.  Until such a time as the vtable work is done to
> make it possible to query an FS or RA module's version after loading it,
> don't add any symbols like this.
> 
> (There's no urgency to doing this work, since the RA/FS ABI version will
> serve in the meantime.)

Righty ho.

I have removed the svn_libname__version() functions from svn_fs_base, 
svn_fs_fs, svn_ra_local, svn_ra_dav.

The function svn_ra_svn_version() function remains, since it is checked 
by svnserve/main.c.  Does that sound ok?

Regards, etc.
Edmund.



Issue 1861: Implement library version functions, and check library
versions in command-line clients.  Each library features a
"svn_libname_version" function that returns information on the
Subversion version the library is compiled as.  Clients call their own
"check_lib_versions" (or a similarly named) function that uses
svn_ver_check_list (from libsvn_subr/version.c) to call the version
functions of a list of libraries, which ensures there are no version
mismatches.

This change is a generalisation of r9697 to other libraries and
clients.

* subversion/libsvn_delta/version.c (svn_delta_version),
   subversion/libsvn_ra_svn/version.c (svn_ra_svn_version):
     New files, with functions for returning library versions.

* subversion/libsvn_diff/util.c (svn_diff_version),
   subversion/libsvn_fs/fs-loader.c (svn_fs_version),
   subversion/libsvn_ra/ra_loader.c (svn_ra_version),
   subversion/libsvn_repos/repos.c (svn_repos_version),
   subversion/libsvn_wc/util.c (svn_wc_version):
     New functions for returning library versions.

* 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/include/svn_ra.h (svn_ra_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.

* subversion/svnadmin/main.c,
   subversion/svnlook/main.c,
   subversion/svndumpfilter/main.c,
   subversion/svnserve/main.c,
   subversion/svnversion/main.c,
   contrib/client-side/svn-push/svn-push.c:
     (check_lib_versions): New function that checks the compiled
         versions of all svn libraries needed by this client.
     (main): Call check_lib_versions() and exit with failure if library
         versions are not adequate.

* subversion/clients/cmdline/main.c:
     (check_lib_versions): Add remaining library version checks.



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

Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:

>On Mon, 2004-07-05 at 02:01, Edmund Horner wrote:
>  
>
>>However I have added svn_libname__version() functions 
>>for the plugin libraries.
>>    
>>
>
>Pointless and messy.  Until such a time as the vtable work is done to
>make it possible to query an FS or RA module's version after loading it,
>don't add any symbols like this.
>
>(There's no urgency to doing this work, since the RA/FS ABI version will
>serve in the meantime.)
>  
>
In fact, the RA ABI version has already been bumped for 1.1, I believe, 
so it would be a really good idea at this point to deprecate the ABI 
version and use the version checks instead. The FS loader's ABI version 
hasn't appeared in any SVN release yet, so I'd vote for ditching it 
completely in favour of version checks.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2004-07-05 at 02:01, Edmund Horner wrote:
> However I have added svn_libname__version() functions 
> for the plugin libraries.

Pointless and messy.  Until such a time as the vtable work is done to
make it possible to query an FS or RA module's version after loading it,
don't add any symbols like this.

(There's no urgency to doing this work, since the RA/FS ABI version will
serve in the meantime.)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Edmund Horner <ch...@chrysophylax.cjb.net>.
Michael W Thelen wrote:
> * Edmund Horner <ch...@chrysophylax.cjb.net> [2004-06-14 05:40]:
>> You should hear from me again tomorrow or so.
> 
> It's been a little while... Edmund, have you been able to work on this
> patch?  If you haven't been able to find time, I'm willing to help out
> if you want it.
> 
> -- Mike

(NB. I started sending this first with an incorrect patch attached, so 
if there's another copy of this mail without this little "NB" notice, 
ignore that one.)

Hi Mike, it sure has been a while!  (So much for hearing from me 
"tomorrow" ...)

Here's the second version of the patch.  I have not implemented anything 
in the RA/FS loaders for checking the versions of plugins; I began to, 
but it got out of hand and I thought the main version checks should be 
finished first.  However I have added svn_libname__version() functions 
for the plugin libraries.  I guess these should eventually be added to 
the vtables or something, and checked from the loaders...?

Proposed log message follows.

-- Edmund.



Implement library version functions, and check library versions in
command-line clients.  Each library features a "svn_libname_version"
function that returns information on the Subversion version the library
is compiled as.  Clients call their own "check_lib_versions" (or a
similarly named) function that uses svn_ver_check_list (from
libsvn_subr/version.c) to call the version functions of a list of
libraries, which ensures there are no version mismatches.

This change is a generalisation of r9697 to other libraries and
clients, and fixes issue 1861.

* subversion/libsvn_delta/version.c (svn_delta_version),
   subversion/libsvn_ra_svn/client.c (svn_ra_svn_version):
     New files, with functions for returning library versions.

* subversion/libsvn_delta/version.c (svn_delta_version),
   subversion/libsvn_diff/util.c (svn_diff_version),
   subversion/libsvn_fs/fs-loader.c (svn_fs_version),
   subversion/libsvn_fs_base/fs.c (svn_fs_base__version),
   subversion/libsvn_fs_fs/fs.c: (svn_fs_fs__version),
   subversion/libsvn_ra/ra_loader.c (svn_ra_version),
   subversion/libsvn_ra_dav/util.c (svn_ra_dav__version),
   subversion/libsvn_ra_local/ra_plugin.c (svn_ra_local__version),
   subversion/libsvn_repos/repos.c (svn_repos_version),
   subversion/libsvn_wc/util.c (svn_wc_version):
     New functions for returning library versions.

* 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),
   subversion/libsvn_fs_fs/fs.h (svn_fs_fs__version),
   subversion/include/svn_ra.h (svn_ra_version),
   subversion/libsvn_ra_dav/ra_dav.h (svn_ra_dav__version),
   subversion/libsvn_ra_local/ra_local.h (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.

* subversion/svnadmin/main.c,
   subversion/svnlook/main.c,
   subversion/svndumpfilter/main.c,
   subversion/svnserve/main.c,
   subversion/svnversion/main.c,
   contrib/client-side/svn-push/svn-push.c:
     (check_lib_versions): New function that checks the compiled
         versions of all svn libraries needed by this client.
     (main): Call check_lib_versions() and exit with failure if library
         versions are not adequate.

* subversion/clients/cmdline/main.c:
     (check_lib_versions): Add remaining library version checks.