This repository has been archived by the owner. It is now read-only.

CMS issue #28864 #1407

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

Octobit commented Jul 24, 2012

CMS issue #28864 An error during extension installation does not properly unwind

adapters/file.php and adapters/module.php are already correct

Contributor

realityking commented Jul 24, 2012

@pasamio @mbabker Could one of you take a look at this? Thanks.

Contributor

Octobit commented Jul 28, 2012

There are 4 files involved.

libraries/joomla/installer/installer.php (line 378)
libraries/joomla/installer/adapters/plugin.php (line 415)
libraries/joomla/installer/adapters/file.php (line 262)
libraries/joomla/installer/adapters/module.php (line 435)

The first 2 name the field 'id', the last 2 name the field 'extension_id'
That is why only two files were pulled.

Contributor

pasamio commented Jul 28, 2012

The other adapters are fine or they just don't feature this functionality? Packages, libraries, components, templates and languages?

Contributor

Octobit commented Jul 28, 2012

The other adapters do not feature this functionality. Perhaps this is so because there are no significant actions between the pushstep() and the installation of the manifest.

This is actually inconsistent. The mechanism was introduced for a reason, and now it is there it is not used. I do not find this correct.

I did not give it deeper thought, but your remark did make me realize. I would vote that all the adapters adopt this behaviour.

Contributor

elinw commented Aug 6, 2012

can you please change the title to something describing the actual issue so it is useful for the change log?

Contributor

Octobit commented Sep 3, 2012

Elin, sorry for the delay as I was abroad on summer holiday and did not have internet access.

I would like to change the title to:
"CMS issue #28864: Fixes that changes to the database be undone when installing a module or file extension fails"

However, I am using the web interface to github and have not found the option to change the title.

Contributor

elinw commented Sep 3, 2012

Usually if it is your pull request you can just click on the pencil icon

Contributor

LouisLandry commented Nov 11, 2012

@Octobit this pull request is no longer mergeable. It seems we have a consensus that the changes are something we would like to include, but perhaps a little cleanup is in order. Would you mind rebasing your branch so that we can get this merged? I'm going to close the pull request until it can be rebased. Once you've gotten that sorted out please leave a comment here asking us to reopen the pull request and we'll get it reopened and merged. I really appreciate your work and patience!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.