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

Declaration of Carbon\Translator after Laravel 7 Upgrade #2033

Closed
stevenhardy-digital opened this issue Mar 7, 2020 · 15 comments
Closed

Declaration of Carbon\Translator after Laravel 7 Upgrade #2033

stevenhardy-digital opened this issue Mar 7, 2020 · 15 comments
Assignees
Labels
wrong version used Trying to use a method on an a version that does not implement it yet

Comments

@stevenhardy-digital
Copy link

Hello,

I encountered an issue with the following code:

Carbon\Translator::trans($id, array $parameters = Array, $domain = NULL, $locale = NULL)

Carbon version: 2.31.0

PHP version: 7.3.9

I expected to get:

Declaration of Carbon\\Translator::trans($id, array $parameters = Array, $domain = NULL, $locale = NULL) must be compatible with Symfony\\Contracts\\Translation\\TranslatorInterface::trans(string $id, array $parameters = Array, ?string $domain = NULL, ?string $locale = NULL) at /home/vagrant/folder/website/vendor/nesbot/carbon/src/Carbon/Translator.php:18)

But I actually get:

Laravel website 500 error

Thanks!

@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Mar 8, 2020

⚠️ For anyone running this issue:

Please triple-check you're really running PHP >= 7.2. This error message is supposed to happen when Symfony >= 5 has been installed and is executed with PHP <= 7.1.

Remember the CLI version of PHP may be different from the version ran in Apache, FastCGI, etc. So what php --version output is not always the php version of you webserver, use phpinfo(); at the top of you public/index.php or equivalent entry point to check your actual settings.

If your production/remote environment run PHP <= 7.1, you simply can't use Symfony 5 or Laravel 7 on this environment and have to upgrade your PHP version or pick a lower version of Symfony/Laravel.

Note that config.platform.php composer setting won't work if the chosen version is greater than the PHP version running in production/remote environment.

Last, code cache like OPCache should be cleaned/regenerated on deploy, or at least on deploy after an upgrade of important dependencies such as your framework.

@kylekatarnls
Copy link
Collaborator

I'm really surprise this throws an exception as our unit tests for Translator::trans succeed using PHP 7.3 and Symfony 5.0.5:
https://travis-ci.org/briannesbitt/Carbon/jobs/658182113?utm_medium=notification&utm_source=github_status

@kylekatarnls
Copy link
Collaborator

Could you please try to provide an minimum code chunk to reproduce the Carbon issue outside Laravel?

@sperelson
Copy link

sperelson commented Mar 8, 2020

Just seconding the issue. Brand new install on PHP 7.3.12 has the same errors. Installed on PHP 7.4.1 and it runs without errors. Could be a configuration difference. The Fix commit linked above is what I did to resolve it temporarily.

Won't fixing the declaration mean forcing the use of "symfony/translation": "^5.0"?

Update I think I figured it out. Tinkerwell is not using the same PHP binary as my system. After pointing Tinkerwell to the correct version (7.3.13) it started working. 🤦‍♂

@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Mar 8, 2020

Won't fixing the declaration mean forcing the use of "symfony/translation": "^5.0"?

