Page MenuHomePhabricator

Require "E" be defined in variables_order so $_ENV is correctly populated
Open, NormalPublic

Description

If PHP's variables_order excludes "E", the $_ENV superglobal is not populated.

We currently manage to struggle through this in most cases, but it's a losing battle because we can't reliably build an environment correctly when executing subprocesses.

We should require installs to include "E" in variables_order, and likely require that they set it to the default value, "EGPCS", although we probably do not need that as long as it contains an "E" somewhere.

Event Timeline

One thought here is that updating variables_order is fine on the server, but I really don't want to force arc users to muck with php.ini. Some possible "solutions":

  • Re-execute php with -d variables_order=EGPCS. However: extremely gross.
  • Run export or printenv and parse the output. Also gross. What about Windows? (SET?)
    • Or, run php -d variables_order=E -r "echo json_encode($_ENV);". This is way more dumb but probably a lot less perilous.
  • Continue muddling through this, enumerating problematic variables one-by-one.

The php -d variables_order=E 'echo json_encode($_ENV);' approach seems maybe least-bad but takes 20ms on my machine, so doing it without a performance penalty will be involved.

My testing from D17296 indicates this would also resolve the original problem described in T9639

Via PHI773, filter_input_array(INPUT_ENV, FILTER_UNSAFE_RAW); does not appear to work if variables_order excludes E.

One thought here is that updating variables_order is fine on the server, but I really don't want to force arc users to muck with php.ini.

This seems to be resolved already but just wanted to add a note - I believe Windows users currently need to modify php.ini to enable the curl extension for arcanist to work. This is from our internal documentation for installing PHP for arcanist:

Browse to C:\tools\php[some version number]. Edit php.ini as listed below.

  • Note: The following modifications remove the semi-colon at the start of the line. Semi-colons are comments in .ini files and these changes are uncommenting and updating the lines.
    • Find the line ;extension=curl and remove the semicolon.
    • Locate the line ;date.timezone = and replace it with date.timezone = UTC to prevent PHP from yelling at you.

Though it's possible this is a result of using chocolatey or scoop to install PHP and not using the standard installer. T13230 is probably a new solution for this, which would likely involve shipping a PHP environment as part of Arcanist, somehow.

I think not all versions of Windows + PHP require adjusting php.ini, but some nontrivial subset do -- enough that arc itself emits some flavor of guidance on this:

    if ($is_windows) {
      $need_functions = array(
        'curl_init' => array('builtin-dll', 'php_curl.dll'),
      );
    } else {

...

      if ($what == 'builtin-dll') {
        $problems[] = sprintf(
          'The build of PHP you are running does not have the "%s" extension '.
          'enabled. Edit your php.ini file and uncomment the line which '.
          'reads "extension=%s".',
          $which,
          $which);
        continue;
      }

My hesitance to require adjustment of php.ini is partly because it's more install steps for users, but partly because every PHP program on the system shares the same config (more or less), and a fair amount of PHP software requires that you adjust php.ini in some particular way, and the more requirements we place on php.ini, the more likely we are to conflict with other software the user is already running. In theory, it's reasonable for a user to want to run Phabricator next to something else -- say, Mediawiki or whatever -- on the same server. When we can find a runtime fix rather than a configuration fix for a PHP-language level issue, we avoid anyone doing that showing up on the forums and complaining that updating broke compatibility (or that they can't find their php.ini file, or didn't restart their server properly, or missed the warning, or whatever else).

The forces entangled in T13230 are something like:

  • macOS has deprecated and is removing PHP.
    • Outside of this, it has become more difficult to build a PHP development environment on macOS in recent releases (T13232).
  • PHP is not installed by default (and fairly tricky to install) on Windows.
  • A lot of users really hate PHP "just because" (see also T11371).
  • PHP doesn't provide reasonable access to native application behavior on any system.

I've never been particularly sympathetic to this stuff (I think PHP is a pretty good language, and Phabricator is perfectly fine as a web application on mobile) but none of this is likely to improve in the future.

One significant advantage of writing arc in an interpreted language is that extending it is very easy, but arc could, in theory, be rewritten in Go and ship with a go compiler or whatever.