Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/phpstan.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ jobs:
php-version: ${{ matrix.php }}

- name: Install Dependencies
run: composer update --ignore-platform-reqs
run: |
# copy common.config.php for composer autoloader
cp core/config/common.config.sample.php core/config/common.config.php
# install composer dependencies
composer update --ignore-platform-reqs

- name: Restore PHPStan cache
id: cache-phpstan
Expand Down
16 changes: 16 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,22 @@
"php": "7.4"
}
},
"autoload": {
"psr-4": {
"Jeedom\\Core\\": "src/",
"Jeedom\\Plugins\\": "plugins/"
},
"files": [
"core/config/common.config.php",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Un point gênant sur common.config.php dans files : en mode Docker, ce fichier n'existe pas au moment du docker build.

Le Dockerfile enchaîne step 9 puis step 10 de install.sh. Or step 9 (la copie common.config.sample.php → common.config.php) est skip en mode docker (install.sh:307, if [ "${INSTALLATION_TYPE}" != "docker" ]). Elle est faite plus tard par install/OS_specific/Docker/init.sh:98 — mais c'est au démarrage du conteneur, pas au build.

Du coup step 10 (qui lance composer install) va planter au build de l'image officielle parce que composer ne trouvera pas le fichier listé dans files.

Même souci pour le workflow CI, c'est d'ailleurs pour ça que tu as dû patcher phpstan.yaml.

Je propose de sortir common.config.php de files et de garder un require_once explicite dans core.inc.php avant vendor/autoload.php :

   "files": [
-     "core/config/common.config.php",
      "core/php/utils.inc.php",
      "core/config/compatibility.config.php"
   ],
 date_default_timezone_set('Europe/Brussels');
+require_once __DIR__ . '/../config/common.config.php';
 require_once __DIR__ . '/../../vendor/autoload.php';

Comme ça tu peux aussi annuler la modif du workflow phpstan.yaml.

"core/php/utils.inc.php",
"core/config/compatibility.config.php"
],
"classmap": [
"core/class/",
"core/com/",
"core/repo/"
]
},
"require-dev": {
"phpstan/phpstan": "^2.1"
}
Expand Down
6 changes: 5 additions & 1 deletion core/class/log.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ public function log($level, $message, array $context = array()) {

public static function getConfig($_key, $_default = '') {
if (self::$config === null) {
self::$config = array_merge(config::getLogLevelPlugin(), config::byKeys(array('log::engine', 'log::formatter', 'log::level', 'addMessageForErrorLog', 'maxLineLog', 'maxSizeLog')));
try {
self::$config = array_merge(config::getLogLevelPlugin(), config::byKeys(array('log::engine', 'log::formatter', 'log::level', 'addMessageForErrorLog', 'maxLineLog', 'maxSizeLog')));
} catch (Exception $e) {
self::$config = array();
}
Comment on lines +62 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Deux petites choses :

  1. Le catch (Exception $e) ne rattrape pas les Error (ex. erreur fatale côté DB). Dans le reste de ta PR tu es passé à Throwable, ce serait cohérent de faire pareil ici.
  2. Si le catch avale silencieusement, on ne saura jamais qu'on tourne avec une config log vide → très pénible à diagnostiquer.

Proposition :

Suggested change
try {
self::$config = array_merge(config::getLogLevelPlugin(), config::byKeys(array('log::engine', 'log::formatter', 'log::level', 'addMessageForErrorLog', 'maxLineLog', 'maxSizeLog')));
} catch (Exception $e) {
self::$config = array();
}
try {
self::$config = array_merge(config::getLogLevelPlugin(), config::byKeys(array('log::engine', 'log::formatter', 'log::level', 'addMessageForErrorLog', 'maxLineLog', 'maxSizeLog')));
} catch (Throwable $e) {
self::$config = array();
error_log('Jeedom: log::getConfig failed, using empty config: ' . $e->getMessage());
}

}
if (isset(self::$config[$_key])) {
return self::$config[$_key];
Expand Down
3 changes: 3 additions & 0 deletions core/class/update.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,9 @@ public function doUpdate() {
if (file_exists($cibDir)) {
rrmdir($cibDir);
}
// re generate composer autoloader
$result = shell_exec(system::getCmdSudo() . ' composer dump-autoload --optimize --working-dir $WEBSERVER_HOME');
log::add(__CLASS__, 'debug', "Composer autoloader dump result:\n$result");
Comment on lines +360 to +362

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Salut, quelques soucis sur ce shell_exec :

  1. $WEBSERVER_HOME entre guillemets simples n'est pas interprété par PHP. C'est le shell qui va essayer de lire la variable d'environnement, mais elle n'est pas définie côté Apache/nginx au runtime (seulement pendant install.sh). Donc --working-dir sera vide et composer tournera au mauvais endroit.
  2. Il manque COMPOSER_ALLOW_SUPERUSER=1, comme dans system.class.php:432, sinon composer peut râler en root.
  3. Si composer plante, on ne saura pas pourquoi (pas de 2>&1, pas de try/catch).

Je te propose ça :

Suggested change
// re generate composer autoloader
$result = shell_exec(system::getCmdSudo() . ' composer dump-autoload --optimize --working-dir $WEBSERVER_HOME');
log::add(__CLASS__, 'debug', "Composer autoloader dump result:\n$result");
try {
$workingDir = realpath(__DIR__ . '/../..');
$cmd = 'export COMPOSER_ALLOW_SUPERUSER=1; '
. system::getCmdSudo() . ' composer dump-autoload --optimize --working-dir '
. escapeshellarg($workingDir) . ' 2>&1';
$result = shell_exec($cmd);
log::add(__CLASS__, 'debug', "Composer autoloader dump result:\n" . (string) $result);
} catch (Throwable $e) {
log::add(__CLASS__, 'error', 'Composer dump-autoload failed: ' . $e->getMessage());
}

Comme ça : chemin calculé en PHP, path échappé, erreurs capturées, et si ça plante ça casse pas la suite de l'update.

} else {
throw new Exception(__("Impossible de décompresser l'archive zip", __FILE__) . ' : ' . $tmp . ' => ' . ZipErrorMessage($res));
}
Expand Down
59 changes: 25 additions & 34 deletions core/php/core.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,60 +17,51 @@
*/
date_default_timezone_set('Europe/Brussels');
require_once __DIR__ . '/../../vendor/autoload.php';
require_once __DIR__ . '/../config/common.config.php';
require_once __DIR__ . '/../class/DB.class.php';
require_once __DIR__ . '/../class/config.class.php';
require_once __DIR__ . '/../class/jeedom.class.php';
require_once __DIR__ . '/../class/plugin.class.php';
require_once __DIR__ . '/../class/translate.class.php';
require_once __DIR__ . '/utils.inc.php';
include_file('core', 'jeedom', 'config');
include_file('core', 'compatibility', 'config');
include_file('core', 'utils', 'class');
include_file('core', 'log', 'class');

try {
$configs = config::byKeys(array('timezone', 'log::level'));
if (isset($configs['timezone'])) {
date_default_timezone_set($configs['timezone']);
}
} catch (\Throwable $e) {
log::add('jeedom', 'error', 'Log (level|timezone) configuration failed: ' . $e->getMessage());
}

try {
if (isset($configs['log::level'])) {
log::define_error_reporting($configs['log::level']);
}
} catch (\Throwable $e) {
log::add('jeedom', 'error', 'Log (level|timezone) configuration failed: ' . $e->getMessage());
}

/**
* Autoload function for specific Jeedom classes
* this function is called after the default Composer autoloader
* it will load specific classes such as Cmd, Real, etc.
* for plugins that do not use namespaces or Composer autoloading
*/
function jeedomAutoload($_classname) {
/* core class always in /core/class : */
$path = __DIR__ . "/../../core/class/$_classname.class.php";
if (file_exists($path)) {
include_file('core', $_classname, 'class');
} else if (substr($_classname, 0, 4) === 'com_') {
/* class com_$1 in /core/com/$1.com.php */
include_file('core', substr($_classname, 4), 'com');
} else if (substr($_classname, 0, 5) === 'repo_') {
/* class repo_$1 in /core/repo/$1.repo.php */
include_file('core', substr($_classname, 5), 'repo');
} else if (strpos($_classname, '\\') === false && strpos($_classname, '/') === false) {
/* autoload for plugins : no namespace */
$classname = str_replace(array('Real', 'Cmd'), '', $_classname);
if (strpos($_classname, '\\') !== false || strpos($_classname, '/') !== false) {
return;
}
/* autoload for plugins : no namespace */
$classname = str_replace(array('Real', 'Cmd'), '', $_classname);
$plugin_active = config::byKey('active', $classname, null);
if (($plugin_active === null || $plugin_active == '' || $plugin_active == 0) && strpos($classname, '_') !== false) {
$classname = explode('_', $classname)[0];
$plugin_active = config::byKey('active', $classname, null);
if (($plugin_active === null || $plugin_active == '' || $plugin_active == 0) && strpos($classname, '_') !== false) {
$classname = explode('_', $classname)[0];
$plugin_active = config::byKey('active', $classname, null);
}
if ($plugin_active == 1) {
try {
include_file('core', $classname, 'class', $classname);
} catch (Throwable $e) {
log::add('plugin', 'error', $e->getMessage());
}
}
if ($plugin_active == 1) {
try {
include_file('core', $classname, 'class', $classname);
} catch (\Throwable $e) {
log::add('plugin', 'error', $e->getMessage());
}
}
}

spl_autoload_register('jeedomAutoload', true, true);
// Composer's autoloader (registered via vendor/autoload.php above) runs first.
// jeedomAutoload is a fallback for legacy plugin classes without namespace.
spl_autoload_register('jeedomAutoload');
Comment thread
pifou25 marked this conversation as resolved.
7 changes: 7 additions & 0 deletions core/php/utils.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -1839,3 +1839,10 @@ function implode_recursive($_array, $_separator, $_key = '') {
}
return $result;
}

/**
* alias for translate::sentence
*/
function __($_content, $_name, $_backslash = false) {
return translate::sentence(str_replace("\'", "'", $_content), $_name, $_backslash);
}
Loading