FriendsOfShopware / FroshDevelopmentHelper

Helpful development tools
81 stars 16 forks source link

Composer deps within composer deps leads to error #17

Closed jissereitsma closed 3 years ago

jissereitsma commented 3 years ago

Even though it is not advertised in the installation instructions in README.md, I would rather install free developer tools via composer instead of relying upon them through the Shopware Store. A simple composer require frosh/development-helper allowed me to installed the tool without issues. However, the command bin/console plugin:install FroshDevelopmentHelper --activate caused a really weird error: Warning: require(/data/git/yireo/yireo-sites/www.yireo.com-shopware/vendor/frosh/development-helper/vendor/vendor/autoload.php): failed to open stream: No such file or directory.

To get rid of the issue, I hacked the file src/FroshDevelopmentHelper.php and removed the inclusion of the autoloader (because my installation is composer-driven already and doesn't need this method):

//if (file_exists(__DIR__ . '/../vendor/autoload.php')) {
//    require_once __DIR__ . '/../vendor/autoload.php';
//}

Obviously, core hacks are not good for mental health. So I'm opening this issue to discuss better ways of differentiating between a composer install of this helper and a non-composer install. The if statement could check for this and then determine whether or not including the autoloader is a good idea.

I've tested out a few things. Checking for getenv('COMPOSER_HOME') is not a solid approach. To me the best approach would be to check if the parent parent parent is vendor. And if so, the plugin is installed via composer. It is weird logic, but does the job:

$installedViaComposer = (basename(dirname(dirname(dirname(__DIR__)))) === 'vendor');
if (!$installedViaComposer && file_exists(__DIR__ . '/../vendor/autoload.php')) {
    require_once __DIR__ . '/../vendor/autoload.php';
}

Another mechanism could be to keep the current PHP statement as it is, but simply remove the local autoload.php file when the package is installed via composer. For this, the following section could be added to the composer.json file:

    "scripts": {
        "post-install-cmd": [
            "rm ./vendor/autoload.php"
        ]
    },

What do you think?

shyim commented 3 years ago

The error makes no sense to me so far 😂 . It requires the file only if exists 🤔

shyim commented 3 years ago

Maybe checking existence of the namespace in Class loader to determine if its in vendor 🤔

windaishi commented 3 years ago

Have a look at the method \Shopware\Core\Framework\Plugin::getAdditionalBundles. This is the first method called by Shopware. You can do the autoloading there. The method has a parameter AdditionalBundleParameters $parameters. This class has a property $kernelParameters. This again has a key kernel.plugin_infos that contains an array with all the plugins from the database. This list also has a field managedByComposer. With the help of this field you can decide whether the plugin is managed by composer or not.

jissereitsma commented 3 years ago

@shyim Indeed, it is strange. To explain a bit more, https://github.com/FriendsOfShopware/FroshDevelopmentHelper/blob/master/src/FroshDevelopmentHelper.php#L12 calls upon https://github.com/FriendsOfShopware/FroshDevelopmentHelper/blob/master/vendor/autoload.php which again tries to call upon a file vendor/vendor/autoload.php which is only installed when somebody would navigate into vendor/frosh/development-helper/vendor and run composer install there. Note that the weirdness is not necessarily caused by installing this extension is installed via composer, but that it includes a double vendor/vendor folder in its own source (https://github.com/FriendsOfShopware/FroshDevelopmentHelper/blob/master/vendor/autoload.php#L2).

@windaishi Thanks for the pointer. But I think we first need to determine what the goal is of this duplicate vendor/vendor folder. I can see that some dependencies (https://github.com/FriendsOfShopware/FroshDevelopmentHelper/blob/master/vendor/composer.json) are installed and others are replaced.

I'm able to duplicate this issue simply with the following commands in any fresh Shopware 6 Production Template (maybe I should have mentioned this all along :o):

composer require frosh/development-helper
bin/console plugin:refresh
bin/console plugin:install FroshDevelopmentHelper --activate

The more I hack myself into things (while I have to say I'm quite a newbie here), the more I guess that this extension was never intended for the Production Template, but only for the Development Template. Could that be correct?

breaker92 commented 3 years ago

We've got the same issue, we've added this plugin to require-dev. Checking also in the "https://github.com/FriendsOfShopware/FroshDevelopmentHelper/blob/master/vendor/autoload.php" if the target exists, would solve the problem in all cases i think.

shyim commented 3 years ago

Sorry for the late response. Its released