captainhookphp / plugin-composer

A composer-Plugin to easily use CaptainHook
MIT License
19 stars 4 forks source link

Reduce dependencies #20

Closed Eydamos closed 9 months ago

Eydamos commented 9 months ago

I specifically installed captainhook/captainhook-phar to keep the dependencies small but when I also install captainhook/plugin-composer it additionally installs captainhook/captainhook as it is a dependency.

As composer does not have a feature like require package A or B could you either make a new package like captainhook/plugin-composer-phar which requires the phar or replace the dependency of captainhook/captainhook with captainhook/captainhook-phar?

I know that there is also the possibility to do this in composer:

{
  "scripts": {
    "post-autoload-dump": "vendor/bin/captainhook install -f -s"
  }
}

But this has again issues with CI as for production dev dependencies are not installed and therefore the captainhook executable does not exists in vendor/bin

sebastianfeldmann commented 9 months ago

Uh thats a good point. I never thought of that. So for production you are disabling plugins? Or how do you handle that?

composer-phar is also a Composer plugin.

So what are you missing? The automatic captainhook install?

Eydamos commented 9 months ago

For production I call composer install --no-dev so when the post-autoload-dump script is triggered composer will exit with a non zero exit code and a message like can not find executable vendor/bin/captainhook as I install captainhook as a dev dependency.

So the composer plugin is very convenient as it is also a dev dependency. On production both are not installed so I have neither the executable nor the composer hook which is perfect. On dev both are present and everything is handled automatically.

Only now that you have mentioned that composer-phar also has a composer plugin I had a deeper look into it. Unfortunately captainhook-phar/src/Plugin.php differs a lot from plugin-composer/src/ComposerPlugin.php. E.g. it ignores the extra.captainhook.force-install config.

So I personally would prefer it if the captainhook-phar would only include the phar and no composer plugin. And a separate package either this one or a separate composer-plubin-phar would provide me just the composer plugin

Eydamos commented 9 months ago

P.S. potentially you could also just remove the dependency all together and mention in the documentation that additionally to the composer plugin either captainhook or captainhook-phar must be installed. As both put a captainhook executable into vendor/bin it does not matter which one is installed.

You could also check for the presence of the executable in the hook and if it is not present print out a warning like Please install either captainhook/captainhook or captainhook/captainhook-phar to use this plugin and exit the script.

sebastianfeldmann commented 9 months ago

What do you think about this.

I will create a new Composer plugin captainhook/hook-installer that has similar functionality as composer-plugin without the actual dependency. The only change I would make is that you can even define the captainhook executable path in the extra config section. Then it would be the users responsibility to make sure the Cap'n is installed, but you could even use phive to do it and then link the executable.

Eydamos commented 9 months ago

Sounds perfect for me 👍

Eydamos commented 9 months ago

Just saw that the repo is already present. Works like a charm. Thank you very much for the quick fix. Amazing work, both on the issue and on the captainhook itself

sebastianfeldmann commented 9 months ago

Yes, I'm still experimenting with the repo a bit. but I think you can already use it. I will remove some functionality from composer-phar because it basically does similar things, it just did not support the force option. But since this is now obsolete and handled by hook-installer I can delete it there. I will update the docs and release version 1.0.0 of the hook-installer maybe tomorrow.

Thanks for pointing out that the Composer script solution has its flaws and a plugin is actually the best way to solve the issue.