akeeba / fof

Rapid Application Development framework for Joomla!™ 3 and 4
0 stars 0 forks source link

CLI - Impossible to connect to the database #599

Closed Eighke closed 8 years ago

Eighke commented 8 years ago

Nic,

You answered to my second point but you skipped first one (and more important). I have no access to the database using the CLI : https://github.com/akeeba/fof/issues/596#issuecomment-205263313

I tried to copy the PHAR in the Joomla root, in the /cli folder. I even copied all the PHP inside /cli to use the fof.php instead. But it always fail, it init the database with the nothing (the default one).

/help :cry: :sos:

Eighke commented 8 years ago

Ok found it, it was easy in final. It just missing the include of the configuration file.

require_once JPATH_CONFIGURATION . '/configuration.php';
Eighke commented 8 years ago

Ok I found the problem.

# App.php L.102
$this->loadConfiguration($this->fetchConfigurationData());

It successfully loads the configuration and store it inside $this->config. But it is used nowhere. Everything is using JFactory::getConfig() and loadConfiguration only load the configuration in $this->config.

The problem is that \JFactory::getConfig() is called before CLI::loadConfiguration(). So the JLoader::register($class, $file); of CLI::fetchConfigurationData() is not registered yet. So it will use the default file defined in \JFactory::getConfig()`...

And the default file is JPATH_PLATFORM . '/config.php' (libraries/config.php). Which doesn't exists.

So no config is loaded...


Well, we have a Joomla bug (I guess, config.php was removed but not its call). But I don't know what should be the default behaviour. If it is to load the configuration.php, it will fix our problem.

Else I think we have no choice but to include JPATH_CONFIGURATION . '/configuration.php'; in fof.php. Because I tried to add \JFactory::$config = $this->config; after $this->loadConfiguration() but it is too late the JFactory::getDbo(); is already loaded with the wrong param in JInputCLI(). And we cannot use loadConfiguration before JInputCLI().


So, I'm not sure what to do. :)

nikosdion commented 8 years ago

This is a change in behavior in Joomla! 3.5. Please note how all our CLI scripts are currently loading configuration.php. For example, https://github.com/akeeba/docimport/blob/development/component/cli/docimport-update.php#L52 https://github.com/akeeba/docimport/blob/development/component/cli/docimport-update.php#L52 I believe those five lines tell you all the story :)

Eighke commented 8 years ago

Seriously. :(

I see, it is because of $stripUSC = -1 in JFilterInput that was added. So it use getDbo() before the config could be Registered.

For me it look more like a bug than a behaviour change. Why continue to register JConfig and configuration.php in fetchConfigurationData, else? And it looks like a BC break too... all CLI that was using the doExecute() w/o including configuration.php will fail on J! 3.5+...

I'm pretty sure it was not detected because all Joomla CLI require_once the configuration.php anyway.


if (version_compare(JVERSION, '3.4.9999', 'ge'))
{
    // Joomla! 3.5 and later does not load the configuration.php unless you explicitly tell it to.
    JFactory::getConfig(JPATH_CONFIGURATION . '/configuration.php');
}

Couldn't we replace by:

JLoader::register('JConfig', JPATH_CONFIGURATION . '/configuration.php');

Anyway, do you know why JPATH_PLATFORM . '/config.php' still here? Shouldn't it load configuration.php or just nothing?

nikosdion commented 8 years ago

It is a blatant b/c break. Of course nobody in Joomla! gives half a flying fuck about 3PD software breaking. If you complain about b/c breaks you'll be told it's your fault. Even when, let's say, JMail is completely broken for 5 weeks and they decide not to publish a fix, instead planning on going straight to Joomla! 3.6.

Regarding the config.php, that's a remnant from the Joomla! Platform (the dead as a dodo one, not the Joomla! Framework that's still on life support until they completely cock up Joomla! with "Joomla!" 4).

As for the solution, I chose to stick with the minimum required code. For all I know they might screw up JLoader too. I can't trust them. They have no clue what a mass distributed, open source, software platform means – let alone being anywhere remotely close to being barely competent to maintain and release one.

mbabker commented 8 years ago

Joomla! 3.5 and later does not load the configuration.php unless you explicitly tell it to.

Actually, JFactory::getConfig() has NEVER had a good default for the config file path (it's always used something counter to what the CMS has actually used, sadly I dug back to 2007 commits to see that counter-intuitive thinking). But because of https://github.com/joomla/joomla-cms/blob/3.5.1/includes/framework.php#L46 (and similar for admin), the CMS web apps will never run into a scenario where JConfig doesn't get loaded. CLI apps have pretty much always required manual import of the config file if they do something that needs it, 3.5 because of the USC detection in JFilterInput pretty much makes that a requirement. Fix JFactory and the hacks go away. But alas, making sense isn't something I advocate for anymore since usually the opposite happens.

Eighke commented 8 years ago

@mbabker Well, before the change in 3.5 fetchConfigurationData was registering JConfig and it still doing it : https://github.com/joomla/joomla-cms/blob/3.5.1/libraries/joomla/application/cli.php#L252

So the CLI scripts never had to manual import the config for anybody only using the doExecute(). The default Joomla CLI are all require_once the configuration, but it was never needed.


But that right JFactory::getConfig() has a problem since the start, the default config is wrong as far as you check in the repository...

My question is how it was never detected before? Or did it was but nobody really care? ^^;


@nikosdion Yes, it is sad. :(

I'll push a PR with your code so.

mbabker commented 8 years ago

So the CLI scripts never had to manual import the config for anybody only using the doExecute(). The default Joomla CLI are all require_once the configuration, but it was never needed.

That's still too late for JFilterInput because it's instantiated as a dependency of JInput. It also only loads the configuration into the local application class, it isn't loading into JFactory::$config. So the config could get loaded into the app OK from CLI, but since Joomla is written to work with the config from JFactory it really doesn't matter.

My question is how it was never detected before?

Because the web application bootstraps (which everyone but maybe a couple dozen folks in Joomla world use) preload JConfig before booting the application so it passes the class_exists() check in JFactory::createConfig(). No obvious bug unless you take out that pre-loading step to set the error reporting level that the CLI apps don't have.

Eighke commented 8 years ago

That's still too late for JFilterInput because it's instantiated as a dependency of JInput. It also only loads the configuration into the local application class, it isn't loading into JFactory::$config. So the config could get loaded into the app OK from CLI, but since Joomla is written to work with the config from JFactory it really doesn't matter.

@mbabker Yes I notice that. :)

But Shouldn't the Register in fetchConfigurationData be removed? It is confusing, registering something that will never been used. If you are only checking ApplicationCLI you will think that the configuration is registered.

Because the web application bootstraps (which everyone but maybe a couple dozen folks in Joomla world use) preload JConfig before booting the application so it passes the class_exists() check in JFactory::createConfig(). No obvious bug unless you take out that pre-loading step to set the error reporting level that the CLI apps don't have.

Yes, I know that, my question was more: how this situation could happen (using a non-existant file). libraries/config.php was deleted, but nobody checked if this file was in use somewhere in Joomla? /scary

Eighke commented 8 years ago

I guess we can close this issue. Looks like the problem was fixed in Joomla itself.

https://github.com/joomla/joomla-cms/pull/9996