Permalink
Browse files

Allow setting path constants in app/Console/cake.php as you can in we…

…broot/index.php
  • Loading branch information...
1 parent f63aa92 commit 8150b89aba4e31041eb41c2dc201ea981ebfdb84 @ADmad ADmad committed Aug 11, 2013
Showing with 4 additions and 2 deletions.
  1. +4 −2 lib/Cake/Console/ShellDispatcher.php
@@ -122,8 +122,10 @@ protected function _bootstrap() {
define('ROOT', $this->params['root']);
define('APP_DIR', $this->params['app']);
define('APP', $this->params['working'] . DS);
- define('WWW_ROOT', APP . $this->params['webroot'] . DS);
- if (!is_dir(ROOT . DS . APP_DIR . DS . 'tmp')) {
+ if (!defined('WWW_ROOT')) {
+ define('WWW_ROOT', APP . $this->params['webroot'] . DS);
+ }
+ if (!defined('TMP') && !is_dir(APP . 'tmp')) {
@bar

bar Nov 7, 2013

Contributor

Shouldn`t the TMP dir when in cli mode be located in the same place as the web mode?¿

@markstory

markstory Nov 7, 2013

Owner

This is generally for the case where someone is running console tools from an app-less cak installation, like pear installs. It might be able to use sys_get_temp_dir() now as logging and caching create the required directories should they be missing.

@bar

bar Nov 7, 2013

Contributor

I agree on that, but wouldn't it be better to try on the web TMP dir before defaulting to the skel one?

@markstory

markstory Nov 7, 2013

Owner

Isn't that what APP . 'tmp' would be? The problem here is that bootstrapping often relies on TMP being set. When using a custom TMP directory you also need to set it in the Console/cake in your app dir.

@bar

bar Nov 7, 2013

Contributor

In a default CakePHP, ROOT . DS . APP_DIR . DS is the same as APP, that's why this change does not modify anything.

What I don't understand is the TMP path being defined, so I did this:

My default CakePHP is here:

$ pwd
/var/www/users/bclausen/projects/web

So calling
$ ./lib/Cake/Console/cake -app /var/www/users/bclausen/projects/web/app mysqhell do
or
$ ./app/Console/cake mysqhell do

Ends up with this inside inside $this->params, from ShellDispatcher::_bootstrap():

Array
(
    [app] => projects
    [root] => /var/www/users/bclausen
    [working] => /var/www/users/bclausen/projects
    [webroot] => webroot
)

Which ultimately sets APP . 'tmp' to /var/www/users/bclausen/projects/tmp, outside of my app :/

Edit:
I don't understand why it is supposed to be there by default, outside my own app. And because /var/www/users/bclausen/projects/tmp doesn't exist, TMPends up being defaulted to the skel dir.

There are 3 points here:

  • Is $this->params with correct values?¿
  • Is APP . 'tmp' the correct place to look for the TMP dir when in cli mode?
  • Shouldn't the default dir for TMP be set from sys_get_temp_dir() instead of the more obscure skel dir?¿
@markstory

markstory Nov 7, 2013

Owner

I think the params are correct based on how you're calling it, I don't know why you wouldn't just use Console/cake inside the app directory though.

@bar

bar Nov 7, 2013

Contributor

Alright, now I understand what you meant:

Doing:
$ ./lib/Cake/Console/cake -app app mysqhell do
or
$ ./Console/cake mysqhell do from inside my app dir

gives me the exact same values for $this->params.

@bar

bar Nov 7, 2013

Contributor

I be damned, I found the Shell mangling $this->params parsing different params, which ends up defining those constants.

Now, I do get the correct TMP you meant: /var/www/users/bclausen/projects/web/app/tmp/

@dereuromark

dereuromark Nov 7, 2013

Member

@bar - Actually it is better to ALWAYS call shells from app dir, never with -app.
So your first example - if you really want to use the core one - would be
../lib/Cake/Console/cake myshell

@bar

bar Nov 7, 2013

Contributor

@dereuromark Thanks for the tip, I just wrote it here to exhaust all the possibilities.

BTW, Isn't sys_get_temp_dir() a better/clearer default than CAKE_CORE_INCLUDE_PATH . DS . 'Cake' . DS . 'Console' . DS . 'Templates' . DS . 'skel' . DS . 'tmp' . DS ?¿

@markstory

markstory Nov 7, 2013

Owner

Using the -app param is generally not a good idea, and is only still around because removing it would blow up lots of userland deploy/cron tools. I'm hoping that in 3.0 we can remove that parameter as there won't be systemwide CakePHP installs anymore.

@markstory

markstory Nov 7, 2013

Owner

@bar I think it is a better default, now that caching and logging will continue to work it would be a good idea to switch this code around.

define('TMP', CAKE_CORE_INCLUDE_PATH . DS . 'Cake' . DS . 'Console' . DS . 'Templates' . DS . 'skel' . DS . 'tmp' . DS);
}
$boot = file_exists(ROOT . DS . APP_DIR . DS . 'Config' . DS . 'bootstrap.php');

2 comments on commit 8150b89

Member

AD7six replied Nov 7, 2013

Thats actually a relativly common cause of permission problems, where the cli user and the webserver user end up being unable to delete (or worse - read) tmp files created by the other user.

Contributor

bar replied Nov 7, 2013

I agree on that one too, but I don't think the skel dir should be supposed to contain those files either.

If you config a FileLog without the path parameter, and you use CakePHP as it is... no special cases here, those files should end up somewhere to be found, not in an obscure directory where you can only find digging in the code.

If the webroot TMP is not a place for them, sys_get_temp_dir() is the place where I would expect them to be if I don't define a path.

Please sign in to comment.