Fixes to make arcanist compatible with php8.1 on Linux

[hiraditya]

Summary

php8.1

• introduced strict type checking and error reporting
• removed deprecated APIs

Also has changes suggested in:
https://discourse.llvm.org/t/arcanist-deprecation-errors-on-php-8/63231

PR: https://github.com/llvm/llvm-project/issues/57271

Testing:

sh
cd /usr/share/arcanist
patch -p1 < /path/to/llvm-project/llvm/utils/phabricator/0001-Fix-PHP-8.1-incompatibility-with-arc-patch-D-12345.patch
patch -p1 < https://reviews.llvm.org/file/data/ck5dddpgtnurs5jjivr2/PHID-FILE-krs7drhq43t4p7jciltf/D131699.diff
patch -p1 < https://reviews.llvm.org/file/data/2g2fs6frlcha5cglbdns/PHID-FILE-laajtlfzcpnharrrp7un/D132523.diff

arc diff

[hiraditya]

Adding more fixes.

[hiraditya]

Added all the changes

[hiraditya]

Make it git compatible.

[nickdesaulniers]

Does the description in phab need updating? It suggests multiple invocations of patch?

[hiraditya]

In D132523#3749655, @nickdesaulniers wrote:

Does the description in phab need updating? It suggests multiple invocations of patch?

Yes. The instructions were for anyone to try this patch while in review.

I guess once https://reviews.llvm.org/D131699 is merged, i'll update the description.

[pmatos]

Thanks for this. I needed this patch for arc to work again.

[nickdesaulniers]

The resulting patch fails to apply to arcanist for me. STR:

