Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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
Owner

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
Owner

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 :smile:

@markstory markstory commented on the diff
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 Owner

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
Commits on Nov 15, 2012
  1. @tenebrousedge
This page is out of date. Refresh to see the latest.
Showing with 12 additions and 11 deletions.
  1. +12 −11 lib/Cake/Console/Command/UpgradeShell.php
View
23 lib/Cake/Console/Command/UpgradeShell.php
@@ -103,7 +103,7 @@ public function all() {
public function tests() {
$this->_paths = array(APP . 'tests' . DS);
if (!empty($this->params['plugin'])) {
- $this->_paths = array(App::pluginPath($this->params['plugin']) . 'tests' . DS);
+ $this->_paths = array($this->_pluginPath($this->params['plugin']) . 'tests' . DS);
}
$patterns = array(
array(
@@ -129,7 +129,7 @@ public function locations() {
$cwd = getcwd();
if (!empty($this->params['plugin'])) {
- chdir(App::pluginPath($this->params['plugin']));
+ chdir($this->_pluginPath($this->params['plugin']));
}
if (is_dir('plugins')) {
@@ -216,7 +216,7 @@ public function helpers() {
$this->_paths = array_diff(App::path('views'), App::core('views'));
if (!empty($this->params['plugin'])) {
- $this->_paths = array(App::pluginPath($this->params['plugin']) . 'views' . DS);
+ $this->_paths = array($this->_pluginPath($this->params['plugin']) . 'views' . DS);
}
$patterns = array();
@@ -230,7 +230,7 @@ public function helpers() {
CakePlugin::load($plugin);
$pluginHelpers = array_merge(
$pluginHelpers,
- App::objects('helper', App::pluginPath($plugin) . DS . 'views' . DS . 'helpers' . DS, false)
+ App::objects('helper', $this->_pluginPath($plugin) . DS . 'views' . DS . 'helpers' . DS, false)
);
}
$helpers = array_merge($pluginHelpers, $helpers);
@@ -261,7 +261,7 @@ public function i18n() {
APP
);
if (!empty($this->params['plugin'])) {
- $this->_paths = array(App::pluginPath($this->params['plugin']));
+ $this->_paths = array($this->_pluginPath($this->params['plugin']));
}
$patterns = array(
@@ -300,7 +300,7 @@ public function basics() {
APP
);
if (!empty($this->params['plugin'])) {
- $this->_paths = array(App::pluginPath($this->params['plugin']));
+ $this->_paths = array($this->_pluginPath($this->params['plugin']));
}
$patterns = array(
array(
@@ -355,7 +355,7 @@ public function request() {
$this->_paths = array_merge($views, $controllers, $components);
if (!empty($this->params['plugin'])) {
- $pluginPath = App::pluginPath($this->params['plugin']);
+ $pluginPath = $this->_pluginPath($this->params['plugin']);
$this->_paths = array(
$pluginPath . 'controllers' . DS,
$pluginPath . 'controllers' . DS . 'components' . DS,
@@ -407,7 +407,7 @@ public function configure() {
APP
);
if (!empty($this->params['plugin'])) {
- $this->_paths = array(App::pluginPath($this->params['plugin']));
+ $this->_paths = array($this->_pluginPath($this->params['plugin']));
}
$patterns = array(
array(
@@ -429,7 +429,7 @@ public function constants() {
APP
);
if (!empty($this->params['plugin'])) {
- $this->_paths = array(App::pluginPath($this->params['plugin']));
+ $this->_paths = array($this->_pluginPath($this->params['plugin']));
}
$patterns = array(
array(
@@ -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 Owner

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
}
$patterns = array(
array(
@@ -536,7 +536,7 @@ public function exceptions() {
$this->_paths = array_merge($controllers, $components);
if (!empty($this->params['plugin'])) {
- $pluginPath = App::pluginPath($this->params['plugin']);
+ $pluginPath = $this->_pluginPath($this->params['plugin']);
$this->_paths = array(
$pluginPath . 'controllers' . DS,
$pluginPath . 'controllers' . DS . 'components' . DS,
@@ -859,3 +859,4 @@ public function getOptionParser() {
}
}
+
Something went wrong with that request. Please try again.