Page MenuHomePhabricator

Repository mirroring fails due to new `git ls-remote` usage
Closed, ResolvedPublic

Description

Since the introduction of rP2b0ad243d179: Use "git ls-remote" to guess if "git fetch" is a no-op mirroring git repositories fails here:

[18-Mar-2017 22:27:03 UTC] [2017-03-18 22:27:03] EXCEPTION: (PhutilProxyException) Error while updating the "R10" repository. {>} (CommandException) Command failed with error #255!
COMMAND
'/var/www/phabricator.company.tld/phabricator/builds/69261d34/bin/repository' update  -- 'R10'

STDOUT
(empty)

STDERR
[2017-03-18 22:27:03] EXCEPTION: (CommandException) Command failed with error #129!
COMMAND
git ls-remote -- '********'

STDOUT
(empty)

STDERR
usage: git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]
                     [-q|--quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
 at [<phutil>/src/future/exec/ExecFuture.php:369]
arcanist(head=master, ref.master=3b6b523c2b23), phabricator(head=master, ref.master=688c120f9f33), phutil(head=master, ref.master=91ab940c3979)
  #0 ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:491]
  #1 PhabricatorRepository::execxRemoteCommand(string, PhutilOpaqueEnvelope) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:397]
  #2 PhabricatorRepositoryPullEngine::loadGitRemoteRefs(PhabricatorRepository) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:343]
  #3 PhabricatorRepos... (869 more bytes) ... at [<phutil>/src/future/exec/ExecFuture.php:369]
[18-Mar-2017 22:27:03 UTC] arcanist(head=master, ref.master=3b6b523c2b23), phabricator(head=master, ref.master=688c120f9f33), phutil(head=master, ref.master=91ab940c3979)
[18-Mar-2017 22:27:03 UTC]   #0 <#3> ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:442]
[18-Mar-2017 22:27:03 UTC]   #1 phlog(PhutilProxyException) called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:449]
[18-Mar-2017 22:27:03 UTC]   #2 PhabricatorRepositoryPullLocalDaemon::resolveUpdateFuture(PhabricatorRepository, ExecFuture, integer) called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:221]
[18-Mar-2017 22:27:03 UTC]   #3 PhabricatorRepositoryPullLocalDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:219]
[18-Mar-2017 22:27:03 UTC]   #4 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:131]

Version Information

Event Timeline

I can't immediately reproduce this, although the systems I have immediate access to are all running Git 2.11.0.

Does this command work on your system?

$ git ls-remote -- https://github.com/phacility/libphutil.git

I can't immediately reproduce this, although the systems I have immediate access to are all running Git 2.11.0.
Does this command work on your system?

$ git ls-remote -- https://github.com/phacility/libphutil.git

No, the result is:

phabricator@prod-phab-infra-4608e666:~$ git ls-remote -- https://github.com/phacility/libphutil.git
usage: git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]
                     [-q|--quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]

Does it work without the --?

$ git ls-remote https://github.com/phacility/libphutil.git

Just tried that too, and strangely: it does.

I wonder why that is as I also often use this approach to clearly separate args from values for shell commands.

I think this commit in the Git upstream is probably the culprit (January 2016):

https://github.com/git/git/commit/ba5f28bf79ea652fbe2be12f5ca6b351bb6ad591

I'm not terribly familiar with the git internals but it looks like that replaced hand-rolled flag parsing with standard flag parsing, and presumably added support for --.

I'll remove the -- from our command. The risk is that users could possibly configure a remote URI like --destroy-everything-everywere and cause us to run git ls-remote --destory-everything-everywere, but none of the flags available to git ls-remote appear to be dangerous and there are other checks on the URI values.

This should now be fixed in HEAD of master. I've also cherry-picked it to stable. Thanks for the report!