You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Alexander L. Belikoff" <ab...@vallinor4.com> on 2004/09/15 14:01:56 UTC
Re: PATCH: $Header$ keyword support (now with proper log message)
I'm sorry for not chiming in earlier (was a bit busy), but could anyone
explain to me the public nature of this particular structure? I am not sure I
see where this structure is being passed between the client and the server.
Because if it is not, then there should hardly be any problem (except for the
case where headers are inconsistent with the libraries, which is a major
screw-up anyway).
My intention for this patch was to make it as backward compatible as possible.
Please let me know if I am missing something here...
On Monday 26 July 2004 18:36, Garrett Rooney wrote:
> kfogel@collab.net wrote:
> > Tobias Ringström <to...@ringstrom.mine.nu> writes:
> >>>Index: subversion/include/svn_subst.h
> >>>===================================================================
> >>>- --- subversion/include/svn_subst.h (revision 10373)
> >>>+++ subversion/include/svn_subst.h (working copy)
> >>>@@ -83,6 +83,7 @@
> >>> const svn_string_t *author;
> >>> const svn_string_t *url;
> >>> const svn_string_t *id;
> >>>+ const svn_string_t *header;
> >>>} svn_subst_keywords_t;
> >>
> >>Incompatible changes to the API such as this one cannot be applied
> >>before the next major version (2.x). If you want this change to be
> >>acceptable for 1.x, you must create a new type, svn_subst_keywords2_t
> >>and also make new versions of the public functions using the type,
> >>e.g. you need to create svn_subst_build_keywords2 that takes
> >>svn_subst_keywords2_t arguments, etc. The old functions should then be
> >>marked as deprecated. I'm afraid that this will complicate the patch a
> >>lot. :-(
> >
> > Why is adding a member to the end of a structure incompatible? Maybe
> > there are other incompatible things about the patch, but this
> > particular change seems compatible to me.
>
> It's a public structure definition, you can't change the size by adding
> a new field because people might be depending on the size, declaring
> them on the stack, etc.
>
> -garrett
--
Alexander L. Belikoff
PGP/GPG fingerprint: 0D58 A804 1AB1 4CD8 8DA9 424B A86E CD0D 8424 2701
(http://pgp5.ai.mit.edu for the key)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by John Peacock <jp...@rowman.com>.
Alexander L. Belikoff wrote:
> - Wait for the announced 'generic' way to expand keywords sometime in the
> future.
The keyword-as-hash patch is a much better way to handle this
(structures are so 1990 ;), which is of course my bias seeing that I
rewrote it in it's current form:
http://subversion.tigris.org/issues/show_bug.cgi?id=890
The thing holding it back right now is that it doesn't do anything
_else_ useful without the ability to have repository-wide definitions of
custom keywords. That and needing a way to handle error callbacks from
the client if the properties-as-keywords portion gets added, too...
> - Stage a coup and get the aforementioned structure out of the public API
That's a 2.0 question, and I would strongly disagree with replacing the
current fixed size structure with a slightly larger fixed size
structure... :(
John
--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5748
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by "Alexander L. Belikoff" <ab...@vallinor4.com>.
On Wednesday 15 September 2004 11:33, Folker Schamel wrote:
> If you upgrade svn, you would have to recompiler ALL applications
> using the svn lib.
Well, that's the beauty of the non-finalized API. ;-) I think the way they
solve it in Windows is by allowing each app using such an API to redistribute
the dll (assuming that the functionality is compatible of course). Not sure
whether this the best thing. I'd rather use the API versioning, where each
'public' data structure has a version associated with it which allows the API
implementation to handle the structures passed by the caller according to the
version of the structure. This is complicated and mostly used in the
client-server systems with multiple distributed clients that must withstand
server upgrades.
OK, you've convinced me that the way I implemented it is not good for 1.x.
Which kind of sucks - after all my change is supposed to be strictly
internal. Maybe this structure shouldn't be public in the first place?
Now, what are my options WRT getting this feature into SVN? IMHO, this feature
is useful enough to be implemented as enhancement (let alone for backward
compatibility). So far I see only four options:
- Wait till the post 1.x development and try my luck again.
- Implement the ugly svn_keywords2_t structure and a parallel set of the API
(blech) which according to the current rules would become a *public* API as
well, which scares the living shit our of me.
- Wait for the announced 'generic' way to expand keywords sometime in the
future.
- Stage a coup and get the aforementioned structure out of the public API
saving us a lot of headache in the future at the expense of some headache for
the current SVN-based app builders ;-) (how many such apps are there anyway?
How many of them use this data structure?)
Suggestions?
--
Alexander L. Belikoff
PGP/GPG fingerprint: 0D58 A804 1AB1 4CD8 8DA9 424B A86E CD0D 8424 2701
(http://pgp5.ai.mit.edu for the key)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by Folker Schamel <sc...@spinor.com>.
> On Wednesday 15 September 2004 10:43, Garrett Rooney wrote:
>
>>The problem is that the client migh thave built with a different version
>>of the header files, and according to our versioning rules it's
>>perfectly valid for them to have done so.
>
>
> Ah, I see. That's the key point here. If you allow me to ask, why is this
> allowed? This doesn't add much value (I mean, how often would you grab a new
> version of the SDK libraries without also grabbing the new headers EVEN if
> they were claimed to be compatible.
Its not about grabbing; its about recompiling.
If you upgrade svn, you would have to recompiler ALL applications
using the svn lib.
And under windows there is no user compiling the applications
he is using by himself ;-)
And also under Linux more and more users install packages without
compiling by themself.
In other words, for windows users and such linux users,
in practise this would mean that different svn versions
are completely incompatible:
If there is a new svn version (e.g. a minor bugfix),
then they have to wait util ALL applications using svn
they are using provide a new binary package, too.
Cheers,
FOlker
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by "Alexander L. Belikoff" <ab...@vallinor4.com>.
On Wednesday 15 September 2004 10:43, Garrett Rooney wrote:
> The problem is that the client migh thave built with a different version
> of the header files, and according to our versioning rules it's
> perfectly valid for them to have done so.
Ah, I see. That's the key point here. If you allow me to ask, why is this
allowed? This doesn't add much value (I mean, how often would you grab a new
version of the SDK libraries without also grabbing the new headers EVEN if
they were claimed to be compatible. Call me paranoid, but I never do that)
yet it puts a major showstopper to any enhancement until the next major
release.
I am in no position to question your development style, but I ask you to
consider a slightly different way:
1. The API is comprised of a set of header files and libraries and must be
treated as a whole. A client using a given version of a library MUST also be
compiled with the corresponding set of headers.
2. The API is guaranteed to be 100% backward compatible on ANY release (major,
minor or a bugfix). That means, any client using the API of a given version
is guaranteed to work with any newer version of the API.
3. The API is NOT guaranteed to be forward compatible (even though the
development team will attempt to prevent incompatibilities as long as it
doesn't hurt the innovation). A client built with a given version of the API
is not guaranteed to *compile* (this part is crucial - incompatibilities must
surface at the compile time) using a previous version of the API.
This is the usual rules of engagement of an API game and it gives enough
flexibility to both the clients and the API providers. I hope there is no
reason Subversion cannot play by those rules. I mean, there should be no
reason, as Subversion is just yet another system with an SDK and a published
API, which is in no way different from other such systems out there be it
Motif, or GTK, or CORBA, or MPI.
P.S. Finally, just playing the Devil's advocate, there is a counterexample -
libc. However, the libc public API has been static for a very long time which
definitely allows for a different treatment of that particular library.
Regards,
--
Alexander L. Belikoff
PGP/GPG fingerprint: 0D58 A804 1AB1 4CD8 8DA9 424B A86E CD0D 8424 2701
(http://pgp5.ai.mit.edu for the key)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by Folker Schamel <sc...@spinor.com>.
ABI compatibibility includes that you can upgrade your library
binary without recompiling the application.
Alexander L. Belikoff wrote:
> On Wednesday 15 September 2004 10:10, Garrett Rooney wrote:
>
>>It's not a matter of the structure being passed between the client and
>>server, it's a matter of the definition of the structure existing in our
>>public header files, so it's part of our ABI/API. If a client of the
>>code (someone who links against the libsvn_* libraries) was depending on
>>the size of the structure (which is perfectly valid for them to do,
>>since we put the definition right out in the open like that) they could
>>be broken if we change that size in a new release of the software.
>
>
> Hmm... So far, the only problem that is evident to me is the case when the
> client is written to take advantage of the new structure member and then it
> has to get built with the older revision that doesn't have that member. This
> situation would result in a compile-time error and it is sometimes (like
> now :-) ) a justified case of forward incompatibility.
>
> However, I don't buy the argument about dependency on the size of the
> structure. In most of the API I know, such a dependency is strictly
> compile-time. For example:
>
> 1. My program uses Motif 1.2. It includes Widget.h and makes use of the Widget
> structure (e.g. by building an in-core array of Widget structures). The size
> of the structure is 68 bytes. These structures are being passed to the Motif
> library, which is compiled with the same set of headers and it expects a
> 68-byte structure for a Widget.
>
> 2. I upgrade my Motif environment to 2.x. This one has extended the Widget
> structure by 'int super_id', making it 72 bytes (alignment issues aside). My
> program is recompiled with a 2.x version of Widget.h and is now using 72
> bytes per structure. *NOWHERE* in my program do I make an explicit use of the
> old '68' number - instead, I always use sizeof(Widget). As I link to the 2.x
> version of the library, I pass the 72-byte structures to the routines that
> are compiled with the same version of the Widget and are expecting that kind
> of data.
>
> 3. My peer Joe Schmoe still has Motif 1.2 installed but he decides to be
> clever so he grabs my .o files to save some recompilation time. As he links
> to the 1.2 libraries (assuming the worst case of no unresolved symbols etc.),
> he gets a binary that mysteriously coredumps every now and then. Go figure!
>
> The long winded point here is that the 'client' of the API should treat the
> data structure just for what it is - an opaque data structure provided by the
> API, where the client is being offered just a bunch of members (potentially
> having other hidden members). If the client then builds with the same version
> of API headers as the API libraries, he should be OK (save for the forward
> incompatibility issue outlined in the beginning).
>
> Let me know if I am wrong in my interpretation of svn_keywords_t.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Alexander L. Belikoff wrote:
>The long winded point here is that the 'client' of the API should treat the
>data structure just for what it is - an opaque data structure provided by the
>API, where the client is being offered just a bunch of members (potentially
>having other hidden members). If the client then builds with the same version
>of API headers as the API libraries, he should be OK (save for the forward
>incompatibility issue outlined in the beginning).
>
>
The problem is that the client migh thave built with a different version
of the header files, and according to our versioning rules it's
perfectly valid for them to have done so.
>Let me know if I am wrong in my interpretation of svn_keywords_t.
>
>
Client of the subversion libraries writes code like this:
svn_subst_keywords_t kw;
int some_other_variable;
svn_subst_build_keyworkds (&kw, ... (other args here) ...);
Note that the svn_subst_keywords_t is declared on the stack. They then
build it with a version of subversion that uses the old definition of
svn_subst_keywords_t, so their binary is created assuming that size for
svn_subst_keywords_t. They ship this binary to thier users who already
have Subversion isntalled on their machine, and everything works.
Then your patch gets applied, which changes the size of
svn_subst_keywords_t, and several of the underlying functions in
libsvn_subr (or wherever the subst functions live, I forget) now assume
the new size. The users (who only have the binary compiled with the old
definition of svn_subst_keywords_t) upgrade to the new version of
Subversion, which assumes the new size.
The program now calls the new version of svn_subst_build_keywords, which
assumes the svn_subst_keywords_t pointer it's given points to an area of
memory large enough to hold the new version. In reality the area of
memory is not large enough, so we just smashed the variables allocated
on the stack after the kw variable, causing their program to crash and
everyone to become afraid of upgrading to new versions of Subversion
because their old programs will break when they do.
We don't want people to be afraid of upgrading within the same major
version, so we don't make those kind of changes.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by "Alexander L. Belikoff" <ab...@vallinor4.com>.
On Wednesday 15 September 2004 10:10, Garrett Rooney wrote:
> It's not a matter of the structure being passed between the client and
> server, it's a matter of the definition of the structure existing in our
> public header files, so it's part of our ABI/API. If a client of the
> code (someone who links against the libsvn_* libraries) was depending on
> the size of the structure (which is perfectly valid for them to do,
> since we put the definition right out in the open like that) they could
> be broken if we change that size in a new release of the software.
Hmm... So far, the only problem that is evident to me is the case when the
client is written to take advantage of the new structure member and then it
has to get built with the older revision that doesn't have that member. This
situation would result in a compile-time error and it is sometimes (like
now :-) ) a justified case of forward incompatibility.
However, I don't buy the argument about dependency on the size of the
structure. In most of the API I know, such a dependency is strictly
compile-time. For example:
1. My program uses Motif 1.2. It includes Widget.h and makes use of the Widget
structure (e.g. by building an in-core array of Widget structures). The size
of the structure is 68 bytes. These structures are being passed to the Motif
library, which is compiled with the same set of headers and it expects a
68-byte structure for a Widget.
2. I upgrade my Motif environment to 2.x. This one has extended the Widget
structure by 'int super_id', making it 72 bytes (alignment issues aside). My
program is recompiled with a 2.x version of Widget.h and is now using 72
bytes per structure. *NOWHERE* in my program do I make an explicit use of the
old '68' number - instead, I always use sizeof(Widget). As I link to the 2.x
version of the library, I pass the 72-byte structures to the routines that
are compiled with the same version of the Widget and are expecting that kind
of data.
3. My peer Joe Schmoe still has Motif 1.2 installed but he decides to be
clever so he grabs my .o files to save some recompilation time. As he links
to the 1.2 libraries (assuming the worst case of no unresolved symbols etc.),
he gets a binary that mysteriously coredumps every now and then. Go figure!
The long winded point here is that the 'client' of the API should treat the
data structure just for what it is - an opaque data structure provided by the
API, where the client is being offered just a bunch of members (potentially
having other hidden members). If the client then builds with the same version
of API headers as the API libraries, he should be OK (save for the forward
incompatibility issue outlined in the beginning).
Let me know if I am wrong in my interpretation of svn_keywords_t.
--
Alexander L. Belikoff
PGP/GPG fingerprint: 0D58 A804 1AB1 4CD8 8DA9 424B A86E CD0D 8424 2701
(http://pgp5.ai.mit.edu for the key)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by Ben Reser <be...@reser.org>.
On Wed, Sep 15, 2004 at 10:01:56AM -0400, Alexander L. Belikoff wrote:
> I'm sorry for not chiming in earlier (was a bit busy), but could anyone
> explain to me the public nature of this particular structure? I am not sure I
> see where this structure is being passed between the client and the server.
> Because if it is not, then there should hardly be any problem (except for the
> case where headers are inconsistent with the libraries, which is a major
> screw-up anyway).
One other thing nobody seemed to mention was that svn_subst_keywords_t
is not allocated by the library. It is allocated by the client prior to
passing it to svn_subst_build_keywords. It would have been better to
make it an opaque data structure and allocate it ourselves and not
reveal it's structure the clients directly. Unfortunately, we didn't do
that.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: PATCH: $Header$ keyword support (now with proper log message)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Alexander L. Belikoff wrote:
>I'm sorry for not chiming in earlier (was a bit busy), but could anyone
>explain to me the public nature of this particular structure? I am not sure I
>see where this structure is being passed between the client and the server.
>Because if it is not, then there should hardly be any problem (except for the
>case where headers are inconsistent with the libraries, which is a major
>screw-up anyway).
>
>My intention for this patch was to make it as backward compatible as possible.
>Please let me know if I am missing something here...
>
It's not a matter of the structure being passed between the client and
server, it's a matter of the definition of the structure existing in our
public header files, so it's part of our ABI/API. If a client of the
code (someone who links against the libsvn_* libraries) was depending on
the size of the structure (which is perfectly valid for them to do,
since we put the definition right out in the open like that) they could
be broken if we change that size in a new release of the software.
-garrett
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org