Page MenuHomePhabricator

Add function_exists() guards to play nice together with illuminate/support (for working with eloquent, the laravel database layer)
Closed, WontfixPublic

Description

In a work project, we are trying to use libphutil in the same project as illuminate/database (the eloquent database driver from Laravel). The illuminate/database pulls in illuminate/support and defines the head() and last() functions as seen here, which causes the following error when loading the libphutil classes:

Fatal error: Cannot redeclare head() (previously declared in /var/www/proj/vendor/illuminate/support/helpers.php:482) in /var/www/proj/vendor/phacility/libphutil/src/utils/utils.php on line 679

This patch adds function_exists() guards to libphutil, in order for the two libraries to play nice together.

diff --git a/src/utils/utils.php b/src/utils/utils.php
index 136a354..8c9142c 100644
--- a/src/utils/utils.php
+++ b/src/utils/utils.php
@@ -674,8 +674,10 @@ function newv($class_name, array $argv) {
  * @param    array Array to retrieve the first element from.
  * @return   wild  The first value of the array.
  */
-function head(array $arr) {
-  return reset($arr);
+if (!function_exists('head')) {
+  function head(array $arr) {
+    return reset($arr);
+  }
 }

 /**
@@ -686,8 +688,10 @@ function head(array $arr) {
  * @param    array Array to retrieve the last element from.
  * @return   wild  The last value of the array.
  */
-function last(array $arr) {
-  return end($arr);
+if (!function_exists('last')) {
+  function last(array $arr) {
+    return end($arr);
+  }
 }

Event Timeline

martin.lindhe raised the priority of this task from to Needs Triage.
martin.lindhe updated the task description. (Show Details)
martin.lindhe added a subscriber: martin.lindhe.
epriestley triaged this task as Wishlist priority.Mar 6 2015, 5:02 PM
epriestley added a subscriber: epriestley.

We are very unlikely to pursue this change, specifically. Although the referenced library happens to define head() and last() in the same way, there's no guarantee that existing head() and last() functions are compatible.

We could use namespaces, but not until T7408.

We could rename these to phutil_head() and phutil_last(), but libphutil has little adoption as a general-purpose library (vs purely as a Phabricator dependency) and we're hesitant about making changes which make it a better general-purpose library but worse for Phabricator development.

For the forseeable future, the easiest way forward is probably maintaining a local patch.

We could possibly do something like:

if (function_exists('head')) {
  $ok = verify_function_behavior('head');
  if (!$ok) {
    throw new Exception("incompatible function blah blah");
  }
}

...but that feels very goofy.

epriestley claimed this task.

We haven't seen more interest in this in a long time, and I don't currently plan to make compatibility-focused changes to the stack.