? ? Failure

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
20 Feb 2017

Move the framework Archive package.

Testing instructions

Installer should work as before when installing new extensions.

avatar laoneo laoneo - open - 20 Feb 2017
avatar laoneo laoneo - change - 20 Feb 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 20 Feb 2017
Category Administration com_banners External Library Libraries
avatar laoneo laoneo - change - 20 Feb 2017
Labels Added: ? ?
avatar wilsonge
wilsonge - comment - 20 Feb 2017

You need to fix tests here

avatar laoneo
laoneo - comment - 21 Feb 2017

The problem is that the archive package handles gzip and bzip differently than JArchive. The line here https://github.com/joomla-framework/archive/blob/master/src/Archive.php#L111 has a different logic than the core the equivalent https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/archive/archive.php#L107.

IMO the core is doing it right. @mbabker I guess changing that in the 1. series of archive package would be a BC break?

avatar mbabker
mbabker - comment - 21 Feb 2017

If you're referring to the use streams thing (why are we passing integers to a boolean parameter anyway!?), to be honest, I have no idea how well that's going to work with the Framework's filesystem package.

To be honest, anything depending on it for much probably needs to be shifted toward the end of this migration process. Considering the entire thing is static and has no way to inject services, I fear to make things work we're going to end up subclassing things to use the CMS' existing API which still has the FTP layer in it and inherently still hooks into the CMS' configuration (I think the FTP layer support was ripped out of the Framework's version).

avatar laoneo
laoneo - comment - 21 Feb 2017

It works with the framework package except the gzip and bzip2 which handle the $extractdir argument differently. The rest is working as expected.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 24 Feb 2017

@laoneo is this for Test?

avatar laoneo
laoneo - comment - 24 Feb 2017

Not yet.

avatar zero-24 zero-24 - change - 24 Feb 2017
Milestone Added:
avatar zero-24 zero-24 - change - 24 Feb 2017
Milestone Added:
avatar mbabker
mbabker - comment - 14 Apr 2017

What's the plan here?

avatar laoneo
laoneo - comment - 29 Apr 2017

Sorry, missed your question. The problem is that the archive package handles gzip and bzip files differently than JArchive. The line here https://github.com/joomla-framework/archive/blob/master/src/Archive.php#L111 has a different logic than the core the equivalent https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/archive/archive.php#L107.

IMO the core is doing it right. I guess changing that in the 1. series of archive package would be a BC break?

avatar mbabker
mbabker - comment - 29 Apr 2017

If you mean supporting streams versus not supporting streams, to be honest because the entirety of our filesystem code is still procedural code, it's really difficult to build the needed dependency (the Stream object) in the Framework whereas the CMS goes through JFactory to create one with the needed configuration. So we really need to bite the bullet and refactor the filesystem package to be OOP based and come up with a system for injecting dependencies.

avatar laoneo
laoneo - comment - 29 Apr 2017

Not stream, about the logic. Please compare the logic of the two links. In the fw package the outside worlds needs to be aware if they are zipped gz files or archives. It is different logic for the extract function. That's why the unit tests in the CMS do fail.

avatar mbabker
mbabker - comment - 29 Apr 2017

I'm obviously missing something here. I'm not seeing why the Framework's way is broken to be honest here. Ya, they might behave differently internally, but that doesn't make it wrong necessarily.

avatar laoneo
laoneo - comment - 1 May 2017

I'm not saying that it is broken in the fw, it just has a logic where you need to pass a folder, depending of the input file. For example when you pass a gzip file /my/path/test.txt.gzthen the extractDir in the fw must be /my/extract/path/test.txt while in the CMS you have to pass as extractdir parameter /my/extract/path. That's why for me the cms lib is more consistent and a 1 to 1 replacement can't be done with the fw package in the core.

avatar laoneo
laoneo - comment - 4 Jun 2017

Reopening this one, but first the inconsistency needs to be fixed in the fw package.

avatar laoneo
laoneo - comment - 5 Jun 2017

Opened pr as disscused at JAB 17 joomla-framework/archive#12. If that one gets merged, then this one needs to be adapted as well.

dd1f0f3 12 Jun 2017 avatar laoneo CS
96e3499 12 Jun 2017 avatar laoneo CS
7192e43 12 Jun 2017 avatar laoneo CS
4d95296 12 Jun 2017 avatar laoneo CS
avatar wilsonge
wilsonge - comment - 12 Jun 2017

Just tagged 1.1.5 for you :)

avatar laoneo
laoneo - comment - 12 Jun 2017

Composer package updated, guess it is ready for a test.

avatar mbabker mbabker - test_item - 17 Jun 2017 - Tested successfully
avatar mbabker
mbabker - comment - 17 Jun 2017

I have tested this item successfully on f7bceaa

Installed com_patchtester ZIP package successfully.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14157.

avatar mbabker mbabker - change - 26 Jun 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-06-26 14:49:35
Closed_By mbabker
avatar mbabker mbabker - close - 26 Jun 2017
avatar mbabker mbabker - merge - 26 Jun 2017

Add a Comment

Login with GitHub to post a comment