dependabot-core icon indicating copy to clipboard operation
dependabot-core copied to clipboard

Bump `php` to `8.2`

Open jeffwidman opened this issue 3 years ago • 14 comments

We are currently on 7.4 which has been EOL'd by upstream in Nov 2022:

  • https://www.php.net/eol.php
  • https://www.php.net/supported-versions.php

There is no newer version of the 7.x series, so we have to bump to the 8.x series.

Fix: https://github.com/dependabot/dependabot-core/issues/6527 Fix: https://github.com/dependabot/dependabot-core/issues/6958

jeffwidman avatar Jan 24 '23 04:01 jeffwidman

While I wasn't in a hurry to bump this for fear of accidentally breaking compatibility with php 7.x code, we are starting to see test failures that indicate we will need to bump soon:

[dependabot-core-dev] ~/dependabot-core/composer $ DEBUG_HELPERS=true rspec ./spec/dependabot/composer/update_checker/version_resolver_spec.rb:225
Run options: include {:locations=>{"./spec/dependabot/composer/update_checker/version_resolver_spec.rb"=>[225]}}

Randomized with seed 12699
php -d memory_limit=-1 /opt/composer/v2/bin/run
{"error":"Your requirements could not be resolved to an installable set of packages.\n  Problem 1\n    - monolog\/monolog dev-main requires php >=8.1 -> your php version (7.4.33) does not satisfy that requirement.\n    - monolog\/monolog 3.x-dev is an alias of monolog\/monolog dev-main and thus requires it to be installed too.\n    - Root composer.json requires monolog\/monolog ^3.0|4.1.x-dev as 3.0.0 -> satisfiable by monolog\/monolog[3.0.0-RC1, ..., 3.x-dev (alias of dev-main)].\n"}

So opening this draft PR to see what CI thinks.

jeffwidman avatar Jan 24 '23 04:01 jeffwidman

I don't know enough about PHP and also our usage of composer to know if us bumping to 8.x will break Dependabot on anyone running with 7.4, as that's still reasonably popular.

There's a chance that since we're not installing but only looking for updates that it will not matter the version of PHP that Dependabot uses... I don't really know though.

jeffwidman avatar Jan 24 '23 05:01 jeffwidman

I don't know enough about PHP and also our usage of composer to know if us bumping to 8.x will break Dependabot on anyone running with 7.4, as that's still reasonably popular.

Yeah, same 🤔 can we ask someone that PHP's? I see a fair number of failing tests also, which seems to happen in the build step? Is that related?

jurre avatar Jan 24 '23 08:01 jurre

Let me work through the build failures, and then I'll tag some PHP folks... but the build failures are because of various things that we'll have to fix whenever this actually happens.

jeffwidman avatar Jan 24 '23 08:01 jeffwidman

@stof @Seldaek 👋 from Dependabot.

We are currently running php 7.4 when we composer against users' composer manifest files... if we bump that to 8.2 in our native helpers (which is what this PR does), then will that still allow our composer instance to work fine against users' manifests that are still using 7.4 / 8.0 / 8.1?

I am unclear if composer only looks at the php version specified in the manifest file that it's updating, or also consider its own version of PHP that it's running on.

I can test it out myself, but wanted to also check with the composer maintainers in case there's some gotcha that won't be immediately apparent to us.

jeffwidman avatar Jan 25 '23 06:01 jeffwidman

It might cause some issues for projects not defining a platform config. But right now you are also having issues for people relying on more modern PHP versions, so I'd say running latest is probably a better trade-off.

Seldaek avatar Jan 25 '23 09:01 Seldaek

Thanks @Seldaek, that makes sense.

While fixing UT failures, I also noticed this failure:

