Jump to content

Welcome, Guest!

By registering with us, you'll be able to discuss, share and private message with other members of our community.

musicmaster

Smarty and PHP 7.2

Recommended Posts

@datakick said in Smarty and PHP 7.2:

@musicmaster said in Smarty and PHP 7.2: Do you experience any errors with php 7.2? Or is it just notices / deprecation warnings?

I experience just the just those notices / deprecation warnings. I know enough about software not to be too afraid of where things will go.

My point is that I need to recommend webshop software to customers and other people. And then it is a serious issue when I know that if I recommend Thirty Bees people will have a problem at the end of the year.

Share this post


Link to post
Share on other sites

@musicmaster said in Smarty and PHP 7.2:

I experience just the just those notices / deprecation warnings

That is excellent news.

That means that the debug mode is the only functionality I know of that is causing real problems on php 7.2. The rest of the core system and modules works just fine.

Debug mode is the only reason why I suggest using php 7.1 at the moment. When debug mode is on, those deprecation warnings get mixed into html, or they are injected into ajax responses, and that causes problems.

But notices and deprecation warnings themselves don't mean that the thirtybees (including current smarty lib) is not php 7.2 compatible. They are really just warnings, advising us that future php versions will have problems with our codebase. But because of terrible implementation of debug mode reporting, these warnings causes real issues right now.

That's why I suggest we tackle debug mode first. Once we have that fixed, we can really advice merchants to use php 7.2, as the core code will be 100% compatible.

And then, we can start work on php 7.3 compatibility. That will require fixing all those notices and warnings. We will need to update smarty library, or patch it as you suggested. But we don't have to rush that decision, because php 7.2 will be supported until Nov 2020.

Share this post


Link to post
Share on other sites

The problem I have with recommending php 7.2 right now is module compatibility. @musicmaster are you using any third party modules besides your own? Shipping modules? Real payment modules that connect via an api? These are the types of modules that we are having the most trouble out of. So while the core would be fine, who would use thirty bees if they could not use any 3rd party modules?

Share this post


Link to post
Share on other sites

@datakick check this out for some profiling stuff: https://github.com/gonssal/thirtybees/tree/profiling-bar.

I've been using it for some time, and I have a local version that improves the awful core custom error handler to show a nice profiling page on fatals, but I'm not developing it further. You can use the profiling-bar as a head start.

Share this post


Link to post
Share on other sites

I read this with a lot of interested. But... do you really think every provider will cancel the support or will not offer hosting with PHP 7.0 and 7.1 in the next months? I dont think so. There are still thousands of offers with PHP 5.x with an separate support (without extra costs) for these hosting accounts. I am still using PHP 5.x for my actual webshop (not tb).

Is there really such a hurry for PHP 7.2 ? imho i think PHP 7.1 will accompany us for the next 3-4 years without any problems at hosting !

Share this post


Link to post
Share on other sites

@DRMasterChief I get where he is coming from in that regard, it is the security aspect of things. http://php.net/supported-versions.php

Share this post


Link to post
Share on other sites

Yep, i know about the security aspect.
But maybe i do not understand everything and i am not a dev (have disturbed your dev-discussion a little bit). This is why i have written my knowledge about the support for PHP 5.x my hoster told me few weeks ago. I have still PHP 5.x active for my 'old' shop and other websites and they told me that this will stay available also in the next months.

But now i am listening with interest again to your solution finding for 7.2 ;)

Share this post


Link to post
Share on other sites

In my mind there are two biggest parts, one is security, the second is the default php installation offered by the host. Hosts will move to 7.2 as their default installation. We need to be ready, thirty bees does need to work out of the box with it, because a lot of users will not know to switch versions. Its a complicated problem.

Share this post


Link to post
Share on other sites

@drmasterchief said in Smarty and PHP 7.2:

I read this with a lot of interested. But... do you really think every provider will cancel the support or will not offer hosting with PHP 7.0 and 7.1 in the next months? I dont think so. There are still thousands of offers with PHP 5.x with an separate support (without extra costs) for these hosting accounts. I am still using PHP 5.x for my actual webshop (not tb).

Is there really such a hurry for PHP 7.2 ? imho i think PHP 7.1 will accompany us for the next 3-4 years without any problems at hosting !

I have seen myself several providers that upgraded from 5.6 to 7. Refusing was not an option. There are stories on the Prestashop forum about the issue too.

Sure, if you look around you can still find another hosting provider that does support older PHP versions. But for many people moving a webshop to another provider is major operation. And even if you do everything right you can run into problems with the PHP setup of the new host.

I I bet that a lot of people who are moving to TB now are people who are driven by these PHP version problems.

Share this post


Link to post
Share on other sites

@lesley said in Smarty and PHP 7.2:

