You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Hudson <gh...@MIT.EDU> on 2004/05/10 14:42:02 UTC
Re: svn commit: r9664 - in trunk/subversion: clients/cmdline
include libsvn_client libsvn_subr
On Mon, 2004-05-10 at 02:53, Branko Čibej wrote:
> I committed a prototype implementation of version query functions.
> Please look at it, and if it's acceptable, I'll implement the rest of
> the functions and add version queries to all the command-line tools.
Uh, -1, unless I missed some design discussion on this. Adding a new
utility function here and there is fine, in my opinion, but adding a new
subsystem should really not happen without prior discussion.
I am +0 on adding functions to query a library's version. I am -0 on
adding machinery to check library version mismatches at runtime, because
I don't see a lot of potential for accidental mismatches and because I
haven't seen much of that machinery in other projects.
Specific comments on the implementation:
+#define SVN_VER_GEN_PROTO(name) \
+#define SVN_VER_GEN_IMPL(name) \
+#define SVN_VER_COMPATIBLE(name) \
+#define SVN_VER_CALLBACK_COMPATIBLE(name) \
I do not believe we should be using the preprocessor to this extent.
The preprocessor is not our friend. Suppose I run into a problem
involving svn_subr_version and I want to find out where it's defined? I
can't grep for it, because it was defined with SVN_VER_GEN_IMPL(subr).
+svn_boolean_t svn_ver_compatible (const svn_version_t *versioninfo,
+svn_boolean_t svn_ver_callback_compatible (const svn_version_t *versioninfo,
The asymmetry between the two version arguments (one specified as a
structure pointer, the other split out into four parameters) seems
weird.
What's the definition of "callback compatibility?" The implementation
seems to simply reverse the direction of the compatibility test. To the
best of my knowledge, we treat changing the signature of a callback as
if it were changing the signature of any API which uses that callback,
so we have no concept of "callback compatibility."
Compatibility is directional, but the docstring for svn_ver_compatible
does not say which direction it checks compatibility in.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9664 - in trunk/subversion: clients/cmdline include
libsvn_client libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
>Branko Čibej <br...@xbc.nu> writes:
>
>
>>O.K., just as long as you're all aware that there will be a /lot/ of them.
>>
>>
>
>One for each library? That doesn't seem like a lot, but maybe it's
>just a matter of opinion.
>
>
Indeed it is...
>Note that I'm not objecting to using a macro to generate the bodies of
>the implementations.
>
Ah! O.K., I think we've reached a reasonable compromise. I'll do it this
way, and that still lets me fix the svn_ver_compatible signature in a
nice way.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9664 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_subr
Posted by kf...@collab.net.
Branko Čibej <br...@xbc.nu> writes:
> O.K., just as long as you're all aware that there will be a /lot/ of them.
One for each library? That doesn't seem like a lot, but maybe it's
just a matter of opinion.
Note that I'm not objecting to using a macro to generate the bodies of
the implementations. That would affect tags-searching slightly, but
not in a very bad way. It's not nearly as severe as having the names
themselves be obscured.
#define SVN_VERSION_BODY \
static const svn_version_t versioninfo = \
{ \
SVN_VER_MAJOR, \
SVN_VER_MINOR, \
SVN_VER_MICRO, \
SVN_VER_NUMTAG \
}; \
return &versioninfo
const svn_version_t *svn_foo_version (void)
{
SVN_VERSION_BODY;
}
const svn_version_t *svn_bar_version (void)
{
SVN_VERSION_BODY;
}
...or something.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9664 - in trunk/subversion: clients/cmdline include
libsvn_client libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
>These sorts of preprocessor splices really mess up tags tables, too
>(this has burned me more than once in APR). I'd also prefer just to
>list the functions by name.
>
>
O.K., just as long as you're all aware that there will be a /lot/ of them.
>>>The asymmetry between the two version arguments (one specified as a
>>>structure pointer, the other split out into four parameters) seems
>>>weird.
>>>
>>>
>>Yes, it does. It's there because I didn't see any other way to
>>generate the shorthand macros. I do see a better way now, and will
>>change those prototypes.
>>
>>
>
>Now, when the macro declarations are actually affecting the
>prototypes, that's a sign... :-)
>
>
All right already, you don't have to rub it in... :-\
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9664 - in trunk/subversion: clients/cmdline include
libsvn_client libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Greg Stein wrote:
>There is one per library. That will get to be very painful, and will be
>more prone to error. I certainly don't see the manual writing of these
>things as outweighing the need for tag lookups. But that's just me :-)
>
>
Ooh, I can soo agree with this. Shall we vote, then? :-)
>Regarding runtime checks: as Branko pointed out, we do it already. Though
>I would recommend a *query* function from a library, rather than a check.
>
>
But that's exactly what I proposed. The query is in the library, the
check is the library user's responsiblity.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9664 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_subr
Posted by Greg Stein <gs...@lyra.org>.
On Mon, May 10, 2004 at 02:11:55PM -0500, kfogel@collab.net wrote:
> Branko ?ibej <br...@xbc.nu> writes:
> > >+#define SVN_VER_GEN_PROTO(name) \
> > >+#define SVN_VER_GEN_IMPL(name) \
> > >+#define SVN_VER_COMPATIBLE(name) \
> > >+#define SVN_VER_CALLBACK_COMPATIBLE(name) \
> > >
> > > I do not believe we should be using the preprocessor to this
> > > extent. The preprocessor is not our friend. Suppose I run into a
> > > problem
> > >involving svn_subr_version and I want to find out where it's defined? I
> > >can't grep for it, because it was defined with SVN_VER_GEN_IMPL(subr).
> >
> > I agree this could be a problem, even though GCC at least will point
> > to the macro definition, and GDB will single-step through it. On the
> > other hand, the svn_*_version functions are so simple that I'm
> > prepared to formally prove their correctness, and I decided on the
> > macro approach because we're going to have many of them, and
> > definitely want them all to be the same.
Agreed. For these "brain-dead" types of functions which are built
according to a very specific pattern, then I think auto-generation is just
fine. If people don't like preprocessor templates, then we can always have
gen-make spit out a .c file into each library...
<duck>
> These sorts of preprocessor splices really mess up tags tables, too
> (this has burned me more than once in APR). I'd also prefer just to
> list the functions by name.
There is one per library. That will get to be very painful, and will be
more prone to error. I certainly don't see the manual writing of these
things as outweighing the need for tag lookups. But that's just me :-)
Regarding runtime checks: as Branko pointed out, we do it already. Though
I would recommend a *query* function from a library, rather than a check.
That will let the calling application decide what it wants to do. For
example, the caller might dynamically look up symbols, and it wants the
library's version number to know what is available for lookup. If you only
have a "check", then the caller cannot properly alter its behavior. It
would have to do a bunch of probes to determine what it is talking to.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9664 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_subr
Posted by kf...@collab.net.
Branko Čibej <br...@xbc.nu> writes:
> >+#define SVN_VER_GEN_PROTO(name) \
> >+#define SVN_VER_GEN_IMPL(name) \
> >+#define SVN_VER_COMPATIBLE(name) \
> >+#define SVN_VER_CALLBACK_COMPATIBLE(name) \
> >
> > I do not believe we should be using the preprocessor to this
> > extent. The preprocessor is not our friend. Suppose I run into a
> > problem
> >involving svn_subr_version and I want to find out where it's defined? I
> >can't grep for it, because it was defined with SVN_VER_GEN_IMPL(subr).
> >
> I agree this could be a problem, even though GCC at least will point
> to the macro definition, and GDB will single-step through it. On the
> other hand, the svn_*_version functions are so simple that I'm
> prepared to formally prove their correctness, and I decided on the
> macro approach because we're going to have many of them, and
> definitely want them all to be the same.
These sorts of preprocessor splices really mess up tags tables, too
(this has burned me more than once in APR). I'd also prefer just to
list the functions by name.
> >The asymmetry between the two version arguments (one specified as a
> >structure pointer, the other split out into four parameters) seems
> >weird.
>
> Yes, it does. It's there because I didn't see any other way to
> generate the shorthand macros. I do see a better way now, and will
> change those prototypes.
Now, when the macro declarations are actually affecting the
prototypes, that's a sign... :-)
I had exactly the same thought about the single param vs four separate
params.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9664 - in trunk/subversion: clients/cmdline include
libsvn_client libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:
>On Mon, 2004-05-10 at 02:53, Branko Čibej wrote:
>
>
>>I committed a prototype implementation of version query functions.
>>Please look at it, and if it's acceptable, I'll implement the rest of
>>the functions and add version queries to all the command-line tools.
>>
>>
>
>Uh, -1, unless I missed some design discussion on this.
>
The preliminary discussion happened mostly on IRC, I think, but I recall
some requests to users@ about getting the library version at runtime,
and also a comment in svn_version.h about that. Anyway, my commit was
intended to start this discussion. Since the commit is fairly small, I
thought it would be easier to start with working code and take it from
there. :-)
>Adding a new
>utility function here and there is fine, in my opinion, but adding a new
>subsystem should really not happen without prior discussion.
>
>
"Review after commit," and the old-boy network and stuff. :-)
>I am +0 on adding functions to query a library's version.
>
Cool.
>I am -0 on
>adding machinery to check library version mismatches at runtime, because
>I don't see a lot of potential for accidental mismatches and because I
>haven't seen much of that machinery in other projects.
>
>
* Lots of users have been burned by this, and got obscure error
messages instead of what they'de get with this scheme.
* We check the BDB version at runtime.
* We check RA and FS(A)P versions at runtime.
>Specific comments on the implementation:
>
>+#define SVN_VER_GEN_PROTO(name) \
>+#define SVN_VER_GEN_IMPL(name) \
>+#define SVN_VER_COMPATIBLE(name) \
>+#define SVN_VER_CALLBACK_COMPATIBLE(name) \
>
>I do not believe we should be using the preprocessor to this extent.
>The preprocessor is not our friend. Suppose I run into a problem
>involving svn_subr_version and I want to find out where it's defined? I
>can't grep for it, because it was defined with SVN_VER_GEN_IMPL(subr).
>
>
I agree this could be a problem, even though GCC at least will point to
the macro definition, and GDB will single-step through it. On the other
hand, the svn_*_version functions are so simple that I'm prepared to
formally prove their correctness, and I decided on the macro approach
because we're going to have many of them, and definitely want them all
to be the same.
I'd be prepared to reconsider if problems show up in SWIG and/or Doxygen.
>+svn_boolean_t svn_ver_compatible (const svn_version_t *versioninfo,
>+svn_boolean_t svn_ver_callback_compatible (const svn_version_t *versioninfo,
>
>The asymmetry between the two version arguments (one specified as a
>structure pointer, the other split out into four parameters) seems
>weird.
>
>
Yes, it does. It's there because I didn't see any other way to generate
the shorthand macros. I do see a better way now, and will change those
prototypes.
>What's the definition of "callback compatibility?" The implementation
>seems to simply reverse the direction of the compatibility test. To the
>best of my knowledge, we treat changing the signature of a callback as
>if it were changing the signature of any API which uses that callback,
>so we have no concept of "callback compatibility."
>
>
This is one of the things I wanted to discuss, as it hinges on the
interpretation of our versioning rules. We have a number of callback
functions that the user of a library has to register in many batons. If
our versioning guarantees cover such callbacks in the same "direction"
as ordinary calls, then the callback-compatibility check can go away --
I'd like to see that happen, too. But I need a second opinion regarding
the interpretation of the versioning guarantees.
>Compatibility is directional, but the docstring for svn_ver_compatible
>does not say which direction it checks compatibility in.
>
>
Hm? Yes it does; "for calls into the library", and callback
compatibility is "for callbacks from the library", that is, in the other
direction.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
callback compat (was: svn commit: r9664 ...)
Posted by Greg Stein <gs...@lyra.org>.
On Mon, May 10, 2004 at 10:42:02AM -0400, Greg Hudson wrote:
>...
> What's the definition of "callback compatibility?" The implementation
> seems to simply reverse the direction of the compatibility test. To the
> best of my knowledge, we treat changing the signature of a callback as
> if it were changing the signature of any API which uses that callback,
> so we have no concept of "callback compatibility."
Correct.
The versioning guidelines are all about, "what happens when I update this
library? will my code continue to work? what do I need to do to make it
work post-upgrade?" So a callback signature cannot change, except across
major API upgrades of the library. Across minor upgrades, you can *add*
callbacks.
Consider that you have to get a callback *into* the library. Thus,
somewhere in the library's API is a signature holding the callback.
Changing the signature on the callback implies a signature change on the
function receiving the callback. Thus, you have the answer to how the
versioning applies here.
I hope that was clear enough.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org