Skip to content

Commit

Permalink
Bump php to 8.2 (#6506)
Browse files Browse the repository at this point in the history
* Bump `php` to `8.2`

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.

* pre-compiled geoip package isn't available under PHP 8

* https://stackoverflow.com/questions/72278262/unable-to-locate-package-php8-1-geoip
* https://stackoverflow.com/questions/74701602/install-pecl-geoip-for-php-8-1-ubuntu-22

* json is included with php 8

* Workaround `php-cs-fixer`/`php 8`/`composer1` incompatibility

`php-cs-fixer` < `3.15` doesn't support `8.2` and instead throw ts the
following error/warning:

```
 > php-cs-fixer fix --diff --verbose '--dry-run'
  PHP needs to be a minimum version of PHP 7.4.0 and maximum version of PHP 8.1.*.
  Current PHP version: 8.2.1.
  To ignore this requirement please set `PHP_CS_FIXER_IGNORE_ENV`.
  If you use PHP version higher than supported, you may experience code modified in a wrong way.
  Please report such cases at https://github.com/PHP-CS-Fixer/PHP-CS-Fixer .
```

However, when I tried to upgrade it, I ran into a problem that the newer
version of `php-cs-fixer` has a dependency on a `composer` library
version that is incompatible with `composer1`.

So this silences the error so that the build doesn't fail. Note that
we're only doing this for `composer 1`, as our `composer 2`-based
helper/environment works fine. So the long term solution here will be
deprecating/dropping support for `composer 1`.

* Update test fixture transitive dep to be php 8 compatible

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

Randomized with seed 14137
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    - doctrine\/instantiator 1.0.5 requires php >=5.3,<8.0-DEV -> your php version (8.2.1) does not satisfy that requirement.\n    - phpunit\/phpunit 5.1.3 requires phpunit\/phpunit-mock-objects >=3.0.5 -> satisfiable by phpunit\/phpunit-mock-objects[3.0.6].\n    - phpunit\/phpunit-mock-objects 3.0.6 requires doctrine\/instantiator ^1.0.2 -> satisfiable by doctrine\/instantiator[1.0.5].\n    - phpunit\/phpunit is locked to version 5.1.3 and an update of this package was not requested.\n"}
```

`doctrine/instantiator` is an indirect dep, so fine to bump w/o affecting
the unit test. So I ran `composer update doctrine/instantiator` to only
bump that dep (and any pins it required bumping) and then committing the results.

The test now passes.

* Update direct dep in test fixture to be PHP 8 compatible

This test was failing with:
```
[dependabot-core-dev] ~/dependabot-core/composer $ DEBUG_HELPERS=true rspec ./spec/dependabot/composer/file_updater/lockfile_updater_spec.rb:191
Run options: include {:locations=>{"./spec/dependabot/composer/file_updater/lockfile_updater_spec.rb"=>[191]}}

Randomized with seed 9856
{}
php -d memory_limit=-1 /opt/composer/v1/bin/run
update
{"error":"Your requirements could not be resolved to an installable set of packages.\n  Problem 1\n    - dealerdirect\/phpcodesniffer-composer-installer v0.4.4 requires php ^5.3|^7 -> your PHP version (8.2.3) does not satisfy that requirement.\n    - dealerdirect\/phpcodesniffer-composer-installer v0.4.4 requires php ^5.3|^7 -> your PHP version (8.2.3) does not satisfy that requirement.\n    - Installation request for dealerdirect\/phpcodesniffer-composer-installer 0.4.4 -> satisfiable by dealerdirect\/phpcodesniffer-composer-installer[v0.4.4].\n"}
```

So I manually bumped it up to `1.0.0` in `composer.json`, then ran
`composer update dealerdirect/phpcodesniffer-composer-installer` to only
bump that package and its deps.

The test now passes. This test was asserting that a package that was
incompatible with `composer` v1 would throw an error. And the newer
version is still incompatible so the behavior under test didn't change.

* Fix unit test for PEAR deps under composer v1

Initially the test was failing because some of the indirect deps in
`composer.lock` enforced a PHP constraint < `8.0.0`. I was able to
workaround that by a simple `composer1 update`, which pruned the
lockfile heavily.

Then I noticed the test was failing because when it tried to extract the
version from the return value of the native helper that was failing.
Which I eventually tracked down to a difference in behavior of the
`getPrettyVersion` depending on the PHP version:
 - PHP 7 returns: "2.4.1"
 - PHP 8 returns: "2.4.1@stable"

Fixing this test may be as simple as tweaking the anchoring regex. Or we
could drop support for PEAR, but I'm a little hesitant to do that until
we drop support for `composer` `1`, as that's the easy time to drop it.

* temp hack to fix unit test in previous commit until I hear from composer authors

* Fix test that didn't bump dep to php 8 compatible version

This test also didn't have a target version that was PHP 8 compatible.

Interestingly, it failed when I set the test expectation to `v4.22.1`,
but passes at `4.22.1` because upstream dropped the `v` prefix:
https://nova.laravel.com/releases/4.22.1

* Fix test of a subdependency to php 8 compatibility

This one was a doozy.

```
DEBUG_HELPERS=true rspec ./spec/dependabot/composer/file_updater/lockfile_updater_spec.rb:404
```

results in:
```
Dependabot::DependencyFileNotResolvable:
    Your requirements could not be resolved to an installable set of packages.
        Problem 1
        - doctrine/inflector v1.3.0 requires php ^7.1 -> your php version (8.2.3) does not satisfy that requirement.
        - illuminate/support v5.2.0 requires doctrine/inflector ~1.0 -> satisfiable by doctrine/inflector[v1.3.0].
        - illuminate/support is locked to version v5.2.0 and an update of this package was not requested.
```

So we need a version of `doctrine/inflector` that is php 8, which landed in `1.4.2` via doctrine/inflector#159. So then we need to figure out what version of `illuminate/support` allows a new enough `doctrine/inflector` version. illuminate/support@7ac4794 bumped it up which says the 6.x series.

`composer update illuminate/support doctrine/inflector` also required unlocking bumping `illuminate/contracts` to allow `composer` to generate a lockfile that satisfied all constraints.

I was a little concerned because the point of this unit test is to ensure that `illuminate/contracts` bumps to the expected version, so I found the oldest version of `illuminate/contracts` on their `6.x` branch is `6.20.0` which landed in illuminate/contracts@55a3d76. And they're now up to `6.20.44` so this test is still workable.

So I manually downgraded `illuminate/contracts` to `6.20.0` in `composer.lock` and then ran `composer update --lock` to regenerate all the accompanying metadata.

I realize the actual versions probably don't matter as these are all stubs, but still I feel better when the stubs match reality.

However, once I updated the `composer.json`/`composer.lock`, then two
other tests were failing.

So I had to bump their desired version to new versions that matched
their tests expected behavior, as well as the `previous_version` to
match the new lockfile content.

* Remove custom error handling for when plugins don't support composer v1

The test was failing because the dependencies used didn't support PHP 8.

However, when I updated them to the oldest version that still supported
PHP 8, then they worked fine against the newest version of composer.

When I dug into it, I realized that because composer 1 was deprecated a
while back, any plugins that don't support the newest version of
composer v1 won't support PHP 8.2, and so the emitted error message is
about deps not working with PHP 8. While still technically possible, the
reality is that the error of allowing a modern PHP version but not a new
version of a deprecated composer major version just isn't going to
happen. And in the worst case that I'm somehow wrong, this isn't that
big of a deal, as the job will fail either way, it just now fails with
a subprocess unknown failure and prints the warning message for a human
to interpret.

So I removed the custom error handling and the associated test.

* Lint fixes.

* RSpec fixes added.

* Removing the vulnerable composer version.

* Updating the vulnerable version symfony/process@5.4.26

* Junit fixes for composer v1

---------

Co-authored-by: Hariharan Thavachelvam <164553783+thavaahariharangit@users.noreply.github.com>
Co-authored-by: “Thavachelvam <“thavaahariharangit@git.com”>
  • Loading branch information
3 people authored and amazimbe committed Jan 13, 2025
1 parent 21a9544 commit 1ad0c6a
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 5 deletions.
15 changes: 10 additions & 5 deletions updater/lib/dependabot/update_files_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,16 @@ def perform_job
#
# As above, we can remove the responsibility for handling fatal/job halting
# errors from Dependabot::Updater entirely.
Dependabot::Updater.new(
service: service,
job: job,
dependency_snapshot: dependency_snapshot
).run
begin
Dependabot::Updater.new(
service: service,
job: job,
dependency_snapshot: dependency_snapshot
).run
rescue StandardError => e
handle_parser_error(e)
return service.mark_job_as_processed(Environment.job_definition["base_commit_sha"])
end

# Wait for all PRs to be created
service.wait_for_calls_to_finish
Expand Down
32 changes: 32 additions & 0 deletions updater/spec/dependabot/update_files_command_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,38 @@
end
end

context "with a Dependabot::DependencyFileNotParseable error" do
let(:error) { Dependabot::DependencyFileNotParseable.new("path/to/file", "a") }

let(:snapshot) do
instance_double(Dependabot::DependencySnapshot,
base_commit_sha: "1c6331732c41e4557a16dacb82534f1d1c831848")
end

let(:updater) do
instance_double(Dependabot::Updater,
service: service,
job: job,
dependency_snapshot: snapshot)
end

before do
allow(Dependabot::DependencySnapshot).to receive(:create_from_job_definition).and_return(snapshot)
allow(Dependabot::Updater).to receive(:new).and_return(updater)
allow(updater).to receive(:run).and_raise(error)
end

it "only records a job error" do
expect(service).not_to receive(:capture_exception)
expect(service).to receive(:record_update_job_error).with(
error_type: "dependency_file_not_parseable",
error_details: { "file-path": "path/to/file", message: "a" }
)

perform_job
end
end

context "with a Dependabot::DependencyFileNotFound error" do
let(:error) { Dependabot::DependencyFileNotFound.new("path/to/file") }

Expand Down

0 comments on commit 1ad0c6a

Please sign in to comment.