Page MenuHomePhabricator

repo create recovery fails if empty workspace exists
Closed, ResolvedPublic

Description

From https://secure.phabricator.com/conpherence/1336/#16787, if an imported workspace finds an empty dir where the workdir should be, it doesn't recover.

$ '/home/avive/devtools/phabricator/bin/repository' update  -- LST
[2015-11-08 15:10:25] EXCEPTION: (CommandException) Command failed with error #128!
COMMAND
git remote show -n origin

STDOUT
(empty)

STDERR
fatal: Not a git repository (or any of the parent directories): .git
 at [<phutil>/src/future/exec/ExecFuture.php:416]
arcanist(head=master, ref.master=2db40f995337, custom=1), phabricator(head=master, ref.master=a2f909f0bd0a), phutil(head=master, ref.master=610ca9eba906)
  #0 ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:346]
  #1 PhabricatorRepository::execxLocalCommand(string) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryEngine.php:66]
  #2 PhabricatorRepositoryEngine::verifyGitOrigin(PhabricatorRepository) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:92]
  #3 PhabricatorRepositoryPullEngine::pullRepository() called at [<phabricator>/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php:80]
  #4 PhabricatorRepositoryManagementUpdateWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:406]
  #5 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:301]
  #6 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/repository/manage_repositories.php:22]

This also fails for hosted repos, but in the case of imported ones we can actually recover (By deleting whatever we find there. We do that for other failed cases).

Related Objects

Event Timeline

avivey raised the priority of this task from to Needs Triage.
avivey updated the task description. (Show Details)
avivey added a project: Diffusion.
avivey added a subscriber: avivey.

I think it's correct that we're very conservative about destroying data. It would be incredibly bad to remove something important because a user set repository.default-local-path to something silly like /home/important_files/ and we decided to destroy /home/important_files/CRITICAL/ because it wasn't a repository.

If there's some way to get the daemons to clone a broken directory, we should fix that (e.g., clone to a temporary directory and do an atomic move on success), but I can't reproduce this (no matter when I interrupt git clone or what bad credentials I give it, I don't seem to ever get an empty/invalid directory).

We could maybe be more specific about the error.