modmore / VersionX

Resource & Element Versioning Extra for MODX Revolution (supports 2.2 and up). Extends the core in a future-proof manner to keep copies of every change to resources, templates, template variables, chunks, snippets and plugins.
https://modmore.com/extras/versionx/
40 stars 20 forks source link

migrate.php doesn't work if the core directory is not in the site root directory #139

Closed SnowCreative closed 1 day ago

SnowCreative commented 4 days ago

migrate.php looks for "config.core.php" just above the core directory. If the core directory has been moved up out of the website root directory (a common practice for MODX 2), then this file doesn't exist in that location. I had to manually copy that file and paste it in the place the migrate script is looking for it. Why not use "../../config/config.inc.php" instead, since the migrate script will always already be inside the core folder?

Mark-H commented 1 day ago

The whole purpose of the config.core.php file in MODX is to instruct where the core directory is in case it has been moved. In addition, hardcoding config/config.inc.php would mean it is impossible to use a different config key (like config/live.inc.php, which again the config.core.php file is meant to inform us about.

So the solution here is to make sure there is a config.core.php in the root.

SnowCreative commented 1 day ago

But why does this script need to know where the core directory is? It is, necessarily, already in the core directory.

Without some way to fix this problem so that people don't have to manually copy a file to a new place, some users won't have an easy time upgrading to the new branch of VersionX without knowing stuff like this!

Mark-H commented 1 day ago

But why does this script need to know where the core directory is? It is, necessarily, already in the core directory.

That's fair. This code that instantiates the MODX service externally is pretty much boilerplate stuff that is more commonly used from an assets path, and in this specific case we could indeed infer the core path.

But we still need to know the config key as well as I mentioned in my earlier edit. The only place to get that is a config.core.php file which MODX itself also expects in the root.

SnowCreative commented 1 day ago

I'm working on a solution . . .

SnowCreative commented 1 day ago

OK, here's what I did that should work in most cases. I changed line 6:

require_once dirname(__DIR__, 3) . '/config.core.php';

to this:

$foundconfig = false;
if(file_exists(dirname(__DIR__, 3) . '/config.core.php')) {
    $foundconfig = true;
    require_once dirname(__DIR__, 3) . '/config.core.php';
} else if(file_exists(dirname(__DIR__, 2) . '/config/config.inc.php')) {
    // core directory is not in the site root 
    require_once dirname(__DIR__, 2) . '/config/config.inc.php';
    if(file_exists(MODX_BASE_PATH . 'config.core.php')) {
        $foundconfig = true;
        require_once MODX_BASE_PATH . 'config.core.php';
    }
}
if(!$foundconfig) {
    // can't find config.core.php 
    $modx->log(modX::LOG_LEVEL_ERROR, "Can't find config.core.php");
    exit;
}

I tested that, and it works on one of my sites. It still won't work if a custom config key is entered, but at least it will log an error if it can't find config.core.php and then stop processing.

The one scenario that might be problematic is if there are multiple config files inside the "config" folder, including "config.inc.php", and the config key is pointed to a different config file. In this case, if "config.inc.php" links to a different root directory than the file linked to the custom config key, then the script may start operating on the wrong database. I think that setup should be rare. Perhaps it would be a good idea to put a note about linking to custom config files when the core folder isn't in the site root in the "Migrating from 2.x" section at the bottom of the main Github page for this plugin.