Namespace clash between local and system modules

[gwillem]

When local (in ~/.n98-magerun) and system modules (in /usr/local/share/n98-magerun) have the same namespace defined in n98-magerun.yaml, I would expect that one of them (ideally, the one in the homedir) takes precedence. However, this is not the case in the situation that the system yaml defines a command that has no corresponding path in ~/.n98-magerun.

Effectively, this breaks local installations when modules are installed globally.

$ magerun
PHP Fatal error:  Uncaught Error: Class 'Elgentos\Magento\Command\Media\Images\CleanCacheCommand' not found in phar:///usr/local/bin/n98-magerun/src/N98/Magento/Application.php:330
Stack trace:
#0 phar:///usr/local/bin/n98-magerun/src/N98/Magento/Application.php(719): N98\Magento\Application->registerCustomCommands()
#1 phar:///usr/local/bin/n98-magerun/src/N98/Magento/Application.php(663): N98\Magento\Application->init(Array, Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#2 /usr/local/bin/n98-magerun(8): N98\Magento\Application->run()
#3 {main}
  thrown in phar:///usr/local/bin/n98-magerun/src/N98/Magento/Application.php on line 330

~/.n98-magerun/modules/magerun-addons/n98-magerun.yaml:

autoloaders:
  Elgentos: %module%/src

commands:
  customCommands:
    - Elgentos\Magento\Command\System\Store\Config\BaseUrlSetCommand
    - Elgentos\Magento\Command\Media\SyncCommand
    - Elgentos\Magento\Command\Extension\TranslationsCommand
    - Elgentos\Magento\Command\Dev\TemplateVars
    - Elgentos\Magento\Command\Dev\OldAdminRouting
    - Elgentos\Magento\Command\Dev\Events\ListCommand
    - Elgentos\Magento\Command\Dev\Events\FireCommand
    - Elgentos\Magento\Command\Dev\Events\ListenCommand
    - Elgentos\Magento\Command\Customer\CleanTaxvatCommand
    - Elgentos\Magento\Command\System\Url\Rewrites\ExportCommand
    - Elgentos\Magento\Command\Media\Images\CleanTablesCommand
    - Elgentos\Magento\Command\Media\Images\DefaultImageCommand
    - Elgentos\Magento\Command\Media\Images\RemoveDuplicatesCommand
    - Elgentos\Magento\Command\Media\Images\RemoveOrphansCommand
    - Elgentos\Magento\Command\Dev\PossibleSqlInjection

/usr/local/share/n98-magerun/modules/Elgentos/n98-magerun.yaml

autoloaders:
  Elgentos: %module%/src

commands:
  customCommands:
    - Elgentos\Magento\Command\System\Store\Config\BaseUrlSetCommand
    - Elgentos\Magento\Command\Media\SyncCommand
    - Elgentos\Magento\Command\Extension\TranslationsCommand
    - Elgentos\Magento\Command\Dev\TemplateVars
    - Elgentos\Magento\Command\Dev\OldAdminRouting
    - Elgentos\Magento\Command\Dev\Events\ListCommand
    - Elgentos\Magento\Command\Dev\Events\FireCommand
    - Elgentos\Magento\Command\Dev\Events\ListenCommand
    - Elgentos\Magento\Command\Customer\CleanTaxvatCommand
    - Elgentos\Magento\Command\System\Url\Rewrites\ExportCommand
    - Elgentos\Magento\Command\Media\Images\CleanTablesCommand
    - Elgentos\Magento\Command\Media\Images\CleanCacheCommand
    - Elgentos\Magento\Command\Media\Images\DefaultImageCommand
    - Elgentos\Magento\Command\Media\Images\RemoveDuplicatesCommand
    - Elgentos\Magento\Command\Media\Images\RemoveOrphansCommand
    - Elgentos\Magento\Command\Dev\PossibleSqlInjection

Note that the system yaml has a new command (CleanCacheCommand) (cc @peterjaap)

[tkn98]

This might be because we don't recursively merge here so only one of the two is taken (on the level of the module, not on the level of the command) and the last module wins in the loading order. That's just a quick assumption.

Which order do you favor and why? homedir over global (the homedir is a mix-in into global, e.g. on a per command basis) or the other way round (and perhaps why).

[gwillem]

IMHO homedir over global is the most intuitive. User is more specific than system, and the most specific should take precedence. Also, global over homedir would not allow a user to overrule system modules (which are quite possibly beyond his control).

[tkn98]

The folder /usr/local/share/n98-magerun/modules is a pre-configured (via config.yaml in the repo) plugin folder.

It is not the system folder as you write - just for the wording.

Can you please run with the -vvv switch as it should show when each plugin config is loaded. This should show the loading order (so in case you have modules you don't want to show, just short the output).

From a first look into the code it's not yet clear to me if the order is wrong (which I think is not so far) or if it isn't something with the auto-loader.

[gwillem]

Hi Tom, thanks for your reply.

$ n98-magerun -vvv
Load dist config
Search for Magento in folder public
Found Magento in folder public
Load dist config
Load plugin config /usr/local/share/n98-magerun/modules/Elgentos/n98-magerun.yaml
Load plugin config /data/web/.n98-magerun/modules/Elgentos/n98-magerun.yaml
PHP Fatal error:  Class 'Elgentos\Magento\Command\Media\Images\CleanCacheCommand' not found in phar:///usr/local/bin/n98-magerun/src/N98/Magento/Application.php on line 330

To reproduce:

# have some system wide modules installed in /usr/local
mkdir -p ~/.n98-magerun
cp -a /usr/local/share/n98-magerun/modules ~/.n98-magerun
# remove random php file under ~/.n98-magerun and also the corresponding line in the home n98-magerun.yaml

[tkn98]

So if home-dir over a the previous usr-local one, this sound like a namespace clash for the auto-loader. w/o hacking it (composer autolader that is IIRC) we could only decide to let one of the same named plugins win. We perhaps stand on our own foots by recursive merging the configs at that level.

Do you have by chance the source of that plugin online on Github?

[gwillem]

The modules in this case are already on Github: https://github.com/peterjaap/magerun-addons

The initial error was caused because our customer (@peterjaap ;)) had an old version installed in his homedir, while we had a newer version installed system-wide.

[tkn98]

Now this makes sense. The home-dir config was merged, so additional commands to which there were no classes to be auto-loaded given then. These lead to the error of the not-found class as it was looking in the wrong directory.

That's a bummer for the override when it comes later.

We could check autoloaders and if a duplicate throw a configuration warning/exception and do not load the (second) plugin.

Then with the correct order of loading, the first could win (e.g. home-dir).

https://github.com/peterjaap/magerun-addons/blob/master/n98-magerun.yaml#L2

[ktomk]

I could now further look into this issue. It's not as straight forward as thought because the home-dir module did already replace the system-module, the problem is, it only did in part. The home-dir (later) module can add more commands (even the same again) but it can only replace the autoloader prefix (PSR-0). So unless the home-dir module ~does not~ provides all files for all existing classes, these are gone.

I now put a check into develop so that the fatal error is not triggered any longer. Instead it will give an error message naming the class-name of the command in question that can not be instantiated (happens when registering the command with the CLI application) any longer. So effectively the commands are still gone but the new ones should work as intended. Also the fatal error is gone.

[tkn98]

@gwillem: If you can please review the changes and provide your feedback. I assume it is now much more stable than before. And sorry for the delay.

[cmuench]

Please re-open if needed.