@musicmaster are you using any third party modules besides your own? Shipping modules? Real payment modules that connect via an api? These are the types of modules that we are having the most trouble out of. So while the core would be fine, who would use thirty bees if they could not use any 3rd party modules?

At the moment I am busy setting up the main part of a shop. I expect to work on the modules next week. But I expect more problems from PS-TB differences than from PHP versions.

Share this post


Link to post
Share on other sites

I read on another post:

In 1.0.9 error reporting was revamped, and the display_errors php settings in dev mode was disabled. 
That means that the theme will work just fine on php7.2 (you will still see the warnings / notices 
in error log, but not in the page itself). 

That doesn't sound like a solution to me. It will make you miss errors so that you can't solve them as soon as they appear and it makes development a lot more work as you need to download the error log each time you make a change. For me it will be reason enough to disable this feature and keep my own solution. I hope that it won't be too difficult to do that.

Note that in PHP 7.4 or 7.5 the "each()" function will disappear and this 1.09 kludge will no longer work.

The solution I proposed - on the other hand - will keep working. I use it in production and I haven't seen any signal yet of problems. Not on my site and also not on Stackoverflow.

As for solutions with a new Smarty version: I have yet to see them working. Until now there isn't even a test version available. 

The idea of a inserting a new version with a switch back option (like PS 1.4) strikes me as a bit optimistic. I would rather start with a version where the old Smarty version is the default and the new is presented as a kind of "turbo" option. That way you avoid mixing up the existing TB-PS compatibility problems with those caused by the Smarty version incompatibilities. It is also a good way to get a better view on the problems that might be involved. 

 

Share this post


Link to post
Share on other sites
1 hour ago, musicmaster said:

That doesn't sound like a solution to me. It will make you miss errors so that you can't solve them as soon as they appear and it makes development a lot more work as you need to download the error log each time you make a change. For me it will be reason enough to disable this feature and keep my own solution. I hope that it won't be too difficult to do that.

Error reporting, as originally implemented in php language, is one of the worst design they could choose. It worked somewhat at the time they invented it, but 20 years later it just doesn't fit well into modern stack. 

When this error_reporting *feature* is turned on, then any error or warning will be printed into the standard output -- into the currently rendering document. Why is this bad? It breaks the document. Note that php can generate different types of responses, and this feature can break all of them

  1. html output -- error message text, which can include html special characters like </>, will be part of the response. Depending on the place in the code where the error happened, the error message can be at the very beginning of the response (before the <html> tag), anywhere inside it, or after. If it's before, then the response is not valid html page, and browsers react accordingly. If it's inside the page, than it still can break the layout, resulting in javascript not working correctly,... Also, we can never be sure that we actually see the error message on the page. It is very common that the error message is hidden somewhere (inside display:none divs, in overflow areas, etc...)
  2. json -- many ajax requests expects json response. When there is error message mixed into the response, then the javascript will not be able to parse the response, which breaks the basic interaction functionality. 
  3. css - if php generates css, than any error message inside it will break design
  4. images - yes, php can mix these error messages into binary streams

These problems makes debug mode very hard to work with. Because responses generated in debug mode are different than the response generated in production mode. That's just stupid.

The new thirtybeees error reporting feature suppress error outputting, but collects the message anyway. When the page is rendered, then these errors are (in debug mode), emitted as javascript objects into the page, and displayed into javascript console. So you don't need to download error log each time you make a change, you can simply open javascript console in your browser. Also, there will be modules (I personally work on one) that will display these collected errors in more user friendly way -- for example in some debug bar. 

Note that this feature has nothing to do with php72 compatibility, or with smarty. It's a generic problem that was part of the prestashop/thirtybees from the beginning. 

2 hours ago, musicmaster said:

Note that in PHP 7.4 or 7.5 the "each()" function will disappear and this 1.09 kludge will no longer work.

The solution I proposed - on the other hand - will keep working. I use it in production and I haven't seen any signal yet of problems. Not on my site and also not on Stackoverflow.

Thirtybees core is fully compatible with php7.2, but it is NOT compatible with php7.3 and higher. When we run thirtybees on php72 in debug mode, then deprecation warnings are displayed. These warnings are harmless, they just inform developers that something will have to be changed in the future. And we know about it, there are definitely plans to make thirtybees fully php7.3 compatible. But because of the error_reporting described above, these warnings breaks about everything in your store. With the error reporting redesign, your store will work as expected, even in debug mode on php72.

While php73 compatibility is definitely a high priority task, it's not the most pressing at the moment. There's no need to rush into any decisions regarding smarty. Current version of smarty, as the rest of thirtybees, is fully php72 compatible. Any decision needs to be carefully evaluated, because y'all will have to live with that. 

  • Like 3

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×