Remove calls to getCallsign() in bin/repository scripts

Summary: Ref T4245. Prepare these scripts for a callsign-free world. This also makes them more flexible and easier to use. The following are now valid ways to identify a repository for these scripts: ID (`3`), PHID (`PHID-REPO-wxyz`), R<ID> (`R3`), r<CALLSIGN> (`rSKYNET`), CALLSIGN (`SKYNET`). In the future, a human-readable label (`skynet`) may also become valid. Test Plan: - Ran `bin/repository reparse --all ...` with `rX`, `X`, `3`, `R3`. - Ran `bin/repository reparse --change ...` with `rXaaa`, including short versions. - Ran `bin/repository update ...` with `rX`, `X`, `3`, `R3`. - Ran `bin/repository refs ...` with various identifiers. - Ran `bin/repository pull ...` with various identifiers. - Ran `bin/repository mirror ...` with various identifiers. - Ran `bin/repository mark-imported ...` with various identifiers. - Ran `bin/repository list`. - Ran `bin/repository importing ...` with various identifiers and examined output. - Ran `bin/repository edit ...` with various identifiers. - Ran `bin/repository discover ...` with various identifiers. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4245 Differential Revision: https://secure.phabricator.com/D14924
2024-09-20 01:08:50 +02:00 · 2016-01-01 18:39:12 -08:00 · 2016-01-01 18:39:12 -08:00 · edcc3232aa
commit edcc3232aa
parent ff1bfb64dd
14 changed files with 156 additions and 90 deletions
--- a/src/applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php
@ -7,7 +7,7 @@ final class PhabricatorRepositoryManagementDiscoverWorkflow
    $this
      ->setName('discover')
      ->setExamples('**discover** [__options__] __repository__ ...')
