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.

Would this be reasonable to address with a new server-side setup check, similar to the checks for post_max_size and op_cache values?

With the caveat that I mostly dig holes in the dirt now and no longer remember how computers work:

For Arcanist, I think we repair variables_order since D19685 (which didn't land in that revision, but did land elsewhere in part of a larger merge). This is a bit clumsy (since we re-exec a PHP subprocess to get the values for ENV), but no one has to edit any config.

For Phabricator, I think (?) I "fixed" all the known issues by selectively passing variables, as in D16988. This is a bit clumsy too, and leaves Phabricator chasing all variables that Git/Mercurial/whatever care about, but the list is presumably finite and mostly-stable, no one has to edit anything, and we build some resistance to "Shellshock" class vulnerabilities (see T6185 previously) by not passing all environmental variables around, so that's at least a hand-wavy argument in favor of enumerating.

This could reasonably be a server-side setup check, but it would be asking users to make configuration changes for no known specific benefit. There's no technical reason not to do this and it's likely to make Phabricator work better in a subset of cases in the future when Git or Mercurial introduce GIT_NEW_WHATEVER_COMMIT_HOOK_THING, my policy was just to really try to avoid asking users to configure anything if I could possibly get away with it.

This could reasonably be a client-side setup check too ("Optionally, adjust variables_order to slightly improve performance.") but there's no great place to deliver it to users today (Arcanist has no other "tuning" sorts of warnings that I recall) and I would want it to be clearly delivered as lowest-possible-priority tuning advice.

For https://we.phorge.it/T15281, consider modifying DiffusionGitCommandEngine->newFormattedCommand() to pass explicit configuration to Git (as we do in Mercurial) rather than requiring administrators correctly configure Git via .gitconfig via $HOME.

I was ready to suggest that after further investigation was made, recalling the "recent" refactor work of the mercurial command-with-extensions changes we looked at 😄