1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00

Rename "IMPORTED_CLOSEABLE" to "IMPORTED_PERMANENT" to clarify the meaning of the flag

Summary:
Ref T13591. This is an old flag with an old name, and there's an import bug because the outdated concept of "closable" is confusing two different behaviors.

This flag should mean only "is this commit reachable from a permanent ref?". Rename it to "IMPORTED_PERMANENT" to make that more clear.

Rename the "Unpublished" query to "Permanent" to make that more clear, as well.

Test Plan:
  - Grepped for all affected symbols.
  - Queried for all commmits, permament commits, and impermanent commits.
  - Ran repository discovery.
  - See also further changes in this change series for more extensive tests.

Maniphest Tasks: T13591

Differential Revision: https://secure.phabricator.com/D21514
This commit is contained in:
epriestley 2021-01-22 13:32:37 -08:00
parent 16a14af2bb
commit 2d0e7c37e1
6 changed files with 49 additions and 49 deletions

View file

@ -54,8 +54,8 @@ final class PhabricatorCommitSearchEngine
$query->withUnreachable($map['unreachable']);
}
if ($map['unpublished'] !== null) {
$query->withUnpublished($map['unpublished']);
if ($map['permanent'] !== null) {
$query->withPermanent($map['permanent']);
}
if ($map['ancestorsOf']) {
@ -132,15 +132,15 @@ final class PhabricatorCommitSearchEngine
'Find or exclude unreachable commits which are not ancestors of '.
'any branch, tag, or ref.')),
id(new PhabricatorSearchThreeStateField())
->setLabel(pht('Unpublished'))
->setKey('unpublished')
->setLabel(pht('Permanent'))
->setKey('permanent')
->setOptions(
pht('(Show All)'),
pht('Show Only Unpublished Commits'),
pht('Hide Unpublished Commits'))
pht('Show Only Permanent Commits'),
pht('Hide Permanent Commits'))
->setDescription(
pht(
'Find or exclude unpublished commits which are not ancestors of '.
'Find or exclude permanent commits which are ancestors of '.
'any permanent branch, tag, or ref.')),
id(new PhabricatorSearchStringListField())
->setLabel(pht('Ancestors Of'))

View file