-      ->setSynopsis(pht('Discover __repository__, named by callsign.'))
+      ->setSynopsis(pht('Discover __repository__.'))
      ->setArguments(
        array(
          array(
@ -31,14 +31,16 @@ final class PhabricatorRepositoryManagementDiscoverWorkflow

    if (!$repos) {
      throw new PhutilArgumentUsageException(
-        pht('Specify one or more repositories to discover, by callsign.'));
+        pht('Specify one or more repositories to discover.'));
    }

    $console = PhutilConsole::getConsole();
    foreach ($repos as $repo) {
      $console->writeOut(
        "%s\n",
-        pht("Discovering '%s'...", $repo->getCallsign()));
+        pht(
+          'Discovering "%s"...',
+          $repo->getDisplayName()));

      id(new PhabricatorRepositoryDiscoveryEngine())
        ->setRepository($repo)
--- a/src/applications/repository/management/PhabricatorRepositoryManagementEditWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementEditWorkflow.php
@ -9,8 +9,7 @@ final class PhabricatorRepositoryManagementEditWorkflow
      ->setExamples('**edit** --as __username__ __repository__ ...')
      ->setSynopsis(
        pht(
-          'Edit __repository__, named by callsign '.
-          '(will eventually be deprecated by Conduit).'))
+          'Edit __repository__ (will eventually be deprecated by Conduit).'))
      ->setArguments(
        array(
          array(
@ -45,7 +44,7 @@ final class PhabricatorRepositoryManagementEditWorkflow

    if (!$repos) {
      throw new PhutilArgumentUsageException(
-        pht('Specify one or more repositories to edit, by callsign.'));
+        pht('Specify one or more repositories to edit.'));
    }

    $console = PhutilConsole::getConsole();
@ -76,7 +75,11 @@ final class PhabricatorRepositoryManagementEditWorkflow
    }

    foreach ($repos as $repo) {
-      $console->writeOut("%s\n", pht("Editing '%s'...", $repo->getCallsign()));
+      $console->writeOut(
+        "%s\n",
+        pht(
+          'Editing "%s"...',
+          $repo->getDisplayName()));

      $xactions = array();

--- a/src/applications/repository/management/PhabricatorRepositoryManagementImportingWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementImportingWorkflow.php
@ -9,8 +9,7 @@ final class PhabricatorRepositoryManagementImportingWorkflow
      ->setExamples('**importing** __repository__ ...')
      ->setSynopsis(
        pht(
-          'Show commits in __repository__, named by callsign, which are '.
-          'still importing.'))
+          'Show commits in __repository__ which are still importing.'))
      ->setArguments(
        array(
          array(
@ -30,8 +29,7 @@ final class PhabricatorRepositoryManagementImportingWorkflow
    if (!$repos) {
      throw new PhutilArgumentUsageException(
        pht(
-          'Specify one or more repositories to find importing commits for, '.
-          'by callsign.'));
+          'Specify one or more repositories to find importing commits for.'));
    }

    $repos = mpull($repos, null, 'getID');
@ -54,7 +52,7 @@ final class PhabricatorRepositoryManagementImportingWorkflow
        $repo = $repos[$row['repositoryID']];
        $identifier = $row['commitIdentifier'];

-        $console->writeOut('%s', 'r'.$repo->getCallsign().$identifier);
+        $console->writeOut('%s', $repo->formatCommitName($identifier));

        if (!$args->getArg('simple')) {
          $status = $row['importStatus'];
--- a/src/applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php
@ -18,7 +18,7 @@ final class PhabricatorRepositoryManagementListWorkflow
      ->execute();
    if ($repos) {
      foreach ($repos as $repo) {
-        $console->writeOut("%s\n", $repo->getCallsign());
+        $console->writeOut("%s\n", $repo->getMonogram());
      }
    } else {
      $console->writeErr("%s\n", pht('There are no repositories.'));
--- a/src/applications/repository/management/PhabricatorRepositoryManagementMarkImportedWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementMarkImportedWorkflow.php
@ -7,7 +7,7 @@ final class PhabricatorRepositoryManagementMarkImportedWorkflow
    $this
      ->setName('mark-imported')
      ->setExamples('**mark-imported** __repository__ ...')
-      ->setSynopsis(pht('Mark __repository__, named by callsign, as imported.'))
+      ->setSynopsis(pht('Mark __repository__ as imported.'))
      ->setArguments(
        array(
          array(
@ -26,32 +26,40 @@ final class PhabricatorRepositoryManagementMarkImportedWorkflow

    if (!$repos) {
      throw new PhutilArgumentUsageException(
-        pht('Specify one or more repositories to mark imported, by callsign.'));
+        pht('Specify one or more repositories to mark imported.'));
    }

    $new_importing_value = (bool)$args->getArg('mark-not-imported');

    $console = PhutilConsole::getConsole();
    foreach ($repos as $repo) {
-      $callsign = $repo->getCallsign();
+      $name = $repo->getDisplayName();

      if ($repo->isImporting() && $new_importing_value) {
        $console->writeOut(
          "%s\n",
-          pht("Repository '%s' is already importing.", $callsign));
+          pht(
+            'Repository "%s" is already importing.',
+            $name));
      } else if (!$repo->isImporting() && !$new_importing_value) {
        $console->writeOut(
          "%s\n",
-          pht("Repository '%s' is already imported.", $callsign));
+          pht(
+            'Repository "%s" is already imported.',
+            $name));
      } else {
        if ($new_importing_value) {
          $console->writeOut(
            "%s\n",
-            pht("Marking repository '%s' as importing.", $callsign));
+            pht(
+              'Marking repository "%s" as importing.',
+              $name));
        } else {
          $console->writeOut(
            "%s\n",
-            pht("Marking repository '%s' as imported.", $callsign));
+            pht(
+              'Marking repository "%s" as imported.',
+              $name));
        }

        $repo->setDetail('importing', $new_importing_value);
--- a/src/applications/repository/management/PhabricatorRepositoryManagementMirrorWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementMirrorWorkflow.php
@ -8,7 +8,7 @@ final class PhabricatorRepositoryManagementMirrorWorkflow
      ->setName('mirror')
      ->setExamples('**mirror** [__options__] __repository__ ...')
      ->setSynopsis(
-        pht('Push __repository__, named by callsign, to mirrors.'))
+        pht('Push __repository__ to mirrors.'))
      ->setArguments(
        array(
          array(
@ -28,14 +28,16 @@ final class PhabricatorRepositoryManagementMirrorWorkflow
    if (!$repos) {
      throw new PhutilArgumentUsageException(
        pht(
-          'Specify one or more repositories to push to mirrors, by callsign.'));
+          'Specify one or more repositories to push to mirrors.'));
    }

    $console = PhutilConsole::getConsole();
    foreach ($repos as $repo) {
      $console->writeOut(
        "%s\n",
-        pht('Pushing "%s" to mirrors...', $repo->getCallsign()));
+        pht(
+          "Pushing '%s' to mirrors...",
+          $repo->getDisplayName()));

      $engine = id(new PhabricatorRepositoryMirrorEngine())
        ->setRepository($repo)
@ -43,7 +45,7 @@ final class PhabricatorRepositoryManagementMirrorWorkflow
        ->pushToMirrors();
    }

-    $console->writeOut('%s\b', pht('Done.'));
+    $console->writeOut("%s\n", pht('Done.'));

    return 0;
  }
--- a/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php
@ -7,7 +7,7 @@ final class PhabricatorRepositoryManagementPullWorkflow
    $this
      ->setName('pull')
      ->setExamples('**pull** __repository__ ...')
-      ->setSynopsis(pht('Pull __repository__, named by callsign.'))
+      ->setSynopsis(pht('Pull __repository__.'))
      ->setArguments(
        array(
          array(
@ -26,12 +26,16 @@ final class PhabricatorRepositoryManagementPullWorkflow

    if (!$repos) {
      throw new PhutilArgumentUsageException(
-        pht('Specify one or more repositories to pull, by callsign.'));
+        pht('Specify one or more repositories to pull.'));
    }

    $console = PhutilConsole::getConsole();
    foreach ($repos as $repo) {
-      $console->writeOut("%s\n", pht("Pulling '%s'...", $repo->getCallsign()));
+      $console->writeOut(
+        "%s\n",
+        pht(
+          'Pulling "%s"...',
+          $repo->getDisplayName()));

      id(new PhabricatorRepositoryPullEngine())
        ->setRepository($repo)
--- a/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php
@ -7,7 +7,7 @@ final class PhabricatorRepositoryManagementRefsWorkflow
    $this
      ->setName('refs')
      ->setExamples('**refs** [__options__] __repository__ ...')
-      ->setSynopsis(pht('Update refs in __repository__, named by callsign.'))
+      ->setSynopsis(pht('Update refs in __repository__.'))
      ->setArguments(
        array(
          array(
@ -27,15 +27,16 @@ final class PhabricatorRepositoryManagementRefsWorkflow
    if (!$repos) {
      throw new PhutilArgumentUsageException(
        pht(
-          'Specify one or more repositories to update refs for, '.
-          'by callsign.'));
+          'Specify one or more repositories to update refs for.'));
    }

    $console = PhutilConsole::getConsole();
    foreach ($repos as $repo) {
      $console->writeOut(
        "%s\n",
-        pht("Updating refs in '%s'...", $repo->getCallsign()));
+        pht(
+          'Updating refs in "%s"...',
+          $repo->getDisplayName()));

      $engine = id(new PhabricatorRepositoryRefEngine())
        ->setRepository($repo)
--- a/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php
@ -28,7 +28,7 @@ final class PhabricatorRepositoryManagementReparseWorkflow
          ),
          array(
            'name'     => 'all',
-            'param'    => 'callsign or phid',
+            'param'    => 'repository',
            'help'     => pht(
              'Reparse all commits in the specified repository. This mode '.
              'queues parsers into the task queue; you must run taskmasters '.
@ -192,16 +192,16 @@ final class PhabricatorRepositoryManagementReparseWorkflow

    $commits = array();
    if ($all_from_repo) {
-      $repository = id(new PhabricatorRepository())->loadOneWhere(
-        'callsign = %s OR phid = %s',
-        $all_from_repo,
-        $all_from_repo);
+      $repository = id(new PhabricatorRepositoryQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withIdentifiers(array($all_from_repo))
+        ->executeOne();
+
      if (!$repository) {
        throw new PhutilArgumentUsageException(
-          pht('Unknown repository %s!', $all_from_repo));
+          pht('Unknown repository "%s"!', $all_from_repo));
      }

-
      $query = id(new DiffusionCommitQuery())
        ->setViewer(PhabricatorUser::getOmnipotentUser())
        ->withRepository($repository);
@ -216,46 +216,14 @@ final class PhabricatorRepositoryManagementReparseWorkflow

      $commits = $query->execute();

-      $callsign = $repository->getCallsign();
      if (!$commits) {
        throw new PhutilArgumentUsageException(
          pht(
-            'No commits have been discovered in %s repository!',
-            $callsign));
+            'No commits have been discovered in the "%s" repository!',
+            $repository->getDisplayName()));
      }
    } else {
-      $commits = array();
-      foreach ($reparse_what as $identifier) {
-        $matches = null;
-        if (!preg_match('/r([A-Z]+)([a-z0-9]+)/', $identifier, $matches)) {
-          throw new PhutilArgumentUsageException(pht(
-            "Can't parse commit identifier: %s",
-            $identifier));
-        }
-        $callsign = $matches[1];
-        $commit_identifier = $matches[2];
-        $repository = id(new PhabricatorRepository())->loadOneWhere(
-          'callsign = %s',
-          $callsign);
-        if (!$repository) {
-          throw new PhutilArgumentUsageException(pht(
-            "No repository with callsign '%s'!",
-            $callsign));
-        }
-        $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
-          'repositoryID = %d AND commitIdentifier = %s',
-          $repository->getID(),
-          $commit_identifier);
-        if (!$commit) {
-          throw new PhutilArgumentUsageException(pht(
-            "No matching commit '%s' in repository '%s'. ".
-            "(For git and mercurial repositories, you must specify the entire ".
-            "commit hash.)",
-            $commit_identifier,
-            $callsign));
-        }
-        $commits[] = $commit;
-      }
+      $commits = $this->loadNamedCommits($reparse_what);
    }

    if ($all_from_repo && !$force_local) {
@ -273,6 +241,8 @@ final class PhabricatorRepositoryManagementReparseWorkflow

    $tasks = array();
    foreach ($commits as $commit) {
+      $repository = $commit->getRepository();
+
      if ($importing) {
        $status = $commit->getImportStatus();
        // Find the first missing import step and queue that up.
--- a/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php
@ -20,9 +20,9 @@ final class PhabricatorRepositoryManagementUpdateWorkflow
      ->setExamples('**update** [options] __repository__')
      ->setSynopsis(
        pht(
-          'Update __repository__, named by callsign. '.
-          'This performs the __pull__, __discover__, __ref__ and __mirror__ '.
-          'operations and is primarily an internal workflow.'))
+          'Update __repository__. This performs the __pull__, __discover__, '.
+          '__ref__ and __mirror__ operations and is primarily an internal '.
+          'workflow.'))
      ->setArguments(
        array(
          array(
@ -47,7 +47,7 @@ final class PhabricatorRepositoryManagementUpdateWorkflow
    $repos = $this->loadRepositories($args, 'repos');
    if (count($repos) !== 1) {
      throw new PhutilArgumentUsageException(
-        pht('Specify exactly one repository to update, by callsign.'));
+        pht('Specify exactly one repository to update.'));
    }

    $repository = head($repos);
--- a/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php
@ -4,26 +4,32 @@ abstract class PhabricatorRepositoryManagementWorkflow
  extends PhabricatorManagementWorkflow {

  protected function loadRepositories(PhutilArgumentParser $args, $param) {
-    $callsigns = $args->getArg($param);
+    $identifiers = $args->getArg($param);

-    if (!$callsigns) {
+    if (!$identifiers) {
      return null;
    }

-    $repos = id(new PhabricatorRepositoryQuery())
+    $query = id(new PhabricatorRepositoryQuery())
      ->setViewer($this->getViewer())
-      ->withCallsigns($callsigns)
-      ->execute();
+      ->withIdentifiers($identifiers);

-    $repos = mpull($repos, null, 'getCallsign');
-    foreach ($callsigns as $callsign) {
-      if (empty($repos[$callsign])) {
+    $query->execute();
+
+    $map = $query->getIdentifierMap();
+    foreach ($identifiers as $identifier) {
+      if (empty($map[$identifier])) {
        throw new PhutilArgumentUsageException(
-          pht("No repository with callsign '%s' exists!", $callsign));
+          pht(
+            'Repository "%s" does not exist!',
+            $identifier));
      }
    }

-    return $repos;
+    // Reorder repositories according to argument order.
+    $repositories = array_select_keys($map, $identifiers);
+
+    return array_values($repositories);
  }

  protected function loadCommits(PhutilArgumentParser $args, $param) {
--- a/src/applications/repository/phid/PhabricatorRepositoryRepositoryPHIDType.php
+++ b/src/applications/repository/phid/PhabricatorRepositoryRepositoryPHIDType.php
@ -49,7 +49,7 @@ final class PhabricatorRepositoryRepositoryPHIDType
  }

  public function canLoadNamedObject($name) {
-    return preg_match('/^r[A-Z]+|R[0-9]+$/', $name);
+    return preg_match('/^r[A-Z]+|R[1-9]\d*\z/', $name);
  }

  public function loadNamedObjects(
--- a/src/applications/repository/query/PhabricatorRepositoryQuery.php
+++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php
@ -15,6 +15,7 @@ final class PhabricatorRepositoryQuery
  private $numericIdentifiers;
  private $callsignIdentifiers;
  private $phidIdentifiers;
+  private $monogramIdentifiers;

  private $identifierMap;

@ -48,10 +49,16 @@ final class PhabricatorRepositoryQuery
  }

  public function withIdentifiers(array $identifiers) {
-    $ids = array(); $callsigns = array(); $phids = array();
+    $ids = array();
+    $callsigns = array();
+    $phids = array();
+    $monograms = array();
+
    foreach ($identifiers as $identifier) {
      if (ctype_digit($identifier)) {
        $ids[$identifier] = $identifier;
+      } else if (preg_match('/^(r[A-Z]+)|(R[1-9]\d*)\z/', $identifier)) {
+        $monograms[$identifier] = $identifier;
      } else {
        $repository_type = PhabricatorRepositoryRepositoryPHIDType::TYPECONST;
        if (phid_get_type($identifier) === $repository_type) {
@ -65,6 +72,8 @@ final class PhabricatorRepositoryQuery
    $this->numericIdentifiers = $ids;
    $this->callsignIdentifiers = $callsigns;
    $this->phidIdentifiers = $phids;
+    $this->monogramIdentifiers = $monograms;
+
    return $this;
  }

@ -273,6 +282,21 @@ final class PhabricatorRepositoryQuery
      }
    }

+    if ($this->monogramIdentifiers) {
+      $monogram_map = array();
+      foreach ($repositories as $repository) {
+        foreach ($repository->getAllMonograms() as $monogram) {
+          $monogram_map[$monogram] = $repository;
+        }
+      }
+
+      foreach ($this->monogramIdentifiers as $monogram) {
+        if (isset($monogram_map[$monogram])) {
+          $this->identifierMap[$monogram] = $monogram_map[$monogram];
+        }
+      }
+    }
+
    return $repositories;
  }

@ -447,7 +471,8 @@ final class PhabricatorRepositoryQuery

    if ($this->numericIdentifiers ||
      $this->callsignIdentifiers ||
-      $this->phidIdentifiers) {
+      $this->phidIdentifiers ||
+      $this->monogramIdentifiers) {
      $identifier_clause = array();

      if ($this->numericIdentifiers) {
@ -471,6 +496,33 @@ final class PhabricatorRepositoryQuery
          $this->phidIdentifiers);
      }

+      if ($this->monogramIdentifiers) {
+        $monogram_callsigns = array();
+        $monogram_ids = array();
+
+        foreach ($this->monogramIdentifiers as $identifier) {
+          if ($identifier[0] == 'r') {
+            $monogram_callsigns[] = substr($identifier, 1);
+          } else {
+            $monogram_ids[] = substr($identifier, 1);
+          }
+        }
+
+        if ($monogram_ids) {
+          $identifier_clause[] = qsprintf(
+            $conn,
+            'r.id IN (%Ld)',
+            $monogram_ids);
+        }
+
+        if ($monogram_callsigns) {
+          $identifier_clause[] = qsprintf(
+            $conn,
+            'r.callsign IN (%Ls)',
+            $monogram_callsigns);
+        }
+      }
+
      $where = array('('.implode(' OR ', $identifier_clause).')');
    }

--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@ -151,6 +151,26 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
    return 'r'.$this->getCallsign();
  }

+  public function getDisplayName() {
+    // TODO: This is intended to produce a human-readable name that is not
+    // necessarily a global, unique identifier. Eventually, it may just return
+    // a string like "skynet" instead of "rSKYNET".
+    return $this->getMonogram();
+  }
+
+  public function getAllMonograms() {
+    $monograms = array();
+
+    $monograms[] = 'R'.$this->getID();
+
+    $callsign = $this->getCallsign();
+    if (strlen($callsign)) {
+      $monograms[] = 'r'.$callsign;
+    }
+
+    return $monograms;
+  }
+
  public function getDetail($key, $default = null) {
    return idx($this->details, $key, $default);
  }