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

Phar for 1.3.1 does not contain version 1.3.1 #69

Closed
jrfnl opened this issue Oct 9, 2021 · 5 comments · Fixed by #70
Closed

Phar for 1.3.1 does not contain version 1.3.1 #69

jrfnl opened this issue Oct 9, 2021 · 5 comments · Fixed by #70

Comments

@jrfnl
Copy link
Collaborator

jrfnl commented Oct 9, 2021

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
Copy link
Collaborator Author

jrfnl commented Oct 9, 2021

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
Copy link
Collaborator Author

jrfnl commented Oct 9, 2021

@jrfnl
Copy link
Collaborator Author

jrfnl commented Oct 10, 2021

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
Copy link
Collaborator Author

jrfnl commented Oct 10, 2021

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
Copy link
Collaborator Author

jrfnl commented Dec 27, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

1 participant