php-parallel-lint/PHP-Parallel-Lint

Phar for 1.3.1 does not contain version 1.3.1

jrfnl opened this issue · 5 comments

jrfnl commented

When running with the Parallel Lint Phar for version 1.3.1, you are greeted by notices like the below, while these notices where fixed in PR #64 which is supposed to be part of the 1.3.1 release.

Feels like the Phar which was uploaded with the release needs to be replaced.

Deprecated: Return type of JakubOnderka\PhpParallelLint\RecursiveDirectoryFilterIterator::hasChildren() should either be compatible with RecursiveFilterIterator::hasChildren(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///phars/parallel-lint-1.3.1.phar/src/Manager.php on line 251

Deprecated: Return type of JakubOnderka\PhpParallelLint\RecursiveDirectoryFilterIterator::getChildren() should either be compatible with RecursiveFilterIterator::getChildren(): ?RecursiveFilterIterator, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///phars/parallel-lint-1.3.1.phar/src/Manager.php on line 264

Deprecated: Return type of JakubOnderka\PhpParallelLint\RecursiveDirectoryFilterIterator::accept() should either be compatible with FilterIterator::accept(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///phars/parallel-lint-1.3.1.phar/src/Manager.php on line 231

@grogy could you please have a look ?

P.S.: while you're looking, I'd also appreciate the changelog creds being fixed. #65 was a PR by me (not that I really care about the creds, but giving them to someone else is taking things a bit far).

- GH Actions: set error reporting to E_ALL [#65] from [@glensc].

jrfnl commented

FYI: I've just tested with the latest Phar artifact from master and it is afflicted by the same problem, so it looks like there is a problem in the Phar generation process.

jrfnl commented

Okay, so I think I've figured out what's going on. The Phar generation step is using Box 2, while Box 2 was last updated in 2016 and is therefore not necessarily compatible with PHP 8 or higher.

Based on the box.json configuration, the "Herrera\\Box\\Compactor\\Php" compactor is used which strips all comments and prior to PHP 8, attributes would be seen as comment and are therefore stripped.

I'm proposing to upgrade to Box 3. Box 3 does have a higher minimum PHP version to run, but still claims to generate Phar files which are compatible all the way back to PHP 5.3, which is what's needed.

I'm working on a PR for this and to add some extra tests as safeguards against future issues like this.

jrfnl commented

Hmm.. turns out it's even worse. Box 3 doesn't properly support PHP 8.0, nor PHP 8.1 either.

Problems discovered:

  1. The build-in requirements checker shipped with Box 3 and included by default in Phar files, is throwing deprecation warnings on PHP 8.1.
    While looking at this, I realized I'd actually fixed those already a while back, but there has been no release of Box 3 yet with those fixes included.
  2. The PHP Compactor which is included with Box (2/3) does not account for PHP 8.0+ attributes properly.
    This means that the Phar:
    • Either needs to be generated on PHP 8.0 in which the attributes are no longer tokenized as comments.
    • Or if the Phar generation is done on PHP < 8.0, the PHP compactor needs to be turned off for the time being until the compactor code in Box has been fixed.

I've opened a new issue in Box about the second problem.

jrfnl commented

Box has released a new version: https://github.com/box-project/box/releases/tag/3.14.0

Generation on any supported PHP version should work fine again.

Note: if the Requirements Checker is enabled and the PHAR is generated on PHP < 8.0 and then run with PHP 8.1, I'm still seeing the deprecations from the Requirements Checker, so there is more to be fixed upstream.

For now, I'd recommend leaving the changes previously made to work around this in place.