On the contrary, the declaration is currently covariant (it's valid and the unit tests are passing for all versions), if I update the declaration to be exactly the same as the current 5.0 implementation, it will break Symfony < 5.

You always need to be sure that composer run the same PHP version when it installs the dependencies that the one your run for the webserver and indeed I guess the issue is the same for @stevenhardy-digital.

@Jaspur
Copy link

Jaspur commented Mar 10, 2020

This occurs also to me.
I force my PHP-version of the server in my composer file:

"config": {
        "optimize-autoloader": true,
        "preferred-install": "dist",
        "platform": {
            "php": "7.2.24"
        },
        "sort-packages": true
    },

But still getting those errors.

2020/03/10 20:48:25 [error] 12466#12466: *18700 FastCGI sent in stderr: "PHP message: PHP Fatal error:  Declaration of Carbon\Translator::setLocale($locale) must be compatible with Symfony\Contracts\Translation\LocaleAwareInterface::setLocale(string $locale) in /home/forge/foo.app/vendor/nesbot/carbon/src/Carbon/Translator.php on line 18
PHP message: PHP Fatal error:  Uncaught ErrorException: Declaration of Illuminate\Http\Response::setContent($content) should be compatible with Symfony\Component\HttpFoundation\Response::setContent(?string $content) in /home/forge/foo.app/vendor/laravel/framework/src/Illuminate/Http/Response.php:14

2020/03/10 20:50:35 [error] 12466#12466: *18700 FastCGI sent in stderr: "PHP message: PHP Fatal error:  Declaration of Carbon\Translator::trans($id, array $parameters = Array, $domain = NULL, $locale = NULL) must be compatible with Symfony\Contracts\Translation\TranslatorInterface::trans(string $id, array $parameters = Array, ?string $domain = NULL, ?string $locale = NULL) in /home/forge/foo.app/vendor/nesbot/carbon/src/Carbon/Translator.php on line 18

@rimace
Copy link

rimace commented Mar 11, 2020

Basically it's line 314 in Translator.php.
That needs to be public function setLocale(string $locale) (notice the type-hint) and the errors would be fixed in my opinion, right?
It may mean that support for symfony/translation 4 and 5 at the same time can't be provided.

@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Mar 11, 2020

As you can see there: https://travis-ci.org/github/kylekatarnls/Carbon/builds/659770462

Aligning the typehint will break Symfony < 5 compatibility and PHP < 7.2 compatibility and as you can see here: https://travis-ci.org/github/kylekatarnls/Carbon/builds/659768977

The library on its own with the current typing is compatible with all versions of PHP and Symfony.

So I won't break anything until I have the exact cause of the error (a way to reproduce it in a sandbox with Carbon alone).

Mabe FastCGI behaves differently regarding typing covariance. Can you compare with standard PHP (php artisan serve) if the same app runs please?

Or does someone get the same error without FastCGI? (In Apache maybe?)

@tai1030
Copy link

tai1030 commented Mar 13, 2020

Hi all,
I have same issues too.

PHP version: 7.4.0
Carbon version: 2.31.0
Laravel version: 7.1.1
Symfony/translation version: 5.0.5

Error: Laravel 500 with below error log:

Declaration of Carbon\Translator::setLocale($locale) must be compatible with Symfony\Contracts\Translation\LocaleAwareInterface::setLocale(string $locale)

But my environment is a bit special, I am not running Laravel on Apache / FastCGI / PHP-FPM, etc.
I am running Laravel on CLI with "LaravelS" package. I am not sure is it related.

However, my workaround is forked this repository, add typehint to both Translator::trans() and Translator::setLocale() by follow Symfony 5, and use composer VCS to require my own Carbon package to my project, now everything is good for my own project. I know that is a bad idea, but I can't do anything if I don't do that. If anyone facing same issue, can consider to use this workaround too.
https://github.com/tai1030/Carbon/commits/2.31.0-patch2

Agree @kylekatarnls 's point, that will break Symfony < 5 compatibility and PHP < 7.2 compatibility. Therefore, I don't submit PR, just solve my own issue first.
Hope the future official version update can solve this issue.

@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Mar 13, 2020

As proven by Travis-CI (and many successful Laravel 7 deployments), Carbon implement is compatible with PHP 7.2 to 7.4 and Symfony 3.4 to 5 :

See this code: https://3v4l.org/FnW20

Please try this simple chunk on your own environment. If your PHP program works as expected (respect the PHP documentation and source code) it should not throw any declaration compatibility error.

I (and most libraries) use PHP standard CLI to test their library and consider than if a bug occurs in some environment but not in the CI platform (Travis-CI for Carbon) then it's an environment issue.

As soon as someone could provide a way to reproduce the bug (using a particular settings, PHP implementation, dependency list) then I would be able to add the bug to our tests and fix it without breaking their app to other users.

@kylekatarnls
Copy link
Collaborator

For the record, this message "Declaration of ... must be compatible" is the one you were supposed to see before covariance was introduced (PHP < 7.2), but with PHP < 7.2, composer install Symfony 3 or 4 but not 5 which requires PHP >= 7.2.

So if you have this error, you should double-check you are really running the composer installation and the environment that throws the exception with the same PHP version.

@kylekatarnls kylekatarnls added wrong version used Trying to use a method on an a version that does not implement it yet and removed details needed We do not have enough input to solve this issue needs investigation labels Mar 16, 2020
@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Mar 16, 2020

Regarding the lack of elements that would allow to reproduce the bug in a CLI environment or could confirm it really happen with PHP >= 7.2, I close this issue.

Please refer #2033 (comment)

If you get this error message, there are great chances you're not running the PHP version you think, you're more likely running a PHP version lower than 7.2, and so that is actually not compatible with Laravel 7 / Symfony 5.

If sufficient clues may be provided there is a different issue, I will re-open this issue.

@zachweix
Copy link

I am using Apache2 with PHP 7.4.8 and I am having the exact same issue where setLocale is incompatible with the parent

@kylekatarnls
Copy link
Collaborator

The possible causes and solutions have been given yet: #2033 (comment)

@zachweix
Copy link

Yeah, I realized afterwards. Once I fixed everything so that it was actually running on PHP 7.4 all was good again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wrong version used Trying to use a method on an a version that does not implement it yet
Projects
None yet
Development

No branches or pull requests

7 participants