Page 1 of 2

major release upgrading bundled libs?

Posted: Wed Mar 19, 2014 11:58 am
by Timbalu
I know we have discussed this somewhere else before, but this is something I still have in mind as a todo, without remembering in special "a certain decision" was definite.

Since 2.0 is called a major update, which btw in my eyes was 1.7 too, fixing and changing a lot internally, but kept compat, I remember this to be allowed to break certain old things. The bundled-libs dir includes some very old libs, except Smarty(!), for certain access and hasn't been upgraded generally beside possible fixes for deprecated PHP 5.5 notices by now.
So IMO a 2.0 release presents the opportunity to think this over again.

Could we discuss the need of each lib getting a lib upgrade and its possible breaks?

Just as an example, the wiki lib itself is 8 years old, and as far as I know an unmaintained (dead) project (http://pear.php.net/package/Text_Wiki/redirected). Why do we keep this? Is s9y.org the possible user for this?

Re: major release upgrading bundled libs?

Posted: Wed Mar 19, 2014 4:27 pm
by onli
+1

You are absolutely right, 2.0 should review and upgrade those.

Re: major release upgrading bundled libs?

Posted: Fri Mar 21, 2014 12:48 pm
by garvinhicking
Sure! My thoughts:

Cache/Lite: Should be upgradable, I'd like to keep it
HTTP: Should be kept, should be upgradable?
Net: Should be kept, should be upgradable?
Smarty: duh
Text/Wiki: Should be moved to the wiki plugin instead of within the core
XML/RPC: Should be moved to the xmlrpc plugin (I believe it is already? We need to see if the core uses this someplace yet)
composer: keep
simplepie: keep RSS syndication
Onyx: Make sure plugins and core use simplepie instead?
zendframework: keep

HTH,
Garvin

Re: major release upgrading bundled libs?

Posted: Fri Mar 21, 2014 1:14 pm
by Timbalu
Is that a beta milestone?

I just checked in my local 2.0 dev for XML/RPC (searched for RPC.php) and found:

Code: Select all

/plugins/serendipity_event_weblogping/serendipity_event_weblogping.php (1 hit)
	Line 149:                         include_once(S9Y_PEAR_PATH . "XML/RPC.php");
as the only core one.

Re: major release upgrading bundled libs?

Posted: Fri Mar 21, 2014 4:25 pm
by garvinhicking
Timbalu wrote:Is that a beta milestone?
For me that's a non-blocking thing actually. I'd rather release 2.0 than wait for someone to do the tidying up ;)
I just checked in my local 2.0 dev for XML/RPC (searched for RPC.php) and found:

Code: Select all

/plugins/serendipity_event_weblogping/serendipity_event_weblogping.php (1 hit)
	Line 149:                         include_once(S9Y_PEAR_PATH . "XML/RPC.php");
as the only core one.
Hm, considering we'd like to keep that plugin, then we would move it from the bundled-libs to the plugin, that wouldn't really help. Then we'd probably better keep XML-RPC in the core in bundled-libs.

Regards,
Garvin

Re: major release upgrading bundled libs?

Posted: Sun Apr 27, 2014 10:37 am
by mattsches
Hi, I'm bringing up this thread again because it showed up when I searched for "bundled-libs AND old" :lol:

As mentioned elsewhere I'm playing around with running unit tests for plugins on Travis CI. The service offers to run the tests against HHVM, Facebook's new, PHP compatible runtime. Turns out these tests fail because of an issue with the good old PEAR HTTP_Request library.

This library was last updated five and a half years ago.
This package has been superseded, but is still maintained for bugs and security fixes. Use HTTP_Request2 instead.
I subsequently downloaded HTTP_Request2 to my bundled-libs/HTTP directory and made some small changes to the affected code in my plugin under test. Definitely not a big deal, but seems to work correctly.

Since we all agree that the libs should be updated eventually: Maybe we could use this approach for a "soft" migration at least for this lib: Ship both versions as bundled libs, and update our code base step by step. Opinions?

Mattsches

Re: major release upgrading bundled libs?

Posted: Sun Apr 27, 2014 5:13 pm
by onli
Imho, we just should update all of them, if possible right now and deal with eventual errors.

Re: major release upgrading bundled libs?

Posted: Sun Apr 27, 2014 7:45 pm
by mattsches
Keep in mind that some plugins are using HTTP_Request et al, too. So if we replace/update the libs, we have to think about a switch or fallback, something along the lines of

Code: Select all

//pseudocode
if (file_exists(HTTP_Request2)) {…} else {…}

Re: major release upgrading bundled libs?

Posted: Mon Apr 28, 2014 7:12 pm
by mattsches
It works!

For the HTTP_Request2 lib and its dependencies, check today's commits to my S9y fork.

For a possible solution in a plugin, check this commit.

We could even extract the if/else/try/catch-stuff to a method somewhere in serendipity_plugin or so.

All in all, it should not be too much of a hassle, although some edge cases might come up where the solution is less straightforward.

Mattsches

Re: major release upgrading bundled libs?

