User tests: Successful: Unsuccessful:
Move the framework Archive package.
Installer should work as before when installing new extensions.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_banners External Library Libraries |
Labels |
Added:
?
?
|
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?
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).
It works with the framework package except the gzip and bzip2 which handle the $extractdir argument differently. The rest is working as expected.
Not yet.
Milestone |
Added: |
Milestone |
Added: |
What's the plan here?
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?
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.
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.
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.
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.gz
then 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.
Reopening this one, but first the inconsistency needs to be fixed in the fw package.
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.
Just tagged 1.1.5 for you :)
Composer package updated, guess it is ready for a test.
I have tested this item
Installed com_patchtester ZIP package successfully.
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-06-26 14:49:35 |
Closed_By | ⇒ | mbabker |
You need to fix tests here