Minify's dependency `pear/net_url2` triggers a PHPUnit test failure on PHP 8.4 in vendor
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Jdforrester-WMF
	Mar 7 2025, 8:47 PM

Description

Running the (not-yet-voting) PHP 8.4 test suite via quibble for vendor results in two errors:

00:02:25.089 There were 2 errors:
00:02:25.089 
00:02:25.089 1) MediaWiki\Tests\ResourceLoader\FileModuleTest::testMixedCssAnnotations
00:02:25.090 Net_URL2::_queryArrayByBrackets(): Implicitly marking parameter $array as nullable is deprecated, the explicit nullable type must be used instead
00:02:25.090 
00:02:25.090 /workspace/src/vendor/pear/net_url2/Net/URL2.php:572
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:418
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:471
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:332
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:321
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:297
00:02:25.090 /workspace/src/includes/ResourceLoader/FileModule.php:1024
00:02:25.090 /workspace/src/includes/ResourceLoader/FileModule.php:974
00:02:25.090 /workspace/src/includes/ResourceLoader/FileModule.php:952
00:02:25.090 /workspace/src/includes/ResourceLoader/FileModule.php:392
00:02:25.090 /workspace/src/tests/phpunit/includes/ResourceLoader/FileModuleTest.php:298
Logs generated by test
00:02:25.090 
00:02:25.090 2) WebInstallerOutputTest::testGetCSS
00:02:25.090 Net_URL2::_queryArrayByBrackets(): Implicitly marking parameter $array as nullable is deprecated, the explicit nullable type must be used instead
00:02:25.090 
00:02:25.090 /workspace/src/vendor/pear/net_url2/Net/URL2.php:572
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:418
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:471
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:332
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:321
00:02:25.090 /workspace/src/vendor/wikimedia/minify/src/CSSMin.php:297
00:02:25.091 /workspace/src/includes/ResourceLoader/FileModule.php:1024
00:02:25.091 /workspace/src/includes/ResourceLoader/FileModule.php:974
00:02:25.091 /workspace/src/includes/ResourceLoader/FileModule.php:952
00:02:25.091 /workspace/src/includes/installer/WebInstallerOutput.php:143
00:02:25.091 /workspace/src/tests/phpunit/includes/installer/WebInstallerOutputTest.php:14

If these two were fixed (upstream?), I think we could make CI voting in PHP 8.4, which would be rather ahead of my expectations (for T386108: Make PHP 8.4 voting on development (master) branch of MW ecosystem (core, vendor, extensions, skins, libraries) in CI).

Details

Related Changes in Gerrit:

Subject	Repo	Branch	Lines +/-
Upgrading pear/net_url2 (v2.2.2 => v2.2.3)	mediawiki/vendor	master	+79 -1 K
Upgrading pear/net_url2 (v2.2.2 => v2.2.3)	mediawiki/vendor	REL1_43	+78 -1 K
Upgrading pear/net_url2 (v2.2.2 => v2.2.3)	mediawiki/vendor	REL1_42	+93 -1 K
Upgrading pear/net_url2 (v2.2.2 => v2.2.3)	mediawiki/vendor	REL1_39	+92 -1 K
build: Update pear/net_url2 to ~2.2.2 to include 2.2.3	mediawiki/libs/Minify	master	+1 -1
ResourceLoader: Remove unused ResourceLoader::expandUrl() method	mediawiki/core	master	+0 -29

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Open	None	T376276 PHP 8.4: Implicitly nullable parameter declarations deprecated
Open	None	T379587 PHP 8.4: Implicitly nullable parameter declarations deprecated in 3rd party vendor libs
Resolved	• Krinkle	T390063 Minify release for PCRE 10.45 fix
Resolved	Reedy	T388273 Minify's dependency `pear/net_url2` triggers a PHPUnit test failure on PHP 8.4 in vendor

Event Timeline

Jdforrester-WMF created this task.Mar 7 2025, 8:47 PM

Restricted Application added a project: MediaWiki-Platform-Team. · View Herald TranscriptMar 7 2025, 8:47 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Aha, reported upstream, fixed, but as @Reedy points out, not yet released.

Reedy moved this task from Backlog to Patch merged upstream on the Upstream board.Mar 7 2025, 8:56 PM

Jdforrester-WMF mentioned this in T386108: Make PHP 8.4 voting on development (master) branch of MW ecosystem (core, vendor, extensions, skins, libraries) in CI.Mar 7 2025, 8:57 PM

Reedy changed the task status from Open to Stalled.Mar 7 2025, 8:59 PM

Net_URL2 is also tracked via T379587: PHP 8.4: Implicitly nullable parameter declarations deprecated in 3rd party vendor libs

Reedy moved this task from Backlog to Other on the PHP 8.4 support board.Mar 8 2025, 1:00 AM

Reedy mentioned this in T379587: PHP 8.4: Implicitly nullable parameter declarations deprecated in 3rd party vendor libs.Mar 9 2025, 11:40 PM

Reedy added a parent task: T379587: PHP 8.4: Implicitly nullable parameter declarations deprecated in 3rd party vendor libs.

@Reedy has requested upstream to release the merged fix, at: https://pear.php.net/bugs/bug.php?id=29032

• larissagaulia moved this task from Inbox, needs triage to Waiting on the MediaWiki-Platform-Team board.Mar 10 2025, 12:30 PM

Given T388335 and the lack of responses from PEAR maintainers, I wonder if we should remove the dependency—apart from Minify, it's only used by ResourceLoader::expandUrl() in core which is itself unused since the removal of RL debug mode v1.

In T388273#10623351, @mszabo wrote:

Given T388335 and the lack of responses from PEAR maintainers, I wonder if we should remove the dependency—apart from Minify, it's only used by ResourceLoader::expandUrl() in core which is itself unused since the removal of RL debug mode v1.

Happy to hear we're about to drop its direct use in core, so I defer to the Minify team (@Hokwelum / @Krinkle) on whether they want to fork it or re-write it to avoid the dependency.

Change #1126566 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/core@master] Remove unused ResourceLoader::expandUrl() method

https://gerrit.wikimedia.org/r/1126566

gerritbot added a project: Patch-For-Review.Mar 11 2025, 2:00 PM

In T388273#10623788, @gerritbot wrote:

Change #1126566 had a related patch set uploaded (by Máté Szabó; author: Máté Szabó):

[mediawiki/core@master] Remove unused ResourceLoader::expandUrl() method

https://gerrit.wikimedia.org/r/1126566

Thanks for finding and removing the unused code after T367441. Much appreciated!

In T388273#10623432, @Jdforrester-WMF wrote:

In T388273#10623351, @mszabo wrote:

Given T388335 and the lack of responses from PEAR maintainers, […]

[…] I defer to the Minify team (@Hokwelum / @Krinkle) on whether they want to fork it or re-write it to avoid the dependency.

Let's review the last four interactions between MediaWiki maintainers and this upstream dependency:

https://github.com/pear/Net_URL2/pull/13 (Krinkle, Feb 2020) - Merged in a week, by ktomk.
https://github.com/pear/Net_URL2/pull/14 (Reedy, Nov 2024) - Reviewed by hakre, and merged in a week.
https://github.com/pear/Net_URL2/pull/15 (Reedy, Nov 2024) - Merged in a week, by ashnazg.
https://pear.php.net/bugs/bug.php?id=29032 (Reedy, Mar 2025) - Acknowledged and assigned after 4 days by tkli (aka ktomk).

With consistent responses under a week, and involvement from three distinct maintainers, this seems like a healthy and well-maintained package. I don't think 4 days is a reasonable timeframe to declare a package in need of removal/forking?

I will note the replies on some of the bug reports in the pear bug tracker haven’t been the best (in terms of response rate).

But looks like there’s been more activity at late at least…

A release is also being prepared now too!

In T388273#10627844, @Reedy wrote:

A release is also being prepared now too!

https://github.com/ktomk/Net_URL2/commit/2fbbbeaff7df3b76b02c3f65337f7da7d8d64302
https://github.com/pear/Net_URL2/compare/master...ktomk:Net_URL2:develop

Change #1126566 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Remove unused ResourceLoader::expandUrl() method

https://gerrit.wikimedia.org/r/1126566

https://github.com/pear/Net_URL2/pull/17#issuecomment-2747407295

Change #1131319 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/libs/Minify@master] composer.json: Update to pear/net_url2

https://gerrit.wikimedia.org/r/1131319

Reedy changed the task status from Stalled to In Progress.Mar 26 2025, 1:37 PM

Reedy triaged this task as High priority.

We need this in versions used for MW-1.39-release (2.3.0), MW-1.42-release (2.7.0), MW-1.43-release (2.8.0).

For 1.43, this is relatively trivial, as it'll be a patch release.

For 1.39 this is more complex.. Can we just update to 2.8.2 (suspected new release)? Or do we need to make a 2.3.1 sub-branch?

Reedy added a parent task: T390063: Minify release for PCRE 10.45 fix.Mar 26 2025, 2:28 PM

For 1.39 this is more complex.. Can we just update to 2.8.2 (suspected new release)? Or do we need to make a 2.3.1 sub-branch?

2.9.0 is about to ship (with T277675); not sure how keen people will be to backport that, but I think the new functionality won't break anything in older release branches?

It's not much effort to create some branches at the old tags, cherry pick the net_url2 patch, create the changelog and tag...

But yeah, if we can simplify it in one way or another...

MW 1.39 requires PHP 7.4 and Minify started requiring PHP 7.4+ from v2.8.0. Should be fine to update to latest all the way. The minify improvements and bug fixes certainly benefit MW 1.39-1.42 as well, especially the more lax token handling we have avoids several known cases where it could break pre-minified payloads generated by Webpack in older extensions.

Change #1131319 merged by jenkins-bot:

[mediawiki/libs/Minify@master] build: Update pear/net_url2 to ~2.2.2 to include 2.2.3

https://gerrit.wikimedia.org/r/1131319

In T388273#10679625, @Krinkle wrote:

MW 1.39 requires PHP 7.4 and Minify started requiring PHP 7.4+ from v2.8.0. Should be fine to update to latest all the way. The minify improvements and bug fixes certainly benefit MW 1.39-1.42 as well, especially the more lax token handling we have avoids several known cases where it could break pre-minified payloads generated by Webpack in older extensions.

I guess when master is done, we (I) can backport, and can see if/what any test failures obviously come up and need remediating.

Reedy moved this task from Patch merged upstream to Patch released upstream on the Upstream board.Mar 26 2025, 5:56 PM

Change #1131437 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/vendor@master] Upgrading pear/net_url2 (v2.2.2 => v2.2.3)

https://gerrit.wikimedia.org/r/1131437

Change #1131437 merged by jenkins-bot:

[mediawiki/vendor@master] Upgrading pear/net_url2 (v2.2.2 => v2.2.3)

https://gerrit.wikimedia.org/r/1131437

Change #1131449 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/vendor@REL1_43] Upgrading pear/net_url2 (v2.2.2 => v2.2.3)

https://gerrit.wikimedia.org/r/1131449

Change #1131450 had a related patch set uploaded (by Reedy; author: Reedy):