Conversation
Add render helper, use HttpException sendHeaders(), clarify severity codes.
Wild to think this was optional once.
The only time I did see this get used it was broken.
This just slipped in years ago and there's no evidence it has ever been used in any project.
This flatten/removes the web bootstrap.
To separate the 'app init' and the bootstrap actions.
Also defines ENVIRONMENT=test.
There was a problem hiding this comment.
Pull request overview
This PR refactors the legacy Kohana-centric bootstrap into smaller, dedicated components (Config, Errors, I18n) and reorganises the application startup sequence to reduce responsibilities inside Kohana::setup().
Changes:
- Introduces new
Sprout\Helpers\ConfigandSprout\Helpers\Errorshelpers, and moves several legacy Kohana responsibilities into them. - Reorganises the global bootstrap (
src/bootstrap.php) and splits bootstrap concerns intosrc/bootstrap/config.phpandsrc/bootstrap/kohana.php. - Removes legacy SSL enforcement helper/config and removes origin cleanup logic from
Router/BootstrapConfig.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/web/index.php | Renames entrypoint constant to ENTRYPOINT for new bootstrap flow. |
| tests/bootstrap.php | Aligns test bootstrap constants with new bootstrap expectations (ENTRYPOINT, ENVIRONMENT). |
| src/sprout/sprout_load.php | Stops calling I18n::init() here (moved earlier into global bootstrap). |
| src/sprout/core/utf8.php | Removes implicit Utf8::setup() side-effect to rely on bootstrap. |
| src/sprout/core/Kohana.php | “Guts” Kohana: delegates config/i18n/errors to new helpers and reduces setup() responsibilities. |
| src/sprout/core/Bootstrap.php | Removes SSL/origin-cleanup hooks; relies on new global bootstrap arrangement. |
| src/sprout/config/require_ssl.php | Removes legacy SSL config stub. |
| src/sprout/Helpers/Ssl.php | Removes legacy SSL enforcement helper. |
| src/sprout/Helpers/Sprout.php | Moves closeConnection() here; delegates backtrace helper to Errors. |
| src/sprout/Helpers/SessionStats.php | Re-attaches pageview tracking via events in init() (not via Kohana setup). |
| src/sprout/Helpers/Session.php | Replaces deprecated Kohana::keyString() with karmabunny\kb\Arrays::value(). |
| src/sprout/Helpers/Router.php | Removes origin-cleanup and apache error handling from router. |
| src/sprout/Helpers/Request.php | Adds Request::userAgent() (moved from legacy Kohana). |
| src/sprout/Helpers/Modules.php | Adds a type assertion comment for module instance creation. |
| src/sprout/Helpers/I18n.php | Moves setlocale() into I18n and adds new lang()/t() APIs. |
| src/sprout/Helpers/FileTransform.php | Routes image-resize exception handling through new Errors handler. |
| src/sprout/Helpers/Errors.php | New centralised error/exception handling and logging implementation. |
| src/sprout/Helpers/Config.php | New centralised config loading and resource finding implementation. |
| src/sprout/Exceptions/HttpNotFoundException.php | Adds a dedicated 404 exception type. |
| src/sprout/Exceptions/HttpException.php | Adds sendHeaders() helper and implements HTTP exception interface behaviour. |
| src/bootstrap/web.php | Removes the old web bootstrap shim. |
| src/bootstrap/phpunit.php | Removes duplicate core file loading and Kohana setup (now handled globally). |
| src/bootstrap/kohana.php | New Kohana-compat bootstrap segment (legacy constants + core includes). |
| src/bootstrap/config.php | New early bootstrap configuration (timezone/reporting/fatal errors toggle). |
| src/bootstrap/cliserver.php | Adjusts CLI-server flow to rely on global bootstrap. |
| src/bootstrap/BootstrapConfig.php | Removes origin-cleanup API from BootstrapConfig. |
| src/bootstrap.php | Major rework of overall bootstrap sequencing and responsibilities. |
| phpunit.xml.dist | Defines ENVIRONMENT=test for test runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self::$configuration['config'] = self::load('config'); | ||
|
|
||
| // Re-parse the include paths | ||
| self::includePaths(TRUE); | ||
| } |
There was a problem hiding this comment.
Config::get() initialises includePaths(TRUE) only once (when self::$configuration is first loaded). In the current bootstrap order, config is accessed (via I18n::init() etc) before modules are registered/loaded, so module paths won't be present in self::$include_paths for the remainder of the request and module config files/resources may not be discoverable. Consider rebuilding include paths (and clearing relevant caches) when modules are registered/loaded, or lazily recomputing include paths when Modules::getModules() changes.
There was a problem hiding this comment.
See this is why I rewrote the config helper in #178. It's such a mess and hard
No, any call to Config::get() will load the "core" config first, which is where module registering happens. It doesn't matter which config is first called, be it locale or sprout - the config loader always enforces "core first".
| require __DIR__ . '/bootstrap/config.php'; | ||
|
|
||
| Utf8::setup(); | ||
| I18n::init(); | ||
|
|
||
| @mkdir(STORAGE_PATH . 'cache', 0755, true); | ||
| @mkdir(STORAGE_PATH . 'temp', 0755, true); | ||
|
|
||
| require __DIR__ . '/bootstrap/kohana.php'; |
There was a problem hiding this comment.
The global error/exception handlers are registered after Utf8::setup(), I18n::init(), directory creation, and bootstrap/kohana.php. Any exceptions/errors thrown during those earlier steps will bypass Errors::exceptionHandler() and result in an unhandled fatal. Register the handlers immediately after bootstrap/config.php (or before any code that can throw), then proceed with Utf8/I18n/Kohana setup.
There was a problem hiding this comment.
That's not really a simple option, nor is is any better than what we had.
- We can't load our error handlers in PHPUNIT or when BOOTSTRAP_ONLY is active.
- We want UTF8 + i18n guarantees when rendering errors.
- Error pages may (and do) use the
Kohana::config- so this must be loaded first.
Any errors here really are critical application errors. Perhaps we wrap it and echo something friendly. Certainly configuring error_log early on is a good idea.
We can improve elsewhere. But the position of the error handlers is very considered.
Now there's never a question if these are declared or not because they get loaded with the autoloader.
This PR aims to do two things:
Kohanaclass into maintainable chunksOur bootstrap has been a bit of a mess for a while. This co-locates all consts, header editing, error handling. The Kohana
setup()method is left to manage only what it needs to (the output buffer + lifecycle events).New components:
ConfigclassErrorsclassI18nnow managessetlocale()Removed:
This is PR is largely a pre-step to fully removing Kohana in Sprout 5.0. It could be merged into v4.4 if we're feeling confident.