Skip to content

Commit

Permalink
Full url prefix is better defined before auto loading
Browse files Browse the repository at this point in the history
  • Loading branch information
@zoghal
zoghal committed Aug 9, 2012
1 parent 1ff6375 commit 36243b4
Showing 1 changed file with 14 additions and 11 deletions.
25 changes: 14 additions & 11 deletions lib/Cake/bootstrap.php
View file
Expand Up @@ -126,21 +126,12 @@
}




require CAKE . 'basics.php';
require CAKE . 'Core' . DS . 'App.php';
require CAKE . 'Error' . DS . 'exceptions.php';

spl_autoload_register(array('App', 'load'));

App::uses('ErrorHandler', 'Error');
App::uses('Configure', 'Core');
App::uses('CakePlugin', 'Core');
App::uses('Cache', 'Cache');
App::uses('Object', 'Core');
App::$bootstrapping = true;

Configure::bootstrap(isset($boot) ? $boot : true);


/**
* Full url prefix
Expand All @@ -158,3 +149,15 @@
}
unset($httpHost, $s);
}

spl_autoload_register(array('App', 'load'));

App::uses('ErrorHandler', 'Error');
App::uses('Configure', 'Core');
App::uses('CakePlugin', 'Core');
App::uses('Cache', 'Cache');
App::uses('Object', 'Core');
App::$bootstrapping = true;

Configure::bootstrap(isset($boot) ? $boot : true);

10 comments on commit 36243b4

@bar
Copy link
Contributor

@bar bar commented on 36243b4 Jun 13, 2013

Choose a reason for hiding this comment

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

Hello @zoghal, can you explain me the rationale behind this change?¿

FULL_BASE_URL is a Cake construction, and will likely not be defined before bootstrapping, attempting to do it before will not allow the user to modify it inside core.php, and that is a major problem when called from the console, where ShellDispatcher::_bootstrap() will always define FULL_BASE_URL unless already defined.

Before this change, you could do something like this inside core.php

/**
 * HTTP_HOST is set so that Router can build valid links when called from php-cli.
 */
if (empty($_SERVER['HTTP_HOST'])) {
    $_SERVER['HTTP_HOST'] = $hostname;
}

and Cake/bootstrap.php will handle it nicely.

@ADmad
Copy link
Member

@ADmad ADmad commented on 36243b4 Jun 13, 2013

Choose a reason for hiding this comment

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

@bar This might please you 😄

@bar
Copy link
Contributor

@bar bar commented on 36243b4 Jun 13, 2013

Choose a reason for hiding this comment

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

Thanks @ADmad! I saw this in 2.4, but still has the same issue because the test is done before calling Configure::bootstrap() which forces the user to copy the code and paste it inside App/core.php, so that ShellDispatcher::_bootstrap() don't define it.

Making the test after Configure::bootstrap() eased that, allowing the user to set HTTP_HOST in App/core.php if needed and Cake will handle it later in a cleaner way.

I think this is more evident when making calls from php-cli, and even more when using queues, like Resque, and attempting to create valid links using the Router, which was a mid-field goal :p

@lorenzo
Copy link
Member

Choose a reason for hiding this comment

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

I actually had the same problem today as @bar, I think we need to revert this commit

@bar
Copy link
Contributor

@bar bar commented on 36243b4 Jun 13, 2013

Choose a reason for hiding this comment

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

👍 as a bugfix in 2.3.x. It is actually quite difficult to get if not familiarized with the web/cli dispatching workflow.
BTW, I'm already loving App.fullbaseURL.

@markstory
Copy link
Member

Choose a reason for hiding this comment

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

Instead of defining $_SERVER['HTTP_HOST'] could you not just define the constant?

@bar
Copy link
Contributor

@bar bar commented on 36243b4 Jun 13, 2013

Choose a reason for hiding this comment

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

Yep @markstory but that basically involves duplicating the code, and because HTTP_HOST tends to be more usable, sometimes you need it defined, which can be helpful.

Also, setting a PHP var like HTTP_HOST when called from php-cli, and relying on Cake to seamlessly do the job later, seems to be cleaner and more transparent IMHO.

@bar
Copy link
Contributor

@bar bar commented on 36243b4 Jun 13, 2013

Choose a reason for hiding this comment

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

Besides, I don't recall a use case where this can be useful before Configure::bootstrap(), where FULL_BASE_URL will never be defined, either by php here or php-cli here, am I right?¿

I think it was useful when being after it, so that if not bootsrapped there was still a chance to define it.

@markstory
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really too fussy either way on this one. Both approaches have drawbacks. Bootstrapping before the constant is defined means you cannot use the constant in bootstraping, but can define it as part of the app/Config/bootstrap.php. Bootstrapping after the constant is defined means you can use it in your application bootstrap, but cannot define it in the bootstrap process, and must rely on the automatic value.

@bar
Copy link
Contributor

@bar bar commented on 36243b4 Jun 14, 2013

Choose a reason for hiding this comment

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

@markstory sorry, but I can't imagine a use case where you'd what to use the constant in bootstrapping, that was my question to @zoghal in the first place :p, can you give me a hint on that? I do believe it presents useful when building urls, though.

Please sign in to comment.