joomlatools / joomlatools-composer

Composer extension installer for Joomla.
https://www.joomlatools.com/developer/tools/composer/
GNU General Public License v3.0
52 stars 13 forks source link

Check if path is available and then set file, this lets us handle zipped plugins in a package correctly #38

Closed danielsmink closed 1 year ago

danielsmink commented 6 years ago

`<?xml version="1.0" encoding="UTF-8" ?>

PKG_JCE Ryan Demmer 21-03-2018 jce 2.6.28 https://www.joomlacontenteditor.net Widget Factory Limited https://www.joomlacontenteditor.net PKG_JCE_XML_DESCRIPTION install.pkg.php com_jce.zip plg_editors_jce.zip plg_content_jce.zip plg_extension_jce.zip plg_fields_mediajce.zip plg_installer_jce.zip plg_quickicon_jce.zip plg_system_jce.zip en-GB/en-GB.pkg_jce.sys.ini pro 2 3 ` The following package wouldn't install correctly because of the zips. This change handles a manifest file for a package correctly.
greenleafmedia commented 6 years ago

Excellent! I just ran into this as well. Thanks for the fix!

EDIT: I take that back. I'm still having troubles with this in JEvents:

<?xml version="1.0" encoding="UTF-8" ?>
<extension type="package" version="2.5" method="upgrade">
    <name>JEV_PACKAGE_TITLE</name>
    <packagename>jevents</packagename>
    <version>3.4.46</version>
    <url>http://www.jevents.net</url>
    <creationDate>March 2018</creationDate>
    <author>GWE Systems Ltd</author>
    <copyright>(C) 2012-2018 GWE Systems Ltd</copyright>
    <authorEmail></authorEmail>
    <authorUrl>www.gwesystems.com</authorUrl>
    <packager>GWE Systems Ltd</packager>
    <packagerurl>http://www.jevents.net</packagerurl>
    <description>JEV_PACKAGE_DESC</description>
    <update></update>
    <scriptfile>install.php</scriptfile>
    <languages folder="language">
        <language tag="en-GB">en-GB/en-GB.pkg_jevents.sys.ini</language>
    </languages>
    <files>
        <file type="component" id="com_jevents" >com_jevents.zip</file>
        <file type="module" id="mod_jevents_cal" client="site">mod_jevents_cal.zip</file>
        <file type="module" id="mod_jevents_legend" client="site">mod_jevents_legend.zip</file>
        <file type="module" id="mod_jevents_latest" client="site">mod_jevents_latest.zip</file>
        <file type="module" id="mod_jevents_filter" client="site">mod_jevents_filter.zip</file>
        <file type="module" id="mod_jevents_custom" client="site">mod_jevents_custom.zip</file>
        <file type="module" id="mod_jevents_switchview" client="site">mod_jevents_switchview.zip</file>
        <file type="plugin" id="jevents" group="finder">finder.zip</file>
        <file type="plugin" id="eventsearch" group="search">search.zip</file>
        <file type="plugin" id="jeventsinstaller" group="content">installer.zip</file>
        <file type="plugin" id="jevents" group="content">jevents.zip</file>
        <file type="library" id="googl">googl.zip</file>
        <file type="library" id="jevmodal">jevmodal.zip</file>
        <file type="library" id="gwejson">gwejson.zip</file>
        <file type="library" id="jevtypeahead">jevtypeahead.zip</file>
    </files>
    <updateservers>
        <server type="extension" name="JEvents Updates">https://www.jevents.net/updates/-/pkg_jevents-update-3.4.46.xml</server>
    </updateservers>
</extension>

What's happening for me is that ExtensionInstaller.php (329) is passing a link to the zipped file for the plugins which in turn fails on Util.php (90). This PR doesn't solve that because realpath is returning true (with the zipped file) on ExtensionInstaller (296). Perhaps we should be attacking this on Util.php (90) by adding a try/catch block and then failing gracefully?

That being said, I'm having a hard time figuring out a time when ExtensionInstaller.php (329) is ever going to work. Won't all packages be referring to zipped files in their files attribute?

danielsmink commented 6 years ago

@greenleafmedia yes exactly I was wondering when that part of the code actually works as well.

Looking at the JEvents manifest I'm surprised it doesn't work though, I'll try it locally.

danielsmink commented 6 years ago

As expected it installs perfectly fine without calling _enablePlugin. I'll alter my PR to completely remove this method.

greenleafmedia commented 6 years ago

@danielsmink - Great! I think that will do the trick (though we may want to leave in the method for non-package installs). I think the reason the original patch was working for the JCE plugin is that there's also another bug in ExtensionInstaller.php where it wasn't reading the folder attribute of the files tag in the manifest, so it wasn't even finding the zipped files. Your most recent patch should take care of both situations.

stevenrombauts commented 6 years ago

@danielsmink @greenleafmedia Thanks for looking into this issue. I have not yet had a chance to test this properly, but as you pointed out, it does look like _enablePlugin() can never work for a package.

We do require _enablePlugin to make sure our custom plugins are ready to go. We could start by simply not running it for a package type. I'm not sure if we can rely on the id arguments for each listed file to enable it (instead of the filename), any idea?

greenleafmedia commented 6 years ago

@stevenrombauts - I actually think there are a couple of things going on here when you try to install 3rd party components.

  1. In the JCH Optimize example that started this ticket, I think part of the problem is that ExtensionInstaller.php never checks the folder property on the <files> tag. I think that causes things to fail because the install path winds up being incorrect when you don't account for that.

  2. For my Jevents example the problems seems to be from the getPackageManifest method over in Util.php. What was happening for me is that when _enablePlugin would loop back around to install the plugins of the package the RecursiveDirectoryIterator was failing because it was getting the path of the zip file and not of the unzipped "install_*" directories. So, you'd also want to do a check before using those iterators in getPackageManifest.

I spent some time trying to get it to work, but I could not reliably get the location of the unzipped plugin for manifest parsing. I wound up just commenting out the _enablePlugin call and displaying a message that said some plugins may not have been enabled.