Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Was about to follow #2274543-120: [7.x-1.x] Remove usage of deprecated create_function() ( thanks @josepholstad ) but seems simple update from 1.x to 2.x branch has fixed the issue for me.
Comment | File | Size | Author |
---|---|---|---|
#193 | views_php-remove_create_function-2274543-193-D7-interdiff.txt | 624 bytes | Berdir |
#193 | views_php-remove_create_function-2274543-193-D7.patch | 9.97 KB | Berdir |
#180 | interdiff-175-180.txt | 195 bytes | Liam Morland |
#180 | views_php-remove_create_function-2274543-180-D7.patch | 9.97 KB | Liam Morland |
#176 | interdiff-172-175.txt | 1.46 KB | Liam Morland |
Issue fork views_php-2274543
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #1
jienckebd CreditAttribution: jienckebd commentedComment #2
bkosborneI've changed the title to reflect that this should be removed for PHP 7.2+ future proofing as well.
PHP's create_function() has been deprecated as of PHP 7.2.
The patch provided uses the new method of declaring an anonymous function, but that's only supported as of PHP 5.3, and Drupal 7 officially supports PHP 5.2.5+.
I wonder if the module can just declare real functions to use instead of anonymous functions. I think that's the only method that will work with PHP 5.2.5+.
If not, this module will have to declare that it requires PHP 5.3.
Comment #3
NWOM CreditAttribution: NWOM commentedThank you very much for the patch!
However, it appears the above patch is based on 7.x-2.x-dev from the module page, but not from HEAD (https://www.drupal.org/node/1043008/git-instructions/7.x-2.x/nonmaintainer).
The following error is shown when attempting to apply the patch:
I have updated the patch, and also took liberty to make it more drupal conform (the patch had tabs and spaces, rather than double spaced).
Please review.
Comment #4
bkat CreditAttribution: bkat commentedThis patch saved me a ton of time on a Fedora 28 system with PHP 7.2. I did find one problem though. The value was always null because the code for evaluating 'php_value' was missing a return
in php_post_execute
should be
I'm attaching an updated patch file.
Comment #5
akolahi CreditAttribution: akolahi commentedI am getting Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /includes/cache.inc) with Patch #4.
If I revert the patch the error goes away.
Digging a bit and it seems to only be an issue on Views that have both Views Bulk Operations and vews_php.
To replicate:
Comment #6
NWOM CreditAttribution: NWOM commentedSetting to Needs Work due to the problems mentioned in #5.
Comment #7
pipicom CreditAttribution: pipicom commented#4 works for me. I do not have Views Bulk Operations installed.
Comment #8
AndreyMaximov CreditAttribution: AndreyMaximov commentedIt is just like @akolahi described it.
The patch should help.
Comment #9
AndreyMaximov CreditAttribution: AndreyMaximov commentedComment #10
sistro CreditAttribution: sistro as a volunteer commentedI still was having warnings on line 212 on views_handler_field#pre_render
I removed line:
$this->php_output_lamda_function = create_function('$view, $handler, &$static, $row, $data, $value', ' ?>' . $this->options['php_output'] . '<?php ');
And added this lines:
$code = $this->options['php_output'];
$this->php_output_lamda_function = function($view, $handler, &$static, $row, $data, $value) use ($code) {
return eval($code);
};
Seems to work.
Comment #11
monnerat CreditAttribution: monnerat commentedThis patch for 7.x-2.x emulates
create_function
() without ause
() clause, thus reducing the number ofeval
() calls to recompile the code.It takes care of unsetting non-local Closure objects to avoid serialization errors.
Comment #12
joe_pixelrow CreditAttribution: joe_pixelrow commented#11 patch eliminated all of the errors for me
Thank You monnerat
Comment #13
alb404 CreditAttribution: alb404 commentedHi.
I made a patch that combines #8 and #6 from this issue: https://www.drupal.org/project/views_php/issues/2300573
Comment #14
ab_early@hotmail.com CreditAttribution: ab_early@hotmail.com as a volunteer commented#11 patch eliminated all of the errors for me
Thanks monnerat
Comment #15
skylord CreditAttribution: skylord commentedHere is #11 for 7.x-1.0-alpha3
Comment #16
lgough CreditAttribution: lgough commentedThanks so much skylord (and mannerot), #15 worked perfectly on 7.x-1.0-alpha3
Comment #17
Neo13 CreditAttribution: Neo13 commented#15 works for me too
Comment #18
cirrus3d CreditAttribution: cirrus3d commentedHere is #15 for 7.x-1.x-dev
Comment #19
glynster CreditAttribution: glynster commented+1 latest patch works a treat!
Comment #20
TrevorBradley CreditAttribution: TrevorBradley commented+1 on #15 here as well!
Comment #21
lne_topos CreditAttribution: lne_topos commented+1 on #18. Thanks for providing for 7.x-1.x-dev.
Comment #22
PeterStubRaindrop CreditAttribution: PeterStubRaindrop commentedI'm a bit confused here...Using 7.x-1.x-dev - and php 5.6.32.
Used patch #18 and got this...:
Hunk #1 succeeded at 63 (offset 7 lines).
Hunk #2 succeeded at 84 (offset 7 lines).
patching file views_php.module
Hunk #2 succeeded at 158 with fuzz 1 (offset 10 lines).
I get tons of these:
Notice: Undefined variable: params i {closure}() (linje 6 af /Applications/.../modules/contrib/views_php/views_php.module(166) : eval()'d code).
I also did a test run with php 7.2 and 7.1 - not luck...
Cant get all those Notices out of here...Can somebody help out?
EDITED: Solved - it was our own mistakes. In the view that used php there was an undefined variable - as cirrus3d / #23 pointed out - THANKS ALOT.... So feel free to delete my comment as its not really relevant for this issue?
Comment #23
cirrus3d CreditAttribution: cirrus3d commentedHi,
It seems to me that these notices are being generated from invalid code inside your View (for example if you have a PHP field or filter). Could you check Line 6 of your custom code?
Comment #24
PeterStubRaindrop CreditAttribution: PeterStubRaindrop commentedThank you very much...@cirrus3d #23
Comment #25
youngwolf0 CreditAttribution: youngwolf0 as a volunteer and commentedjust wondering if this patch will ever be rolled into the main module since it was last updated on the 13th of November 2015. Our infrastructure is not set up in a way that makes patching easy so would be better for me if it was in a proper update.
Comment #26
Welsby CreditAttribution: Welsby commented#23 works a charm, thanks.
And a +1 for this to be rolled into a release please.
Comment #27
cmseasy CreditAttribution: cmseasy commented#15 for 7.x-1.x-dev works ok.
Please commit
Comment #28
steveoriol#15 for 7.x-1.0-alpha3 works.
Comment #29
rachelf CreditAttribution: rachelf commented#15 for 7.x-1.0-alpha3 works for me too
Comment #30
fred1978 CreditAttribution: fred1978 commentedFor me, using #15 for 7.x-1.0-alpha3 breaks the sorting of my view. I use PHP to calculate a new variable, and then sort the results according to this new variable and also existing variables.
The sort code looks like this
if ($row1->php < $row2->php) {
return -1;
}
else if ($row1->php > $row2->php) {
return 1;
}
...(sort on other fields results)...
else
return 0;
Could this be related to the "unset($this->php_sort_function); /* Closure objects are not serializable. */" in the ob_start part of the code ?
Comment #31
monnerat CreditAttribution: monnerat commentedWhat do you mean by "breaks"? Is it wrongly sorted or does it crash ?
If you have a crash, then probably you have a syntax error in your sort code, or an exception occurring in it.
No:
$this->php_sort_function
is set just before thisob_start()
sequence and only used by functionphp_sort()
that is called by theusort()
before theunset()
. Unsetting$this->php_sort function
only prevents it from being serialized later and should not impact the logic.Note that your problem might be related to
$row->php
being a structure and you have to extract the comparison value from its subfields.Comment #32
fred1978 CreditAttribution: fred1978 commentedThank you monnerat, I will investigate based on your comment. No crash in my views result, but results are wrongly sorted.
Comment #33
fred1978 CreditAttribution: fred1978 commentedMy mistake : there has been a problem while patching my file, the usort() line before the unset() has been removed. Thanks monnerat for pointing the function to me. Patch #15 for 7.x-1.0-alpha3 works well then.
Comment #34
salvisThe RTBC refers to #15 and #18 for 7.x-1.x-dev — please commit #18 to 7.x-1.x-dev!
There are ten times as many users on 1.x than on 2.x and 1.x has at least an alpha release. The 2.x branch only has a snapshot and its state is uncertain, according to the front page.
Comment #35
MustangGB CreditAttribution: MustangGB commented#18 has some formatting and coding style issues, also the removal of
php_output_lamda_function()
seems like a API breaking change that's not needed.So here's a version I'm testing against 7.x-1.x-dev to address these things.
Comment #36
MustangGB CreditAttribution: MustangGB commentedThe future is now.
Comment #37
salvisI agree about the formatting and coding style issues, but you've introduced a new glitch of your own:
While #35 looks siimilar to #18, the changes aren't just cosmetic — this needs reviewing and testing from scratch.
Comment #38
MustangGB CreditAttribution: MustangGB commentedDarn it, nice catch.
Comment #39
akzahid CreditAttribution: akzahid commentedI am getting Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /home/project/public_html/drupal_site/includes/cache.inc) with Patch views_php-2274543-38.patch
I did the same work as akolahi #5 to reprduce the issue
[ If I revert the patch the error goes away.
Digging a bit and it seems to only be an issue on Views that have both Views Bulk Operations and vews_php.
To replicate:
Apply Patch
Create View with a views php field and VBO
Visit the View
]
Comment #40
DamienMcKenna@akzahid: If you're writing code that sophisticated you might want to consider moving it into a module instead of burying it in the Views interface.
Comment #41
akzahid CreditAttribution: akzahid commented@DamienMcKenna Thanks, Yes that what I am working on and redesigning it in drupal 8 . But for the time being I am working to make sure the current used site work properly using pHP 7.2
Comment #42
nickonom CreditAttribution: nickonom commentedI was pulling the dev version with `drush dl views_php --dev` command and failing to get the patch applied several times, until I paid enough attention to see the patch was for 7.x.-1.x. But then the project page says:
and that's really confusing.
Comment #43
joelpittetYes this is back to RTBC thanks @MustangGB
Comment #44
salvisTrust the facts rather than the text...
7.x-2.x is just not there yet and may never get "there".
7.x-1.x hasn't quite gotten to 7.x-1.0 yet either, but it's what people are using and what needs to be fixed.
Comment #45
nickonom CreditAttribution: nickonom commentedsalvis, so every new user has to get to the facts only after sweating some time over this. imho, it's more usable to get the project description aligned with the "facts". i am not editing it only because i am not sure about the project developer's stance on this.
Comment #46
salvis<offtopic>
I agree with you, nickonom — in a perfect world the maps always match the terrain.
In open source, however, with a lot of volunteer work, "customers" should look at the wares before downloading them.
I'm not picking on open source, quite the contrary, because in open source you can look at the stuff before you "buy", which is a huge advantage over commercial software, but open source is not free like free beer. Customers need to realize that they also have to "pay" for open source offerings, e.g. by taking on the responsibility for selecting what they download. It's just part of doing your homework.
</offtopic>
Comment #47
nickonom CreditAttribution: nickonom commentedI do confirm #4's finding:
Comment #48
akzahid CreditAttribution: akzahid commentedI Still have this following issue. I use the latest patch #38
"Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /includes/cache.inc)"
Akzahid
Comment #49
akzahid CreditAttribution: akzahid commentedComment #50
DamienMcKenna@akzahid: Please see if you can track down the view and then custom PHP code which triggers the above error.
Comment #51
akzahid CreditAttribution: akzahid commentedI use the follwing php code in global php in view
and also using
Global: Send e-mail (Send e-mail)
and this view shows the following error: (used the latest patch #38)
"Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /includes/cache.inc)"
Thanks
Comment #52
gravisrs CreditAttribution: gravisrs commentedPatch won't work due to caching.
eval() created functions are Anonymous functions that are implemented using the Closure class - and thus cannot be serialized for the purpose of caching.
So if you use a view within a block, or anywhere where Drupal caches the output - it will throw an Exception like in #48
We need different approach
Dirty hack to use for now is
@create_function(...
Comment #53
MustangGB CreditAttribution: MustangGB commentedIn response to #52 maybe the approach to take, rather than trying to work around closures not being serializable, is to allow them to be serializable, and therefore keep cache functionality and such.
Here is a patch to try this strategy, it uses the Opis Closure library to do this.
Perhaps a hard dependency on Libraries API isn't wanted or needed, but it served as suitable for testing the idea at least.
To test, make sure you download the library and put it somewhere like:
sites/all/libraries/closure/autoload.php
Also for review purposes it's worth noting that although the library, which serves as a wrapper, says it allows direct access to the closure, it recommends going through
getClosure()
, and indeed this was needed in our case due to parameters being passed by reference, might be something that could be resolved upstream, but not the end of the world at any rate.Comment #54
MustangGB CreditAttribution: MustangGB commentedFixed typo.
Comment #55
khaldoon_masud CreditAttribution: khaldoon_masud as a volunteer commentedThe patches #15 #18 and #54 are still reporting below warning on my local machine:
Deprecated function: Function create_function() is deprecated in views_php_handler_field->pre_render() (line 202 of /app/drupal/sites/all/modules/contrib_modifed/views_php/plugins/views/views_php_handler_field.inc).
I am using a linux alpine distribution on a docker container on MacOs. It doesn't make sense cause I can't find create_function anywhere in the code, BUT when I pushed same code to our dev server running Ubuntu OS, it started working without errors.
I applied patch #15 on 7.x-1.0-alpha3. Couldn't apply patch #18, there was a spacing issue.
Comment #56
stacypendell CreditAttribution: stacypendell commented#54 is working for me - "Exception: Serialization of 'Closure'" errors stopped appearing.
Comment #57
akzahid CreditAttribution: akzahid commented#54 is working for me - "Exception: Serialization of 'Closure'" errors stopped appearing for me as well.
Note: After this my site is not working on PHP 5.6 - Works fine on PHP 7.2
resaons:
Parse error: syntax error, unexpected '(' in /root/---/---/drupal_folder/sites/all/modules/views_php/views_php.module on line 34
Thanks
akzahid
Comment #58
hiramanpatil CreditAttribution: hiramanpatil commentedI am getting these output after applying patch #53 and #54 both.
Skipped patch 'plugins/views/views_php_handler_area.inc'.
Skipped patch 'plugins/views/views_php_handler_field.inc'.
Skipped patch 'plugins/views/views_php_handler_filter.inc'.
Skipped patch 'plugins/views/views_php_handler_sort.inc'.
Skipped patch 'plugins/views/views_php_plugin_cache.inc'.
Skipped patch 'views_php.info'.
Skipped patch 'views_php.module'.
Comment #59
MustangGB CreditAttribution: MustangGB commented@hiramanpatil
Probably you're trying to apply the patch from the wrong folder or to the wrong branch, either way your comment is unrelated to this issue.
Comment #60
alimc29 CreditAttribution: alimc29 commented... apparently I can't delete my own comment - this was posted in error
Comment #61
spelcheck CreditAttribution: spelcheck commented@MustangGB - I'm currently on 2.x-dev (sadly) and there's no easy way for me to get back to 1.x-dev. I reworked your latest patch to work for 2.x-dev but unsure whether you'd want me to post in here, as it might be confusing to the thread. Let me know what you'd like to do. As you're a major contributor to this fix, I'll leave it to your judgement. Thank you!
Comment #62
MustangGB CreditAttribution: MustangGB commented@spelcheck
The done thing these days seems to be to create a new issue and add a relationship between the two, so I'd say to go with that.
Comment #63
MustangGB CreditAttribution: MustangGB commentedRegarding PHP 5.6 backwards compatibility as per #57.
Probably we must make changes along these lines:
And so on for the rest of the
getClosure()
references.I tried to keep it as similar to the original code as possible, and to make it an easy switch to remove the explicit
getClosure()
calls (if/when the aforementioned "by reference" issue is addressed), however backwards compatibility is more important than looking pretty.Comment #64
MustangGB CreditAttribution: MustangGB commentedNeeds work, to make the changes noted in #63.
Comment #65
spelcheck CreditAttribution: spelcheck commented2.x-dev ports of contributed patches @ https://www.drupal.org/project/views_php/issues/3055655. I'll do my best to keep them updated with whatever is posted here.
Comment #66
monnerat CreditAttribution: monnerat commentedThis is an interesting comment in an issue that was originally created for 7.x-2.x and hijacked for 7.x-1.x, then simply reassigned to 7.x-1.x with all 7.x-2.x attachments hidden !!!
Comment #67
spelcheck CreditAttribution: spelcheck commentedConcerning #5 and #39, I also run into issues when using VBO. Was there any workaround or recommendation?
On simply visiting the view with VBO:
Warning: file_get_contents(/var/www/d7site/web/sites/all/modules/contrib/views_php/views_php.module(197) : eval()'d code): failed to open stream: No such file or directory in Opis\Closure\ReflectionClosure->getFileTokens() (line 631 of /var/www/d7site/web/sites/all/libraries/closure/src/ReflectionClosure.php).
It's attempting to file_get_contents() on "....views_php/views_php.module(197) : eval()'d code" instead of the filename by itself.
On attempt to actually use VBO within the view:
Notice: unserialize(): Error at offset 1711218 of 3039193 bytes in DrupalDatabaseCache->prepareItem() (line 449 of /var/www/d7site/web/includes/cache.inc).
Error: Unsupported operand types in form_get_cache() (line 525 of /var/www/d7site/web/includes/form.inc).
Comment #68
spelcheck CreditAttribution: spelcheck commented@MustangGB - Updated your patch
views_php-2274543-54.patch
according to instructions on #63. Forgive me for hijacking the patch, I'm hoping to keep momentum so that there might be some time to attack VBO issues.Comment #69
MustangGB CreditAttribution: MustangGB commentedNo problem, I didn't have time yet, so was hoping someone would roll in the changes.
Comment #70
tt12 CreditAttribution: tt12 commentedAfter applying patch #68 and visiting the view with views_php, i encounter an error:
Error: Class 'Opis\Closure\SerializableClosure' not found in function views_php_create_function() (line 188 in file (...)/views_php/views_php.module).
Comment #71
MustangGB CreditAttribution: MustangGB commented@tt12
You need to install the library, as mentioned in #53.
Comment #72
newtoidConfirming that the patch also works for the latest alpha version of views_php.
Comment #73
James Feng CreditAttribution: James Feng commentedviews_php-2274543-38.patch ,It suits me very well. my version:php7.2; Views PHP
7.x-1.0-alpha3; Thanks.
Comment #74
patoshi CreditAttribution: patoshi commentedSo i need to install the Libraries api module? And then what before doing the #68 patch?
Or do I just use patch #38?
i tried patching #38 with v1.x dev views_php but i'm getting error of:
can't find file to patch at input line 5
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
-------------------------- |diff --git a/plugins/views/views_php_handler_area.inc b/plugins/views/views_php_handler_area.inc |index 0296373..ed14bcf 100644 |--- a/plugins/views/views_php_handler_area.inc |+++ b/plugins/views/views_php_handler_area.inc --------------------------
File to patch: ^C
Comment #75
liquidcms CreditAttribution: liquidcms commentedI'm surprised that this patch is for 1.x branch and that people are still using this. The 1.x branch never really worked as advertised and the 2.x branch is a major improvement (as it does what the module suggests). Ahh.. but i see a link to 2.x patch for this.. yay!! :)
Comment #76
patoshi CreditAttribution: patoshi commented@liquidcms so your saying 1.x doesn't work at all with the patches above?
FYI, I've applied patch #38 to the 1.x dev code base as I needed to patch it manually as the patch file wasnt working.
Here is the file: https://transfer.sh/Qc3we/views_php_201921_38.tar --- nevermind it doesnt work...
Comment #77
MustangGB CreditAttribution: MustangGB commentedThe patches work fine with 1.x, I believe he's simply speaking in the abstract.
Perhaps if some of the people using 2.x in the wild would chime in on what the release blockers might be we could all get back on the same page, but this is not really the issue for such discussions me thinks.
Comment #78
joseph.olstadI am using patch #68 with the library mentioned in #53 and it works in both php 7.2.x and php 7.3.x
however the patch breaks php 5.6.x with the following error:
Parse error: syntax error, unexpected '(' in sites/all/modules/contrib/views_php/views_php.module on line 34
line 34 is this:
here is what I found on php.net might be related, if anyone is really keen they might figure this out:
https://www.php.net/manual/fr/functions.anonymous.php#119388
here is a sandbox php 5.6 environment with php 7.1.x as well, shows precisely the same issue:
// php 5.6.x is BARFING ON ->getClosure()();
see example code:
https://3v4l.org/P1MRc
source code from the 3v4l.org example:
Output for php 7.1.x / 7.2.x / 7.3.x
foo
Output for php 5.6.x
Comment #79
FeyP CreditAttribution: FeyP as a volunteer and at werk21 commentedThanks Joseph for testing the patch with different versions of PHP. I'm setting the issue status back to needs work, since we have a patch and we just need to work on it a bit more to fix the remaining issues with it.
Comment #80
MustangGB CreditAttribution: MustangGB commentedThe solution to #78 (and #57) was already posted in #63.
These were intended to be resolved in #68, however it looks like one got missed, so should be straightforward to resolve.
Comment #81
joseph.olstadThanks @MustangGB,
here is the new patch based on the solution posted in #63.
interdiff is included for readability.
Comment #82
joseph.olstadOk, back to RTBC, I tested patch 81 with both php 5.6.x AND php 7.3.x , works beautifully in both cases.
Comment #83
joseph.olstadcrap ya I'm having the same issue as spelcheck with VBO.
see #2274543-67: [7.x-1.x] Remove usage of deprecated create_function()
with that said, patch 81 is the best so far.
Comment #84
joseph.olstadOk I've patched the library, the warning no longer shows up, HOWEVER, I'm still reviewing the results....
Here's the patch for the closure project.
I've forked the closure project if you prefer you may use my fork:
https://github.com/joejoseph00/closure
Comment #85
MustangGB CreditAttribution: MustangGB commentedNice job, thanks for chasing this one, I noticed you submitted a pull request upstream as well.
Comment #86
joseph.olstadya the author of the closure library already replied, some minor formatting and code style changes were made to the solution, either way works.
https://github.com/opis/closure/pull/38
he is reviewing this , the travis ci is passing.
I haven't found any regressions with this solution "yet" however I do not really entirely understand what this closure library is doing. If I find a regression I will report back, however so far so good.
Comment #87
joseph.olstadI had to adjust the patch for the closure library.. It's improved.
still have not found a regression with this.
use the closure library with the closure library patch:
and views_php version 7.x-1.x with this patch from #81
works on php 5.6.x AND php 7.3.x
with or without views_bulk_operations
I am still searching for possible regressions however have not yet found any.
Comment #88
joseph.olstad***EDIT***
NEVERMIND this comment... I re-read this entire thread... Cross fingers that the patch I provided for the closure library does not have any regressions.......
ALTERNATIVELY:check the Drupal 8 version of this solution. It doesn't use the closure library AT ALL....
see the custom function it uses instead:
Why can't we use this function instead of getClosure() and avoid using the closure library altogether?
Please check the Drupal 8 version of this solution: #3026201-3: Function create_function() is deprecated in Drupal\views_php\Plugin\views\field\ViewsPhp->render() (line 122 of modules/contrib/views_php/src/Plugin/views/field/ViewsPhp.php patch number 3, no closure library is required.. might be a lot simpler and a lot lower risk.Comment #89
DamienMcKennaSomething to consider..
Create a new branch of the module that only works with PHP 7.x (pick a branch) and mark 7.x-1.x as unsupported.
Comment #90
joseph.olstad@DamienMcKenna, can you please elaborate on this very interesting idea? I like the discussion.
Comment #91
DamienMcKennaLeave the 7.x-1.x branch focused on PHP 5.x with complete disregard for PHP 7.
Create a 7.x-2.x branch. Decide which specific version of PHP to stick with - 7.0, 7.1, 7.2, etc, and list that in the info file.
Write this patch for the 7.x-2.x branch, i.e. PHP 5 compatibility doesn't matter anymore.
Profit.
(well, no, not really, but YKWIM)
Comment #92
joseph.olstadUpdate: I spent more time working on the suggestions from the upstream developers of the opis/closure library and the opis/stream library.
The TESTED successful approach:
patch 81 and my forked closure library is working.
I had to adjust the patch for the closure library.. It's improved.
still have not found a regression with this.
OR if you like patching, use the non-forked opis/closure library with the closure library patch:
OR use this forked closure repo that includes the closure patch (no need to patch this fork of the opis/closure library):
https://github.com/entreprise7pro/closure
and views_php version 7.x-1.x with this patch from #81
works on php 5.6.x AND php 7.3.x
with or without views_bulk_operations
The FAILED but suggested by library maintainers approach.
The opis/stream library approach does not work, it is passing seralizable functions and causes a core crash here:
Exception: Serialization of 'Closure' is not allowed in serialize() (line 465 of /path/to/drupal/includes/cache.inc).
or get this from another one of their suggestions
Warning: Parameter 3 to Opis\Closure\SerializableClosure::{closure}() expected to be a reference, value given in Opis\Closure\SerializableClosure->__invoke() (line 109 of web/sites/all/libraries/closure/src/SerializableClosure.php).
Comment #93
joseph.olstadOk slight change to the patch
see interdiff, I am changing the vendor of the closure library from opis to entreprise7pro.
this patch 94 is intended to be used with the entreprise7pro forked version of the closure library
Comment #94
joseph.olstadnew patch forthcomming.
Comment #95
joseph.olstadok, the upstream closure library maintainers were very helpful and pointed out how to implement this without needing to patch their closure library.
So, use the opis/closure library version 3.4.0 or newer (you must upgrade if you are using an older version of this)
and use this patch:
Comment #96
ibakayoko CreditAttribution: ibakayoko as a volunteer commented@joseph.olstad thanks for the patch.
After applying the patch i get a new error when i try to run a VBO action
Error: Unsupported operand types in form_get_cache() (line 525 of /Users/../../docroot/includes/form.inc).
Comment #97
joseph.olstad@ibakayoko , what version of php are you running when this error occurs ?
Comment #98
joseph.olstadalso, what version of the closure library are you using? 3.4.0 ?
Comment #99
joseph.olstadibakayoko , if you're using memcache, did you flush your memcache after applying the patch and correctly installing the closure library?
Comment #100
MustangGB CreditAttribution: MustangGB commentedAwesome, I think it's a bit clearer if we write it this way.
Also standardised the int cast spacing with the rest of the codebase, and restored some whitespace that was removed unnecessarily.
Comment #101
DrupalYedi CreditAttribution: DrupalYedi commentedThe patch #81 is working for me to hide the "deprecated error message"... - BUT in my case individual fields under "available variables" are not working. The value of individual fields is always the node ID instead of the field value. Only the Avaiable variables "$row->nid" and "$row->title" are working correct.
Does anyone have the same problem? Any idea how to solve it?
Comment #102
joseph.olstad@DrupalYedi, you should use the patch # 95 with the closure library, install the closure library as mentioned in the above thread, get the latest release of the closure library, apply the patch 95, should work.
I haven't tested MustangGBs patch however it could also do the job.
I have been using patch 95 in combination with VBO , works fine as does regular views php without vbo.
We're using php 7.3.x
re-patch from 7.x-1.x head of dev branch
Comment #103
joseph.olstadSome related issue with php 7.3.x (7.2.x) compatibility
I have a VBO form with views_php field in an ajax form
1st clue is this error message:
I put a debug into line 525 of includes/form.inc (core) and dumped the '$cached' object, the 'data' attribute is empty..
Going further back, it appears to be something with the cache system not being able to unserialize our closure.
2nd clue:
3rd clue:
So the cache system is unable to unserialize our closure as per includes/cache.inc
If someone has a solution for this, please share.
I think it is related to the cache system in core not liking php 7.3.x / 7.2.x and unserializing closures.
I'm looking at what looks to be a related core patch #2819375-94: Do not make entries in "cache_form" when viewing forms that use #ajax['callback'] (Drupal 7 port)
however it is an 'opt-in' patch, which allows configuring which forms get 'excluded' from cache, unfortunately this is impossible with the views_php vbo form ajax I'm using because the form id changes every cache clear.
Comment #104
joseph.olstadWow , I cleared caches, re-applied the core patch, WOOT fist pumps, it works!
#2819375-94: Do not make entries in "cache_form" when viewing forms that use #ajax['callback'] (Drupal 7 port)
I did some more testing, php 5.6.x does not have this problem, works fine with the closure library. the issue I was observing was only occuring for me in the php 7.3.x (I didn't test 7.2.x but it probably same thing happens there).
SO, for those of you that use this, add the core patch too
Comment #105
joseph.olstadah Sorry for the bad news, but my last test I was running php 5.6.x , after I switched back to php 7.3.x I got the error described in comment #103
So no, that core patch, I'd have to configure the random form id in the variable as per the core patch, which changes dynamically, so meh, need to review this some more.
Comment #106
joseph.olstadOk, update, good news, the core patch 42 by Quicksketch does the job.
here it is: https://www.drupal.org/files/issues/drupal-no_cache_form-2819375-42.patch
#2819375-42: Do not make entries in "cache_form" when viewing forms that use #ajax['callback'] (Drupal 7 port)
So to summarize, use patch 100 by MustangGB here
and the closure library as discussed above
and core patch 42 from 2819375
Comment #107
joseph.olstadOk, status summary:
To use views_php with php 7.2 is pretty easy
Step 1) download the latest 7.x-1.x branch (see other issue for the 2.x branch) dev release or latest release probably works too
Step 2) apply MustangGBs patch 100 above #2274543: [7.x-1.x] Remove usage of deprecated create_function() to the views_php module
Step 3) install the latest opis closure library To test, make sure you download the library and put it somewhere like:
sites/all/libraries/closure/autoload.php
Step 4) Make sire to have prerequisite libraries module installed and enabled.
Optional steps
Step 5) if you have issues with ajax forms in vbo , apply this core patch which resolves massive form caching issues.
Step 6) if you are using php 7.3 or higher you will need this diff applied to core AFTER the patch from step 5 is applied after applying that core patch:
Step 7) clear caches, have fun.
Comment #108
joseph.olstadok everybody, more followup.
I repeat, no errors in php 5.6 , php 7.0 , php 7.1 , php 7.2
everything is just peachy in those versions of php
With php 7.3 I found the following:
I'm seeing this on Drupal 7 with php 7.3.x using views_php and the closure library.
The error happens in includes/cache.inc
the error is as follows:
Warning: Class __PHP_Incomplete_Class has no unserializer in DrupalDatabaseCache->prepareItem() (line 451 of /var/web/includes/cache.inc).
next is a notice:
Notice: unserialize(): Error at offset 255029 of 338002 bytes in DrupalDatabaseCache->prepareItem() (line 451 of /var/web/includes/cache.inc).
I originally got the error at offset 133000 something, then I cranked up my php memory limit to 2048 MB , it went to a higher number offset, then I cranked it up to 4096 MB as a test, the offset seems to be similar
but it's definately the same serialized data that is not wanting to unserialize.
Wwe're using the opis/closure library to replace create_function in views_php at the above latest patch of course .
I've debugged far enough to get the serialized nastiness that throws this, I had to copy all the lines into a test.php file and use the multiline variable technique and managed to unserialize it without errors using the unserialize() function. The serialized output had too many escape characters and unquotes to be handled another way. Weird that only php 7.3 barfs though.
$var = << ALL THE NASTY STUFF GOES HERE
more nasty stuff
NASTYMULTILINESERIALIZATIONWITHVIEWS_PHP;
then
echo unserialize($var); // works fine
but it crashes in cache.inc , ONLY in php 7.3 , does not crash in php 7.2.
Attached is what I'm 99% sure the serialized data that is blowing up on, it's got views_php serialized , see test_php.tgz
however, run the php from the command line, there's no issue, it's only an issue in includes/cache.inc
see the reported similar core issue: #2882456: Drupal 8.4.x and PHP7 unserialize compiled route - __PHP_Incomplete_Class error
people noticed this in D8 at some point, but it was not really the same issue. Same symptoms but different source of the issue.
***EDIT***
the previous comment refers to errors observed with php 7.3.8.1
I'm hopeful that php 7.3.9.x will resolve the issues, seems like a bug in php, why would it all of a sudden have problems with unserialize when all the previous versions didn't have this problem?
***END EDIT***
Comment #109
MustangGB CreditAttribution: MustangGB commented@joseph.olstad Related to https://bugs.php.net/bug.php?id=77302 perhaps?
If so:
Might be explained by:
Comment #110
joseph.olstad@MustangGB , ya , sounds like the same bug for sure, there's a lot of bug fixes in 7.3.9.0 , as soon as I upgrade I'll re-test after I upgrade from 7.3.8.1
Comment #111
joseph.olstadIn D8 there is a core issue that looks related, so I made a D7 child issue from that one.
#3093110: D7 - EntityType objects cannot be reliably serialized without DependencySerializationTrait
Comment #112
rgpublicIt seems on Symfony they also talked about this:
https://github.com/symfony/symfony/issues/29459
And they also finally point to the bug already mentioned by MustangGB:
https://bugs.php.net/bug.php?id=77302
I think it's all the same bug. It's not in Drupal 7/8/whatever but definitively in PHP. AFAIU the bug is not introduced in PHP 7.3 but this version is more strict and crashes. In earlier version the unserialization is wrong but probably as long as the unserialized data isn't used somehow, the error doesn't cause any user-visible problems.
The import comment is probably:
I cannot imagine, though, that at least D8 will remain incompatible with all PHP 7.3 versions due to this. For D7 I wonder, wouldn't it be enough to apply the workaround for those classes where the error is most prevalent?
Comment #113
joseph.olstadFrom what I gather, in D8 they work around this by using DependencySerializationTrait
see #3093110: D7 - EntityType objects cannot be reliably serialized without DependencySerializationTrait and the parent D8 issue, the original.
I'm not sure what all this means though, sounds like a workaround is needed in some cases. What boggles me is according to my tests I don't see any issue until I switch to php 7.3.8.1. php 7.2.x does not throw any warning or error messages.
Something changed in the php 7.3.x serialize() function that makes it behave differently in some cases.
Comment #114
rgpublicBTW: I wonder whether it might be advisable to not use the Closure library to serialize the closure because actually the code should be available as a string before we even turn it into a Closure. Perhaps the closure could be replaced with a proxy class that holds the closure and the original code. When it's time to serialize/sleep we only serialize the code string we already have and discard the closure. And on wake-up we create the closure from the code string again. This way we'd avoid having to serialize/unserialize the closure in the first place - removing the dependency on OPIS lib as well. Wouldn't that work? Don't know much about this module though, so it's just a thought. Came here from the general PHP 7.3 support issue.
Comment #115
joseph.olstad@rgpublic , ya I think you might be on to something, upstream they mentioned problems occur when something gets serialized twice, once maybe by closure, then again by the cache.inc when something gets shoved into the cache?
I'm guessing of course on this.
Comment #116
rgpublic@joseph.olstad: Yeah, that's weird. But I guess to achieve a *solution* for those problems it doesn't actually matter whether the problem is during serialization or unserialization - even though I understand it'd be interesting from a "scientific" POV.
I think that the important part from DependencySerializationTrait that avoids this error is just that __sleep() is implemented and the object is somehow cleaned of anything that's not strictly needed during __wakeup(). In D8 I also encountered some similar problems in the past when there are e.g. variables pointing to services etc. But, of course, you can remove those and add them back on wake-up because the services are readily available (just an example). So IMHO it boils down to: If you have problems serializing objects of a specific class, make sure to implement __sleep (be it via a trait or directly) in that class and use it to remove any cruft (i.e. variables) that are on your object (variables pointing to other objects in particular) to avoid serializing a huge complicated interdependent "network" of connected objects which will inadvertetly "stress-test" PHP's serialiation engine . This should also cut down on the size of the final serialization string.
Comment #117
MustangGB CreditAttribution: MustangGB commentedIf the issue is to do with double encoding and/or storing in the database, then the base64 wrapper trick might work: https://davidwalsh.name/php-serialize-unserialize-issues
Would be good to have some steps to reproduce this though, as it's not something I've seen on my own sites.
Comment #118
joseph.olstad@MustangGB, thanks for the suggestion, I'm trying to figure out an easy way to reproduce this bug.
Not entirely sure if it's related to the closure or not, there's a lot of things going on and a very complicated setup.
With that said, I did try the base64_decode and base64_encode in the closure library, it didn't seem to make any difference , even after flush cache / cache rebuild.
so for that reason, I think that the issue I'm observing is not actually related to the closure library OR views_php but rather some entity relationship of some kind.
, the site I'm debugging is using workflow (rules) , entity , editablefields (ajax form in here, I'm thinking it's more editablefields that is the culprit now that I'm getting closer, but I haven't created yet clear steps to reproduce, so it's all tentative guessing at this point).
so I'm going to look at editablefields and zero in on this.
Thanks for your assistance.
I'm thinking that this views_php patch is good, the closure library is good, with that said, I think this base 64 encoding is probably a good idea after serialize and base 64 decode before unserialize.
See the above patch code, might be worth reviewing just for kicks, see what the closure guys say upstream. However, ya probably not even needed at this point, it might however be a good safeguard.
Comment #119
MustangGB CreditAttribution: MustangGB commentedIt might produce the same result as what you posted, but I was thinking more like this:
Comment #120
joseph.olstadOk, status summary update again:
The php 7.3.x issue I mentioned above is most likely not related to views_php or the closure library, I am still investigating but now I'm looking into the editablefields module which is being used on the site that I observed the issue..
I have tested patch 100 and the opis/closure library to work with views_php and php 5.6.x , 7.0, 7.1, 7.2, 7.3.
Step 1) download the latest 7.x-1.x branch (see other issue for the 2.x branch) dev release or latest release probably works too
Step 2) apply MustangGBs patch 100 above #2274543: [7.x-1.x] Remove usage of deprecated create_function() to the views_php module
Step 3) install the latest opis closure library To test, make sure you download the library and put it somewhere like:
sites/all/libraries/closure/autoload.php
Step 4) Make sure to have prerequisite libraries module installed and enabled.
Step 5) clear caches, have fun.
Comment #121
joseph.olstad@mustanggb, I am really looking at all solutions and so I did try your most recent suggestion just now, it throws the following:
so I get :
Warning: base64_encode() expects parameter 1 to be string, object given in views_php_create_function() (line 190 of /web/sites/all/modules/contrib/views_php/views_php.module).
I'm actually thinking that the problem I'm observing with serialize/unserialize is related to editablefields which caches an ajax form and some entity data along with the views_php closure, possibly a double serialize somewhere, not sure.
Comment #122
joseph.olstad@MustangGB, and everyone else, thanks for your assistance, I ended up finding a way to work around the unserialize() / serialize() issue with php 7.3.x in my complicated ajax form which also happens to be using the editablefields module in addition to views_php.
It turns out that the editablefields module is rebuilding forms from cache, something happens which I'm not sure of somewhere, that blows up in php 7.3.x
to work around the issue, I set the form_state 'rebuild' array index value to FALSE
this totally avoids the form_cache and no longer needs to serialize / unserialize whatever it was trying to do (probably a double serialize() )
so, using the above views_php patch, the opis/closure library, and the core patch for ajax forms, plus a patch to editablefields , my php 7.3.x environment is working prestinely.
here's the patch for editablefields
Sorry for the noise, refer to comment #120 for how to use views_php with the new opis/closure library which works wonderfully with php 7.2 and php 7.3
Thanks!
Comment #123
joseph.olstadah, actually, during my test I was using a small core patch to includes/form.inc which works around a php 7.3.x
, there still may be an empty cache in php 7.2 and previous versions of php, but they do not throw an unsupported operand types like php 7.3.x does.
not sure if this is closure related or not but seems to affect only php 7.3.x
here's the patch to core:
https://www.drupal.org/files/issues/2019-11-04/php7.3_edge_case_closure_...
basically , I changed one line in includes/form.inc
So this, in addition to the editablefields patch for editablefields resolved my issue.
see the full explanation here: #3093423: php 7.3.x compatibility, unexpected operands in form_get_cache and too few arguments
Comment #124
MustangGB CreditAttribution: MustangGB commentedAh of course, yea ignore me, was just a thought, but you're right, it would have to be after the serialization.
Comment #125
joseph.olstadComment #126
joseph.olstadOk, the patch that I thought fixed the issue, just masked it again, it all came back to the opis/closure library and the views_php module.
As soon as I disable the views_php module, the editablefields module works fine in all ways.
I've openned a support request with the opis/closure team.
https://github.com/opis/closure/issues/43
Comment #127
rgpublicStill, i don't understand at all why opis/closure is used at all. It seems to me way overkill to bring in this library for no reason. It is far easier to get a serializable closure in PHP. See the following proof-of-concept:
https://gist.github.com/rgpublic/499071d10bfe067ffc271f60f75ec132
The library, as far as i can see, extracts the code of the closure from wherever it is in the source code. This is why it is so complicated. IMHO we don't need that here as the code is already readily available as a string. The original problem was only to replace the create_function() call because it's deprecated. See my example above how that is easily achievable without any huge additional library/dependency.
Comment #128
MustangGB CreditAttribution: MustangGB commentedHow can it be done without
eval()
though?Comment #129
rgpublic@MustangGB: Why should it be done without "eval"? "create_function" is deprecated in PHP 7.2 - not "eval".
Comment #130
MustangGB CreditAttribution: MustangGB commentedAs per the issue summary and PHP 7.2 release notes,
create_function()
was deprecated because it useseval()
internally, being that it has performance, memory, and security implications and the world has been migrating away fromeval()
for sometime, it doesn't seem logically to me to be introducing another instance of what we're trying to be rid of when another viable option is available.Also I wouldn't be surprised if
eval()
were to itself be deprecated at some point, for the same reasoning, it'd be nice to have this issue resolved, without needing to comeback to it again in the near future.I've not testing it, but opis/closure also notes:
You can serialize/unserialize any closure unlimited times, even those previously unserialized (this is possible because eval() is not used for unserialization)
Comment #131
rgpublicWell, eval probably doesn't go anywhere for the time being. Many things in the PHP world are relying on this. And nothing like that has even been officially announced or created as an RFC. So, the idea that eval is going away is currently your very own speculation. Yes, eval has indeed major security implications but it is a misunderstanding that they are magically solved by opis/closure. If you see the documentation of that library you will notice that it doesn't claim to do that.
What this library does (AFAICT) is: Create a temporary in-memory php file and include that. And if we speculate that eval is going away then we might as well speculate that the support for including files from in-memory php files via a stream wrapper is going away. And to be honest, I think that support for the latter is very ugly and support for that might leave us sooner than eval. What the library does is cleverly masking the fact that untrusted code is run. It's no better than "eval".
The security problem of eval is that a string/user data is turned into code. I agree, this is quite terrible for a multitude of reasons. But, then again, this is what this modules (views_php) is all about. If you want to avoid the security hole of mixing user data and code execution that you cannot use this module. It's as simple as that. This module is a very bad idea security-wise and I myself don't use it for this very reason but there's nothing to be gained whatsoever by using the opis/closure library. It just brings in LOTS of new code that might have additional security problems as well and it doesn't solve any current actual problem.
Comment #132
MustangGB CreditAttribution: MustangGB commentedFair enough, if we accept that
eval()
is the way to go, then perhaps we can look at adding some of the ideas from SuperClosure to the #127 code, for starters I'm wondering if it shouldimplement Serializable
and if__invoke()
could usefunc_get_args()
.Although SuperClosure does again mention the issue of:
Cannot serialize closures that are defined within eval()'d code. This includes re-serializing a closure that has been unserialized.
Are we sure this won't be something that we hit when caching for example?
Comment #133
rgpublic@MustangGB: I think (might be wrong) __invoke cannot use func_get_args() because from the source-code it looks we need call-by-reference. The references are destroyed with func_get_args. That's why I'm faking this with arg1...arg9.
Serializable (or __sleep/__wakeup) must be implemented if you store the closure in a variable of that object. I'm currently recreating it everytime in my GIST example. It might in fact have performance benefits if the closure is only created once and then cached. OTOH you can also just store the cached closure as a function-local static variable:
I think(!) it won't be serialized in that case and you can save yourself from implementing Serializable completely. Implementing Serializable is only necessary if the variables you want to serialize are somehow different from the variables the object has on it naturally.
For the other features in super-closure: What features do you particularly find interesting? For the most part I guess they are not needed since in our case it's *us* who are actually building the closure. The user-provided code we make our closure out of probably shouldn't/wouldn't use e.g. $this etc. because this could not possibly have worked before with create_function anyway.
Comment #134
DamienMcKennaJust to mention it, if folks are worried about whether or not to use eval() maybe they it shouldn't be possible to store PHP code in a Views configuration field?
Comment #135
DamienMcKennaIt's also worth noting that Views itself uses eval() in several files:
So using eval() is safe for now.
Comment #136
skylord CreditAttribution: skylord commentedAgree with previous posts. Tools like Closure library are certainly overkill for discussed purpose. Actually the whole views_php module is just a UI wrapper over eval() invoсation. If someone wants to avoid eval() - he has to write usual views field handler.
@rgpublic is right - the way Closure works seems like a rough hack and mentioned problems with php 7.3 and etc are proofs of that.
Comment #137
MustangGB CreditAttribution: MustangGB commentedFor the record, I'm not "worried" per se, only asking the questions as to why one approach might be preferable or not over the other.
I don't see the use of a stream wrapper based include as hacky, each implementation has pros and cons, if none of the cons of
eval()
effect us, and it's a lighter solution, then so be it.Comment #138
ruslan_drupaler CreditAttribution: ruslan_drupaler as a volunteer commented#4 Worked smoothly for me. Thanks a lot!
Comment #139
yurg CreditAttribution: yurg as a volunteer commented#2274543-120: [7.x-1.x] Remove usage of deprecated create_function() ( thanks @josepholstad ) works perfectly.
Comment #140
joseph.olstadeval() is probably the way to go. Either solution probably has similar related limitation that occurs when serializing certain objects . if a view has a form for instance like when editablefields module provides an ajax interface on a view and is used with views_php, the form is cached by core and serialized when cached and unserialized after loading from cache.
Forms like this contained in views_php result in or expose a limitation of the serialize function.
In versions of php prior to php 7.3, the serialize failures were silent, but as of 7.3 now they throw warnings.
This is pretty much what I observed. So to resolve I moved the views_php code out of the views ajax form that is created by the editablefields module.
Comment #141
DrupalYedi CreditAttribution: DrupalYedi commented@joseph.olstad #102...thank you for your support and your engagement in that thread!
For me patch #95 or#100 are both working (PHP 7.2) :-) but aren't showing the values of the fields in the view correct :-(:
Example:
Vou have a view with:
field_myPicture
field_myDescription
field_myButton
field_myPHP1 //Print something like print 'xyz';
and then you have another PHP field in the view and try to render the individual fields from that field with the placeholder normal avaiable variables:
print $row->field_myPicture; //RESULT: node ID
print $row->field_myDescription; // RESULT: node ID
print $row->field_myButton; // RESULT: node ID
print $row->myPHP1;// RESULT: EMTPY
...
only $row->nid and $row->title have the correct values.
with print_r($row) one can see, that the values in the $row Array are not correct. Any Idea?
Comment #142
eit2103 CreditAttribution: eit2103 commentedCame in here for this issue. Would there be a new release soon with this patch or do you suggest that I just use the patch.
Comment #143
eit2103 CreditAttribution: eit2103 commentedBy the way, patch #4 throws the following error.
ParseError: syntax error, unexpected end of file in _registry_check_code() (line 249 of /home/eit/public_html/sites/all/modules/views_php/plugins/views/views_php_handler_field.inc).
Comment #144
phrk CreditAttribution: phrk commentedI have tried not only patch #100, but almost all patches posted above. But I simply cannot make it work with php 7.3 and DrupaI 7.69. I keep getting this error:
I have tested all forks of Closure library mentioned above. I have views_php 7.x-1.x-dev installed. I have cleared all caches every time. I have also tested suggestions mentioned in comments of similar issue 3055655 with views_php 7.x-2.x-dev.
I have been trying to solve that for many hours with almost no success... I would be very grateful for any idea anyone may have :)
Comment #145
MustangGB CreditAttribution: MustangGB commented@phrk
Sounds like you haven't got the library installed correctly, if it helps version 3.4.1 is what I currently have.
You should have it installed such that the following file exists
sites/all/libraries/closure/autoload.php
, it's just an example to show you the correct directory structure, all the files should be included.When you visit
/admin/reports/libraries
it should also appear as installed.Comment #146
phrk CreditAttribution: phrk commentedThank you, MustangGB! I downgraded closure library from current 3.5.1 to 3.4.1 and that solved my problem.
Comment #147
joseph.olstad@phrk could you please open a followup issue on github in the opis closure project reporting the regression and the fact that you had to downgrade to 3.4.1 from 3.5.1?
The developers there should respond and they have been helpful previously for us.
Thanks
Comment #148
phrk CreditAttribution: phrk commentedSure. https://github.com/opis/closure/issues
Comment #149
AnybodyHi all, is #100 the test to review now to take things forward here?
Is there an active maintainer in this project willing to commit this after RTBC?
Comment #150
MustangGB CreditAttribution: MustangGB commented@Anybody
#100 is working fine, as mentioned in #146 there might be a bit of tweaking to get it working with newer library versions.
However I think the next thing to do would be to try @rgpublic's approach from #112-#133 to see if this works without the need of an additional library. I haven't bothered to try this because as mentioned, #100 is working fine for my purposes at the moment.
As mentioned in #89, if we go the library route, it might make more sense to do it as a new branch.
Doesn't look like there are any active maintainers at the moment :'(
Comment #151
Mitnick CreditAttribution: Mitnick commented+1 on #18. Thanks a lot for providing the patch for 7.x-1.x-dev, @cirrus3d.
Comment #152
shubhangi1995Comment #153
quak CreditAttribution: quak commentedI created this simple patch to solve problem without use Opis Closure library.
Comment #154
shubhangi1995patch #15 as well as patch #153 solved the error for me without the library.
Comment #155
shubhangi1995Comment #156
sgroundwater CreditAttribution: sgroundwater as a volunteer commentedPatch #153 worked for me. I am thankful for not needing any additional library files. Thanks for the help quak.
Comment #157
cmseasy CreditAttribution: cmseasy commentedPatch 153 is a patch from the root of drupal. Please create a patch from the module folder, https://www.drupal.org/node/707484
Comment #158
bisonbleu CreditAttribution: bisonbleu commentedRefactoring @quak's patch in #153 from the module's directory.
Comment #159
cmseasy CreditAttribution: cmseasy commentedAfter applying patch #158 to latest dev and
echo "Hello World";
in the output code construction field the 'Hello World' was not rendered. Using php 7.3. Removed the patch and rendering is back.Changed status to 'Needs work'.
Comment #160
guistoll CreditAttribution: guistoll commented#15 for 7.x-1.0-alpha3 works.
Comment #161
jfuentes CreditAttribution: jfuentes as a volunteer and commentedI've applied the patches, but i still have the issue Warning: Class __PHP_Incomplete_Class has no unserializer a DrupalDatabaseCache->prepareItem()
In my case the view has views_php & editablefields, and the error happens when the editable field is configured to be editable on click
Comment #162
siramsay CreditAttribution: siramsay as a volunteer commentedI tested #158, even though the Deprecated function message stopped, the views PHP no longer worked correctly, it changed the PHP code into sanitized values for output.
Both recipe at #120 with opis closure library installed and #15 worked.
I went with #15 with 7.x-1.0-alpha3 for ease of use.
Comment #163
chill5-0 CreditAttribution: chill5-0 commented+1 on #15 for 7.x-1.0-alpha3
Comment #164
frazac CreditAttribution: frazac commented+1 on #15 for 7.x-1.0-alpha3
Comment #165
MustangGB CreditAttribution: MustangGB commentedIf you read the issue you'll know what the problem with #15 is, namely, as mentioned in #52, that it breaks when using caches, which are used everywhere.
If you'd like to move the issue forwards the next steps are to implement rgpublic's suggestion, see #133 and previous comments.
Comment #166
puddyglumUntil there is a stable version available (using method from #133), using instructions from #120 is working for us.
Comment #167
Liam MorlandComment #168
Liam MorlandWhat is the problem with #13? That looks like the most straight-forward replacement of create_function() with anonymous functions.
Comment #169
rgpublic@Liam-Morland: See #165
Comment #170
Liam MorlandSo #13 has the same problem as #15. Which caches are the problem? In
#11#8, the anonymous functions are created right before they are used. They are not stored in object properties. I had the impression that storing closures in object properties was the problem.Comment #171
rgpublic@Liam Morland. I see. Ha! Funny. Could it really be that simple? That the solution was always right in front of us in the first place? I have do admit that I also came quite late to the party. When reading the comments again now, it seems like the solution #11 was just ignored and everyone kept going on as if #11 never happened. Anyone's really seeing any problems with #11? I don't have a decent way to test it at the moment, cause I'm not using the module. Anyone getting caching errors?
Comment #172
Liam MorlandHere is a patch most similar to #8. It uses regular anonymous functions, declared just before they are used and not stored in object properties.
This patch is also similar to #158 except that this one runs eval() on each code block instead of just returning the code. It also uses
use
to pass the code into the anonymous functions.Comment #173
rgpublic@Liam Morland - on a cursory glance, that approach makes a lot of sense to me. In hindsight, it's terrible what kind of a detour this has taken. Unreal. Due to the long discussion that ensued and the proposal to even add the dependency to a full closure serialization library, I was always under the impression, that it's unavoidable that the closure ends up in the serialized data because of some obscure reasons how caches in Drupal work.
Just to make sure: Did you test this with cached views and dev mode disabled that the view is still properly cached?
BTW: There're still some minor spelling errors in the comments: "Ecexute"
Comment #174
Liam MorlandYes, I noticed the spelling errors. I am inclined to fix those in a separate commit so that this patch only addresses this issue.
My testing of this has not been extensive. I don't know what all has to be tested to be sure there are no problems with caching, etc. This patch is informed mostly by my understanding that the modern equivalent to create_function() is the anonymous functions. The long detour suggests that I may have missed something.
Comment #175
Liam MorlandThis is #172 with a couple of small errors fixed.
I have tested it and it works for all the Views that we use, but we only use some of the features of views_php, so this cannot be considered a comprehensive test.
Comment #176
Liam MorlandComment #177
zipymonkey CreditAttribution: zipymonkey commentedI tested the patch in #175 once several of our sites and the patch works for me. These sites are using the views_php_handler_field.inc and views_php_handler_field.inc plugins. I did not test the area, filter or cache plugins.
Comment #178
JGReidy CreditAttribution: JGReidy as a volunteer commentedAfter applying patch #175, I got
Error: Call to undefined function is_boolean() in views_php_handler_field->php_click_sort() (line 179 of .../sites/all/modules/views_php/plugins/views/views_php_handler_field.inc).
Comment #179
Liam MorlandThanks. Please replace
is_boolean()
withis_bool()
.Comment #180
Liam MorlandThis is #175 but with
is_bool()
.@JGReidy: Please give it a try and post your results. Thanks.
Comment #181
spjsche CreditAttribution: spjsche commentedI am currently using Views PHP 7.x-1.0-alpha3 with PHP 7.1.32 with no issues on production.
I have updated my DEV system to PHP 7.3.16 and I am now getting the dreaded ->
"Deprecated function: Function create_function() is deprecated in views_php_handler_field->pre_render() (line 202 of modules\views_php\plugins\views\views_php_handler_field.inc)."
My query is will the associated patch mentioned be committed once testing is finalised, or will it stay as a patch.
I will implement the patch on our Dev environment to test it, and will comment accordingly.
Thanking you in anticipation...
Comment #182
kevster CreditAttribution: kevster commented#180 worked for me thank you after getting
Error: Call to undefined function views_php_create_function() in views_php_handler_sort->php_post_execute() (line 73 views_php/plugins/views/views_php_handler_sort.inc).
Comment #183
whthat CreditAttribution: whthat at College of Western Idaho commented#180 Looks great here, moving from PHP 7.0.27 to 7.2.11
Comment #184
benjamin_dk CreditAttribution: benjamin_dk commentedWith the patch in #180 applied to version = "7.x-1.0-alpha3" my filters no longer work. Even though TRUE is returned the row is not removed.
PHP 7.2.18
Comment #185
Liam MorlandCan you identify which of the changes from the patch is the problem?
Comment #186
benjamin_dk CreditAttribution: benjamin_dk commentedThe whole function creation logic is over my head. I would guess the post_execute part, as this is the one I am using.
Comment #187
BasH CreditAttribution: BasH commentedI am currently testing Views PHP 7.x-1.0-alpha3 with PHP 7.4.7, my issue before applying the patch is:
Deprecated function: Function create_function() is deprecated in views_php_handler_field->pre_render() (regel 202 van C:\..\drupal\htdocs\sites\all\modules\views_php\plugins\views\views_php_handler_field.inc).
my issue after applying the patch is:
ParseError: syntax error, unexpected '?>', expecting function (T_FUNCTION) or const (T_CONST) in _registry_check_code() (regel 219 van C:\..\drupal\htdocs\sites\all\modules\views_php\plugins\views\views_php_handler_field.inc).
Line 219 however is in the pre-render function that is commented out. Clearing the cache before and after this patch doesn't change the issue, after clearing the "views" cache the site doesn't work at all anymore (every page only showing "something went wrong, try again later" and the syntax-error in the watchdog-table)
Sorry, I had to completely remove the pre-render function instead of comment it out, the patch in #180 works for me also now.
Comment #188
c_archer CreditAttribution: c_archer as a volunteer and commentedI can confirm that the patch in #180 worked for me against version 7.x-1.0-alpha3
Comment #189
BasH CreditAttribution: BasH commentedI was having the same problem as @benjamin_dk in #186 : the php code in the filter was correct but TRUE was never (or only the first time?) returned. My solution was to add a return statement before the eval statement in the post_execute function:
after this change my filter worked again. Really don't know when to use the return statement or not in the other proposed changes, but so far in the other cases I didn't find any problems (yet), I will continue to test my views.
Comment #190
kwfinken CreditAttribution: kwfinken at Michigan State University for Michigan State University commentedComment #191
lazzyvn CreditAttribution: lazzyvn commentedi have warning in php 7.4
I debug it comes from
sites/all/modules/contrib/views_php/plugins/views/views_php_handler_field.inc
so
it must be
please add it in your patch
Comment #192
Liam Morland@#191 This should be a separate ticket.
Comment #193
BerdirAdding the missing return to the filter plugin as suggested by others.
Comment #194
joseph.olstadThanks @Berdir
we could probably put this latest patch into dev
Comment #195
John Pitcairn CreditAttribution: John Pitcairn commentedThanks. Patch also applies to 7.x-1.0-alpha3 and fixes the deprecation warning.
Comment #196
kwfinken CreditAttribution: kwfinken at Michigan State University for Michigan State University commentedIt appears that there are now 8 patches RTBC, but there has been no update of the module for almost 5 years. Is there any active maintainer of the module? Is it a abandoned? Does anyone want to claim ownership (see https://www.drupal.org/node/251466 for details)?
Patch Last update on patch
[7.x-1.x] Remove usage of deprecated create_function() 2 weeks 1 day
Function create_function() is deprecated in Drupal\views_php\Plugin\views\field\ViewsPhp->render() (line 122 of modules/contrib/views_php/src/Plugin/views/field/ViewsPhp.php 3 months 3 weeks
Global php does not render available variable in drupal 8.2 versions 4 months 3 weeks
Missing argument 1 4 months 3 weeks
Coding standard changes 1 year 4 days
Pager fix causes performance and memory regression 1 year 6 days
Update Dependencies to new Format in .info.yml 1 year 11 months
Remove depreciated methods in code base. 2 years 8 months
Latest module release: 13 November 2015
I really don't have time to go through all 8 patches and make sure they work with each other, but perhaps someone else does?
Comment #197
Liam MorlandBesides this issue, there is only one other RTBC patch for 7.x-1.x: #2628014: Pager fix causes performance and memory regression. All other RTBC patches are for 8.x-1.x.
Comment #198
AnybodyConfirming RTBC for #193. Looking forward to a new release with that fix.
Comment #199
yookoala CreditAttribution: yookoala commentedAny news on this change?
Comment #200
Liam MorlandThere is no active maintainer, so there is no one to make the commit.
Comment #201
kwfinken CreditAttribution: kwfinken as a volunteer and at Michigan State University for Michigan State University commentedIs someone willing to take ownership? If not, I will try to squeeze it in.
Comment #202
rar CreditAttribution: rar as a volunteer commentedPatch #193 worked for me with PHP-7.4.3
Comment #203
joseph.olstad193 seems like the right patch
if someone generously becomes maintainer here, please put 193 in and also would be nice to also get the RTBC patch in #2628014: Pager fix causes performance and memory regression
Comment #204
Anybody@kwfinken I guess you will have to try to squeeze this in. ;) Also see #203.
Thanks a lot in advance, I already did the same for many other abandoned projects, so I'm out this time.
Comment #205
SilviuChingaru CreditAttribution: SilviuChingaru at MagazinulCuScule.ro for MagazinulCuScule.ro commentedI've posted patch #2274543-193: [7.x-1.x] Remove usage of deprecated create_function() for 7.x-2.x branch here #3055655-19: [7.x-2.x] Remove usage of deprecated create_function().
Comment #207
Liam MorlandI created an issue fork and merge request with the patch in #193.
Comment #208
noopal CreditAttribution: noopal commentedCan someone upload the 7.x-1.x version with the patch applied?
Thanks
Comment #209
yookoala CreditAttribution: yookoala commentedIs there any way to adopt this orphan module? There are still many people using this module, and it needs to be maintained.
Comment #210
DamienMcKennaThe standard process still applies: https://www.drupal.org/node/251466
Comment #212
Liam MorlandComment #213
DamienMcKennaThanks Liam!
Comment #214
MustangGB CreditAttribution: MustangGB commentedWhy only Liam Morland and Berdir given credit?
Comment #215
Liam MorlandSorry, adding credit.
Comment #216
MustangGB CreditAttribution: MustangGB commented<3
Also thanks for taking this on.
Comment #217
noopal CreditAttribution: noopal commentedThank you for committing this.
After years I now no longer get that error in my logs.