Homec4science

Simplify Repository remote and local command construction

Authored by epriestley <git@epriestley.com> on Nov 20 2013, 19:41.

Description

Simplify Repository remote and local command construction

Summary:
This cleans up some garbage:

  • We were specifying environmental variables with X=y git ..., but now have setEnv() on both ExecFuture and PhutilExecPassthru. Use setEnv().
  • We were specifying the working directory with (cd %s && git ...), but now have setCWD() on both ExecFuture and PhutilExecPassthru. Use setCWD().
  • We were specifying the Git credentials with ssh-agent -c (ssh-add ... && git ...). We can do this more cleanly with GIT_SSH. Use GIT_SSH.
  • Since we have to write a script for GIT_SSH anyway, use the same script for Subversion and Mercurial.

This fixes two specific issues:

  • Previously, we were not able to set -o StrictHostKeyChecking=no on Git commands, so the first time you cloned a git repo the daemons would generally prompt you to add github.com or whatever to known_hosts. Since this was non-interactive, things would mysteriously hang, in effect. With GIT_SSH, we can specify the flag, reducing the number of ways things can go wrong.
  • This adds LANG=C, which probably (?) forces the language to English for all commands. Apparently you need to install special language packs or something, so I don't know that this actually works, but at least two users with non-English languages have claimed it does (see https://github.com/facebook/arcanist/pull/114 for a similar issue in Arcanist).

At some point in the future I might want to combine the Arcanist code for command execution with the Phabricator code for command execution (they share some stuff like LANG and HGPLAIN). However, credential management is kind of messy, so I'm adopting a "wait and see" approach for now. I expect to split this at least somewhat in the future, for Drydock/Automerge if nothing else.

Also I'm not sure if we use the passthru stuff at all anymore, I may just be able to delete that. I'll check in a future diff.

Test Plan: Browsed and pulled Git, Subversion and Mercurial repositories.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7600

Details

Committed
epriestley <git@epriestley.com>Nov 20 2013, 19:41
Pushed
aubortJan 31 2017, 17:16
Parents
rPH08bdfacff39f: Make Subversion URI construction more consistent
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPHff8b48979eb7: Simplify Repository remote and local command construction (authored by epriestley <git@epriestley.com>).Nov 20 2013, 19:41