$ cd llvm-project
$ wget https://reviews.llvm.org/file/data/nrt6tovpa573cdwctpyc/PHID-FILE-kgufiuwoyczdbfqdezq5/D132523.diff
$ git apply D132523.diff
$ cd arcanist
$ git am /android0/llvm-project/llvm/utils/phabricator/*.patch
Applying: Fix PHP 8.1 incompatibility with `arc patch D<12345>`
Applying: Fixes to make arcanist compatible with php8.1
error: patch failed: src/error/PhutilErrorHandler.php:180
error: src/error/PhutilErrorHandler.php: patch does not apply
error: patch failed: src/lint/engine/ArcanistLintEngine.php:274
error: src/lint/engine/ArcanistLintEngine.php: patch does not apply
error: patch failed: src/lint/linter/ArcanistLinter.php:291
error: src/lint/linter/ArcanistLinter.php: patch does not apply
error: patch failed: src/moduleutils/PhutilLibraryMapBuilder.php:122
error: src/moduleutils/PhutilLibraryMapBuilder.php: patch does not apply
error: patch failed: src/object/Phobject.php:33
error: src/object/Phobject.php: patch does not apply
error: patch failed: src/parser/PhutilTypeSpec.php:76
error: src/parser/PhutilTypeSpec.php: patch does not apply
error: patch failed: src/unit/engine/phutil/PhutilTestCase.php:408
error: src/unit/engine/phutil/PhutilTestCase.php: patch does not apply
error: patch failed: src/utils/PhutilArray.php:29
error: src/utils/PhutilArray.php: patch does not apply
error: patch failed: src/utils/utils.php:2003
error: src/utils/utils.php: patch does not apply
error: patch failed: src/workflow/ArcanistWorkflow.php:728
error: src/workflow/ArcanistWorkflow.php: patch does not apply
Patch failed at 0002 Fixes to make arcanist compatible with php8.1
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

This patch needs to be able to be applied on top of llvm/utils/phabricator/0001-Fix-PHP-8.1-incompatibility-with-arc-patch-D-12345.patch.

[hiraditya]

not needed anymore.

llvm/utils/phabricator/0002-Fixes-to-make-arcanist-compatible-with-php8.1.patch

+--- a/src/error/PhutilErrorHandler.php  2020-09-25 00:59:46.000000000 -0700
++++ b/src/error/PhutilErrorHandler.php  2022-08-23 18:56:21.058339739 -0700
+@@ -173,21 +173,21 @@
+    * into @{class:RuntimeException}s.
+    *
+    * @param int Error code.
+    * @param string Error message.
+    * @param string File where the error occurred.
+    * @param int Line on which the error occurred.
+    * @param wild Error context information.
+    * @return void
+    * @task internal
+    */
+-  public static function handleError($num, $str, $file, $line, $ctx) {
++  public static function handleError($num, $str, $file, $line, $ctx = 'E_NOCONTEXT') {
+ 
+     foreach (self::$traps as $trap) {
+       $trap->addError($num, $str, $file, $line, $ctx);
+     }
+ 
+     if ((error_reporting() & $num) == 0) {
+       // Respect the use of "@" to silence warnings: if this error was
+       // emitted from a context where "@" was in effect, the
+       // value returned by error_reporting() will be 0. This is the
+       // recommended way to check for this, see set_error_handler() docs
+--- a/src/lint/engine/ArcanistLintEngine.php  2020-09-25 00:59:46.000000000 -0700
++++ b/src/lint/engine/ArcanistLintEngine.php  2022-08-23 19:16:59.278408091 -0700
+@@ -267,21 +267,21 @@
+     }
+ 
+     return $this->results;
+   }
+ 
+   final public function isSeverityEnabled($severity) {
+     $minimum = $this->minimumSeverity;
+     return ArcanistLintSeverity::isAtLeastAsSevere($severity, $minimum);
+   }
+ 
+-  final private function shouldUseCache(
++  /*final*/ private function shouldUseCache(
+     $cache_granularity,
+     $repository_version) {
+ 
+     switch ($cache_granularity) {
+       case ArcanistLinter::GRANULARITY_FILE:
+         return true;
+       case ArcanistLinter::GRANULARITY_DIRECTORY:
+       case ArcanistLinter::GRANULARITY_REPOSITORY:
+         return ($this->repositoryVersion == $repository_version);
+       default:
+@@ -306,21 +306,21 @@
+     return $this->stopped;
+   }
+ 
+   abstract public function buildLinters();
+ 
+   final public function setRepositoryVersion($version) {
+     $this->repositoryVersion = $version;
+     return $this;
+   }
+ 
+-  final private function isRelevantMessage(ArcanistLintMessage $message) {
++  /*final*/ private function isRelevantMessage(ArcanistLintMessage $message) {
+     // When a user runs "arc lint", we default to raising only warnings on
+     // lines they have changed (errors are still raised anywhere in the
+     // file). The list of $changed lines may be null, to indicate that the
+     // path is a directory or a binary file so we should not exclude
+     // warnings.
+ 
+     if (!$this->changedLines ||
+         $message->isError() ||
+         $message->shouldBypassChangedLineFiltering()) {
+       return true;
+--- a/src/lint/linter/ArcanistLinter.php  2020-09-25 00:59:46.000000000 -0700
++++ b/src/lint/linter/ArcanistLinter.php  2022-08-23 19:17:46.135912295 -0700
+@@ -284,21 +284,21 @@
+     return $this;
+   }
+ 
+   /**
+    * Filter out paths which this linter doesn't act on (for example, because
+    * they are binaries and the linter doesn't apply to binaries).
+    *
+    * @param  list<string>
+    * @return list<string>
+    */
+-  final private function filterPaths(array $paths) {
++  /*final*/ private function filterPaths(array $paths) {
+     $engine = $this->getEngine();
+ 
+     $keep = array();
+     foreach ($paths as $path) {
+       if (!$this->shouldLintDeletedFiles() && !$engine->pathExists($path)) {
+         continue;
+       }
+ 
+       if (!$this->shouldLintDirectories() && $engine->isDirectory($path)) {
+         continue;
+--- a/src/moduleutils/PhutilLibraryMapBuilder.php 2020-09-25 00:59:46.000000000 -0700
++++ b/src/moduleutils/PhutilLibraryMapBuilder.php 2022-08-23 19:06:18.972594751 -0700
+@@ -115,21 +115,21 @@
+   /**
+    * Write a status message to the user, if not running in quiet mode.
+    *
+    * @param  string  Message to write.
+    * @return this
+    *
+    * @task map
+    */
+   private function log($message) {
+     if (!$this->quiet) {
+-      @fwrite(STDERR, "%s\n", $message);
++      @fwrite(STDERR, sprintf("%s\n", $message));
+     }
+     return $this;
+   }
+ 
+ 
+ /* -(  Path Management  )---------------------------------------------------- */
+ 
+   /**
+    * Get the path to some file in the library.
+    *
+--- a/src/object/Phobject.php 2020-09-25 00:59:46.000000000 -0700
++++ b/src/object/Phobject.php 2022-08-23 18:56:21.170342346 -0700
+@@ -26,36 +26,41 @@
+         get_class($this).'::'.$name));
+     }
+ 
+   public function __set($name, $value) {
+     throw new DomainException(
+       pht(
+         'Attempt to write to undeclared property %s.',
+         get_class($this).'::'.$name));
+   }
+ 
++  #[\ReturnTypeWillChange]
+   public function current() {
+     $this->throwOnAttemptedIteration();
+   }
+ 
++  #[\ReturnTypeWillChange]
+   public function key() {
+     $this->throwOnAttemptedIteration();
+   }
+ 
++  #[\ReturnTypeWillChange]
+   public function next() {
+     $this->throwOnAttemptedIteration();
+   }
+ 
++  #[\ReturnTypeWillChange]
+   public function rewind() {
+     $this->throwOnAttemptedIteration();
+   }
+ 
++  #[\ReturnTypeWillChange]
+   public function valid() {
+     $this->throwOnAttemptedIteration();
+   }
+ 
+   private function throwOnAttemptedIteration() {
+     throw new DomainException(
+       pht(
+         'Attempting to iterate an object (of class %s) which is not iterable.',
+         get_class($this)));
+   }
+--- a/src/parser/PhutilTypeSpec.php 2020-09-25 00:59:46.000000000 -0700
++++ b/src/parser/PhutilTypeSpec.php 2022-08-23 19:24:59.461543991 -0700
+@@ -69,22 +69,25 @@
+           throw new PhutilTypeCheckException($this, $value, $name);
+         }
+         break;
+       case 'string':
+         if (!is_string($value)) {
+           throw new PhutilTypeCheckException($this, $value, $name);
+         }
+         break;
+       case 'regex':
+         $trap = new PhutilErrorTrap();
+-          $ok = @preg_match($value, '');
+-          $err = $trap->getErrorsAsString();
++        if (!is_string($value)) {
++          $value = implode(', ', $value);
++        }
++        $ok = @preg_match($value, '');
++        $err = $trap->getErrorsAsString();
+         $trap->destroy();
+ 
+         if ($ok === false) {
+           throw new PhutilTypeCheckException($this, $value, $name, $err);
+         }
+         break;
+       case 'null':
+         if (!is_null($value)) {
+           throw new PhutilTypeCheckException($this, $value, $name);
+         }
+--- a/src/repository/api/ArcanistGitAPI.php 2022-08-23 19:49:34.714466391 -0700
++++ b/src/repository/api/ArcanistGitAPI.php 2022-08-23 18:56:21.182342625 -0700
+@@ -1133,20 +1133,23 @@
+ 
+   public function supportsCommitRanges() {
+     return true;
+   }
+ 
+   public function supportsLocalCommits() {
+     return true;
+   }
+ 
+   public function hasLocalCommit($commit) {
++    if (!$commit) {
++      return false;
++    }
+     try {
+       if (!phutil_nonempty_string($commit) ||
+           !$this->getCanonicalRevisionName($commit)) {
+         return false;
+       }
+     } catch (CommandException $exception) {
+       return false;
+     }
+     return true;
+   }
+--- a/src/unit/engine/phutil/PhutilTestCase.php 2020-09-25 00:59:46.000000000 -0700
++++ b/src/unit/engine/phutil/PhutilTestCase.php 2022-08-23 19:03:08.849311825 -0700
+@@ -401,51 +401,51 @@
+ 
+ 
+   /**
+    * Mark the currently-running test as a failure.
+    *
+    * @param string  Human-readable description of problems.
+    * @return void
+    *
+    * @task internal
+    */
+-  final private function failTest($reason) {
++  /*final*/ private function failTest($reason) {
+     $this->resultTest(ArcanistUnitTestResult::RESULT_FAIL, $reason);
+   }
+ 
+ 
+   /**
+    * This was a triumph. I'm making a note here: HUGE SUCCESS.
+    *
+    * @param string  Human-readable overstatement of satisfaction.
+    * @return void
+    *
+    * @task internal
+    */
+-  final private function passTest($reason) {
++  /*final*/ private function passTest($reason) {
+     $this->resultTest(ArcanistUnitTestResult::RESULT_PASS, $reason);
+   }
+ 
+ 
+   /**
+    * Mark the current running test as skipped.
+    *
+    * @param string  Description for why this test was skipped.
+    * @return void
+    * @task internal
+    */
+-  final private function skipTest($reason) {
++  /*final*/ private function skipTest($reason) {
+     $this->resultTest(ArcanistUnitTestResult::RESULT_SKIP, $reason);
+   }
+ 
+ 
+-  final private function resultTest($test_result, $reason) {
++  /*final*/ private function resultTest($test_result, $reason) {
+     $coverage = $this->endCoverage();
+ 
+     $result = new ArcanistUnitTestResult();
+     $result->setCoverage($coverage);
+     $result->setNamespace(get_class($this));
+     $result->setName($this->runningTest);
+     $result->setLink($this->getLink($this->runningTest));
+     $result->setResult($test_result);
+     $result->setDuration(microtime(true) - $this->testStartTime);
+     $result->setUserData($reason);
+@@ -546,34 +546,34 @@
+   }
+ 
+   final public function setEnableCoverage($enable_coverage) {
+     $this->enableCoverage = $enable_coverage;
+     return $this;
+   }
+ 
+   /**
+    * @phutil-external-symbol function xdebug_start_code_coverage
+    */
+-  final private function beginCoverage() {
++  /*final*/ private function beginCoverage() {
+     if (!$this->enableCoverage) {
+       return;
+     }
+ 
+     $this->assertCoverageAvailable();
+     xdebug_start_code_coverage(XDEBUG_CC_UNUSED | XDEBUG_CC_DEAD_CODE);
+   }
+ 
+   /**
+    * @phutil-external-symbol function xdebug_get_code_coverage
+    * @phutil-external-symbol function xdebug_stop_code_coverage
+    */
+-  final private function endCoverage() {
++  /*final*/ private function endCoverage() {
+     if (!$this->enableCoverage) {
+       return;
+     }
+ 
+     $result = xdebug_get_code_coverage();
+     xdebug_stop_code_coverage($cleanup = false);
+ 
+     $coverage = array();
+ 
+     foreach ($result as $file => $report) {
+@@ -611,21 +611,21 @@
+     // Only keep coverage information for files modified by the change. In
+     // the case of --everything, we won't have paths, so just return all the
+     // coverage data.
+     if ($this->paths) {
+       $coverage = array_select_keys($coverage, $this->paths);
+     }
+ 
+     return $coverage;
+   }
+ 
+-  final private function assertCoverageAvailable() {
++  /*final*/ private function assertCoverageAvailable() {
+     if (!function_exists('xdebug_start_code_coverage')) {
+       throw new Exception(
+         pht("You've enabled code coverage but XDebug is not installed."));
+     }
+   }
+ 
+   final public function getWorkingCopy() {
+     return $this->workingCopy;
+   }
+ 
+@@ -668,21 +668,21 @@
+   final public function setRenderer(ArcanistUnitRenderer $renderer) {
+     $this->renderer = $renderer;
+     return $this;
+   }
+ 
+   /**
+    * Returns info about the caller function.
+    *
+    * @return map
+    */
+-  final private static function getCallerInfo() {
++  /*final*/ private static function getCallerInfo() {
+     $callee = array();
+     $caller = array();
+     $seen = false;
+ 
+     foreach (array_slice(debug_backtrace(), 1) as $location) {
+       $function = idx($location, 'function');
+ 
+       if (!$seen && preg_match('/^assert[A-Z]/', $function)) {
+         $seen = true;
+         $caller = $location;
+--- a/src/utils/PhutilArray.php 2020-09-25 00:59:46.000000000 -0700
++++ b/src/utils/PhutilArray.php 2022-08-23 19:52:11.731494704 -0700
+@@ -22,20 +22,21 @@
+ 
+ 
+   public function toArray() {
+     return iterator_to_array($this, true);
+   }
+ 
+ 
+ /* -(  Countable Interface  )------------------------------------------------ */
+ 
+ 
++  #[\ReturnTypeWillChange]
+   public function count() {
+     return count($this->data);
+   }
+ 
+ 
+ /* -(  Iterator Interface  )------------------------------------------------- */
+ 
+ 
+   public function current() {
+     return current($this->data);
+@@ -54,27 +55,31 @@
+   }
+ 
+   public function valid() {
+     return (key($this->data) !== null);
+   }
+ 
+ 
+ /* -(  ArrayAccess Interface  )---------------------------------------------- */
+ 
+ 
++  #[\ReturnTypeWillChange]
+   public function offsetExists($key) {
+     return array_key_exists($key, $this->data);
+   }
+ 
++  #[\ReturnTypeWillChange]
+   public function offsetGet($key) {
+     return $this->data[$key];
+   }
+ 
++  #[\ReturnTypeWillChange]
+   public function offsetSet($key, $value) {
+     $this->data[$key] = $value;
+   }
+ 
++  #[\ReturnTypeWillChange]
+   public function offsetUnset($key) {
+     unset($this->data[$key]);
+   }
+ 
+ }
+--- a/src/utils/utils.php 2020-09-25 00:59:46.000000000 -0700
++++ b/src/utils/utils.php 2022-08-23 19:32:14.898952009 -0700
+@@ -1996,10 +1996,135 @@
+     $partition = array($key => $value);
+     $partition_value = $value;
+   }
+ 
+   if ($partition) {
+     $partitions[] = $partition;
+   }
+ 
+   return $partitions;
+ }
++
++/**
++ * Test if a value is a nonempty string.
++ *
++ * The value "null" and the empty string are considered empty; all other
++ * strings are considered nonempty.
++ *
++ * This method raises an exception if passed a value which is neither null
++ * nor a string.
++ *
++ * @param Value to test.
++ * @return bool True if the parameter is a nonempty string.
++ */
++function phutil_nonempty_string($value) {
++  if ($value === null) {
++    return false;
++  }
++
++  if ($value === '') {
++    return false;
++  }
++
++  if (is_string($value)) {
++    return true;
++  }
++
++  throw new InvalidArgumentException(
++    pht(
++      'Call to phutil_nonempty_string() expected null or a string, got: %s.',
++      phutil_describe_type($value)));
++}
++
++
++/**
++ * Test if a value is a nonempty, stringlike value.
++ *
++ * The value "null", the empty string, and objects which have a "__toString()"
++ * method which returns the empty string are empty.
++ *
++ * Other strings, and objects with a "__toString()" method that returns a
++ * string other than the empty string are considered nonempty.
++ *
++ * This method raises an exception if passed any other value.
++ *
++ * @param Value to test.
++ * @return bool True if the parameter is a nonempty, stringlike value.
++ */
++function phutil_nonempty_stringlike($value) {
++  if ($value === null) {
++    return false;
++  }
++
++  if ($value === '') {
++    return false;
++  }
++
++  if (is_string($value)) {
++    return true;
++  }
++
++  if (is_object($value)) {
++    try {
++      $string = phutil_string_cast($value);
++      return phutil_nonempty_string($string);
++    } catch (Exception $ex) {
++      // Continue below.
++    } catch (Throwable $ex) {
++      // Continue below.
++    }
++  }
++
++  throw new InvalidArgumentException(
++    pht(
++      'Call to phutil_nonempty_stringlike() expected a string or stringlike '.
++      'object, got: %s.',
++      phutil_describe_type($value)));
++}
++
++
++/**
++ * Test if a value is a nonempty, scalar value.
++ *
++ * The value "null", the empty string, and objects which have a "__toString()"
++ * method which returns the empty string are empty.
++ *
++ * Other strings, objects with a "__toString()" method which returns a
++ * string other than the empty string, integers, and floats are considered
++ * scalar.
++ *
++ * This method raises an exception if passed any other value.
++ *
++ * @param Value to test.
++ * @return bool True if the parameter is a nonempty, scalar value.
++ */
++function phutil_nonempty_scalar($value) {
++  if ($value === null) {
++    return false;
++  }
++
++  if ($value === '') {
++    return false;
++  }
++
++  if (is_string($value) || is_int($value) || is_float($value)) {
++    return true;
++  }
++
++  if (is_object($value)) {
++    try {
++      $string = phutil_string_cast($value);
++      return phutil_nonempty_string($string);
++    } catch (Exception $ex) {
++      // Continue below.
++    } catch (Throwable $ex) {
++      // Continue below.
++    }
++  }
++
++  throw new InvalidArgumentException(
++    pht(
++      'Call to phutil_nonempty_scalar() expected: a string; or stringlike '.
++      'object; or int; or float. Got: %s.',
++      phutil_describe_type($value)));
++}
++
+--- a/src/workflow/ArcanistWorkflow.php 2020-09-25 00:59:46.000000000 -0700
++++ b/src/workflow/ArcanistWorkflow.php 2022-08-23 19:52:35.740260200 -0700
+@@ -721,21 +721,21 @@
+ 
+   final public function setWorkingDirectory($working_directory) {
+     $this->workingDirectory = $working_directory;
+     return $this;
+   }
+ 
+   final public function getWorkingDirectory() {
+     return $this->workingDirectory;
+   }
+ 
+-  final private function setParentWorkflow($parent_workflow) {
++  /*final*/ private function setParentWorkflow($parent_workflow) {
+     $this->parentWorkflow = $parent_workflow;
+     return $this;
+   }
+ 
+   final protected function getParentWorkflow() {
+     return $this->parentWorkflow;
+   }
+ 
+   final public function buildChildWorkflow($command, array $argv) {
+     $arc_config = $this->getArcanistConfiguration();
+@@ -1370,21 +1370,21 @@
+     ConduitClient $conduit,
+     $revision_id) {
+ 
+     return $this->loadBundleFromConduit(
+       $conduit,
+       array(
+       'revisionIDs' => array($revision_id),
+     ));
+   }
+ 
+-  final private function loadBundleFromConduit(
++  /*final*/ private function loadBundleFromConduit(
+     ConduitClient $conduit,
+     $params) {
+ 
+     $future = $conduit->callMethod('differential.querydiffs', $params);
+     $diff = head($future->resolve());
+ 
+     if ($diff == null) {
+       throw new Exception(
+         phutil_console_wrap(
+           pht("The diff or revision you specified is either invalid or you ".
+--- a/support/init/init-script.php  2020-09-25 00:59:46.000000000 -0700
++++ b/support/init/init-script.php  2022-08-23 18:56:21.186342719 -0700
+@@ -8,21 +8,21 @@
+   // output buffer. Discard any such output buffers so messages can be sent to
+   // stdout (if a user wants to capture output from a script, there are a large
+   // number of ways they can accomplish it legitimately; historically, we ran
+   // into this on only one install which had some bizarre configuration, but it
+   // was difficult to diagnose because the symptom is "no messages of any
+   // kind").
+   while (ob_get_level() > 0) {
+     ob_end_clean();
+   }
+ 
+-  error_reporting(E_ALL | E_STRICT);
++  // error_reporting(E_ALL | E_STRICT);
+ 
+   $config_map = array(
+     // Always display script errors. Without this, they may not appear, which is
+     // unhelpful when users encounter a problem. On the web this is a security
+     // concern because you don't want to expose errors to clients, but in a
+     // script context we always want to show errors.
+     'display_errors'              => true,
+ 
+     // Send script error messages to the server's `error_log` setting.
+     'log_errors'                  => true,