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

strftime deprecated in PHP 8.1 #672

Closed
mambax7 opened this issue Jul 27, 2021 · 21 comments · Fixed by #811
Closed

strftime deprecated in PHP 8.1 #672

mambax7 opened this issue Jul 27, 2021 · 21 comments · Fixed by #811
Labels
waiting Waiting for answer

Comments

@mambax7
Copy link

mambax7 commented Jul 27, 2021

Unknown: Function strftime() is deprecated in file /Smarty_Compiler.class.php line 395
Unknown: preg_match_all(): Passing null to parameter #2 ($subject) of type string is deprecated in file /Smarty_Compiler.class.php line 1531
@Bigbear7
Copy link

The strftime deprecation was actually a bit controversial where the RFC author voted against it ;-) Also the benefit of deprecating was debated on the internals list, see: https://externals.io/message/113657

Anyway, is this also an issue for Smarty 3?

@mambax7
Copy link
Author

mambax7 commented Jul 29, 2021

  1. Nikita was not the author of it, he just collected the various deprecation requests:

"This is a collection of minor deprecations that various people have put together over the last ~2 years."

  1. I don't know about Smarty 3 - we're still using Smarty 2

@Bigbear7
Copy link

Well, when checking for the prevalence of strftime in Smarty 3 I found it in the following files:

  • /plugins/function.html_select_date.php
  • /plugins/modifier.date_format.php
  • /Smarty.class.php
  • /sysplugins/smarty_internal_config_file_compiler.php
  • /sysplugins/smarty_internal_runtime_codeframe.php

Probably the same for Smarty 2.

@wisskid wisskid changed the title PHP 8.1 and Smarty 2 issues strftime deprecated in PHP 8.1 Aug 18, 2021
@wisskid
Copy link
Contributor

wisskid commented Aug 18, 2021

Thank you for your report. We'll fix this in due time. But let's get a PHP8.0 release out first :)

@wisskid
Copy link
Contributor

wisskid commented Oct 13, 2021

See #678

@mambax7
Copy link
Author

mambax7 commented Nov 25, 2021

@wisskid Thank you!
Now that PHP 8.1 is available, I hope that this update will be available soon too! :)

@mambax7
Copy link
Author

mambax7 commented Feb 6, 2022

I see that 4.x is moving very quickly with PHP 8.1 updates.
When could we expect an update for Smarty 2.x?
Pretty Please? :)

@wisskid
Copy link
Contributor

wisskid commented Feb 6, 2022

@mambax7 I have no such intentions, but feel free to provide a PR!

@mambax7
Copy link
Author

mambax7 commented Feb 6, 2022

So is Smarty 2.x now officially "Abandonware"?

@wisskid
Copy link
Contributor

wisskid commented Feb 6, 2022

@mambax7 no, Smarty has a v3 and a v4 you could upgrade to.

@mambax7
Copy link
Author

mambax7 commented Feb 6, 2022

I understand that, but is Smarty 2.x now officially "Abandonware"?

@wisskid
Copy link
Contributor

wisskid commented Feb 6, 2022

@mambax7 I don't think you can call a specific version "abandonware". But semantics aside: support for v2 has been very limited over the past years and I don't see that improving. Since this is getting off-topic: maybe you could start a Discussion at https://github.com/smarty-php/smarty/discussions if you want to discuss the pros/cons of supporting v2 (and v3?) further into the future?

@mambax7
Copy link
Author

mambax7 commented Feb 6, 2022

This is how I see it:
If 2.x is officially still supported, then this discussion is on-topic, and the original question about update to PHP 8.1 is still valid.

If 2.x is officially not anymore supported, then I agree with you - let's make an official announcement that 2.x is NOT supported anymore, and then people can start a Discussion at https://github.com/smarty-php/smarty/discussions about it

I think, it's important that we have a clear situation and no misunderstandings about the status of 2.x

@noriellecruz
Copy link

I understand that, but is Smarty 2.x now officially "Abandonware"?

It's a legacy version, it won't receive compatibility update anymore. You should use smarty v4 now.

@wisskid
Copy link
Contributor

wisskid commented Feb 7, 2022

@mambax7 please see https://github.com/smarty-php/smarty/blob/master/SECURITY.md for our current policy as regarding to security support. Smarty 2 is officially not getting security support. We'll try to keep Smarty 3 usable and secure for the time being. New features (such as support for newer PHP-versions) are for Smarty 4+.

@mambax7
Copy link
Author

mambax7 commented Feb 7, 2022

Thanks for the clarification!
It would be good to make a note about it on https://www.smarty.net/

@mfettig
Copy link
Contributor

mfettig commented Jun 18, 2022

I had some time and took another look at the issue here. From comments in #678 it sounded like relying on IntlDateFormatter with symfony/polyfill-intl-icu as a fallback was the most interesting path forward so I tested some with that combination

The biggest issue I found with the approach is that the formatting syntax is markedly different between strftime() and IntlDateFormatter::format - as an example %D for strftime is m/d/Y for IntlDateFormatter. I expect that would be a pretty drastic change for a lot of existing code.

One alternative I came across was another BC bridge like this one: https://gist.github.com/bohwaz/42fc223031e2b2dd2585aab159a20f30 or the package/repo derived from it: https://packagist.org/packages/php81_bc/strftime. These replace the old strftime function with a custom implementation which uses IntlDateFormatter under the hood with a translation table for converting the formatting syntax. That approach has worked well enough in my testing so far so it could be an option to add that as another composer dependency, or implement a similar fix directly in this project.

@wisskid what are your thoughts on this? Is the syntax change acceptable switching to IntlDateFormatter, or would another BC layer be better?

@wisskid
Copy link
Contributor

wisskid commented Jun 18, 2022 via email

@wisskid
Copy link
Contributor

wisskid commented Sep 9, 2022

https://packagist.org/packages/php81_bc/strftime seems like a great bridge to solve the strftime compatibility issue, but it requires the Intl extension, so now Smarty requires the Intl extension, so that is a breaking change and a new major release. Smarty 5 would be born and we'd have to maintain an extra version just to be able to suppress deprecation notices.

symfony/polyfill-intl-icu could fill this gap, but it only supports the 'en' locale, so that doesn't really help all that much.

So, for now, I propose people read https://stitcher.io/blog/dealing-with-deprecations and relax while we wait for a PHP9 release candidate. When PHP9 arrives, we'll release Smarty v5 and use whatever we now by then to make the transition as smooth as possible.

@wisskid wisskid added the waiting Waiting for answer label Sep 9, 2022
@lkppo
Copy link

lkppo commented Sep 9, 2022

Hello Simon,

Month after month, notices about deprecated functions come back to pollute the bugtracker. Maybe it would be good to release a minor version to silence these notices, instead of constantly bothering you while waiting for PHP 9. I admire your patience.

Regards,

@wisskid
Copy link
Contributor

wisskid commented Sep 22, 2022

@lkppo thank you for your admiration ;)
After some fruitful discussion with @doekenorg I came to my senses and decided that we don't need to bug users with deprecation notices when we plan to fix the problem the notice is warning them for. So I'll silence the strftime calls and we will provide an alternative implementation for PHP9 (#810) when a release candidate becomes available.

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

Successfully merging a pull request may close this issue.

6 participants