PHP 7 compatibility

What changes (minimal) could make the code-base work on PHP 7.

Added php7 label

+1 for this!

From my point of view, I can only see it in this order:

1. Making it work under 5.6 first, so following http://php.net/manual/en/migration54.php and http://php.net/manual/en/migration55.php
2. Then follow http://php.net/manual/en/migration70.php

On the way we will find out that half of the libraries being used in vtiger are either outdated or actually completely deprecated. I would say, we just start... there is no minimal changes that can be planned and outlined in advance, it's too much of a mess for that. I would like to throw in this existing merge request as being related to this issue already: !25 (merged)

Interesting read about upcoming gotchas can also be found in: https://techblog.badoo.com/blog/2016/03/14/how-badoo-saved-one-million-dollars-switching-to-php7/

Well the good news is that it does work at a basic level on PHP 7 in that it runs, I can log in, look around, save stuff, so it is a matter of finding actually broken things, and recoding stuff to avoid deprecated warnings.

@preexo You started well updating HTTP/Session2 Agree with you updating libraries

Some minimal changes can work but a wider improvement is preferable.

@alanbell i think installation is not possible (you installed with lower php version didn't you?)

@prasad we all know what needs to be changed. Lets start with replaces for deprecated functions split() to preg_split() that are all over the place

Minimum changes at least to allow open records:

modules Vtiger models ListView.php line 338

$split = spliti(' from ', $listQuery);

to

$split = preg_split(' from ', $listQuery);

modules Vtiger models Module.php line 1352

$newQuery = spliti('FROM', $query);

to

$newQuery = preg_split('FROM', $query);

yes, I didn't try a fresh install yet, I was just quite pleased it worked at a basic level after an upgrade.

I was not able to get pass the installation - will update as I make the move.

php7.0-xml dependency was needed to proceed with installation.

@manuelgit - Thanks for the headsup, defining split function through explode could work.

Succeeded with installation and basic working on PHP7 with (db3ef7f9) Please share your feedback.

@prasad: Good stuff, thanks for trying hard and succeeding with PHP7! Regarding your fix in db3ef7f9, yes this will make split work. But I don't like it at all... it's dodgy and just working around the problem instead of tackling it properly. Instead of bloating up vtiger with code that will make deprecated code work, why not change the original cause of the problem and get rid of all split in vtiger code?

I hope you can comment why you chose this approach to fix things in this particular case... Looking forward to it.

@preexo - This was the minimal path I could think off. Cleanup is a big-effort that could easily take couple of months, it deserves time (hoping to be taken ahead).

@preexo - Being backward compatible sometimes help to keep up existing code work. Think of extensions and customizations that is already in production and using split - this solution helps to keep thing work after upgade without a trouble than demanding or surprising - isn't?

@preexo - Absolutely right! The possible explanation was just a quick fix to allow installation success. I hope that workaround in not definitive because as you said feels dodgy and doesn't solve other areas.

Milestone changed to 6.5.0

this would be a quick fix for me: http://code.vtiger.com/preexo/vtigercrm/commit/6a87fcfd6326b398b898c7cf6d5750c3012198b7 and http://code.vtiger.com/preexo/vtigercrm/commit/fc12cc3505c3a65423f051ebd7530bc81addab5c

Please let me know what would be wrong with this.

I understand that vtiger is using some libraries that come from PHP4 age and which are still using those deprecated functions, but I don't think that vtiger can accept that and instead should update those libraries, go native or with completely different solutions. Backwards compability for deprecated libraries should not be an issue.

the PHPExcel library has some issues that cause 'break' not in the 'loop' or 'switch' context errors, where they do a return from a switch followed by a break (which is redundant and can never be reached)

            switch (gettype($value)) {
                    case 'integer'  :
                            return $value;
                            break;

These could be fixed, but probably best to pull from upstream PHPExcel, however they don't appear to have a stable release for PHP 7 https://github.com/PHPOffice/PHPExcel

@preexo - @alanbell - I did some extensive tests with smarty3 ===== 3.1.30-dev ===== and it works out of the box. Just for reference in case you want to skip that library from your tests.

So, maybe that worth an update also.

There are some incompatibility that could be easily solved with PDFMaker in sidebar widget doesn't list properly the templates list for export, only shows an "A" in the (maybe if PDFMaker Developers see this post can fix)

mentioned in issue #218 (closed)

@manuelgit - I tried adding Smarty through composer, maybe you checkout my branch on this issue and give me some feedback: http://code.vtiger.com/preexo/vtigercrm/commits/introduce_composer

There are quite a lot of notices generated by the cache, by using indexes that don't exist. I wonder if that is bad usage of the smarty template engine by the vtiger code or if the smarty engine is actually coded in such a dirty way as well... Haven't found time to investigate this further.

E.g.:

Notice: Undefined index: bill_city in /var/lib/shared/websites/custom/www/crmfresh/test/templates_c/vlayout/db44585e27becb6866d2e68c19bc4039a003481d_0.file.ListViewContents.tpl.php on line 119

@preexo - I just checked with 3.1.30-dev and 3.1.29 but i see the same with original Smarty

Last check after clean installation, same result, no more Notices than before

+1

@alanbell - as far as the issues about PHP7 go in the github repro from https://github.com/PHPOffice/PHPExcel it says that 1.8.1 is a stable release, which is PHP7 compatible. But I would again advise to not make this manually, but to move this into composer http://code.vtiger.com/preexo/vtigercrm/commits/introduce_composer

I am just not sure if vtiger would let us go this route...

Composer approach would not fit well in the current framework. We would not be switching to it immediately.

@prasad - I agree, an immediate switch might be too much changes at once, but what about introducing it as suggested in my merge request !68 (closed) with only one or two libraries first. Then migrating all used libraries over to composer, one after another.

How does the approach not fit in the current framework? Which current framework are you referring to?

Thanks for following up, much appreciated!

nusoap + pear Request needed syntax fix - 9a422f5a

dUnzip + dZip needed syntax fix - d23ddb8f

split* compat improved - 1adf437f

HTMLSax3 needed syntax fix - c5ba6f07

Status changed to closed

7.1.0 works on PHP 7

Added bug label

Milestone removed