PHP7 compatibility #82
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Archon's object composition technique makes heavy use of a PHP feature allowing method calls from an incompatible context. However, this ability was deprecated in PHP 5.6 and removed from PHP 7.0 [1]. Ultimately, it seems like moving over to Traits may be a logical path forward. This patch instead attempts to allow current Archon to run on PHP 7+ with the fewest changes possible. Rather than simply attempt the method call in the wrong context, we instead create a closure in the desired class, then insert our current context into that closure using bindTo. There may, of course, be a simpler way to do this. In particular, it seems messy to need to create an object only to get the closure. I do not claim to be an expert in PHP internals, but this functions, and is decently boiled down. This new technique should be supported for PHP >= 5.4.0, but this is untested. One questionable area would be the use of the getClosure method call on a parenthesized "new" object statement. I believe it is supported syntax on at least 5.6, and would be very easy to work around in any case. That said, it may be better to retain the old technique and branch within the code to suit the running version. [1] http://php.net/manual/en/migration56.deprecated.php#migration56.deprecated.incompatible-context Signed-off-by: Dan Wells <dbw2@calvin.edu>
$OutputQueries is not static, so we need $this here. Older versions of PHP (<7.0) were more lax about this, particular the way we were mixing contexts. Signed-off-by: Dan Wells <dbw2@calvin.edu>
PHP no longer supports "<?" as an opening tag; make it read "<?php" instead. Signed-off-by: Dan Wells <dbw2@calvin.edu>
Author
|
Original pull request, for reference: LibraryHost#9 |
|
I copied this code out but was having some difficulty running it(it produces a blank page, with the following log output). The issue seems to stem from the bindTo($this) call trying to change scopes. Cannot bind method Core_QueryLog::logQuery() to object of class QueryLog in /var/www/html/archon_sc/packages/core/lib/archonobject.inc.php on line 283 Cannot bind method Core_Archon::initialize() to object of class Archon in /var/www/html/archon_sc/packages/core/lib/archonobject.inc.php on line 283 I'm not entirely sure that I'm not missing something in my configuration though. I did have to change the Server type to MySQLi: $_ARCHON->db->ServerType = 'MySQLi'; since MySQL is now dead. Is this something you've run into? Full log output: |
Author
|
Thank you for testing tpwalsh! Unforunately, you are right the technique we were using successfully on 7.0 no longer works on 7.1+. I closing this pull request in favor of changes on #90 which have better compatibility. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(This pull request was originally posted for LibraryHost/archon, but it was suggested to move the conversation over here instead.)
We are in the process of adopting Archon at Hekman Library, Calvin College. After getting it up an running on a PHP 5.6 test server, we then ran into some compatibility issues with PHP 7 on our production server.
The contents of this pullrequest got us to a working state. There may be more changes lurking in areas of the code we haven't exercised (similar to the small tag fix in the EAD export, included here), but we are now able to browse, search, and do DB imports successfully on our production server.
See the commit comments for much more detail on these changes, and why they are needed.