-
Notifications
You must be signed in to change notification settings - Fork 55
Add ability to specify config file for modx:install #219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Currently, the required configuration options are passed through the command line during Gitify modx:install. This prevents the installation process from being automated further, as described in modmore#193. To solve this, a physical config file can now be defined in the modx:install command as follows: Gitify modx:install latest /absolute/path/to/config.xml
|
Getting some errors now, so it needs a bit of tweaking still.. |
|
Never mind, it's working fine. Used the same variable name twice in a bash script, so it was feeding Gitify the wrong path.. |
src/Command/InstallModxCommand.php
Outdated
|
|
||
| // Create the XML config | ||
| $config = $this->createMODXConfig(); | ||
| if ($configFile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably check if it can find the file, and not just "is it set"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good one. I updated that, so it aborts when the file is not found. I think checking for valid XML is a bit too much here, also because there's no need for the config file to start with an XML declaration (meaning more difficult to check).
I also noticed that the config file was being deleted automatically by the unlink function down the road, so added an exception there too.
|
How would you use this? Can this move the core path (like #223) or would you need to manually move those directories? |
And prevent it from being deleted after installation..
|
Usage: by providing an XML file with all the config variables. For example: I tried changing the core path this way, but that doesn't move the entire core folder for me. Only core/components is added in the designated location (empty), but I assume the goal would be to move the whole core folder? The config.core.php file also points to the default location, so I guess this feature would need a little more work still.. |
|
By the way, is it possible to add flags to the command line arguments somehow? So that the command can be something like |
|
I tested this patch today on Please, implement accept this PR, and change command format to Thanks for @hugopeek for this important work! |
|
Yes flags should be possible, please look at the symfony console docs for details. |
sebastian-marinescu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make it a flag
|
Directly after $cp_option = "";
$doc = new \DOMDocument;
if ($doc->load($config)) {
$cp = $doc->getElementsByTagName('core_path')->item(0)->nodeValue;
$cp_option = "--core_path=" . $cp;
} else {
$output->writeln("<error>Provided config file is not valid.</error>");
}
// If core_path is provided, move the core directory to specified new path
if (!empty($cp) && !file_exists($cp)) {
$output->writeln("Config provided a non-standard core_path, moving directory now...");
exec("mv {$wd}core {$cp}", $cpOutput, $cpReturn);
if ($cpReturn) {
$output->writeln("<error>Moving core-directory failed - maybe a permissions issue?</error>");
}
$output->writeln("<comment>{$cpOutput[0]}</comment>");
}
// Actually run the CLI setup
$output->writeln("Running MODX Setup...");
exec("php -d date.timezone={$tz} {$wd}setup/index.php --installmode=new --config={$config} {$cp_option}", $setupOutput); |
|
@sebastian-marinescu I'm not that familiar with advanced installs and don't have energy left to test your suggestion.. Shall we create a separate PR for that, and try to get this merged first? |
|
@hugopeek If I remember it correctly, my thinking was that now that a user can provide a different core-path through the config-xml, Gitify and the InstallCommand should acknowledge that and move the extracted core/ directory to the new specified location. I'll test your branch soon with two config-files and then also with one changing the core-path. |
|
@Mark-H What's keeping this PR from being merged? I added a flag as @sebastian-marinescu suggested and I've been using it for years without issue. This change is not intended to be used for moving the core folder. Only to run the installer without user interaction, which is necessary when using Gitify through a CLI tool like an automated installer or a Gitlab runner (as @alexander-mart pointed out). And from what I understood, MODX3 won't support moving the core folder anyway, so.. Good to go? :) |
|
Oh sorry, I didn't see that there are conflicts. Will sort that out first. |
Currently, the required configuration options are passed through the command line during Gitify modx:install. This prevents the installation process from being automated further, as described in modmore#193. To solve this, a physical config file can now be defined in the modx:install command as follows: Gitify modx:install latest /absolute/path/to/config.xml
And prevent it from being deleted after installation..
… into modx-install-config-file
|
Hi @hugopeek , I'll be doing a bunch of testing and merging towards the end of this week. This one is top of the list! :) |
|
Thanks @muzzwood :) I was also looking at it already. It has become incompatible after this pull request was merged: #223 That adds some code after I'll try to move things around a little so these 2 functionalities can co-exist. Would be nice if the corePathParameter can also be used in a physical config file. |
|
Perhaps in the InstallModxCommand, replace: with and then in createMODXConfig, only interactively ask for values not present in |
The createMODXConfig function now evaluates any provided config data first, and only becomes interactive if a certain value is not provided. This way, it integrates nicely with the ability to move the core. If you specify an alternative location in config.xml, the core will be moved there. The Core Path and Manager Directory questions are also shortened to match the formatting of the others. I'm also scratching my own itch by adding variables for DB charset and collation. See: modmore#265
|
@Mark-H That's a good idea! I did that, and now it integrates nicely with the option to specify an alternative location for the core. If a path is specified outside of the project root in config.xml, it will be moved. And only properties not set in config.xml will invoke a question. |
muzzwood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works great, @hugopeek . Nice work!
What does it do ?
This adds the ability to define a physical config file for the modx:install command as follows:
Gitify modx:install latest /absolute/path/to/config.xmlIt doesn't need to be a .xml file btw, can also be a temp file like /tmp/tmp.CDyHpH4jYP. Just make sure the file has proper read permissions..
Why is it needed ?
Currently, the required configuration options are passed through the command line during Gitify modx:install. This prevents the installation process from being automated further.
Related issue(s)/PR(s)
As described in #193.