revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-27 09:12:41 +01:00
Code Issues Releases Wiki Activity

Diffusion - add verifyGitOrigin check to git fetch operation

Browse source 
Summary: Fixes T4946. Theoretically.

Test Plan:
iiam

also unit tests.

also

```
cd /var/repo/X
git remote remove origin # simulates origin-missing clone under 1.7.1
cd /path/to/phabricator
./bin/repository pull X
```

and observed no errors

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T4946, T5938

Differential Revision: https://secure.phabricator.com/D10855
This commit is contained in:
Bob Trahan 2014-11-14 18:02:27 -08:00
parent 00b245a566
commit d2ea0bc5f0
3 changed files with 95 additions and 93 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

93
src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
View file

@ -135,99 +135,6 @@ final class PhabricatorRepositoryDiscoveryEngine
}
/**
* Verify that the "origin" remote exists, and points at the correct URI.
*
* This catches or corrects some types of misconfiguration, and also repairs
* an issue where Git 1.7.1 does not create an "origin" for `--bare` clones.
* See T4041.
*
* @param PhabricatorRepository Repository to verify.
* @return void
*/
private function verifyGitOrigin(PhabricatorRepository $repository) {
list($remotes) = $repository->execxLocalCommand(
'remote show -n origin');
$matches = null;
if (!preg_match('/^\s*Fetch URL:\s*(.*?)\s*$/m', $remotes, $matches)) {
throw new Exception(
"Expected 'Fetch URL' in 'git remote show -n origin'.");
}
$remote_uri = $matches[1];
$expect_remote = $repository->getRemoteURI();
if ($remote_uri == 'origin') {
// If a remote does not exist, git pretends it does and prints out a
// made up remote where the URI is the same as the remote name. This is
// definitely not correct.
// Possibly, we should use `git remote --verbose` instead, which does not
// suffer from this problem (but is a little more complicated to parse).
$valid = false;
$exists = false;
} else {
$normal_type_git = PhabricatorRepositoryURINormalizer::TYPE_GIT;
$remote_normal = id(new PhabricatorRepositoryURINormalizer(
$normal_type_git,
$remote_uri))->getNormalizedPath();
$expect_normal = id(new PhabricatorRepositoryURINormalizer(
$normal_type_git,
$expect_remote))->getNormalizedPath();
$valid = ($remote_normal == $expect_normal);
$exists = true;
}
if (!$valid) {
if (!$exists) {
// If there's no "origin" remote, just create it regardless of how
// strongly we own the working copy. There is almost no conceivable
// scenario in which this could do damage.
$this->log(
pht(
'Remote "origin" does not exist. Creating "origin", with '.
'URI "%s".',
$expect_remote));
$repository->execxLocalCommand(
'remote add origin %P',
$repository->getRemoteURIEnvelope());
// NOTE: This doesn't fetch the origin (it just creates it), so we won't
// know about origin branches until the next "pull" happens. That's fine
// for our purposes, but might impact things in the future.
} else {
if ($repository->canDestroyWorkingCopy()) {
// Bad remote, but we can try to repair it.
$this->log(
pht(
'Remote "origin" exists, but is pointed at the wrong URI, "%s". '.
'Resetting origin URI to "%s.',
$remote_uri,
$expect_remote));
$repository->execxLocalCommand(
'remote set-url origin %P',
$repository->getRemoteURIEnvelope());
} else {
// Bad remote and we aren't comfortable repairing it.
$message = pht(
'Working copy at "%s" has a mismatched origin URI, "%s". '.
'The expected origin URI is "%s". Fix your configuration, or '.
'set the remote URI correctly. To avoid breaking anything, '.
'Phabricator will not automatically fix this.',
$repository->getLocalPath(),
$remote_uri,
$expect_remote);
throw new Exception($message);
}
}
}
}
/* -( Discovering Subversion Repositories )-------------------------------- */

94
src/applications/repository/engine/PhabricatorRepositoryEngine.php
View file

@ -51,6 +51,100 @@ abstract class PhabricatorRepositoryEngine {
return PhabricatorUser::getOmnipotentUser();
}
/**
* Verify that the "origin" remote exists, and points at the correct URI.
*
* This catches or corrects some types of misconfiguration, and also repairs
* an issue where Git 1.7.1 does not create an "origin" for `--bare` clones.
* See T4041.
*
* @param PhabricatorRepository Repository to verify.
* @return void
*/
protected function verifyGitOrigin(PhabricatorRepository $repository) {
list($remotes) = $repository->execxLocalCommand(
'remote show -n origin');
$matches = null;
if (!preg_match('/^\s*Fetch URL:\s*(.*?)\s*$/m', $remotes, $matches)) {
throw new Exception(
"Expected 'Fetch URL' in 'git remote show -n origin'.");
}
$remote_uri = $matches[1];
$expect_remote = $repository->getRemoteURI();
if ($remote_uri == 'origin') {
// If a remote does not exist, git pretends it does and prints out a
// made up remote where the URI is the same as the remote name. This is
// definitely not correct.
// Possibly, we should use `git remote --verbose` instead, which does not
// suffer from this problem (but is a little more complicated to parse).
$valid = false;
$exists = false;
} else {
$normal_type_git = PhabricatorRepositoryURINormalizer::TYPE_GIT;
$remote_normal = id(new PhabricatorRepositoryURINormalizer(
$normal_type_git,
$remote_uri))->getNormalizedPath();
$expect_normal = id(new PhabricatorRepositoryURINormalizer(
$normal_type_git,
$expect_remote))->getNormalizedPath();
$valid = ($remote_normal == $expect_normal);
$exists = true;
}
if (!$valid) {
if (!$exists) {
// If there's no "origin" remote, just create it regardless of how
// strongly we own the working copy. There is almost no conceivable
// scenario in which this could do damage.
$this->log(
pht(
'Remote "origin" does not exist. Creating "origin", with '.
'URI "%s".',
$expect_remote));
$repository->execxLocalCommand(
'remote add origin %P',
$repository->getRemoteURIEnvelope());
// NOTE: This doesn't fetch the origin (it just creates it), so we won't
// know about origin branches until the next "pull" happens. That's fine
// for our purposes, but might impact things in the future.
} else {
if ($repository->canDestroyWorkingCopy()) {
// Bad remote, but we can try to repair it.
$this->log(
pht(
'Remote "origin" exists, but is pointed at the wrong URI, "%s". '.
'Resetting origin URI to "%s.',
$remote_uri,
$expect_remote));
$repository->execxLocalCommand(
'remote set-url origin %P',
$repository->getRemoteURIEnvelope());
} else {
// Bad remote and we aren't comfortable repairing it.
$message = pht(
'Working copy at "%s" has a mismatched origin URI, "%s". '.
'The expected origin URI is "%s". Fix your configuration, or '.
'set the remote URI correctly. To avoid breaking anything, '.
'Phabricator will not automatically fix this.',
$repository->getLocalPath(),
$remote_uri,
$expect_remote);
throw new Exception($message);
}
}
}
}
/**
* @task internal

1
src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
View file

@ -88,6 +88,7 @@ final class PhabricatorRepositoryPullEngine
"Updating the working copy for repository '%s'.",
$callsign));
if ($is_git) {
$this->verifyGitOrigin($repository);
$this->executeGitUpdate();
} else if ($is_hg) {
$this->executeMercurialUpdate();