Page MenuHomePhabricator
Create Task
Maniphest T11137

Unable to update Git repositories with latest stable
Closed, ResolvedPublic
Actions

Description

We've upgraded to the latest stable version of Phabricator, but are now unable to pull in any changes from our Git repositories into Phabricator. The daemons.log shows the following error:

[13-Jun-2016 11:00:30 Europe/Amsterdam] [2016-06-13 11:00:30] EXCEPTION: (PhutilProxyException) Error while updating the "rMB" repository. {>} (CommandException) Command failed with error #255!
COMMAND
'/home/ubuntu/phabricator/bin/repository' update  -- 'rMB'

STDOUT
(empty)

STDERR
[2016-06-13 11:00:30] EXCEPTION: (InvalidArgumentException) Argument 1 passed to DiffusionCommandEngine::setURI() must be an instance of PhutilURI, instance of PhutilGitURI given, called in /home/ubuntu/phabricator/src/applications/repository/storage/PhabricatorRepository.php on line 511 and defined at [<phutil>/src/error/PhutilErrorHandler.php:200]
arcanist(head=stable, ref.master=e8a0ebaeffaa, ref.stable=7b0aac5c6f31), phabricator(head=stable, ref.master=55767aac0f86, ref.stable=1558175ec84c, custom=1), phutil(head=stable, ref.master=dad3ab8d7e87, ref.stable=ad458fb7df59)
  #0 PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phabricator>/src/applications/diffusion/protocol/DiffusionCommandEngine.php:52]
  #1 DiffusionCommandEngine::setURI(PhutilGitURI) called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:511]
  #2 PhabricatorRepository::newRemoteCommandEngine(array) called at [<phabricator>/src/applications/reposit... (1,252 more bytes) ... at [<phutil>/src/future/exec/ExecFuture.php:361]
[13-Jun-2016 11:00:30 Europe/Amsterdam] arcanist(head=stable, ref.master=e8a0ebaeffaa, ref.stable=7b0aac5c6f31), phabricator(head=stable, ref.master=55767aac0f86, ref.stable=1558175ec84c, custom=1), phutil(head=stable, ref.master=dad3ab8d7e87, ref.stable=ad458fb7df59)
[13-Jun-2016 11:00:30 Europe/Amsterdam]   #0 <#3> ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:393]
[13-Jun-2016 11:00:30 Europe/Amsterdam]   #1 phlog(PhutilProxyException) called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:400]
[13-Jun-2016 11:00:30 Europe/Amsterdam]   #2 PhabricatorRepositoryPullLocalDaemon::resolveUpdateFuture(PhabricatorRepository, ExecFuture, integer) called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:203]
[13-Jun-2016 11:00:30 Europe/Amsterdam]   #3 PhabricatorRepositoryPullLocalDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:184]
[13-Jun-2016 11:00:30 Europe/Amsterdam]   #4 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:127]

We've tried to restart the webserver and the daemons a few times, but that doesn't seem to help. Is it possible to revert the code to a previous version (i.e. does that work without reverting the storage upgrade)?

Revisions and Commits

rPHU libphutil
D16103rPHU0dca72380253 (stable) Don't detect long sentences which happen to have a colon in them as…
D16103rPHUa815d2d850b8 Don't detect long sentences which happen to have a colon in them as Git URIs
D16099rPHU52748950bb36 Merge PhutilGitURI into PhutilURI
D16099rPHU8bb124c37de3 Merge PhutilGitURI into PhutilURI
rP Phabricator
D16100rPb26ecb189e48 (stable) Remove all uses of PhutilGitURI in Phabricator
D16100rPbba53205de4a Remove all uses of PhutilGitURI in Phabricator

Related Objects

Event Timeline

ivo created this task.Jun 13 2016, 9:12 AM
ivo added a comment.Jun 13 2016, 9:24 AM

Reverted commit rP55a698a28a750017e787979a72dd125858997fa5 on our install and repository updates now work again.

epriestley mentioned this in T11004: git URI normalization is not reliable.Jun 13 2016, 12:32 PM
epriestley merged a task: T11004: git URI normalization is not reliable.
epriestley added subscribers: joshuaspence, alexmv, epriestley and 2 others.
epriestley added a comment.Jun 13 2016, 12:39 PM

This issue describes a narrow issue with typehinting URIs, introduced by T10227.

T11004 describes a broader issue with Git URI handling (some Git-style URIs have no standard representation).

