upgrade shell: attempt at making --plugin param actually work. #960

Closed
wants to merge 1 commit into
from

2 participants

@tenebrousedge

I am not entirely sure that this is a correct solution.

Current behavior is: when --plugin param is given, individual methods in UpgradeShell will call App::pluginPath() to determine plugin paths. This in turn calls CakePlugin::pluginPaths, which seems not to have been initialized and is probably looking for correctly-named plugins anyway. In either case it returns empty no matter how the files are named.

I replaced the call to App::pluginPath with a call to $this->_pluginPaths() from the Shell class, which also checks CakePlugin, but then defaults to the legacy path when that returns nothing. This method does not actually check whether the files exist, although is_dir is called a couple times in the upgrade shell and will return an error if the directory /plugins/pluginname does not exist.

I'm not sure, due to unfamiliarity with the shell code, how the shell classes are actually supposed to be loading plugins.

@markstory
CakePHP member

Won't this just hit the first pluginPath that is configured?

@tenebrousedge

Markstory, I don't think so. Like I said, I don't think any are loaded. CakePlugin::load is never called, so CakePlugin::loaded should always return false, and $this->_pluginPath should return something like 'Plugin'.DS.$pluginname.DS;

What's it supposed to do? Not loading plugins actually sounds reasonable under the circumstances, since the upgrade shell won't actually use them and they're probably misnamed for the rest of the app.

@markstory
CakePHP member

I think that's what it does as well, but it still runs the risk of being wrong as applications could have more than one plugin path set. With that said it is an improvement 😄

@markstory markstory commented on the diff Nov 16, 2012
lib/Cake/Console/Command/UpgradeShell.php
@@ -511,7 +511,7 @@ public function constants() {
public function components() {
$this->_paths = App::Path('Controller/Component');
if (!empty($this->params['plugin'])) {
- $this->_paths = App::Path('Controller/Component', $this->params['plugin']);
+ $this->_paths = $this->_pluginPath($this->params['plugin']) . 'controllers' . DS . 'components' . DS;
@markstory
CakePHP member
markstory added a line comment Nov 16, 2012

This assumes the old paths which I don't think it should.

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