Posted: Wed Apr 30, 2014 9:51 am
by garvinhicking
Hi!

I believe we need a wrapper for HTTP_Request2. We should edit the current "HTTP/Request.php" to actually include HTTP_Request2, and use the old API to map to the new one. Everything else would lead to too many incompatibilities, as several plugins and also the core uses the include to HTTP/Request.

Regards,
Garvin

Re: major release upgrading bundled libs?

Posted: Wed Apr 30, 2014 11:46 am
by mattsches
Editing external libs usually is not a good idea. However, HTTP_Request was last updated on 2008-11-17. So I don't expect a lot of security fixes soon :lol:

Which means that I could totally live with this approach. But for any *new* contributions, HTTP_Request2 should of course be called directly.

Also, it buys us time to switch the calls from HTTP_Request to HTTP_Request2 - I mean, if so. is editing a plugin or a core function which calls the old lib, it should be updated to the new one right away.

Mattsches

Re: major release upgrading bundled libs?

Posted: Mon May 05, 2014 10:46 am
by garvinhicking
Hi!

Yeah, we wouldn't actually edit HTTP_Request, we would need to create our own HTTP_Request which is just a wrapper for HTTP_Request2.

However I do see that the HTTP_Request class actually has a lot of functions we would need to wrap:

Code: Select all

<?php

// @ref PEAR::isError
// @ref serendipity_request_start
// @ref serendipity_request_end

class HTTP_Request {
	function HTTP_Request($file, $options) {
		// $options: allowRedirects, maxRedirects
	}

	function sendRequest() {
	}

	function getResponseCode() {
	}

	function getResponseBody() {
	}

	function getResponseHeader() {
	}

	function setMethod() {
	}

	function addHeader() {
	}

	function addRawPostData() {
	}
}
So maybe the better idea would be to both bundle the current HTTP_Request AND also bundle HTTP_Request2, and then we can use HTTP_Request2 for new code, and the old lib will also still work for older plugins using the API above. This would make plugins still work in 1.7 environments, and it's not a big library so not that much overhead.

HTTP_Request has quite a solid basic API, and it's proven, so I think we can keep it around? Of course the core 2.0 plugins and code would no longer use HTTP_Request1 and we would change all of that code to HTTP_Request2. Only spartacus plugins would then have that if/else construct checking for serendipity 2.0?

Regards,
Garvin

Re: major release upgrading bundled libs?

Posted: Tue May 06, 2014 8:44 pm
by mattsches
garvinhicking wrote:HTTP_Request has quite a solid basic API, and it's proven, so I think we can keep it around? Of course the core 2.0 plugins and code would no longer use HTTP_Request1 and we would change all of that code to HTTP_Request2. Only spartacus plugins would then have that if/else construct checking for serendipity 2.0?
Yes, that's what I meant. We would include both versions as bundled libs.

Core 2.0 plugins/code will use HTTP_Request2.

Spartacus plugins can either continue to use HTTP_Request1 or check if HTTP_Request2 is available (using an if/else switch).

As mentioned, we could also add a small method (e.g. isHTTPRequest2Available()) to Core which provides this check (and maybe require_once() the correct lib) - so that plugin authors can just call this method to check if v2 is available.

Mattsches

Re: major release upgrading bundled libs?

Posted: Wed May 07, 2014 1:26 pm
by garvinhicking
Hi!
As mentioned, we could also add a small method (e.g. isHTTPRequest2Available()) to Core which provides this check (and maybe require_once() the correct lib) - so that plugin authors can just call this method to check if v2 is available.
But this would defeat the purpose since using this check would mean that serendipity 2.0 would be required to call it; then again, http_request2 would always be available *g*

I think checking for $serendipity['version'] >= 2 (pseudocode) is the only way to go with inside plugins...

Re: major release upgrading bundled libs?

Posted: Wed May 07, 2014 7:56 pm
by mattsches
Ok ok, just brainstorming here ;)

So, let's try to do this? One issue might be that we need to update PEAR.php to a newer version, and I'm not sure whether or not this breaks compatibility with the old HTTP_Request (I hope it won't).

Here is a list of files we'll probably need to add:

Code: Select all

bundled-libs/HTTP/Request2.php
bundled-libs/HTTP/Request2/Adapter.php
bundled-libs/HTTP/Request2/Adapter/Curl.php
bundled-libs/HTTP/Request2/Adapter/Mock.php
bundled-libs/HTTP/Request2/Adapter/Socket.php
bundled-libs/HTTP/Request2/CookieJar.php
bundled-libs/HTTP/Request2/Exception.php
bundled-libs/HTTP/Request2/MultipartBody.php
bundled-libs/HTTP/Request2/Observer/Log.php
bundled-libs/HTTP/Request2/Response.php
bundled-libs/HTTP/Request2/SOCKS5.php
bundled-libs/HTTP/Request2/SocketWrapper.php
bundled-libs/Net/URL2.php
bundled-libs/PEAR/Exception.php
bundled-libs/PEAR.php (updated)