You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by je...@apache.org on 2010/08/21 04:30:31 UTC
svn commit: r987689 - /subversion/branches/1.6.x/STATUS
Author: jerenkrantz
Date: Sat Aug 21 02:30:31 2010
New Revision: 987689
URL: http://svn.apache.org/viewvc?rev=987689&view=rev
Log:
Propose r879757 & r880320 for backport to 1.6.x.
Modified:
subversion/branches/1.6.x/STATUS
Modified: subversion/branches/1.6.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=987689&r1=987688&r2=987689&view=diff
==============================================================================
--- subversion/branches/1.6.x/STATUS (original)
+++ subversion/branches/1.6.x/STATUS Sat Aug 21 02:30:31 2010
@@ -233,6 +233,17 @@ Candidate changes:
Votes:
+1: danielsh
+ * r879757, r880320
+ Let ra_serf work with current serf releases.
+ Justification:
+ Having a dud client is bad. This seems to be the minimal required changes.
+ Backport branch:
+ ^/subversion/branches/1.6.x-r879757
+ Notes:
+ r879757 is the main fix. r880320 is a follow up fix.
+ Votes:
+ +1: jerenkrantz
+
Veto-blocked changes:
=====================
Re: svn commit: r987689 - /subversion/branches/1.6.x/STATUS
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sun, Aug 22, 2010 at 7:25 PM, Greg Stein <gs...@gmail.com> wrote:
> I also added an API to serf to facilitate runtime version checks (ie.
> hopefully before a call to a bogus signature is performed). But that
> call is only in later versions :-P
Yup - I noticed that when I added the minimum version auto-fu check
today. I'm not sure how critical it is to do that run-time check at
configure-time - I know that APR does it for BDB, but...
If we decide not to do the proposed backport, then 1.6.x might
actually need a *maximum* version check. But, I'd rather just make
1.6.x work with a current serf instead...and probably force Subversion
to require 0.6.2/0.7.0 (depending upon what Greg rolls soon) as
0.4.0-0.6.1 won't work with 1.6.x, but 0.3.1 might.
My head hurts. =) -- justin
Re: svn commit: r987689 - /subversion/branches/1.6.x/STATUS
Posted by Greg Stein <gs...@gmail.com>.
On Sun, Aug 22, 2010 at 17:30, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> On Sun, Aug 22, 2010 at 1:02 PM, Lieven Govaerts <sv...@mobsol.be> wrote:
>> Hey Justin,
>>
>> On Sat, Aug 21, 2010 at 4:30 AM, <je...@apache.org> wrote:
>>> Author: jerenkrantz
>>> Date: Sat Aug 21 02:30:31 2010
>>> New Revision: 987689
>>>
>>> URL: http://svn.apache.org/viewvc?rev=987689&view=rev
>>> Log:
>>> Propose r879757 & r880320 for backport to 1.6.x.
>>>
>>> Modified:
>>> subversion/branches/1.6.x/STATUS
>>>
>>> Modified: subversion/branches/1.6.x/STATUS
>>> URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=987689&r1=987688&r2=987689&view=diff
>>> ==============================================================================
>>> --- subversion/branches/1.6.x/STATUS (original)
>>> +++ subversion/branches/1.6.x/STATUS Sat Aug 21 02:30:31 2010
>>> @@ -233,6 +233,17 @@ Candidate changes:
>>> Votes:
>>> +1: danielsh
>>>
>>> + * r879757, r880320
>>> + Let ra_serf work with current serf releases.
>>> + Justification:
>>> + Having a dud client is bad. This seems to be the minimal required changes.
>>> + Backport branch:
>>> + ^/subversion/branches/1.6.x-r879757
>>> + Notes:
>>> + r879757 is the main fix. r880320 is a follow up fix.
>>> + Votes:
>>> + +1: jerenkrantz
>>
>> I didn't want to propose r879757 for backport because it changes the
>> svn_ra_serf__conn_setup function declaration, which is used as a
>> callback function for serf, in an incompatible way with serf 0.3. As
>> long as one builds and runs svn with the same serf version there is no
>> problem. The idea was to just raise the minimum serf version with svn
>> 1.7 release, so this problem couldn't happen.
>>
>> Is this something we make promises about?
>
> It has the ifdef so older serf's should work fine. Or, am I missing
> something? Is the issue compiling against 0.4.x+ and running with
> 0.3.x+? If so, I'm not sure that's worth worrying about. (Serf
> doesn't have a promise of binary API compatibility...)
>
> The core issue that I'm trying to address in this backport is that we
> don't have enough auto-fu checks currently in place to block
> combinations of 1.6.x with current releases of serf. We have no
> version checks at configure-time - only at compile-time; and the
> compile-time checks in 1.6.x don't complain if it sees a serf version
> it doesn't know about. So, right now, 1.6.x (without r879757) builds
> "successfully", but due to the API mismatches, we get a dud client
> with serf 0.4.0+ and 1.6.x. -- justin
I also added an API to serf to facilitate runtime version checks (ie.
hopefully before a call to a bogus signature is performed). But that
call is only in later versions :-P
Cheers,
-g
Re: svn commit: r987689 - /subversion/branches/1.6.x/STATUS
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sun, Aug 22, 2010 at 1:02 PM, Lieven Govaerts <sv...@mobsol.be> wrote:
> Hey Justin,
>
> On Sat, Aug 21, 2010 at 4:30 AM, <je...@apache.org> wrote:
>> Author: jerenkrantz
>> Date: Sat Aug 21 02:30:31 2010
>> New Revision: 987689
>>
>> URL: http://svn.apache.org/viewvc?rev=987689&view=rev
>> Log:
>> Propose r879757 & r880320 for backport to 1.6.x.
>>
>> Modified:
>> subversion/branches/1.6.x/STATUS
>>
>> Modified: subversion/branches/1.6.x/STATUS
>> URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=987689&r1=987688&r2=987689&view=diff
>> ==============================================================================
>> --- subversion/branches/1.6.x/STATUS (original)
>> +++ subversion/branches/1.6.x/STATUS Sat Aug 21 02:30:31 2010
>> @@ -233,6 +233,17 @@ Candidate changes:
>> Votes:
>> +1: danielsh
>>
>> + * r879757, r880320
>> + Let ra_serf work with current serf releases.
>> + Justification:
>> + Having a dud client is bad. This seems to be the minimal required changes.
>> + Backport branch:
>> + ^/subversion/branches/1.6.x-r879757
>> + Notes:
>> + r879757 is the main fix. r880320 is a follow up fix.
>> + Votes:
>> + +1: jerenkrantz
>
> I didn't want to propose r879757 for backport because it changes the
> svn_ra_serf__conn_setup function declaration, which is used as a
> callback function for serf, in an incompatible way with serf 0.3. As
> long as one builds and runs svn with the same serf version there is no
> problem. The idea was to just raise the minimum serf version with svn
> 1.7 release, so this problem couldn't happen.
>
> Is this something we make promises about?
It has the ifdef so older serf's should work fine. Or, am I missing
something? Is the issue compiling against 0.4.x+ and running with
0.3.x+? If so, I'm not sure that's worth worrying about. (Serf
doesn't have a promise of binary API compatibility...)
The core issue that I'm trying to address in this backport is that we
don't have enough auto-fu checks currently in place to block
combinations of 1.6.x with current releases of serf. We have no
version checks at configure-time - only at compile-time; and the
compile-time checks in 1.6.x don't complain if it sees a serf version
it doesn't know about. So, right now, 1.6.x (without r879757) builds
"successfully", but due to the API mismatches, we get a dud client
with serf 0.4.0+ and 1.6.x. -- justin
Re: svn commit: r987689 - /subversion/branches/1.6.x/STATUS
Posted by Lieven Govaerts <sv...@mobsol.be>.
Hey Justin,
On Sat, Aug 21, 2010 at 4:30 AM, <je...@apache.org> wrote:
> Author: jerenkrantz
> Date: Sat Aug 21 02:30:31 2010
> New Revision: 987689
>
> URL: http://svn.apache.org/viewvc?rev=987689&view=rev
> Log:
> Propose r879757 & r880320 for backport to 1.6.x.
>
> Modified:
> subversion/branches/1.6.x/STATUS
>
> Modified: subversion/branches/1.6.x/STATUS
> URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=987689&r1=987688&r2=987689&view=diff
> ==============================================================================
> --- subversion/branches/1.6.x/STATUS (original)
> +++ subversion/branches/1.6.x/STATUS Sat Aug 21 02:30:31 2010
> @@ -233,6 +233,17 @@ Candidate changes:
> Votes:
> +1: danielsh
>
> + * r879757, r880320
> + Let ra_serf work with current serf releases.
> + Justification:
> + Having a dud client is bad. This seems to be the minimal required changes.
> + Backport branch:
> + ^/subversion/branches/1.6.x-r879757
> + Notes:
> + r879757 is the main fix. r880320 is a follow up fix.
> + Votes:
> + +1: jerenkrantz
I didn't want to propose r879757 for backport because it changes the
svn_ra_serf__conn_setup function declaration, which is used as a
callback function for serf, in an incompatible way with serf 0.3. As
long as one builds and runs svn with the same serf version there is no
problem. The idea was to just raise the minimum serf version with svn
1.7 release, so this problem couldn't happen.
Is this something we make promises about?
Lieven