(stable) Promote 2018 Week 32

resources/sql/autopatches/20180809.repo_identities.activity.php

@@ -0,0 +1,20 @@
+<?php
+
+// Advise installs to rebuild the repository identities.
+
+// If the install has no commits (or no commits that lack an
+// authorIdentityPHID), don't require a rebuild.
+$commits = id(new PhabricatorRepositoryCommit())
+  ->loadAllWhere('authorIdentityPHID IS NULL LIMIT 1');
+
+if (!$commits) {
+  return;
+}
+
+try {
+  id(new PhabricatorConfigManualActivity())
+    ->setActivityType(PhabricatorConfigManualActivity::TYPE_IDENTITIES)
+    ->save();
+} catch (AphrontDuplicateKeyQueryException $ex) {
+  // If we've already noted that this activity is required, just move on.
+}

scripts/ssh/ssh-auth.php

@@ -2,13 +2,27 @@
 <?php
 
 $root = dirname(dirname(dirname(__FILE__)));
-require_once $root.'/scripts/__init_script__.php';
+require_once $root.'/scripts/init/init-script.php';
+
+// NOTE: We are caching a datastructure rather than the flat key file because
+// the path on disk to "ssh-exec" is arbitrarily mutable at runtime. See T12397.
 
 $cache = PhabricatorCaches::getMutableCache();
-$authfile_key = PhabricatorAuthSSHKeyQuery::AUTHFILE_CACHEKEY;
-$authfile = $cache->getKey($authfile_key);
+$authstruct_key = PhabricatorAuthSSHKeyQuery::AUTHSTRUCT_CACHEKEY;
+$authstruct_raw = $cache->getKey($authstruct_key);
 
