I'm just making some notes here for context:
- @mbishopim3 reported `git fetch` failing after the introduction of `ssh-connect` because it was trying to use default MySQL credentials
- //EXCEPTION: (AphrontQueryConnectionException) Attempt to connect to root@localhost failed with error #1045: Access denied for user 'root'@'localhost' (using password: NO).//
- I guessed that this is because `PHABRICATOR_ENV` is not propagating correctly, and provided P1002 as a patch.
- This fixed the issue.
Normally, `PHABRICATOR_ENV` should propagate correctly on its own and the patch shouldn't be necessary. However, the implementation of `setEnv()` in `ExecFuture` uses `$env + $_ENV` if we aren't wiping the environment, and I believe `$_ENV` does not always populate correctly (for example, we use `getenv()` in `PhabricatorEnv`). This may be a function of `variables_order` omitting `E`.
I'm moving forward by promoting P1002 toward the upstream rather than dealing with `$_ENV` because this should be the only such variable which matters and being explicit should be safe and reasonable, but an alternate fix may be:
- Require `$_ENV` populate?
- Test for it during configuration/setup?
- Advise users to include `E` in `variables_order` if not present?
This is a more complicated fix to develop and test, but if we apply it at some point in the future we could revert the P1002 fix.