Skip to content

Commit

Permalink
Allow setting path constants in app/Console/cake.php as you can in we…
Browse files Browse the repository at this point in the history 
…broot/index.php
  • Loading branch information
@ADmad
ADmad committed Aug 11, 2013
1 parent f63aa92 commit 8150b89
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions lib/Cake/Console/ShellDispatcher.php
View file
Expand Up @@ -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')) {

This comment has been minimized.

Sign in to view

Copy link
@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?¿

This comment has been minimized.

Sign in to view

Copy link
@markstory

markstory Nov 7, 2013

Member

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.

This comment has been minimized.

Sign in to view

Copy link
@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?

This comment has been minimized.

Sign in to view

Copy link
@markstory

markstory Nov 7, 2013

Member

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.

This comment has been minimized.

Sign in to view

Copy link
@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?¿

This comment has been minimized.

Sign in to view

Copy link
@markstory

markstory Nov 7, 2013

Member

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.

This comment has been minimized.

Sign in to view

Copy link
@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.

This comment has been minimized.

Sign in to view

Copy link
@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/

This comment has been minimized.

Sign in to view

Copy link
@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

This comment has been minimized.

Sign in to view

Copy link
@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 ?¿

This comment has been minimized.

Sign in to view

Copy link
@markstory

markstory Nov 7, 2013

Member

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.

This comment has been minimized.

Sign in to view

Copy link
@markstory

markstory Nov 7, 2013

Member

@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');
Expand Down

2 comments on commit 8150b89

@AD7six
Copy link
Member

@AD7six AD7six commented on 8150b89 Nov 7, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@bar
Copy link
Contributor

@bar bar commented on 8150b89 Nov 7, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.