-if ($authfile === null) {
+$authstruct = null;
+
+if (strlen($authstruct_raw)) {
+  try {
+    $authstruct = phutil_json_decode($authstruct_raw);
+  } catch (Exception $ex) {
+    // Ignore any issues with the cached data; we'll just rebuild the
+    // structure below.
+  }
+}
+
+if ($authstruct === null) {
   $keys = id(new PhabricatorAuthSSHKeyQuery())
     ->setViewer(PhabricatorUser::getOmnipotentUser())
     ->withIsActive(true)
@@ -19,7 +33,7 @@ if ($authfile === null) {
     exit(1);
   }
 
-  $bin = $root.'/bin/ssh-exec';
+  $key_list = array();
   foreach ($keys as $ssh_key) {
     $key_argv = array();
     $object = $ssh_key->getObject();
@@ -42,18 +56,7 @@ if ($authfile === null) {
     $key_argv[] = '--phabricator-ssh-key';
     $key_argv[] = $ssh_key->getID();
 
-    $cmd = csprintf('%s %Ls', $bin, $key_argv);
-
-    $instance = PhabricatorEnv::getEnvConfig('cluster.instance');
-    if (strlen($instance)) {
-      $cmd = csprintf('PHABRICATOR_INSTANCE=%s %C', $instance, $cmd);
-    }
-
-    // This is additional escaping for the SSH 'command="..."' string.
-    $cmd = addcslashes($cmd, '"\\');
-
     // Strip out newlines and other nonsense from the key type and key body.
-
     $type = $ssh_key->getKeyType();
     $type = preg_replace('@[\x00-\x20]+@', '', $type);
     if (!strlen($type)) {
@@ -66,22 +69,54 @@ if ($authfile === null) {
       continue;
     }
 
-    $options = array(
-      'command="'.$cmd.'"',
-      'no-port-forwarding',
-      'no-X11-forwarding',
-      'no-agent-forwarding',
-      'no-pty',
+    $key_list[] = array(
+      'argv' => $key_argv,
+      'type' => $type,
+      'key' => $key,
     );
-    $options = implode(',', $options);
-
-    $lines[] = $options.' '.$type.' '.$key."\n";
   }
 
-  $authfile = implode('', $lines);
+  $authstruct = array(
+    'keys' => $key_list,
+  );
+
+  $authstruct_raw = phutil_json_encode($authstruct);
   $ttl = phutil_units('24 hours in seconds');
-  $cache->setKey($authfile_key, $authfile, $ttl);
+  $cache->setKey($authstruct_key, $authstruct_raw, $ttl);
 }
 
+$bin = $root.'/bin/ssh-exec';
+$instance = PhabricatorEnv::getEnvConfig('cluster.instance');
+
+$lines = array();
+foreach ($authstruct['keys'] as $key_struct) {
+  $key_argv = $key_struct['argv'];
+  $key = $key_struct['key'];
+  $type = $key_struct['type'];
+
+  $cmd = csprintf('%s %Ls', $bin, $key_argv);
+
+  if (strlen($instance)) {
+    $cmd = csprintf('PHABRICATOR_INSTANCE=%s %C', $instance, $cmd);
+  }
+
+  // This is additional escaping for the SSH 'command="..."' string.
+  $cmd = addcslashes($cmd, '"\\');
+
+  $options = array(
+    'command="'.$cmd.'"',
+    'no-port-forwarding',
+    'no-X11-forwarding',
+    'no-agent-forwarding',
+    'no-pty',
+  );
+  $options = implode(',', $options);
+
+  $lines[] = $options.' '.$type.' '.$key."\n";
+}
+
+$authfile = implode('', $lines);
+
 echo $authfile;
+
 exit(0);

src/applications/auth/query/PhabricatorAuthSSHKeyQuery.php

@@ -3,7 +3,7 @@
 final class PhabricatorAuthSSHKeyQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
-  const AUTHFILE_CACHEKEY = 'ssh.authfile';
+  const AUTHSTRUCT_CACHEKEY = 'ssh.authstruct';
 
   private $ids;
   private $phids;
@@ -13,7 +13,7 @@ final class PhabricatorAuthSSHKeyQuery
 
   public static function deleteSSHKeyCache() {
     $cache = PhabricatorCaches::getMutableCache();
-    $authfile_key = self::AUTHFILE_CACHEKEY;
+    $authfile_key = self::AUTHSTRUCT_CACHEKEY;
     $cache->deleteKey($authfile_key);
   }
 

src/applications/cache/spec/PhabricatorDataCacheSpec.php

@@ -106,8 +106,21 @@ final class PhabricatorDataCacheSpec extends PhabricatorCacheSpec {
       $cache = $info['cache_list'];
       $state = array();
       foreach ($cache as $item) {
-        $info = idx($item, 'info', '<unknown-key>');
-        $key = self::getKeyPattern($info);
+        // Some older versions of APCu report the cachekey as "key", while
+        // newer APCu and APC report it as "info". Just check both indexes
+        // for commpatibility. See T13164 for details.
+
+        $info = idx($item, 'info');
+        if ($info === null) {
+          $info = idx($item, 'key');
+        }
+
+        if ($info === null) {
+          $key = '<unknown-key>';
+        } else {
+          $key = self::getKeyPattern($info);
+        }
+
         if (empty($state[$key])) {
           $state[$key] = array(
             'max' => 0,

src/applications/conduit/management/PhabricatorConduitCallManagementWorkflow.php

@@ -21,6 +21,13 @@ final class PhabricatorConduitCallManagementWorkflow
               'File to read parameters from, or "-" to read from '.
               'stdin.'),
           ),
+          array(
+            'name' => 'as',
+            'param' => 'username',
+            'help' => pht(
+              'Execute the call as the given user. (If omitted, the call will '.
+              'be executed as an omnipotent user.)'),
+          ),
         ));
   }
 
@@ -39,6 +46,22 @@ final class PhabricatorConduitCallManagementWorkflow
         pht('Specify a file to read parameters from with "--input".'));
     }
 
+    $as = $args->getArg('as');
+    if (strlen($as)) {
+      $actor = id(new PhabricatorPeopleQuery())
+        ->setViewer($viewer)
+        ->withUsernames(array($as))
+        ->executeOne();
+      if (!$actor) {
+        throw new PhutilArgumentUsageException(
+          pht(
+            'No such user "%s" exists.',
+            $as));
+      }
+    } else {
+      $actor = $viewer;
+    }
+
     if ($input === '-') {
       fprintf(STDERR, tsprintf("%s\n", pht('Reading input from stdin...')));
       $input_json = file_get_contents('php://stdin');
@@ -49,7 +72,7 @@ final class PhabricatorConduitCallManagementWorkflow
     $params = phutil_json_decode($input_json);
 
     $result = id(new ConduitCall($method, $params))
-      ->setUser($viewer)
+      ->setUser($actor)
       ->execute();
 
     $output = array(

src/applications/config/check/PhabricatorManualActivitySetupCheck.php

@@ -13,63 +13,134 @@ final class PhabricatorManualActivitySetupCheck
     foreach ($activities as $activity) {
       $type = $activity->getActivityType();
 
-      // For now, there is only one type of manual activity. It's not clear
-      // if we're really going to have too much more of this stuff so this
-      // is a bit under-designed for now.
+      switch ($type) {
+        case PhabricatorConfigManualActivity::TYPE_REINDEX:
+          $this->raiseSearchReindexIssue();
+          break;
 
-      $activity_name = pht('Rebuild Search Index');
-      $activity_summary = pht(
-        'The search index algorithm has been updated and the index needs '.
-        'be rebuilt.');
+        case PhabricatorConfigManualActivity::TYPE_IDENTITIES:
+          $this->raiseRebuildIdentitiesIssue();
+          break;
 
-      $message = array();
-
-      $message[] = pht(
-        'The indexing algorithm for the fulltext search index has been '.
-        'updated and the index needs to be rebuilt. Until you rebuild the '.
-        'index, global search (and other fulltext search) will not '.
-        'function correctly.');
-
-      $message[] = pht(
-        'You can rebuild the search index while Phabricator is running.');
-
-      $message[] = pht(
-        'To rebuild the index, run this command:');
-
-      $message[] = phutil_tag(
-        'pre',
-        array(),
-        (string)csprintf(
-          'phabricator/ $ ./bin/search index --all --force --background'));
-
-      $message[] = pht(
-        'You can find more information about rebuilding the search '.
-        'index here: %s',
-        phutil_tag(
-          'a',
-          array(
-            'href' => 'https://phurl.io/u/reindex',
-            'target' => '_blank',
-          ),
-          'https://phurl.io/u/reindex'));
-
-      $message[] = pht(
-        'After rebuilding the index, run this command to clear this setup '.
-        'warning:');
-
-      $message[] = phutil_tag(
-        'pre',
-        array(),
-        (string)csprintf('phabricator/ $ ./bin/config done %R', $type));
-
-      $activity_message = phutil_implode_html("\n\n", $message);
-
-      $this->newIssue('manual.'.$type)
-        ->setName($activity_name)
-        ->setSummary($activity_summary)
-        ->setMessage($activity_message);
+        default:
+      }
     }
+  }
 
+  private function raiseSearchReindexIssue() {
+    $activity_name = pht('Rebuild Search Index');
+    $activity_summary = pht(
+      'The search index algorithm has been updated and the index needs '.
+      'be rebuilt.');
+
+    $message = array();
+
+    $message[] = pht(
+      'The indexing algorithm for the fulltext search index has been '.
+      'updated and the index needs to be rebuilt. Until you rebuild the '.
+      'index, global search (and other fulltext search) will not '.
+      'function correctly.');
+
+    $message[] = pht(
+      'You can rebuild the search index while Phabricator is running.');
+
+    $message[] = pht(
+      'To rebuild the index, run this command:');
+
+    $message[] = phutil_tag(
+      'pre',
+      array(),
+      (string)csprintf(
+        'phabricator/ $ ./bin/search index --all --force --background'));
+
+    $message[] = pht(
+      'You can find more information about rebuilding the search '.
+      'index here: %s',
+      phutil_tag(
+        'a',
+        array(
+          'href' => 'https://phurl.io/u/reindex',
+          'target' => '_blank',
+        ),
+        'https://phurl.io/u/reindex'));
+
+    $message[] = pht(
+      'After rebuilding the index, run this command to clear this setup '.
+      'warning:');
+
+    $message[] = phutil_tag(
+      'pre',
+      array(),
+      'phabricator/ $ ./bin/config done reindex');
+
+    $activity_message = phutil_implode_html("\n\n", $message);
+
+    $this->newIssue('manual.reindex')
+      ->setName($activity_name)
+      ->setSummary($activity_summary)
+      ->setMessage($activity_message);
+  }
+
+  private function raiseRebuildIdentitiesIssue() {
+    $activity_name = pht('Rebuild Repository Identities');
+    $activity_summary = pht(
+      'The mapping from VCS users to Phabricator users has changed '.
+      'and must be rebuilt.');
+
+    $message = array();
+
+    $message[] = pht(
+      'The way Phabricator attributes VCS activity to Phabricator users '.
+      'has changed. There is a new indirection layer between the strings '.
+      'that appear as VCS authors and committers (such as "John Developer '.
+      '<johnd@bigcorp.com>") and the Phabricator user that gets associated '.
+      'with VCS commits. This is to support situations where users '.
+      'are incorrectly associated with commits by Phabricator making bad '.
+      'guesses about the identity of the corresponding Phabricator user. '.
+      'This also helps with situations where existing repositories are '.
+      'imported without having created accounts for all the committers to '.
+      'that repository. Until you rebuild these repository identities, you '.
+      'are likely to encounter problems with future Phabricator features '.
+      'which will rely on the existence of these identities.');
+
+    $message[] = pht(
+      'You can rebuild repository identities while Phabricator is running.');
+
+    $message[] = pht(
+      'To rebuild identities, run this command:');
+
+    $message[] = phutil_tag(
+      'pre',
+      array(),
+      (string)csprintf(
+        'phabricator/ $ ./bin/repository rebuild-identities --all'));
+
+    $message[] = pht(
+      'You can find more information about this new identity mapping '.
+      'here: %s',
+      phutil_tag(
+        'a',
+        array(
+          'href' => 'https://phurl.io/u/repoIdentities',
+          'target' => '_blank',
+        ),
+        'https://phurl.io/u/repoIdentities'));
+
+    $message[] = pht(
+      'After rebuilding repository identities, run this command to clear '.
+      'this setup warning:');
+
+    $message[] = phutil_tag(
+      'pre',
+      array(),
+      'phabricator/ $ ./bin/config done identities');
+
+    $activity_message = phutil_implode_html("\n\n", $message);
+
+    $this->newIssue('manual.identities')
+      ->setName($activity_name)
+      ->setSummary($activity_summary)
+      ->setMessage($activity_message);
   }
 
 }

src/applications/config/storage/PhabricatorConfigManualActivity.php

@@ -7,6 +7,8 @@ final class PhabricatorConfigManualActivity
   protected $parameters = array();
 
   const TYPE_REINDEX = 'reindex';
+  const TYPE_IDENTITIES = 'identities';
+
 
   protected function getConfiguration() {
     return array(

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -206,7 +206,7 @@ final class PhabricatorRepositoryCommit
     return $this->assertAttached($this->authorIdentity);
   }
 
-  public function getCommiterIdentity() {
+  public function getCommitterIdentity() {
     return $this->assertAttached($this->committerIdentity);
   }
 

src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php

@@ -56,8 +56,18 @@ final class PhabricatorStorageManagementProbeWorkflow
       ->setShowHeader(false)
       ->setPadding(2)
       ->addColumn('name', array('title' => pht('Database / Table')))
-      ->addColumn('size', array('title' => pht('Size')))
-      ->addColumn('percentage', array('title' => pht('Percentage')));
+      ->addColumn(
+        'size',
+        array(
+          'title' => pht('Size'),
+          'align' => PhutilConsoleTable::ALIGN_RIGHT,
+        ))
+      ->addColumn(
+        'percentage',
+        array(
+          'title' => pht('Percentage'),
+          'align' => PhutilConsoleTable::ALIGN_RIGHT,
+        ));
 
     foreach ($totals as $db => $size) {
       list($database_size, $database_percentage) = $this->formatSize(
@@ -98,7 +108,7 @@ final class PhabricatorStorageManagementProbeWorkflow
 
   private function formatSize($n, $o) {
     return array(
-      sprintf('%8.8s MB', number_format($n / (1024 * 1024), 1)),
+      pht('%s MB', new PhutilNumber($n / (1024 * 1024), 1)),
       sprintf('%3.1f%%', 100 * ($n / $o)),
     );
   }