Repository navigation
feat: Add ability to read the custom location of config.php - #413
NingZhou-NZ wants to merge 1 commit into
Conversation
|
|
||
| # Does the current directory have a site config, either as ./config.php or via TOTARA_CONFIG_PATH? | ||
| has_site_config() { | ||
| if [[ -f './config.php' ]]; then |
There was a problem hiding this comment.
has_site_config checks ./config.php first, but site_config_file checks TOTARA_CONFIG_PATH first. If the env var is set for the whole container, a site that has its own config.php gets read through the env file instead. The DB prompt, behat_dataroot and behat_wwwroot then point at the wrong site. Both helpers should use the same order, and a local config.php should probably win.
| local original_path=$(pwd) | ||
| local current_path="$original_path" | ||
| while [[ ! -f './config.php' || -f './config.php' && -f '../config.php' ]] && | ||
| while { ! has_site_config || [[ -f './config.php' && -f '../config.php' ]]; } && |
There was a problem hiding this comment.
When there's no root config.php and TOTARA_CONFIG_PATH is set, this stops at server/, because server/config.php always exists and there's no ../config.php to push it up. Running unit from server/lib then cds into server/, and the ./test/phpunit, ./server/version.php and ./server/$2 checks all miss.
|
|
||
| # Resolve the config file for the current site root. | ||
| # Totara 21+ supports loading the config from a custom location via the TOTARA_CONFIG_PATH environment variable. | ||
| site_config_file() { |
There was a problem hiding this comment.
behat_parallel_count in shell/behat-aliases.sh:9 still does require("config.php"). With no root config, PHP stops with a fatal error and install_behat passes the error text to --parallel=. It should use site_config_file too.
| fi | ||
| # When the config file lives outside the code directory, identify the site root by the | ||
| # version.php + server directory combination instead (TOTARA_CONFIG_PATH requires Totara 13+ layout). | ||
| if [[ -n "$TOTARA_CONFIG_PATH" && -f "$TOTARA_CONFIG_PATH" && -f './version.php' && -d './server' ]]; then |
There was a problem hiding this comment.
With the env var set, any checkout under src with version.php and server/ counts as configured, even when it has no config of its own. install or upgrade in an unconfigured site would run against the database in the env config instead of failing.
| #OTEL_METRICS_EXPORTER=otlp | ||
|
|
||
| # Custom location of config.php | ||
| #TOTARA_CONFIG_PATH=/your/totara/config/src/path No newline at end of file |
There was a problem hiding this comment.
This is passed into the container as is, with no volume mount. A host path fails the -f check and falls back to ./config.php without any warning. The comment should say it must be a container path, for example under /var/www/totara/src. The file also has no newline at the end.
| OTEL_TRACES_EXPORTER: ${OTEL_TRACES_EXPORTER:-} | ||
| OTEL_LOGS_EXPORTER: ${OTEL_LOGS_EXPORTER:-} | ||
| OTEL_METRICS_EXPORTER: ${OTEL_METRICS_EXPORTER:-} | ||
| TOTARA_CONFIG_PATH: ${TOTARA_CONFIG_PATH:-} |
There was a problem hiding this comment.
This always defines the var in the container, as '' when it isn't set. If core checks getenv('TOTARA_CONFIG_PATH') !== false, every site on 8.4/8.5 tries to load ''. Core should treat an empty value as unset. The same line is at 377, 408 and 445.
| define(\"CLI_SCRIPT\", true); | ||
| define(\"ABORT_AFTER_CONFIG\", true); | ||
| require(\"config.php\"); | ||
| require(\"$config_file\"); |
There was a problem hiding this comment.
The path goes into a PHP double-quoted string without escaping. A path with $, " or \ gets expanded or breaks. Pass it in through $argv or use single quotes.
| return 0 | ||
| fi | ||
| # When the config file lives outside the code directory, identify the site root by the | ||
| # version.php + server directory combination instead (TOTARA_CONFIG_PATH requires Totara 13+ layout). |
There was a problem hiding this comment.
This says 13+, but site_config_file says 21+. Only T21 reads TOTARA_CONFIG_PATH, so this should say T21+.
| # Is this directory the root of a Totara/Moodle site? | ||
| is_site_root() { | ||
| [[ -f './config.php' && -f './version.php' ]] && return 0 || return 1 | ||
| [[ -f './version.php' ]] && has_site_config && return 0 || return 1 |
There was a problem hiding this comment.
version.php is checked here and again in has_site_config, and the env test is repeated in site_config_file. All of this runs on every prompt render. One helper that resolves the config path once, and returns non-zero when there is none, could serve has_site_config, site_config_file and behat_parallel_count. It would also fix the order mismatch on line 44.
2 participants
Description
This PR is about to add the ability to read the config.php from the custom location which is set in the environment variable.
This feature test with the change of TL-50053
Testing Instructions
Checklist
bin/directory run correctly on both MacOS and WSLconfig.phpare compatible with our oldest supported Totara version, our newest Totara version, and Moodle