mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-02 10:51:01 +01:00
Fix git "origin" remote in more circumstances
Summary: Fixes T4041. We currently detect when "origin" is incorrect, but can do better: - When "origin" is missing, we can add it. This happens for Git 1.7.1 -- see T4041. - When "origin" is wrong, we can fix it automatically if we control the repository. We only need to fail when origin exists, is wrong, and we aren't in charge of the repository. Test Plan: Ran `bin/repository discover X` on a repository with a good origin, no origin, a bad-but-under-control origin, and a bad-out-of-control origin. Got the right behavior in all cases. Reviewers: btrahan Reviewed By: btrahan CC: aran, champo Maniphest Tasks: T4041 Differential Revision: https://secure.phabricator.com/D7614
This commit is contained in:
parent
ff8b48979e
commit
a07f444f2a
4 changed files with 105 additions and 45 deletions
src/applications/repository
|
@ -535,19 +535,7 @@ final class PhabricatorRepositoryPullLocalDaemon
|
|||
PhabricatorRepository $repository) {
|
||||
|
||||
if (!$repository->isHosted()) {
|
||||
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'.");
|
||||
}
|
||||
|
||||
self::executeGitVerifySameOrigin(
|
||||
$matches[1],
|
||||
$repository->getRemoteURI(),
|
||||
$repository->getLocalPath());
|
||||
$this->verifyOrigin($repository);
|
||||
}
|
||||
|
||||
$refs = id(new DiffusionLowLevelGitRefQuery())
|
||||
|
@ -690,24 +678,101 @@ final class PhabricatorRepositoryPullLocalDaemon
|
|||
}
|
||||
|
||||
|
||||
/**
|
||||
* 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 verifyOrigin(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 {
|
||||
$valid = self::isSameGitOrigin($remote_uri, $expect_remote);
|
||||
$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 %s',
|
||||
$expect_remote);
|
||||
|
||||
// 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 %s',
|
||||
$expect_remote);
|
||||
} 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 git
|
||||
*/
|
||||
public static function executeGitVerifySameOrigin($remote, $expect, $where) {
|
||||
public static function isSameGitOrigin($remote, $expect) {
|
||||
$remote_path = self::getPathFromGitURI($remote);
|
||||
$expect_path = self::getPathFromGitURI($expect);
|
||||
|
||||
$remote_match = self::executeGitNormalizePath($remote_path);
|
||||
$expect_match = self::executeGitNormalizePath($expect_path);
|
||||
|
||||
if ($remote_match != $expect_match) {
|
||||
throw new Exception(
|
||||
"Working copy at '{$where}' has a mismatched origin URL. It has ".
|
||||
"origin URL '{$remote}' (with remote path '{$remote_path}'), but the ".
|
||||
"configured URL '{$expect}' (with remote path '{$expect_path}') is ".
|
||||
"expected. Refusing to proceed because this may indicate that the ".
|
||||
"working copy is actually some other repository.");
|
||||
}
|
||||
return ($remote_match == $expect_match);
|
||||
}
|
||||
|
||||
private static function getPathFromGitURI($raw_uri) {
|
||||
|
|
|
@ -231,7 +231,7 @@ final class PhabricatorRepositoryPullEngine
|
|||
}
|
||||
}
|
||||
|
||||
if ($err && $this->canDestroyWorkingCopy($path)) {
|
||||
if ($err && $repository->canDestroyWorkingCopy()) {
|
||||
phlog("Repository working copy at '{$path}' failed sanity check; ".
|
||||
"destroying and re-cloning. {$message}");
|
||||
Filesystem::remove($path);
|
||||
|
@ -256,7 +256,7 @@ final class PhabricatorRepositoryPullEngine
|
|||
$future->setCWD($path);
|
||||
list($err, $stdout, $stderr) = $future->resolve();
|
||||
|
||||
if ($err && !$retry && $this->canDestroyWorkingCopy($path)) {
|
||||
if ($err && !$retry && $repository->canDestroyWorkingCopy()) {
|
||||
$retry = true;
|
||||
// Fix remote origin url if it doesn't match our configuration
|
||||
$origin_url = $repository->execLocalCommand(
|
||||
|
@ -357,14 +357,4 @@ final class PhabricatorRepositoryPullEngine
|
|||
execx('svnadmin create -- %s', $path);
|
||||
}
|
||||
|
||||
|
||||
/* -( Internals )---------------------------------------------------------- */
|
||||
|
||||
|
||||
private function canDestroyWorkingCopy($path) {
|
||||
$default_path = PhabricatorEnv::getEnvConfig(
|
||||
'repository.default-local-path');
|
||||
return Filesystem::isDescendant($path, $default_path);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -97,19 +97,12 @@ final class PhabricatorWorkingCopyDiscoveryTestCase
|
|||
foreach ($cases as $case) {
|
||||
list($remote, $config, $expect, $message) = $case;
|
||||
|
||||
$ex = null;
|
||||
try {
|
||||
PhabricatorRepositoryPullLocalDaemon::executeGitverifySameOrigin(
|
||||
$remote,
|
||||
$config,
|
||||
'(a test case)');
|
||||
} catch (Exception $exception) {
|
||||
$ex = $exception;
|
||||
}
|
||||
|
||||
$this->assertEqual(
|
||||
$expect,
|
||||
!$ex,
|
||||
PhabricatorRepositoryPullLocalDaemon::isSameGitOrigin(
|
||||
$remote,
|
||||
$config,
|
||||
'(a test case)'),
|
||||
"Verification that '{$remote}' and '{$config}' are the same origin ".
|
||||
"had a different outcome than expected: {$message}");
|
||||
}
|
||||
|
|
|
@ -898,6 +898,18 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
|
|||
}
|
||||
}
|
||||
|
||||
public function canDestroyWorkingCopy() {
|
||||
if ($this->isHosted()) {
|
||||
// Never destroy hosted working copies.
|
||||
return false;
|
||||
}
|
||||
|
||||
$default_path = PhabricatorEnv::getEnvConfig(
|
||||
'repository.default-local-path');
|
||||
return Filesystem::isDescendant($this->getLocalPath(), $default_path);
|
||||
}
|
||||
|
||||
|
||||
public function writeStatusMessage(
|
||||
$status_type,
|
||||
$status_code,
|
||||
|
|
Loading…
Reference in a new issue