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

fetch_reports.php no longer working after upgrading to php8.4 - Filesystem: Implicitly marking parameter $pathNormalizer as nullable is deprecated #156

Open
brozkeff opened this issue Jan 29, 2025 · 16 comments

Comments

@brozkeff
Copy link

Using latest version from git via git pull 2.2.1, same effect was on previous used version 1.8:

I upgraded from old php7.4 to 8.4 via sury ppa on ubuntu 22.04 and cronjob to fetch reports stopped working.

Error on php8.4:

php8.4 -f ./utils/fetch_reports.php 
dmarc-srg [error]: ErrorException: League\Flysystem\Filesystem::__construct(): Implicitly marking parameter $pathNormalizer as nullable is deprecated, the explicit nullable type must be used instead in /usr/share/nginx/html/dmarc/vendor/league/flysystem/src/Filesystem.php:24
Stack trace:
#0 /usr/share/nginx/html/dmarc/vendor/composer/ClassLoader.php(571): {closure:/usr/share/nginx/html/dmarc/init.php:72}()
#1 /usr/share/nginx/html/dmarc/vendor/composer/ClassLoader.php(571): include()
#2 /usr/share/nginx/html/dmarc/vendor/composer/ClassLoader.php(428): Composer\Autoload\includeFile()
#3 [internal function]: Composer\Autoload\ClassLoader->loadClass()
#4 /usr/share/nginx/html/dmarc/classes/Core.php(310): class_exists()
#5 /usr/share/nginx/html/dmarc/utils/fetch_reports.php(171): Liuch\DmarcSrg\Core->checkDependencies()
#6 {main}
-----
Error: League\Flysystem\Filesystem::__construct(): Implicitly marking parameter $pathNormalizer as nullable is deprecated, the explicit nullable type must be used instead (-1)
-----
ErrorException: League\Flysystem\Filesystem::__construct(): Implicitly marking parameter $pathNormalizer as nullable is deprecated, the explicit nullable type must be used instead in /usr/share/nginx/html/dmarc/vendor/league/flysystem/src/Filesystem.php:24
Stack trace:
#0 /usr/share/nginx/html/dmarc/vendor/composer/ClassLoader.php(571): {closure:/usr/share/nginx/html/dmarc/init.php:72}()
#1 /usr/share/nginx/html/dmarc/vendor/composer/ClassLoader.php(571): include()
#2 /usr/share/nginx/html/dmarc/vendor/composer/ClassLoader.php(428): Composer\Autoload\includeFile()
#3 [internal function]: Composer\Autoload\ClassLoader->loadClass()
#4 /usr/share/nginx/html/dmarc/classes/Core.php(310): class_exists()
#5 /usr/share/nginx/html/dmarc/utils/fetch_reports.php(171): Liuch\DmarcSrg\Core->checkDependencies()
#6 {main}

Running it manually with old php7.4 is OK and works.

Web app via apache and php8.4 seems to work ok.

PHP versions used:

PHP 8.4.3 (cli) (built: Jan 19 2025 14:20:34) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.4.3, Copyright (c) Zend Technologies
    with Zend OPcache v8.4.3, Copyright (c), by Zend Technologies
php7.4 -v
PHP 7.4.33 (cli) (built: Dec 24 2024 07:11:50) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.33, Copyright (c), by Zend Technologies
@williamdes
Copy link
Contributor

I think this issue report should go to League\Flysystem\Filesystem and not DmarcSrg

@williamdes
Copy link
Contributor

Already fixed thephpleague/flysystem@2df5d73
First released in 3.25.1

@williamdes
Copy link
Contributor

williamdes commented Jan 29, 2025

This project should upgrade to v3

"league/flysystem-aws-s3-v3": "^2.5"

Please double check

@liuch
Copy link
Owner

liuch commented Jan 30, 2025

Thank you for the information!

@liuch
Copy link
Owner

liuch commented Feb 2, 2025

This project should upgrade to v3

This will raise the PHP version requirement to 8.0.2 minimum. Perhaps this should have been done a long time ago.

@liuch
Copy link
Owner

liuch commented Feb 3, 2025

Hm. I was unable to reproduce this error on flysystem v2.5.0 and php 8.4. But I will update it.

@williamdes
Copy link
Contributor

williamdes commented Feb 3, 2025

This is a breaking change, maybe require 2 or 3
Else this make php8 required https://github.com/thephpleague/flysystem/blob/abbd664eb4381102c559d358420989f835208f18/composer.json#L20

@williamdes
Copy link
Contributor

I think users appreciate php 7 compatibility
Can you keep it for now?
maybe in some years drop it?

@liuch
Copy link
Owner

liuch commented Feb 3, 2025

This is not a problem for me. I haven't used PHP 8 code anywhere in my project. Is it normal that my code requires PHP 7 and one optional dependencies requires PHP 8?

@williamdes
Copy link
Contributor

This is not a problem for me. I haven't used PHP 8 code anywhere in my project. Is it normal that my code requires PHP 7 and one optional dependencies requires PHP 8?

As long as you use an OR operator it can support both versions
and it's an optional dependency
so that's okay

@liuch
Copy link
Owner

liuch commented Feb 6, 2025

May I close this?

@williamdes
Copy link
Contributor

Is there new commits done for this one?

@liuch
Copy link
Owner

liuch commented Feb 6, 2025

These ones:

The changes in v3 do not affect the code in my project.

@liuch
Copy link
Owner

liuch commented Feb 6, 2025

Are you suggesting to set "league/flysystem-aws-s3-v3": ^2.5 || ^3.25.1 for Flysystem in composer.json?

Added: Honestly, I still haven't figured out why I haven't reproduced this error with v2.5

@williamdes
Copy link
Contributor

Are you suggesting to set "league/flysystem-aws-s3-v3": ^2.5 || ^3.25.1 for Flysystem in composer.json?

Added: Honestly, I still haven't figured out why I haven't reproduced this error with v2.5

Yes exactly, this would ensure the project can be installed on older versions
But without platform version, now the lock file might be an issue. But users can figure it out and run composer update.
Removing support for php 7 is maybe a change to plan for a next major or minor version. It really depends on how you perceive your project and it's users

@liuch
Copy link
Owner

liuch commented Feb 6, 2025

@williamdes Thanks for the detailed response. I think PHP 7.3 is enough for me for now.

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

No branches or pull requests

3 participants