Skip to content
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

Support composer-bin commands like composer bin [folder] install #220

Open
Aerendir opened this issue Jan 11, 2022 · 4 comments
Open

Support composer-bin commands like composer bin [folder] install #220

Aerendir opened this issue Jan 11, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@Aerendir
Copy link

Aerendir commented Jan 11, 2022

We extensively use https://github.com/bamarni/composer-bin-plugin to manage dev dependencies to avoid conflicts and dependencies hells.

bamarni/composer-bin-plugin requires to use the argument bin before the Composer's argument.

Something like:

  • composer bin [folder] install
  • composer bin [folder] update

Currently it is not possible to use this action in conjunction with bamarni/composer-bin-plugin as it is not possible to prepend bin [folder] to the actual Composer's command (install/update).

What we tested

We tried to directly call the action in the folders where the composer-bin composer.json files reside:

...
            -   name: Install Composer bin PHPunit
                uses: ramsey/composer-install@v2
                with:
                    working-directory: "vendor-bin/phpunit"

            -   name: Run PHPunit
                run: |
                    vendor/bin/simple-phpunit
...

Unfortunately this doesn't work as the executable is not moved to the main vendor/bin folder and so it doesn't exist there.

So, we thought we could change the step to actually call the executable directly from the vendor-bin/[folder]:

...
            -   name: Install Composer bin PHPunit
                uses: ramsey/composer-install@v2
                with:
                    working-directory: "vendor-bin/phpunit"

            -   name: Run PHPunit
                run: |
-                    vendor/bin/simple-phpunit
+                    vendor-bin/phpunit/vendor/bin/simple-phpunit
...

Unfortunately this also doesn't work as not all classes are dumped in the autoload, mainly the ones of the project: bamarni/composer-bin-plugin, in facts, merges all the classes from the autoload key in the root composer.json with the ones found in the various bin subfolders in one autoload file, so it is possible to maintain all the dependencies separate, but anyway using them as if they were all installed in the root composer.json.
Obviously, it also has a mechanism to avoid conflicts (but this mechanism is not related to this issue, so I'm skipping it).

Proposal/solution

I think the besto solution is to directly support bamarni/composer-bin-plugin, allowing to set a key for it with the name of the folder in which is the composer.json file to install.

So, continuing with the previous PHPUnit example and assuming the composer.json file we want to install is in vendor-bin/phpunit/composer.json, then we could configure the action with something like this:

            -   name: Install Composer bin PHPunit
                uses: ramsey/composer-install@v2
                with:
                    bin: "phpunit"
@Aerendir Aerendir added the enhancement New feature or request label Jan 11, 2022
@ramsey
Copy link
Owner

ramsey commented Jan 12, 2022

Hi! Thanks for the feature request.

I've never used bamarni/composer-bin-plugin, nor am I familiar with it. I hesitate to add support for a Composer plugin like this, since it means I'll need to maintain support for something I don't use and, therefore, I won't be able to do a good job supporting it.

It sounds like your workflow falls outside the scope of this tool and requires more customization than most users will need. Have you considered forking this action, instead?

@Aerendir
Copy link
Author

First of all, thank you for the fast reply :)

I understand well your point and agree with it.

I've not considered to fork this action as I'm not able to maintain it as I'm not very versed in its language... Actually I don't know it, so I'm not able to modify it to adapt to my use case.

For what I saw, I think it is simply an if condition to check if the key bin is passed and, if it is, add it to command.

If it may help you deciding, I can tell you that the plugin has a very simple "frontend": it only requires to use bin [folder] and nothing else: no options nor parameters, so, also the maintaining is very easy in the long run.

I would appreciate much a direct support for it as I know it is very used in projects that relies on a lot of dev dependencies (rector, phan, phpstan, php cs fixer, etc.).

I'm in your hands! 😬

@theofidry
Copy link

Hi! Glad to find this issue as I was about to open one about it :)

Disclaimer: I'm the current maintainer of https://github.com/bamarni/composer-bin-plugin.


First of all, for clarification an introduction to the plugin: it is only but a tiny wrapper to do a cd before calling composer:

$ composer bin phpstan install phpstan/phpstan
> parses the composer command to see if it starts with "bin "
> parses the namespace (here "phpstan")
> creates the directory "vendor-bin/phpstan" if does not already exist
> do "cd vendor-bin/phpstan"
> strip the command from the bin directory, i.e. the new command is "composer install phpstan/phpstan"
> execute this new composer command (in the directory)

I may have forgotten a detail or two but this is what it boils down to and it doesn't do more because I don't want to start to temper with more internal Composer code/behaviour.


A current possible solution, as highlighted by @Aerendir is to leverage the working-directory option:

            -   name: Install Composer PHPStan dependencies
                uses: ramsey/composer-install@v1
                with:
                    working-directory: 'vendor-bin/phpstan'

will work just fine.

The downside however is the symlinks but as you can see in bamarni/composer-bin-plugin#60 (comment) relying on the symlinks is a bad idea IMO. In fact I always disable symlinks whenever I use this plugin and I'm definitely gonna remove it in 2.0.


Regarding @Aerendir issue: I don't get the autoload part. You can check bamarni/composer-bin-plugin#57 for more details but there is no autoload concatenation of any sort.


In the current form, to support such a plugin, the best way to do it in the most "generic" way would be to offer a way to add a prefix option, for example:

            -   name: Install Composer dependencies
                uses: ramsey/composer-install@v1
                with:
                    composer-prefix-option: 'bin phpstan'

However I must say it does not look very elegant and IMO this is due to the design of this GitHub Actions that adds a layer of abstraction on what command to execute (the "locked", "highest" and "lowest" options). If instead you let the user pass the options, i.e. instead of "lowest" you do:

            -   name: Install Composer dependencies
                uses: ramsey/composer-install@v1
                with:
                    command: 'update'
                    composer-options: '--prefer-lowest'

The adding support for the plugin is already done:

            -   name: Install Composer dependencies
                uses: ramsey/composer-install@v1
                with:
                    command: 'bin phpstan update'
                    composer-options: '--prefer-lowest'

@ramsey
Copy link
Owner

ramsey commented Jan 13, 2022

I think providing a command argument might work. If it's present, it would override dependency-versions, and users would need to use composer-options to specify whether to prefer lowest, etc., but this could be made clear in the documentation.

I'll think on this a bit and ask any questions I have on this thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants