Allow package extension types to use scriptfiles#237
Allow package extension types to use scriptfiles#237chdemko merged 5 commits intojoomla:stagingfrom mbabker:installer
Conversation
|
Do you have a link to a test package that could be included with the installer sample packages? (they're currently living in the CMS project) |
|
I'll take the example code that is already in the SVN and tweak it to use these changes. Give me a day or so to get that done. |
|
I've branched the SVN trunk and tweaked the package already there to demonstrate all the features that I'm aware for this extension type. The scriptfile includes a working example of what I'm proposing with this pull request on postflight. Since I've got the branch going there, I'll hop through each of the extensions and clean them up then get that merged later. The branch is at http://joomlacode.org/svn/joomla/development/branches/sample_exts/ |
|
Michael, I've changed the system so we can generate the changelog from the pull request messages. There's no need to add the changelog anymore. Your branch will also need updating as git reports it can't be automatically merged. Thanks. |
|
I've merged to master and in the merge I pulled my changelog entry since it conflicted anyways. Should be good to go now. |
|
mine found only 3 :P |
|
Build triggered by changes to the base. Test log missing. Tests failed to execute. |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1695 tests and 10608 assertions. |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1704 tests and 10633 assertions. |
|
Build triggered by changes to the base. Test log missing. Tests failed to execute. |
|
Build triggered by changes to the head. Test log missing. Tests failed to execute. |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1707 tests and 10661 assertions. |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1735 tests and 10781 assertions. |
|
Build triggered by changes to the head. Unit testing complete. There were 2 failures and 0 errors from 1873 tests and 11091 assertions. |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1872 tests and 11090 assertions. |
1 similar comment
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1872 tests and 11090 assertions. |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1873 tests and 11091 assertions. |
|
Build triggered by changes to the head. Test log missing. Tests failed to execute. |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1873 tests and 11091 assertions. |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1876 tests and 11094 assertions. |
|
@pasamio what do you think on this one? |
|
Build triggered by changes to the head. Unit testing complete. There were 0 failures and 0 errors from 1971 tests and 11142 assertions. |
|
Seems fine to me, I've always wondered about packages having their own scripts but figured that the components, modules or plugins would take it over. Really have no opinion either way. |
There was a problem hiding this comment.
I would not change this, since this changes the path for all files. The scriptfile should they together with the rest anyway.
There was a problem hiding this comment.
A package extension has no true root. Since it is just a package of extensions, it truly only has a manifest and optional language files. Therefore, logically, a script file should be placed with the single required file for a package extension, the manifest. Well, that's my 2 cents anyways.
-Michael
Please pardon any errors, this message was sent from my iPhone.
On Jan 1, 2012, at 12:59 PM, marcel-kanter reply@reply.github.com wrote:
@@ -90,8 +90,7 @@ public function install()
$group = (string) $this->manifest->packagename;
if (!empty($group))
{
// TODO: Remark this location$this->parent->setPath('extension_root', JPATH_ROOT . '/packages/' . implode(DS, explode('/', $group)));$this->parent->setPath('extension_root', JPATH_MANIFESTS . '/packages/' . implode(DS, explode('/', $group)));I would not change this, since this changes the path for all files. The scriptfile should they together with the rest anyway.
Reply to this email directly or view it on GitHub:
https://github.com/joomla/joomla-platform/pull/237/files#r321672
There was a problem hiding this comment.
We're thinking the same. I did not came to take a closer look into this code. So JPATH_ROOT and JPATH_MANIFESTS point to equal fodlers?
There was a problem hiding this comment.
JPATH_ROOT goes to the site root. There's no packages folder at the site root. JPATH_MANIFESTS points to administrator/manifests, which is the path that manifest files for file, library, and package extensions are placed at. The TODO with the path leads me to think that at one point, there was an intent to use that path, but it's not used, so that was a logical change based on practical use.
-Michael
Please pardon any errors, this message was sent from my iPhone.
On Jan 1, 2012, at 1:17 PM, marcel-kanter reply@reply.github.com wrote:
@@ -90,8 +90,7 @@ public function install()
$group = (string) $this->manifest->packagename;
if (!empty($group))
{
// TODO: Remark this location$this->parent->setPath('extension_root', JPATH_ROOT . '/packages/' . implode(DS, explode('/', $group)));$this->parent->setPath('extension_root', JPATH_MANIFESTS . '/packages/' . implode(DS, explode('/', $group)));We're thinking the same. I did not came to take a closer look into this code. So JPATH_ROOT and JPATH_MANIFESTS point to equal fodlers?
Reply to this email directly or view it on GitHub:
https://github.com/joomla/joomla-platform/pull/237/files#r321677
There was a problem hiding this comment.
Hmm. Then the handling of the manifests of component is different to package and my code is copying the script into the base. Should we equalise this for all extension types? Bend component and the rest into JPATH_MANIFESTS?
There was a problem hiding this comment.
Negative. Component, module, and plugin manifests are fine as is and moving them would require numerous other changes.
-Michael
Please pardon any errors, this message was sent from my iPhone.
On Jan 1, 2012, at 1:52 PM, marcel-kanter reply@reply.github.com wrote:
@@ -90,8 +90,7 @@ public function install()
$group = (string) $this->manifest->packagename;
if (!empty($group))
{
// TODO: Remark this location$this->parent->setPath('extension_root', JPATH_ROOT . '/packages/' . implode(DS, explode('/', $group)));$this->parent->setPath('extension_root', JPATH_MANIFESTS . '/packages/' . implode(DS, explode('/', $group)));Hmm. Then the handling of the manifests of component is different to package and my code is copying the script into the base. Should we equalise this for all extension types? Bend component and the rest into JPATH_MANIFESTS?
Reply to this email directly or view it on GitHub:
https://github.com/joomla/joomla-platform/pull/237/files#r321685
There was a problem hiding this comment.
More important, moving them would break any upgraded site which already has these files which would mean adding extra code to check where they are. Essentially packages, and to a lesser extent libraries and files, don't really have a main extension root like components, modules, templates, plugins and languages do have significant files and it makes more sense to co-locate their manifest files with them, as has traditionally been the case.
And yes, it should have been updated.
There was a problem hiding this comment.
Think about sharing the InstallerScript. I would add:
if(!class_exists($classname))
{
$classname = 'InstallerScript';
}
Allow package extension types to use scriptfiles
|
Take a look into #702 |
This pull request makes changes to the package extension installer to allow for a scriptfile to be included on a package.
This will allow for a package to have a single preflight method, for example to check for any dependencies, such as another extension's presence or a minimum Joomla! version.
Additionally, a $results array has been created with each extension's name and $tmpInstaller->install result (which should be true given where it is compiled). This array is pushed forward to the postflight method, and can be used to display a list of the installed extensions, something many 3PD already do with their custom installation methods (see http://twitpic.com/65xw3i for an example).
Lastly, the TODO concerning the extension_root has been resolved by setting it to JPATH_MANIFESTS/packages/packagename and this is the location the scriptfile is stored in.