[Prev] Thread [Next]  |  [Prev] Date [Next]

Re: [dev] Horde 5 was: Re: [commits] Horde branch develop updated. fa956a4d83a7227e2a5d47bfe87185854e151fbc Michael J Rubinsky Mon Feb 20 07:01:31 2012

Quoting Jan Schneider <[EMAIL PROTECTED]>:

Zitat von Michael M Slusarz <[EMAIL PROTECTED]>:

Quoting Michael J Rubinsky <[EMAIL PROTECTED]>:

Quoting Michael M Slusarz <[EMAIL PROTECTED]>:

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.

..and this requirement is fine. It's the other direction that is the issue.

Can you upgrade Horde_Core without upgrading Kronolith? No. But that's also the reality of the situation.

.and IMO, makes this a BC break.

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.

I guess we will have to agree to disagree. If the MIME encoding is done in a stand alone library, and the behaviour of that library changes in such a way that any existing code written against it is broken due to the change, that is a BC break and warrants a bump of the library's major version number.

It is VERY frustrating as a developer to have your code break after some minor update to a library not under your control. As an example, at one point ImageMagick's utilities were breaking BC with almost every minor (and even with only revision number increments) release they made. They, too, had made arguments that the previous behavior was not totally correct. It didn't much matter, existing scripts broke. Their mailing list archive is full of complaints about this. That's one reason why Horde_Image has so much version sniffing in it. It was a major source of frustration to developers and caused some to move on to other, more stable tools. I expect a jump from 1.0 -> 2.0 to have breaks, not from 1.0 -> 1.1 or even 1.0.1.

We switched to only checking authenticated apps in this commit, a week after Horde 4 was released:

commit dcdd9f52eabc7cd8df5b2ea661097effe039833e
Author: Michael M Slusarz <[EMAIL PROTECTED]>
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.

To be fair, this isn't a problem unique to Horde. I'm sure you are well aware that this is a fact of life when you are programming against any set of published stand-alone libraries. In a more general sense, no one is saying you can't break BC. You just can't break it within the same major version; We need to bump the major version number. As discussed below, this is exactly what we said should be done in these cases during the initial Horde 4 discussions. The problem with the Horde 3 -> Horde 4 change was that it just took *way* too long between those releases. BC was a thorn in everyone's side. That's something we all agreed should not happen again.

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.

This is exactly what we discussed should happen in a case like this. Let's do this and move on.


The Horde Project (www.horde.org)
Horde developers mailing list
Frequently Asked Questions: http://horde.org/faq/
To unsubscribe, mail: [EMAIL PROTECTED]