@ -15,7 +15,7 @@ final class DiffusionCommitQuery
private $statuses;
private $packagePHIDs;
private $unreachable;
private $unpublished;
private $permanent;
private $needAuditRequests;
private $needAuditAuthority;
@ -154,8 +154,8 @@ final class DiffusionCommitQuery
return $this;
}
public function withUnpublished($unpublished) {
$this->unpublished = $unpublished;
public function withPermanent($permanent) {
$this->permanent = $permanent;
return $this;
}
@ -859,18 +859,18 @@ final class DiffusionCommitQuery
}
}
if ($this->unpublished !== null) {
if ($this->unpublished) {
$where[] = qsprintf(
$conn,
'(commit.importStatus & %d) = 0',
PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE);
} else {
if ($this->permanent !== null) {
if ($this->permanent) {
$where[] = qsprintf(
$conn,
'(commit.importStatus & %d) = %d',
PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE,
PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE);
PhabricatorRepositoryCommit::IMPORTED_PERMANENT,
PhabricatorRepositoryCommit::IMPORTED_PERMANENT);
} else {
$where[] = qsprintf(
$conn,
'(commit.importStatus & %d) = 0',
PhabricatorRepositoryCommit::IMPORTED_PERMANENT);
}
}

View file

@ -5,7 +5,7 @@ final class PhabricatorRepositoryCommitRef extends Phobject {
private $identifier;
private $epoch;
private $branch;
private $canCloseImmediately;
private $isPermanent;
private $parents = array();
public function setIdentifier($identifier) {
@ -35,13 +35,13 @@ final class PhabricatorRepositoryCommitRef extends Phobject {
return $this->branch;
}
public function setCanCloseImmediately($can_close_immediately) {
$this->canCloseImmediately = $can_close_immediately;
public function setIsPermanent($is_permanent) {
$this->isPermanent = $is_permanent;
return $this;
}
public function getCanCloseImmediately() {
return $this->canCloseImmediately;
public function getIsPermanent() {
return $this->isPermanent;
}
public function setParents(array $parents) {

View file

@ -101,7 +101,7 @@ final class PhabricatorRepositoryDiscoveryEngine
$repository,
$ref->getIdentifier(),
$ref->getEpoch(),
$ref->getCanCloseImmediately(),
$ref->getIsPermanent(),
$ref->getParents(),
$task_priority);
@ -250,7 +250,7 @@ final class PhabricatorRepositoryDiscoveryEngine
$refs[$identifier] = id(new PhabricatorRepositoryCommitRef())
->setIdentifier($identifier)
->setEpoch($epoch)
->setCanCloseImmediately(true);
->setIsPermanent(true);
if ($upper_bound === null) {
$upper_bound = $identifier;
@ -354,7 +354,7 @@ final class PhabricatorRepositoryDiscoveryEngine
$branch_refs = $this->discoverStreamAncestry(
new PhabricatorMercurialGraphStream($repository, $commit),
$commit,
$close_immediately = true);
$is_permanent = true);
$this->didDiscoverRefs($branch_refs);
@ -371,7 +371,7 @@ final class PhabricatorRepositoryDiscoveryEngine
private function discoverStreamAncestry(
PhabricatorRepositoryGraphStream $stream,
$commit,
$close_immediately) {
$is_permanent) {
$discover = array($commit);
$graph = array();
@ -424,7 +424,7 @@ final class PhabricatorRepositoryDiscoveryEngine
$refs[] = id(new PhabricatorRepositoryCommitRef())
->setIdentifier($commit)
->setEpoch($epoch)
->setCanCloseImmediately($close_immediately)
->setIsPermanent($is_permanent)
->setParents($stream->getParents($commit));
}
@ -507,8 +507,8 @@ final class PhabricatorRepositoryDiscoveryEngine
}
/**
* Sort branches so we process closeable branches first. This makes the
* whole import process a little cheaper, since we can close these commits
* Sort branches so we process permanent branches first. This makes the
* whole import process a little cheaper, since we can publish these commits
* the first time through rather than catching them in the refs step.
*
* @task internal
@ -538,7 +538,7 @@ final class PhabricatorRepositoryDiscoveryEngine
PhabricatorRepository $repository,
$commit_identifier,
$epoch,
$close_immediately,
$is_permanent,
array $parents,
$task_priority) {
@ -570,8 +570,8 @@ final class PhabricatorRepositoryDiscoveryEngine
$commit->setRepositoryID($repository->getID());
$commit->setCommitIdentifier($commit_identifier);
$commit->setEpoch($epoch);
if ($close_immediately) {
$commit->setImportStatus(PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE);
if ($is_permanent) {
$commit->setImportStatus(PhabricatorRepositoryCommit::IMPORTED_PERMANENT);
}
$data = new PhabricatorRepositoryCommitData();

View file

@ -9,7 +9,7 @@ final class PhabricatorRepositoryRefEngine
private $newPositions = array();
private $deadPositions = array();
private $closeCommits = array();
private $permanentCommits = array();
private $rebuild;
public function setRebuild($rebuild) {
@ -24,7 +24,7 @@ final class PhabricatorRepositoryRefEngine
public function updateRefs() {
$this->newPositions = array();
$this->deadPositions = array();
$this->closeCommits = array();
$this->permanentCommits = array();
$repository = $this->getRepository();
$viewer = $this->getViewer();
@ -96,8 +96,8 @@ final class PhabricatorRepositoryRefEngine
$this->updateCursors($cursor_group, $refs, $type, $all_closing_heads);
}
if ($this->closeCommits) {
$this->setCloseFlagOnCommits($this->closeCommits);
if ($this->permanentCommits) {
$this->setPermanentFlagOnCommits($this->permanentCommits);
}
$save_cursors = $this->getCursorsForUpdate($all_cursors);
@ -217,9 +217,9 @@ final class PhabricatorRepositoryRefEngine
return $this;
}
private function markCloseCommits(array $identifiers) {
private function markPermanentCommits(array $identifiers) {
foreach ($identifiers as $identifier) {
$this->closeCommits[$identifier] = $identifier;
$this->permanentCommits[$identifier] = $identifier;
}
return $this;
}
@ -377,7 +377,7 @@ final class PhabricatorRepositoryRefEngine
$identifier,
$exclude);
$this->markCloseCommits($new_identifiers);
$this->markPermanentCommits($new_identifiers);
}
}
}
@ -507,10 +507,10 @@ final class PhabricatorRepositoryRefEngine
}
/**
* Mark a list of commits as closeable, and queue workers for those commits
* Mark a list of commits as permanent, and queue workers for those commits
* which don't already have the flag.
*/
private function setCloseFlagOnCommits(array $identifiers) {
private function setPermanentFlagOnCommits(array $identifiers) {
$repository = $this->getRepository();
$commit_table = new PhabricatorRepositoryCommit();
$conn = $commit_table->establishConnection('w');
@ -552,7 +552,7 @@ final class PhabricatorRepositoryRefEngine
}
}
$closeable_flag = PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE;
$permanent_flag = PhabricatorRepositoryCommit::IMPORTED_PERMANENT;
$published_flag = PhabricatorRepositoryCommit::IMPORTED_PUBLISH;
$all_commits = ipull($all_commits, null, 'commitIdentifier');
@ -568,9 +568,9 @@ final class PhabricatorRepositoryRefEngine
}
$import_status = $row['importStatus'];
if (!($import_status & $closeable_flag)) {
// Set the "closeable" flag.
$import_status = ($import_status | $closeable_flag);
if (!($import_status & $permanent_flag)) {
// Set the "permanent" flag.
$import_status = ($import_status | $permanent_flag);
// See T13580. Clear the "published" flag, so publishing executes
// again. We may have previously performed a no-op "publish" on the

View file

@ -36,7 +36,7 @@ final class PhabricatorRepositoryCommit
const IMPORTED_PUBLISH = 8;
const IMPORTED_ALL = 11;
const IMPORTED_CLOSEABLE = 1024;
const IMPORTED_PERMANENT = 1024;
const IMPORTED_UNREACHABLE = 2048;
private $commitData = self::ATTACHABLE;
@ -467,7 +467,7 @@ final class PhabricatorRepositoryCommit
}
public function isPermanentCommit() {
return (bool)$this->isPartiallyImported(self::IMPORTED_CLOSEABLE);
return (bool)$this->isPartiallyImported(self::IMPORTED_PERMANENT);
}
public function newCommitAuthorView(PhabricatorUser $viewer) {