[PATCH] D129409: [llvm][docs] commit phabricator patch

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers created this revision.
Herald added a project: All.
nickdesaulniers requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Users upgrading to PHP 8.1 might start observing failures with `arc`.
Commit @ychen's suggestions as a patch in tree that can be applied since
arcanist is no longer accepting patches.

Also, remove the suggestion to apply an external patch updating CA
certs. It seems that this was fixed in upstream arcanist before they
stopped accepting patches. Compare
https://github.com/rashkov/arcanist/commit/e3659d43d8911e91739f3b0c5935598bceb859aa
vs
https://github.com/rashkov/arcanist/commit/13d3a3c3b100979c34dda261fe21253e3571bc46

Link: https://secure.phabricator.com/book/phabcontrib/article/contributing_code/


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129409

Files:
  llvm/docs/Phabricator.rst
  llvm/utils/phabricator/0001-Fix-PHP-8.1-incompatibility-with-arc-patch-D-12345.patch


Index: llvm/utils/phabricator/0001-Fix-PHP-8.1-incompatibility-with-arc-patch-D-12345.patch
===================================================================
--- /dev/null
+++ llvm/utils/phabricator/0001-Fix-PHP-8.1-incompatibility-with-arc-patch-D-12345.patch
@@ -0,0 +1,70 @@
+From bb2e9a0394a48f4ef3576e8d91607e8129b2b716 Mon Sep 17 00:00:00 2001
+From: Nick Desaulniers <ndesaulniers at google.com>
+Date: Fri, 8 Jul 2022 14:55:46 -0700
+Subject: [PATCH] Fix PHP 8.1 incompatibility with `arc patch D<12345>`
+
+Fixes the following observed error with PHP 8.1:
+
+  EXCEPTION: (RuntimeException) preg_match(): Passing null to parameter phacility#2 ($subject) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
+  arcanist(head=master, ref.master=acec17af414f)
+    #0 PhutilErrorHandler::handleError(integer, string, string, integer)
+    phacility#1 preg_match(string, NULL, NULL) called at [<arcanist>/src/repository/api/ArcanistGitAPI.php:603]
+    phacility#2 ArcanistGitAPI::getCanonicalRevisionName(NULL) called at [<arcanist>/src/repository/api/ArcanistGitAPI.php:1146]
+    phacility#3 ArcanistGitAPI::hasLocalCommit(NULL) called at [<arcanist>/src/workflow/ArcanistPatchWorkflow.php:433]
+    phacility#4 ArcanistPatchWorkflow::run() called at [<arcanist>/src/workflow/ArcanistPatchWorkflow.php:398]
+    phacility#5 ArcanistPatchWorkflow::run() called at [<arcanist>/scripts/arcanist.php:427]
+
+Link: https://secure.phabricator.com/book/phabcontrib/article/contributing_code/
+Link: https://reviews.llvm.org/D129232#3634072
+Suggested-by: Yuanfang Chen <yuanfang.chen at sony.com>
+Signed-off-by: Nick Desaulniers <ndesaulniers at google.com>
+---
+ src/differential/ArcanistDifferentialDependencyGraph.php | 3 ++-
+ src/lint/linter/ArcanistScriptAndRegexLinter.php         | 2 +-
+ src/repository/api/ArcanistGitAPI.php                    | 3 ++-
+ 3 files changed, 5 insertions(+), 3 deletions(-)
+
+diff --git a/src/differential/ArcanistDifferentialDependencyGraph.php b/src/differential/ArcanistDifferentialDependencyGraph.php
+index 64e2f9c71c20..fbc4e6594f0e 100644
+--- a/src/differential/ArcanistDifferentialDependencyGraph.php
++++ b/src/differential/ArcanistDifferentialDependencyGraph.php
+@@ -42,7 +42,8 @@ final class ArcanistDifferentialDependencyGraph extends AbstractDirectedGraph {
+     $edges = array();
+     foreach ($dependencies as $dependency) {
+       $dependency_revision = $this->getCommitHashFromDict($dependency);
+-      if ($repository_api->hasLocalCommit($dependency_revision)) {
++      if (phutil_nonempty_string($dependency_revision) &&
++          $repository_api->hasLocalCommit($dependency_revision)) {
+         $edges[$dependency['phid']] = array();
+         continue;
+       }
+diff --git a/src/lint/linter/ArcanistScriptAndRegexLinter.php b/src/lint/linter/ArcanistScriptAndRegexLinter.php
+index 0c3d9d9a11ca..b9f6924ec997 100644
+--- a/src/lint/linter/ArcanistScriptAndRegexLinter.php
++++ b/src/lint/linter/ArcanistScriptAndRegexLinter.php
+@@ -338,7 +338,7 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
+     }
+ 
+     $line = idx($match, 'line');
+-    if (strlen($line)) {
++    if (phutil_nonempty_string($line) && strlen($line)) {
+       $line = (int)$line;
+       if (!$line) {
+         $line = 1;
+diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
+index 6c6d2ac42a19..13907d5634be 100644
+--- a/src/repository/api/ArcanistGitAPI.php
++++ b/src/repository/api/ArcanistGitAPI.php
+@@ -1143,7 +1143,8 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
+ 
+   public function hasLocalCommit($commit) {
+     try {
+-      if (!$this->getCanonicalRevisionName($commit)) {
++      if (!phutil_nonempty_string($commit) ||
++          !$this->getCanonicalRevisionName($commit)) {
+         return false;
+       }
+     } catch (CommandException $exception) {
+-- 
+2.37.0.rc0.161.g10f37bed90-goog
+
Index: llvm/docs/Phabricator.rst
===================================================================
--- llvm/docs/Phabricator.rst
+++ llvm/docs/Phabricator.rst
@@ -42,8 +42,7 @@
 .. code-block:: console
 
   % cd arcanist
-  % git fetch https://github.com/rashkov/arcanist update_cacerts
-  % git cherry-pick e3659d43d8911e91739f3b0c5935598bceb859aa
+  % git am <path/to/llvm-project>/llvm/utils/phabricator/*.patch
 
 You can learn more about how to use arc to interact with
 Phabricator in the `Arcanist User Guide`_.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129409.443387.patch
Type: text/x-patch
Size: 4487 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220708/a58321b5/attachment.bin>