[dev] Horde 5 was: Re: [commits] Horde branch develop updated. fa956a4d83a7227e2a5d47bfe87185854e151fbc

[Jan Schneider]

Zitat von Michael M Slusarz <slusarz at horde.org>:

> Quoting Michael J Rubinsky <mrubinsk at horde.org>:
>
>> Quoting Michael M Slusarz <slusarz at horde.org>:
>>
>>> Re-setting this discussion, since I don't even know where we are  
>>> at.  Here's what I see:
>>>
>>> * Notifications and Alarms setup HAVE to be cached going forward.   
>>> Nobody has proven to me that these changes broke anything, or are  
>>> not BC.
>>>
>>> The only thing that has changed (that I can see) in the  
>>> notification/alarm code is as follows:
>>> - Applications are only initialized once per session.
>>> - Non-authenticated apps are initialized (or initialized before  
>>> the entire Horde framework is initialized).
>>
>>> The first shouldn't be an issue (and is the whole reason for the  
>>> change).  The second is NOT A BC BREAK!  Not only was this  
>>> behavior not documented, and thus not part of the API, but this is  
>>> how we did things at the release of Horde 4.
>>
>> I stand by my earlier statement - this *IS* a BC break. How can it  
>> not be? If Core is updated, existing application functionality  
>> breaks. It doesn't much matter what any documentation does or does  
>> not say - a user is only going to care that their synchronization,  
>> or some custom application that uses RPC access is broken.
>
> You are still wrong.  It is NOT a BC break.  Just because KRONOLITH  
> is incorrectly using the API does not make anything in Core wrong.
>
> There are only 2 ways you can break backward compatibility:
>
> 1. You explicitly change an API element.  We all agree that is not  
> the case here (these changes don't change what a method either  
> accepts or returns).
> 2. Alternatively, it could be argued that in the absence of a  
> clearly-defined API, you change some critical system as it was  
> performed hen 4.0 was released.  As already described, in 4.0 the  
> defined behavior is that non-authenticated applications would have  
> init() called BEFORE the full Horde environment was created.
>
> To reiterate: init() is solely a bootstrapping function to allow  
> enough of the application environment to be setup so that it could  
> be accessed by the Registry.  That's all it was ever designed to do.  
>  As amply shown, it was never guaranteed to have the full Horde  
> environment (this is why authAuthenticateCallback() exists).
>
>> For instance, the Ansel uploader plugins for iPhoto and Aperture  
>> rely on existing RPC behavior. If initializing the share system of  
>> applications when accessed via RPC is broken (which is currently  
>> is) - these uploaders will be broken too.
>
> If Ansel is not following the limitations described above, then the  
> correct fix is to fix Ansel.  There's nothing wrong with Core - just  
> how Ansel uses Core.
>
> As far as versions - this is the whole point of package.xml.  Will  
> the next version of Kronolith 3.x require Horde_Core 1.6.0 (or  
> whatever the next release is)?  Yes.  That's just the reality of the  
> situation.
>
> Can you upgrade Horde_Core without upgrading Kronolith?  No.  But  
> that's also the reality of the situation.
>
> Imagine this scenario: MIME encoding of an address.  Let's say an  
> application parsed the results of an encode for the quoted-printable  
> signature to determine if the message was encoded, and performed an  
> action based on the encoding status.  In a future version of the  
> MIME library, it was decided to change quoted-printable encoding  
> into binary encoding.
>
> This change would "break" the existing application if the library is  
> updated but not the Application.  But this is not a BC break.  This  
> is an example of an application making incorrect assumptions about  
> the API.  Nobody could possibly argue that we would have to maintain  
> the old way of MIME encoding just because a downstream application  
> won't be able to handle it properly.
>
>>> We switched to only checking authenticated apps in this commit, a  
>>> week after Horde 4 was released:
>>>
>>> commit dcdd9f52eabc7cd8df5b2ea661097effe039833e
>>> Author: Michael M Slusarz <slusarz at curecanti.org>
>>> Date:   Thu Apr 14 23:48:24 2011 -0600
>>>
>>>   Bug #9733:  Don't setup notification handlers in applications  
>>> that are not yet authenticated
>>
>> Which is a change you committed after commenting that you didn't  
>> know why we were polling unauthenticated apps for notifications.  
>> What changed with the documentation of the API since then that  
>> makes the earlier behavior more appropriate?
>
> Because this was a broken assumption.  This prevents  
> non-authenticated applications from setting notification handlers,  
> which is perfectly acceptable under the original API (as it should  
> be).  So the original bug-fix was flat wrong.
>
>>> Thus, if Kronolith has been initializing things in _init() that  
>>> aren't working because the Horde session itself is not fully  
>>> initialized, KRONOLITH IS BROKEN AND MUST BE FIXED (in 3.0 also).   
>>> FYI: this is the whole reason for the authAuthenticateCallback() -  
>>> to do things that require the ENTIRE Horde framework to be  
>>> initialized.
>>
>> Another issue to consider is that rpc requests initialize Horde  
>> with authentication => 'none' (we delegate authentication to the  
>> Rpc classes), and when I attempted to refactor things using this  
>> callback, it was *never* called from rpc requests.
>
> This seems like a limitation in the Registry_Application API then  
> that was not contemplated before Horde 4 came out.  Sounds like you  
> will have to manually do Authentication checking in Kronolith.  Or  
> add a new method that is called in this situation.  But you can't  
> hijack the init() function to do something it wasn't supposed to do.
>
>
> I'll be honest: revisiting this kind of BC discussion/argument is  
> exactly what I was fearful of a year ago.  This adherence to BC is  
> the single biggest problem of Horde, as it has been for the last 10  
> years.  I have had preliminary thoughts of forking IMP for precisely  
> this reason so I could avoid all this headache.  I spend way too  
> much time worrying about these sorts of issues - something like  
> releasing versions of IMP that require the latest version of all  
> required components would eliminate a tremendous amount of wasted  
> time for me.  There's a bunch of stuff that I have wanted to do, but  
> have not been able to because of these limitations.  Not my  
> intention to sound combative, but it accurately portrays the  
> frustration I have felt the last few months.  Or maybe my goals are  
> simply diverging from others'.

While I'm still with with Mike in this discussion, we don't seem to be  
able to come to a conclusion.

When we discussed the new release model for Horde 4, one of the  
important points was indeed that we don't want to fear breaking BC if  
we consider it necessary to go forward. I say we have reached this  
point, and instead of spending too much time in such discussions and  
causing frustrations, we should make the next release Horde 5.
I already considered to bring this up before this thread started,  
because if we really manage to get the new design in place for the  
next release, this would probably be a large enough change for the end  
users that it warrants and a new major version, even if we didn't  
break BC.

If everyone could agree to this, let's go forward with Horde 5 (and  
let's quickly update any conference, paper, article applications that  
mentioned 4.1).

Jan.

-- 
The Horde Project
http://www.horde.org/