revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2025-01-24 05:28:18 +01:00
Code Issues Releases Wiki Activity

When running "arc land" from a detached HEAD, don't try to delete the source ref

Browse source 
Summary:
Fixes T10321. Some reasonable but less-common workflows involve running `arc land` from a detached HEAD state.

When users do this, we currently try to delete the raw hash as though it were a branch during cleanup. Instead, detect if the thing we're thinking about deleting is a branch or not, and just leave it alone if it isn't.

Test Plan:
  - Ran `git checkout <some hash>`, then `arc land --revision <some revision>`.
  - Before, everything worked but cleanup tried to `git branch -D <some hash>`.
  - After, everything worked and cleanup skipped branch deletion.

Maniphest Tasks: T10321

Differential Revision: https://secure.phabricator.com/D20786
This commit is contained in:
epriestley 2019-09-05 05:22:25 -07:00
parent d92fa96366
commit 3cdfe1fff8
1 changed files with 27 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
src/land/ArcanistGitLandEngine.php
View file

@ -473,8 +473,9 @@ final class ArcanistGitLandEngine
private function destroyLocalBranch() { private function destroyLocalBranch() {
$api = $this->getRepositoryAPI(); $api = $this->getRepositoryAPI();
$source_ref = $this->getSourceRef();
if ($this->getSourceRef() == $this->getTargetOnto()) { if ($source_ref == $this->getTargetOnto()) {
// If we landed a branch into a branch with the same name, so don't // If we landed a branch into a branch with the same name, so don't
// destroy it. This prevents us from cleaning up "master" if you're // destroy it. This prevents us from cleaning up "master" if you're
// landing master into itself. // landing master into itself.
@ -483,20 +484,32 @@ final class ArcanistGitLandEngine
// TODO: Maybe this should also recover the proper upstream? // TODO: Maybe this should also recover the proper upstream?
// See T10321. If we were not landing a branch, don't try to clean it up.
// This happens most often when landing from a detached HEAD.
$is_branch = $this->isBranch($source_ref);
if (!$is_branch) {
echo tsprintf(
"%s\n",
pht(
'(Source "%s" is not a branch, leaving working copy as-is.)',
$source_ref));
return;
}
$recovery_command = csprintf( $recovery_command = csprintf(
'git checkout -b %R %R', 'git checkout -b %R %R',
$this->getSourceRef(), $source_ref,
$this->sourceCommit); $this->sourceCommit);
echo tsprintf( echo tsprintf(
"%s\n", "%s\n",
pht('Cleaning up branch "%s"...', $this->getSourceRef())); pht('Cleaning up branch "%s"...', $source_ref));
echo tsprintf( echo tsprintf(
"%s\n", "%s\n",
pht('(Use `%s` if you want it back.)', $recovery_command)); pht('(Use `%s` if you want it back.)', $recovery_command));
$api->execxLocal('branch -D -- %s', $this->getSourceRef()); $api->execxLocal('branch -D -- %s', $source_ref);
} }
/** /**
@ -592,4 +605,14 @@ final class ArcanistGitLandEngine
'same state as before.')); 'same state as before.'));
} }
private function isBranch($ref) {
$api = $this->getRepositoryAPI();
list($err) = $api->execManualLocal(
'show-ref --verify --quiet -- %R',
'refs/heads/'.$ref);
return !$err;
}
} }