{"error":"Your requirements could not be resolved to an installable set of packages.
Problem 1
 - doctrine\/instantiator 1.0.5 requires php >=5.3,<8.0-DEV -> your php version (8.2.1) does not satisfy that requirement.
 - phpunit\/phpunit 5.1.3 requires phpunit\/phpunit-mock-objects >=3.0.5 -> satisfiable by phpunit\/phpunit-mock-objects[3.0.6].
 - phpunit\/phpunit-mock-objects 3.0.6 requires doctrine\/instantiator ^1.0.2 -> satisfiable by doctrine\/instantiator[1.0.5].
 - phpunit\/phpunit is locked to version 5.1.3 and an update of this package was not requested.
 "}

doctrine/instantiator is an indirect / transient dep, so that's why it's not getting updated. So if we bump to PHP 8, then any transient deps that explicitly do not support PHP 8 will blow up.

However, I don't think that should block us upgrading php, as most packages will either not pin an upper limit on the PHP version, or they'll have released a newer version that added support... for example, doctrine/instantiator removed the blocking pin way back in 2017.

And as @Seldaek mentioned above, we're already in the situation that we're preventing new PR's from being opened where the package requires PHP 8. For example, the aforementioned doctrine/instantiator just released 2.0 which dropped support for php < 8... so Dependabot won't open PR's to bump that package to 2.0 until this PR is merged.

jeffwidman avatar Jan 25 '23 23:01 jeffwidman

This is now ready for review, with the only remaining item from a code standpoint being https://github.com/dependabot/dependabot-core/pull/6506#discussion_r1123899146.

From a user impact standpoint, I still need to pull metrics on what versions of PHP our users are using, and also double-check my current understanding that:

  1. w/o this, manifests that require PHP 8 fail
  2. with this, manifests that require PHP 7 fail

jeffwidman avatar Mar 03 '23 06:03 jeffwidman

I would love to see this merged. I did some testing locally (from the user perspective), and this branch did resolve my problems (PR's not being created by dependabot).

Of course, things will be different for each package of php-code, but most of the larger PHP ecosystems (e.g. I am working a lot with Drupal and Symfony packages) have built in the expectation that their tools should work on a large range of PHP environments, ranging from older 7.4 environments to the newest 8.2 . Most will have some versions that provide some overlap, e.g. working on 7.4 and 8.2, and then introducing a newer major version that drops support for the old 7.4 version. In that case, the user is actively updating their package to a new version.

As I see it, it might be the case that this could break update-checks for packages that rely on php 7.4 and don't support newer versions. But as already stated, 7.4 is EOL and should not be supported any more. Current versions of tools/packages should be expected to work fine with PHP 8.2 (as it is current). Worst case, it could break version-checks for some older versions of packages, which should be resolved if the package is updated outside of dependabot to a version that does support 8.2 .

More importantly, without this change, many updates that should be performed are not mentioned by dependabot, because composer is not able to resolve a set of versions that matches the constraints (because the versions in the constraints require php >= 8.0 or php >= 8.1 , not provided on the dependabot environment without this change). As more packages are updated and dropping support for older PHP versions, dependabot will only list less and less PHP upgrades without this change. I would be in favor of merging this ASAP, and perhaps eventually making the used PHP version more configurable in the future (e.g. it looks like npm_yarn allows using an argument containing the NODEJS_VERSION to override if needed).

Didel avatar Aug 29 '23 22:08 Didel

cc @abdulapopoola ☝️ in case you didn't see this user feedback.

jeffwidman avatar Dec 16 '23 01:12 jeffwidman

What is the status of this ticket? At this point PHP support in Dependabot is non-functional for projects using any of the supported PHP versions (PHP >= 8.1). As a reminder, this is the current state of the supported PHP versions:

image (Taken from php.net/supported-versions ).

What is needed to merge this PR? How can others contribute? I personally would be in favor of making PHP versions user-configurable (as mentioned above), but not before this PR has been merged to make Dependabot usable again.

Didel avatar Feb 07 '24 07:02 Didel

we're already in the situation that we're preventing new PR's from being opened where the package requires PHP 8. For example, the aforementioned doctrine/instantiator just released 2.0 which dropped support for php < 8... so Dependabot won't open PR's to bump that package to 2.0 until this PR is merged.

More importantly, without this change, many updates that should be performed are not mentioned by dependabot, because composer is not able to resolve a set of versions that matches the constraints (because the versions in the constraints require php >= 8.0 or php >= 8.1, not provided on the dependabot environment without this change).

At this point PHP support in Dependabot is non-functional for projects using any of the supported PHP versions (PHP >= 8.1).

@Didel Not sure if this applies to your use case, but just in case this helps anyone: // cc @jeffwidman

We use Dependabot in many PHP 8.1/8.2 projects without any problems, and also get Dependabot updates for packages that only support PHP 8.1+. If I remember correctly, we just define a target PHP version in our composer.json require section for this to work, e.g.:

"require": {
    "php": "^8.2.9",

vintagesucks avatar Feb 07 '24 12:02 vintagesucks

@Didel Not sure if this applies to your use case, but just in case this helps anyone: // cc @jeffwidman

We use Dependabot in many PHP 8.1/8.2 projects without any problems, and also get Dependabot updates for packages that only support PHP 8.1+. If I remember correctly, we just define a target PHP version in our composer.json require section for this to work, e.g.:

"require": {
    "php": "^8.2.9",

That does seem to help, yes! Thank you for the suggestion! I still think that's not a 'fix' but more of a work-around for the underlaying problem (depending on EOL PHP versions), but at least that allows Dependabot to create updates again. That will probably also explain why not every PHP project is impacted, as some (most?) might have the PHP dependency specified in their composer.json file.

Didel avatar Feb 08 '24 10:02 Didel

tagging @jonjanego as part of the release standardization effort

abdulapopoola avatar May 16 '24 05:05 abdulapopoola