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

Composer: allow composer/installers 2.x #65356

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Sep 15, 2024

What?

Composer released the first version of composer/installers 2.x over three years ago.

The 2.x range has a minimum PHP version of PHP 7.2. While WordPress and Gutenberg still supported PHP < 7.2, the requirement could have been set to ^1.0 || ^2.0, but as support for PHP < 7.2 has now been dropped, I see no reason not to require the package at ^2.0 outright.

Having said that, there may still be other plugins out there which don't allow for the composer/installers 2.x package yet and if one such plugins would be installed as a dependency alongside Gutenberg, this would lead to an unresolvable conflict when running composer install for such a project.

So with that in mind, I'm erring on the side of caution and propose to change the requirements to ^1.0 || ^2.0.

Note: this will also get rid of a PHP 8.4 deprecation notice when running various Composer commands.

Refs:

Composer released the first version of `composer/installers` 2.x over three years ago.

The 2.x range has a minimum PHP version of PHP 7.2.
While WordPress and Gutenberg still supported PHP < 7.2, the requirement could have been set to `^1.0 || ^2.0`, but as support for PHP < 7.2 has now been dropped, I see no reason not to require the package at `^2.0` outright.

Having said that, there may still be other plugins out there which don't allow for the `composer/installers` 2.x package yet and if one such plugins would be installed as a dependency alongside Gutenberg, this would lead to an unresolvable conflict when running `composer install` for such a project.

So with that in mind, I'm erring on the side of caution and propose to change the requirements to `^1.0 || ^2.0`.

Note: this will also get rid of a PHP 8.4 deprecation notice when running various Composer commands.

Refs:
* https://github.com/composer/installers/blob/main/CHANGELOG.md
* https://github.com/composer/installers/releases
Copy link

github-actions bot commented Sep 15, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: jrfnl <jrf@git.wordpress.org>
Co-authored-by: desrosj <desrosj@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@jrfnl jrfnl requested a review from desrosj September 15, 2024 16:45
@jrfnl jrfnl added the [Type] Enhancement A suggestion for improvement. label Sep 15, 2024
Copy link
Contributor

@desrosj desrosj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jrfnl. This makes sense to me.

I was trying to think of a way to eventually remove 1.x. Maybe we could:

  • Call this out in the release notes when the next version of the plugin is released.
  • Drop 1.x support along with the next PHP version update, whenever that ends up being.

@jrfnl
Copy link
Member Author

jrfnl commented Sep 16, 2024

I was trying to think of a way to eventually remove 1.x. Maybe we could:

  • Call this out in the release notes when the next version of the plugin is released.

That's always a good thing.

  • Drop 1.x support along with the next PHP version update, whenever that ends up being.

I would be hesitant/reluctant to do so as, for the following reasons:

  • Having it set at ^1.0 || ^2.0 doesn't do any harm. Composer will just install whichever version will work for all dependencies of the "client project".
  • There is no maintenance overhead involved, so there is no reason or urgency to drop support for v 1.x.
    AFAICS this plugin doesn't have any custom code for the integration, so it relies completely on the installer, which is not maintained here.
  • Depending on the skill level of the person running composer install, it may be an insurmountable problem, as a conflict like this cannot easily be solved in the "client project" itself, but would need PRs upstream (and for those to be accepted). (well, it can be with some hacks, but that's not for everyone and, again, hacks require a certain skill level).

I think a better measurement of when support for v 1.x can be dropped, would be to get in touch with WP Packagist and see if they can generate stats about:

  • How many of the plugins hosted there declare a non-dev dependency on composer-installers.
  • How popular those plugins are.
  • What the breakdown is of the required versions for the composer-installers dependency in those plugins.

Only when there are barely any plugins left which have their requirement set to ^1.0 (without || ^2.0) will it be safe to drop support for 1.x.

Copy link
Contributor

@desrosj desrosj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a reasonable and safe approach. We can explore potentially dropping 1.x separately in the future.

@desrosj desrosj merged commit 9220e7a into trunk Sep 26, 2024
66 of 67 checks passed
@desrosj desrosj deleted the fix/composer-update-installers branch September 26, 2024 16:54
@github-actions github-actions bot added this to the Gutenberg 19.4 milestone Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants