(experimental) Merge branch "master" into "experimental".

src/parser/ArcanistBundle.php

@@ -781,37 +781,46 @@ final class ArcanistBundle extends Phobject {
 
   private function emitBinaryDiffBody($data) {
     $eol = $this->getEOL('git');
+    return self::newBase85Data($data, $eol);
+  }
 
-    if (!function_exists('gzcompress')) {
-      throw new Exception(
-        pht(
-          'This patch has binary data. The PHP zlib extension is required to '.
-          'apply patches with binary data to git. Install the PHP zlib '.
-          'extension to continue.'));
+  public static function newBase85Data($data, $eol, $mode = null) {
+    // The "32bit" and "64bit" modes are used by unit tests to verify that all
+    // of the encoding pathways here work identically. In these modes, we skip
+    // compression because `gzcompress()` may not be stable and we just want
+    // to test that the output matches some expected result.
+
+    if ($mode === null) {
+      if (!function_exists('gzcompress')) {
+        throw new Exception(
+          pht(
+            'This patch has binary data. The PHP zlib extension is required '.
+            'to apply patches with binary data to git. Install the PHP zlib '.
+            'extension to continue.'));
+      }
+
+      $input = gzcompress($data);
+      $is_64bit = (PHP_INT_SIZE >= 8);
+    } else {
+      switch ($mode) {
+        case '32bit':
+          $input = $data;
+          $is_64bit = false;
+          break;
+        case '64bit':
+          $input = $data;
+          $is_64bit = true;
+          break;
+        default:
+          throw new Exception(
+            pht(
+              'Unsupported base85 encoding mode "%s".',
+              $mode));
+      }
     }
 
     // See emit_binary_diff_body() in diff.c for git's implementation.
 
-    $buf = '';
-
-    $deflated = gzcompress($data);
-    $lines = str_split($deflated, 52);
-    foreach ($lines as $line) {
-      $len = strlen($line);
-      // The first character encodes the line length.
-      if ($len <= 26) {
-        $buf .= chr($len + ord('A') - 1);
-      } else {
-        $buf .= chr($len - 26 + ord('a') - 1);
-      }
-      $buf .= self::encodeBase85($line);
-      $buf .= $eol;
-    }
-
-    return $buf;
-  }
-
-  public static function encodeBase85($data) {
     // This is implemented awkwardly in order to closely mirror git's
     // implementation in base85.c
 
@@ -839,6 +848,9 @@ final class ArcanistBundle extends Phobject {
     // (Since PHP overflows integer operations into floats, we don't need much
     // additional casting.)
 
+    // On 64 bit systems, we skip all this fanfare and just use integers. This
+    // is significantly faster.
+
     static $map = array(
       '0',
       '1',
@@ -927,27 +939,65 @@ final class ArcanistBundle extends Phobject {
       '~',
     );
 
+    $len_map = array();
+    for ($ii = 0; $ii <= 52; $ii++) {
+      if ($ii <= 26) {
+        $len_map[$ii] = chr($ii + ord('A') - 1);
+      } else {
+        $len_map[$ii] = chr($ii - 26 + ord('a') - 1);
+      }
+    }
+
     $buf = '';
 
-    $pos = 0;
-    $bytes = strlen($data);
-    while ($bytes) {
-      $accum = 0;
-      for ($count = 24; $count >= 0; $count -= 8) {
-        $val = ord($data[$pos++]);
-        $val = $val * (1 << $count);
-        $accum = $accum + $val;
-        if (--$bytes == 0) {
-          break;
+    $lines = str_split($input, 52);
+    $final = (count($lines) - 1);
+
+    foreach ($lines as $idx => $line) {
+      if ($idx === $final) {
+        $len = strlen($line);
+      } else {
+        $len = 52;
+      }
+
+      // The first character encodes the line length.
+      $buf .= $len_map[$len];
+
+      $pos = 0;
+      while ($len) {
+        $accum = 0;
+        for ($count = 24; $count >= 0; $count -= 8) {
+          $val = ord($line[$pos++]);
+          $val = $val * (1 << $count);
+          $accum = $accum + $val;
+          if (--$len == 0) {
+            break;
+          }
         }
+
+        $slice = '';
+
+        // If we're in 64bit mode, we can just use integers. Otherwise, we
+        // need to use floating point math to avoid overflows.
+
+        if ($is_64bit) {
+          for ($count = 4; $count >= 0; $count--) {
+            $val = $accum % 85;
+            $accum = $accum / 85;
+            $slice .= $map[$val];
+          }
+        } else {
+          for ($count = 4; $count >= 0; $count--) {
+            $val = (int)fmod($accum, 85.0);
+            $accum = floor($accum / 85.0);
+            $slice .= $map[$val];
+          }
+        }
+
+        $buf .= strrev($slice);
       }
-      $slice = '';
-      for ($count = 4; $count >= 0; $count--) {
-        $val = (int)fmod($accum, 85.0);
-        $accum = floor($accum / 85.0);
-        $slice .= $map[$val];
-      }
-      $buf .= strrev($slice);
+
+      $buf .= $eol;
     }
 
     return $buf;

src/parser/__tests__/ArcanistBundleTestCase.php

@@ -663,11 +663,7 @@ EODIFF;
     }
 
     $expect = Filesystem::readFile(dirname(__FILE__).'/base85/expect1.txt');
-    $expect = trim($expect);
-
-    $this->assertEqual(
-      $expect,
-      ArcanistBundle::encodeBase85($data));
+    $this->assertBase85($expect, $data, pht('Byte Sequences'));
 
     // This is just a large block of random binary data, it has no special
     // significance.
@@ -948,11 +944,27 @@ EODIFF;
       "\xe8\x1d\xa4\x18\xf3\x73\x82\xb4\x50\x59\xc2\x34\x36\x05\xeb";
 
     $expect = Filesystem::readFile(dirname(__FILE__).'/base85/expect2.txt');
-    $expect = trim($expect);
 
-    $this->assertEqual(
-      $expect,
-      ArcanistBundle::encodeBase85($data));
+    $this->assertBase85($expect, $data, pht('Random Data'));
+  }
+
+  private function assertBase85($expect, $data, $label) {
+    $modes = array(
+      '32bit',
+    );
+
+    // If this is a 64-bit machine, we can also test 64-bit mode.
+    $has_64bit = (PHP_INT_SIZE >= 8);
+    if ($has_64bit) {
+      $modes[] = '64bit';
+    }
+
+    foreach ($modes as $mode) {
+      $this->assertEqual(
+        $expect,
+        ArcanistBundle::newBase85Data($data, "\n", $mode),
+        pht('base85/%s: %s', $mode, $label));
+    }
   }
 
 }

src/parser/__tests__/base85/expect1.txt

@@ -1 +1,10 @@
-009C61O)~M2nh-c3=Iws5D^j+6crX17#SKH9337XAR!_nBqb&%C@Cr{EG;fCFflSSG&MFiI5|2yJUu=?KtV!7L`6nNNJ&adOifNtP*GA-R8>}2SXo+ITwPvYU}0ioWMyV&XlZI|Y;A6DaB*^Tbai%jczJqze0_d@fPsR8goTEOh>41ejE#<ukdcy;l$Dm3n3<ZJoSmMZprN9pq@|{(sHv)}tgWuEu(7hUw6(UkxVgH!yuH4^z`?@9#Kp$P$jQpf%+1cv(9zP<)YaD4*xB0K+}+;a;Njxq<mKk)=;`X~?CtLF@bU8V^!4`l`1$(#{Qds_|Ni~_`}+C#_xAPl^YZcV@9ypF>+0#~=jP?)<Kp4q-`?Ha+uGUK*Vfh4)6&t<&(6)v%gV{f$Hv9P!@|M9zrMY^ySll!x3;ykv$C<Uudc1EtE#D}r>3Q(qoSdppPrqZo0^%JmzI^3lai5;kB*Iui;9VehlYiOgMxv8e|~*@dwO|zcXoAjb8>NTZ*FaDYiem|XJ%z&V`5=oUtV2YTUuFIS5{S2Q&Le-PfkrtOG-&dM@B_NLqb77KR!J?J32WyH#RjiGcqwSFD@-CD=H}{CnhB%BO)OnA08bX8yXoH7Zw#16A}>+4-O3s3knGc2L=TM0|Eg6
+z009C61O)~M2nh-c3=Iws5D^j+6crX17#SKH9337XAR!_nBqb&%C@Cr{EG;fCFflSS
+zG&MFiI5|2yJUu=?KtV!7L`6nNNJ&adOifNtP*GA-R8>}2SXo+ITwPvYU}0ioWMyV&
+zXlZI|Y;A6DaB*^Tbai%jczJqze0_d@fPsR8goTEOh>41ejE#<ukdcy;l$Dm3n3<ZJ
+zoSmMZprN9pq@|{(sHv)}tgWuEu(7hUw6(UkxVgH!yuH4^z`?@9#Kp$P$jQpf%+1cv
+z(9zP<)YaD4*xB0K+}+;a;Njxq<mKk)=;`X~?CtLF@bU8V^!4`l`1$(#{Qds_|Ni~_
+z`}+C#_xAPl^YZcV@9ypF>+0#~=jP?)<Kp4q-`?Ha+uGUK*Vfh4)6&t<&(6)v%gV{f
+z$Hv9P!@|M9zrMY^ySll!x3;ykv$C<Uudc1EtE#D}r>3Q(qoSdppPrqZo0^%JmzI^3
+zlai5;kB*Iui;9VehlYiOgMxv8e|~*@dwO|zcXoAjb8>NTZ*FaDYiem|XJ%z&V`5=o
+zUtV2YTUuFIS5{S2Q&Le-PfkrtOG-&dM@B_NLqb77KR!J?J32WyH#RjiGcqwSFD@-C
+rD=H}{CnhB%BO)OnA08bX8yXoH7Zw#16A}>+4-O3s3knGc2L=TM0|Eg6

src/repository/api/ArcanistGitAPI.php

@@ -660,14 +660,18 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     $parts = explode("\0", $stdout);
     while (count($parts) > 1) {
       $entry = array_shift($parts);
-      $entry_parts = explode(' ', $entry);
+      $entry_parts = explode(' ', $entry, 2);
       if ($entry_parts[0] == '1') {
+        $entry_parts = explode(' ', $entry, 9);
         $path = $entry_parts[8];
       } else if ($entry_parts[0] == '2') {
+        $entry_parts = explode(' ', $entry, 10);
         $path = $entry_parts[9];
       } else if ($entry_parts[0] == 'u') {
+        $entry_parts = explode(' ', $entry, 11);
         $path = $entry_parts[10];
       } else if ($entry_parts[0] == '?') {
+        $entry_parts = explode(' ', $entry, 2);
         $result[$entry_parts[1]] = self::FLAG_UNTRACKED;
         continue;
       }

src/repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php

@@ -7,6 +7,7 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase {
       $this->parseState('git_basic.git.tgz');
       $this->parseState('git_submodules_dirty.git.tgz');
       $this->parseState('git_submodules_staged.git.tgz');
+      $this->parseState('git_spaces.git.tgz');
     } else {
       $this->assertSkipped(pht('Git is not installed'));
     }
@@ -156,6 +157,17 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase {
         );
         $this->assertEqual($expect_uncommitted, $api->getUncommittedStatus());
         break;
+      case 'git_spaces.git.tgz':
+        $expect_working = array(
+          'SPACES ADDED'       => $f_add,
+          'SPACES DELETED'     => $f_del,
+          'SPACES MODIFIED'    => $f_mod,
+          'SPACES UNCOMMITTED' => $f_add | $f_unc,
+          'SPACES UNSTAGED'    => $f_add | $f_mod | $f_uns | $f_unc,
+          'SPACES UNTRACKED'   => $f_unt,
+        );
+        $this->assertEqual($expect_working, $api->getWorkingCopyStatus());
+        break;
       case 'hg_basic.hg.tgz':
         $expect_uncommitted = array(
           'UNCOMMITTED' => $f_mod | $f_unc,

src/workflow/ArcanistDiffWorkflow.php

@@ -994,22 +994,6 @@ EOTEXT
       throw new Exception(pht('Repository API is not supported.'));
     }
 
-    if (count($changes) > 250) {
-      $message = pht(
-        'This diff has a very large number of changes (%s). Differential '.
-        'works best for changes which will receive detailed human review, '.
-        'and not as well for large automated changes or bulk checkins. '.
-        'See %s for information about reviewing big checkins. Continue anyway?',
-        phutil_count($changes),
-        'https://secure.phabricator.com/book/phabricator/article/'.
-          'differential_large_changes/');
-
-      if (!phutil_console_confirm($message)) {
-        throw new ArcanistUsageException(
-          pht('Aborted generation of gigantic diff.'));
-      }
-    }
-
     $limit = 1024 * 1024 * 4;
     foreach ($changes as $change) {
       $size = 0;

src/workflow/ArcanistPatchWorkflow.php

@@ -115,66 +115,59 @@ EOTEXT
   }
 
   protected function didParseArguments() {
-    $source = null;
-    $requested = 0;
-    if ($this->getArgument('revision')) {
-      $source = self::SOURCE_REVISION;
-      $requested++;
-    }
-    if ($this->getArgument('diff')) {
-      $source = self::SOURCE_DIFF;
-      $requested++;
-    }
-    if ($this->getArgument('arcbundle')) {
-      $source = self::SOURCE_BUNDLE;
-      $requested++;
-    }
-    if ($this->getArgument('patch')) {
-      $source = self::SOURCE_PATCH;
-      $requested++;
-    }
+    $arguments = array(
+      'revision' => self::SOURCE_REVISION,
+      'diff' => self::SOURCE_DIFF,
+      'arcbundle' => self::SOURCE_BUNDLE,
+      'patch' => self::SOURCE_PATCH,
+      'name' => self::SOURCE_REVISION,
+    );
 
-    $use_revision_id = null;
-    if ($this->getArgument('name')) {
-      $namev = $this->getArgument('name');
-      if (count($namev) > 1) {
-        throw new ArcanistUsageException(
-          pht('Specify at most one revision name.'));
+    $sources = array();
+    foreach ($arguments as $key => $source_type) {
+      $value = $this->getArgument($key);
+      if (!$value) {
+        continue;
       }
-      $source = self::SOURCE_REVISION;
-      $requested++;
 
-      $use_revision_id = $this->normalizeRevisionID(head($namev));
+      switch ($key) {
+        case 'revision':
+          $value = $this->normalizeRevisionID($value);
+          break;
+        case 'name':
+          if (count($value) > 1) {
+            throw new ArcanistUsageException(
+              pht('Specify at most one revision name.'));
+          }
+          $value = $this->normalizeRevisionID(head($value));
+          break;
+      }
+
+      $sources[] = array(
+        $source_type,
+        $value,
+      );
     }
 
-    if ($requested === 0) {
+    if (!$sources) {
       throw new ArcanistUsageException(
         pht(
-          "Specify one of '%s', '%s' (to select the current changes attached ".
-          "to a Differential revision), '%s' (to select a specific, ".
-          "out-of-date diff or a diff which is not attached to a revision), ".
-          "'%s' or '%s' to choose a patch source.",
-          'D12345',
-          '--revision <revision_id>',
-          '--diff <diff_id>',
-          '--arcbundle <file>',
-          '--patch <file>'));
-    } else if ($requested > 1) {
-      throw new ArcanistUsageException(
-        pht(
-          "Options '%s', '%s', '%s', '%s' and '%s' are not compatible. ".
-          "Choose exactly one patch source.",
-          'D12345',
-          '--revision',
-          '--diff',
-          '--arcbundle',
-          '--patch'));
+          'You must specify changes to apply to the working copy with '.
+          '"D12345", "--revision", "--diff", "--arcbundle", or "--patch".'));
     }
 
-    $this->source = $source;
-    $this->sourceParam = nonempty(
-      $use_revision_id,
-      $this->getArgument($source));
+    if (count($sources) > 1) {
+      throw new ArcanistUsageException(
+        pht(
+          'Options "D12345", "--revision", "--diff", "--arcbundle" and '.
+          '"--patch" are mutually exclusive. Choose exactly one patch '.
+          'source.'));
+    }
+
+    $source = head($sources);
+
+    $this->source = $source[0];
+    $this->sourceParam = $source[1];
   }
 
   public function requiresConduit() {