Generally, we have two separate parser/structure classes for Git URIs (user@domain:relative_path) vs normal URIs (proto://user@domain/absolute/path). This seemed like a plausible architecture to me when I wrote it but I think we're better off trying to consider them to both be the same type of URI object, as this is a frequent source of errors in practice.

I made some effort to limit the damage caused by two forms when rewriting repository URIs by converting them to standard URIs early, but T11004 has examples which show this does not work (the path is interpreted as relative in one scheme and absolute in the other).

Instead, I'm just going to make PhutilURI a monstrosity that parses and represents both styles.

(We could use subclassing to solve typehint issues, but we really face a mixture of typehinting and API issues, and solving this effectively means that both URI types should present APIs that are as similar as possible, e.g. a GitURI should expose a getPort() method which just acts as though no port was configured in the URI, even though no port can be configured in the URI).

epriestley added a revision: D16099: Merge PhutilGitURI into PhutilURI.Jun 13 2016, 12:41 PM
epriestley added a comment.Jun 13 2016, 1:02 PM

@ivo, if you see this before I post a fix, can you show me the output of this command so I can make sure I'm reproducing exactly the same issue?

phabricator/ $ ./bin/repository pull rMB
epriestley added a revision: D16100: Remove all uses of PhutilGitURI in Phabricator.Jun 13 2016, 1:14 PM
epriestley claimed this task.Jun 13 2016, 1:35 PM
epriestley triaged this task as Normal priority.
gregoryp added a subscriber: gregoryp.Jun 13 2016, 2:07 PM
epriestley added a commit: rPHU8bb124c37de3: Merge PhutilGitURI into PhutilURI.Jun 13 2016, 2:20 PM
epriestley added a commit: rPHU52748950bb36: Merge PhutilGitURI into PhutilURI.
epriestley added a commit: rPbba53205de4a: Remove all uses of PhutilGitURI in Phabricator.
epriestley added a commit: rPb26ecb189e48: (stable) Remove all uses of PhutilGitURI in Phabricator.Jun 13 2016, 2:23 PM
epriestley added a comment.Jun 13 2016, 2:25 PM

I'm deploying this to the cluster now.

ivo added a comment.Jun 13 2016, 3:59 PM

The command output was:

[2016-06-13 17:45:26] EXCEPTION: (InvalidArgumentException) Argument 1 passed to DiffusionCommandEngine::setURI() must be an instance of PhutilURI, instance of PhutilGitURI given, called in /home/ubuntu/phabricator/src/applications/repository/storage/PhabricatorRepository.php on line 511 and defined at [<phutil>/src/error/PhutilErrorHandler.php:200]
arcanist(head=stable, ref.master=e8a0ebaeffaa, ref.stable=7b0aac5c6f31), phabricator(head=1558175ec84c7b13ba3f692b897464901bc00578, ref.master=55767aac0f86, custom=2), phutil(head=stable, ref.master=dad3ab8d7e87, ref.stable=ad458fb7df59)
  #0 PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phabricator>/src/applications/diffusion/protocol/DiffusionCommandEngine.php:52]
  #1 DiffusionCommandEngine::setURI(PhutilGitURI) called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:511]
  #2 PhabricatorRepository::newRemoteCommandEngine(array) called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:497]
  #3 PhabricatorRepository::newRemoteCommandFuture(array) called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:488]
  #4 PhabricatorRepository::getRemoteCommandFuture(string, string) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:350]
  #5 PhabricatorRepositoryPullEngine::executeGitUpdate() called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:125]
  #6 PhabricatorRepositoryPullEngine::pullRepositoryWithLock() called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:40]
  #7 PhabricatorRepositoryPullEngine::pullRepository() called at [<phabricator>/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php:43]
  #8 PhabricatorRepositoryManagementPullWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:410]
  #9 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:303]
  #10 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/repository/manage_repositories.php:22]

The latest commit on stable fixes the problem for us 👍

epriestley added a comment.Jun 13 2016, 4:00 PM

Thanks. That trace is consistent with the one I was able to reproduce, so I suspect the changes above fix this. I'll leave this open for a bit in case anyone else shows up, this fix is the kind of fix that's likely to have spooky side effects.

epriestley added a revision: D16103: Don't detect long sentences which happen to have a colon in them as Git URIs.Jun 13 2016, 4:33 PM
epriestley added a commit: rPHUa815d2d850b8: Don't detect long sentences which happen to have a colon in them as Git URIs.Jun 13 2016, 5:25 PM
epriestley added a commit: rPHUa815d2d850b8: Don't detect long sentences which happen to have a colon in them as Git URIs.
epriestley added a commit: rPHU0dca72380253: (stable) Don't detect long sentences which happen to have a colon in them as….Jun 16 2016, 5:42 PM
epriestley closed this task as Resolved.Jun 16 2016, 6:02 PM

This fix appears to be holding.

manoj.katikar added a subscriber: manoj.katikar.Apr 17 2017, 10:41 AM

When I trying to update the hosted repository from the remote GIT installed location. It is throwing following error message:
Error updating working copy: Command failed with error #255!
COMMAND
git fetch origin '+refs/*:refs/*' --prune

STDOUT
(empty)

STDERR
error: cannot open FETCH_HEAD: Permission denied

Phabricator · Built by Phacility