Migrate from phabricator to phorge #11
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=b325304b6e52), phorge(head=SvnRepoMain, ref.master=cb938d869c3f, ref.SvnRepoMain=cb938d869c3f) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionController.php:156] ``` Closes T15446 Test Plan: After applying this change, going to a newly created SVN repository at `/diffusion/1/`, the page renders, showing the repository as Inactive and also "Empty Repository - This repository does not have any commits yet." as expected. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15446 Differential Revision: https://we.phorge.it/D25278
… Hg repo Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=b325304b6e52), phorge(head=HgViewBranches, ref.master=cb938d869c3f, ref.HgViewBranches=cb938d869c3f) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php:83] ``` Closes T15447 Test Plan: After applying this change, going to `/diffusion/1/branches/default/` shows an expected FilesystemException instead of the previous RuntimeException. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15447 Differential Revision: https://we.phorge.it/D25279
… Differential when not entering data Summary: `preg_match()` does not accept passing null as the `$subject` string parameter in PHP 8.1. Thus add a `phutil_nonempty_string()` check if the `$subject` parameter is a non-empty string. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) preg_match(): Passing null to parameter freebsd#2 ($subject) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=18554ea76ceb), phorge(head=diffAttach, ref.master=e11c5486c92b, ref.diffAttach=e11c5486c92b) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261] freebsd#1 <freebsd#2> preg_match(string, NULL, NULL) called at [<phorge>/src/applications/differential/editor/DifferentialTransactionEditor.php:221] ``` Closes T15431 Test Plan: After applying this change on top of D25262, going to `/differential/diff/create/`, creating a diff, and entering nothing on the "Create Revision" page on `/differential/revision/attach/1/to/` correctly shows the errors "You must provide a test plan. Describe the actions you performed to verify the behavior of this change. Revisions must have a title." in web browser instead of an exception. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15431 Differential Revision: https://we.phorge.it/D25263
Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=18554ea76ceb), phorge(head=diff3, ref.master=e11c5486c92b, ref.diff3=e11c5486c92b) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/differential/customfield/DifferentialBranchField.php:42] ``` Closes T15432 Test Plan: After applying these three changes (on top of D25262 and D25263), there is no `strlen()` related exception displayed on `/D1/` anymore, however there are further exceptions to be sorted out in other tasks. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15432 Differential Revision: https://we.phorge.it/D25264
…o after renaming default branch Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=arcpatch-D25250, ref.master=18554ea76ceb, ref.arcpatch-D25250=fade4603a799), phorge(head=master, ref.master=e11c5486c92b) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryDefaultBranchTransaction.php:25] ``` Closes T15434 Test Plan: After applying this change, after changing the name of the (previously empty) default branch of a Diffusion repository, and then going to the repository History at `/diffusion/<someID>/manage/history/`, the page renders correctly in the web browser instead of showing an exception. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15434 Differential Revision: https://we.phorge.it/D25265
…ithout entering assignee Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=b325304b6e52), phorge(head=diffusionCreateIdentity, ref.master=cbc0e661544a, ref.diffusionCreateIdentity=cbc0e661544a) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryIdentityAssignTransaction.php:55] ``` ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=b325304b6e52), phorge(head=diffusionCreateIdentity, ref.master=cbc0e661544a, ref.diffusionCreateIdentity=cbc0e661544a) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:298] ``` ``` EXCEPTION: (RuntimeException) mb_detect_encoding(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=b325304b6e52), phorge(head=diffusionCreateIdentity, ref.master=cbc0e661544a, ref.diffusionCreateIdentity=cbc0e661544a) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261] freebsd#1 <freebsd#2> mb_detect_encoding(NULL, array) called at [<phorge>/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:313] ``` Closes T15440 Test Plan: After applying these three changes, going to `/diffusion/identity/edit/form/default/` and selecting the "Create Identity" button will only show a AphrontQueryException unrelated to PHP 8 and covered in T15439 instead. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15440 Differential Revision: https://we.phorge.it/D25271
…Import Only" Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=b325304b6e52), phorge(head=master, ref.master=cb938d869c3f) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositorySVNSubpathTransaction.php:25] ``` Closes T15445 Test Plan: After applying this change, and after setting some "Import Only" value for a new Subversion repository, the page `/diffusion/1/manage/history/` renders showing `R1` being Inactive and `user set the repository "Import Only" path to main`. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15445 Differential Revision: https://we.phorge.it/D25277
Summary:
On mobile devices like tablets or toasters the "Persistent Chat"
floating widget is already hidden.
So, the related checkbox available from the top navigation bar
is just confusing on tablet and mobile devices / toasters, since
that nice checkbox does nothing there.
On mobile and tablet, this is the graphical change:
| Before | After |
|----------|-----------|
|{F281239} | {F281235} |
This change do not change anything for desktop devices.
So, on desktop, that checkbox is obviously still visible.
Closes T15240
Test Plan:
- test on tablet and below: now the checkbox should be not visible
- test on desktop: the checkbox should still be visible
Reviewers: O1 Blessed Committers, Cigaryno, bfs, speck
Reviewed By: O1 Blessed Committers, Cigaryno, bfs, speck
Subscribers: avivey, bfs, dcog, chris, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15240
Differential Revision: https://we.phorge.it/D25120
Summary:
Premise: the Config page runs git commands. Spoiler: they can fail.
Before this change errors were just suppressed and ignored.
After this change you get at least a log line. Also, you get a tip for a very specific well-known error affecting recent git.
Probably suppressing stuff was fine in the moment git worked here. But nowadays
git doesn't work so easily here, since it introduced very weird additional
configurations in order for a repository to be indicated as "safe" or not.
Error suppression was a problem there, because understanding the error with
"future objects" is not trivial for most users. Really.
After this change, these errors are beautifully mentioned in the standard log
of your webserver, to the best of our communication ability.
This is a cute example of a new log line:
Cannot identify the version of the phorge repository because the webserver does not trust it (more info on Task https://we.phorge.it/T15282).
Try this system resolution:
sudo git config --system --add safe.directory /var/www/phorge
Another:
Cannot identify the version of the phorge repository because the webserver does not trust it (more info on Task https://we.phorge.it/T15282).
Try this system resolution:
sudo git config --system --add safe.directory /var/www/arcanist
Incidentally, these specific errors probably afflict your Phorge/Phabricator, and now
you have some useful resolution tips. You are welcome!
You can also join T15282 to discuss your specific case.
Closes T15243
Test Plan:
- visit the `/config/` page: does it worked before? it still works now
- visit the `/config/` page without `/etc/gitconfig`: you may still see "Unknown" as version - but, you finally get something in the log (instead of nothing)
- visit the `/config/` page after following your log messages: now you should see the library versions! yeeh
Additional tests:
- manually sabotage the command "git log" replacing with "gitfooolog" and visit /config page: see the unexpected 'gitfooolog command not found' log line
- manually sabotage the command "git remove" replacing with "gitremotelog" and visit /config/ page: see the unexpected 'gitremotelog command not found' log line
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, deadalnix, aklapper, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15243
Differential Revision: https://we.phorge.it/D25148
…arbormaster.sendmessage page Summary: Since PHP 8.1, passing a null string to `trim()` is deprecated. Thus first check that `$content` is not null before trimming it. Also since trim() returns a string and never null, we can also simplify the non-empty check, in a more readable and efficient way, avoiding strlen() that was usually used for other "more wild" cases. ``` EXCEPTION: (RuntimeException) trim(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=18554ea76ceb), phorge(head=master, ref.master=0d81da590923) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261] freebsd#1 <freebsd#2> trim(NULL) called at [<phorge>/src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php:117] ``` Closes T15427 Test Plan: Applied this change, afterwards `/conduit/method/harbormaster.sendmessage/` correctly rendered in web browser. Reviewers: O1 Blessed Committers, speck, valerio.bozzolan Reviewed By: O1 Blessed Committers, speck, valerio.bozzolan Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15427 Differential Revision: https://we.phorge.it/D25259
…epo History page Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. Passing null to strlen() is deprecated since PHP 8.1. Thus first check that the string is not null. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=b325304b6e52), phorge(head=master, ref.master=83edbffd521d) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryStagingURITransaction.php:20] ``` This is quite similar to D25277. Closes T15458 Test Plan: After applying this change, editing the Staging URI by saving its already empty value, and going to the repo History page, the page `/diffusion/1/manage/history/` renders without an exception, showing `R1` being Inactive and `user set as the staging area for this repository.` [sic!] Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15458 Differential Revision: https://we.phorge.it/D25282
… comment Summary: Since PHP 8.1, passing a null string to `ltrim(string $string)` is deprecated. Thus first check if After and Below text are not null before trimming. Closes T15379 Test Plan: Applied this change; afterwards several times created a new Pholio mock and added a Meme comment. I could not reproduce the problem anymore (first meme comment in a mock always rendered correctly instead of showing an exception as comment). However, I could not reliably reproduce the problem anyway. - Create a meme without any above/below text - Create a meme with just spaces as above/below text - Create a meme with just above text as "asd", or "0" or a lizard - Create a meme with just below text as "asd", or "0" or a lizard - Create a meme with both above and below texts with "asd", "0" and a lizard and more stuff (doing all 64 combinations) Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15379 Differential Revision: https://we.phorge.it/D25212
… Create Blueprint page Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=e4fd31ec024e), phorge(head=D25240, ref.master=b1edfea09bad, ref.D25240=b1edfea09bad) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php:392] ``` Closes T15413 Test Plan: Applied this change on top of D25239. Afterwards, the "Create Blueprint" page on `/drydock/blueprint/edit/form/default/` after pressing the "Create Blueprint" button the page correctly renders the expected error that some fields cannot be empty. # Enter a commit message. # # Changes: # # src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php Also, try to create extra numeric fields (like in the User application) and try to send zero or 1 using Conduit `user.edit`. Reviewers: O1 Blessed Committers, valerio.bozzolan, speck Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15413 Differential Revision: https://we.phorge.it/D25240
…y without path access permissions Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. Closes T15459 Test Plan: After applying this change, going to `/diffusion/17/manage/`, the actual error (`cannot mkdir /var/repo/17: Permission denied`) is propagated and displayed in the "Working Copy Status" section, instead of being hidden by a `strlen()` exception. See T15459 for details. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15459 Differential Revision: https://we.phorge.it/D25283
…load with no data passed Summary: Since PHP 8.1, `base64_decode()` does not accept passing null as a parameter. Thus first check that `data !== null` before calling `decodeBase64($data)` (which then calls PHP's `base64_decode()`), and throw an exception if it is. ``` EXCEPTION: (RuntimeException) base64_decode(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=18554ea76ceb), phorge(head=conduitDashboardPanelEdit, ref.master=0d81da590923, ref.conduitDashboardPanelEdit=ab4391b30465) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261] freebsd#1 <freebsd#2> base64_decode(NULL, boolean) called at [<phorge>/src/applications/files/conduit/FileConduitAPIMethod.php:84] ``` Closes T15426 Test Plan: Applied this change; afterwards `/api/file.upload` under "Method Result", "error_info" shows "Unable to decode base64 data!" instead of "base64_decode(): Passing null to parameter freebsd#1 ($string) of type string is deprecated" which is more descriptive. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15426 Differential Revision: https://we.phorge.it/D25258
…sitory history Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCode, ref.master=108cbcd09bd3, ref.diffusionBrowseCode=015ffef14b0c) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:50] ``` ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCode, ref.master=108cbcd09bd3, ref.diffusionBrowseCode=015ffef14b0c) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:50] ``` Closes T15463 Test Plan: After applying these two changes, get next expected exceptions about `RuntimeException: file_exists(): Passing null to parameter freebsd#1 ($filename) of type string is deprecated` and `CommandException: Command failed with error #128! fatal: detected dubious ownership in repository at '/var/repo/1'`, but no RuntimeException about `strlen()` anymore. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15463 Differential Revision: https://we.phorge.it/D25287
…when bugtraq.url = null Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCommit, ref.master=9b99123ff931, ref.diffusionBrowseCommit=9b99123ff931) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionCommitController.php:925] ``` Closes T15464 Test Plan: After applying this change, selecting the last commit on http://phorge.localhost/diffusion/query/all/ correctly renders the commit page (e.g. `/R1:1234567890abcdef1234567890abcdef12345678`) Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15464 Differential Revision: https://we.phorge.it/D25288
…po after changing text encoding Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=master, ref.master=108cbcd09bd3) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryEncodingTransaction.php:20] ``` Closes T15460 Test Plan: After applying these two changes, changing the text encoding of a Diffusion repository to a valid encoding and going to `/diffusion/1/manage/history/`, page renders correctly and shows no exception. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15460 Differential Revision: https://we.phorge.it/D25284
…sitory code Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCode, ref.master=108cbcd09bd3, ref.diffusionBrowseCode=108cbcd09bd3) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionBrowseController.php:25] ``` ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionBrowseCode, ref.master=108cbcd09bd3, ref.diffusionBrowseCode=108cbcd09bd3) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php:40] ``` Closes T15462 Test Plan: After applying these two changes, get next expected exceptions about `RuntimeException: file_exists(): Passing null to parameter freebsd#1 ($filename) of type string is deprecated` and `CommandException: Command failed with error #128! fatal: detected dubious ownership in repository at '/var/repo/1'`, but no RuntimeException about `strlen()` anymore. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15462 Differential Revision: https://we.phorge.it/D25286
…ge in Diffusion Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionCallsignHistory, ref.master=9b99123ff931, ref.diffusionCallsignHistory=d5fb6702a49a) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionCommitController.php:31] ``` Closes T15466 Test Plan: After applying this change, going to an existing commit URL like `/diffusion/XYZ/commit/999a2c7d1ff8` shows the exception covered by T15464 instead, as expected. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15466 Differential Revision: https://we.phorge.it/D25290
…conExample) page Summary: Fix three typos in icon names Closes T15454 Test Plan: Go to `/uiexample/view/PHUIIconExample/` before and after applying the change; compare icons displayed for `fa-codiepie`, `fa-user-o`, `fa-thermometer-0` Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15454 Differential Revision: https://we.phorge.it/D25280
…page after setting Callsign Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=diffusionCallsignHistory, ref.master=9b99123ff931, ref.diffusionCallsignHistory=9b99123ff931) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryCallsignTransaction.php:28] ``` Closes T15465 Test Plan: After applying this change, `/diffusion/XYZ/manage/history/` renders correctly and shows `user set the callsign for this repository to XYZ.` Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15465 Differential Revision: https://we.phorge.it/D25289
…mber function getAppliedTransactionPHIDs() on bool Summary: In some cases, older Herald transcripts which got garbage collected display an exception. For unknown reasons, calling `getObjectTranscript()` on the HeraldTranscript object `$xscript` can return a bool with the value `false`. Afterwards, calling `getAppliedTransactionPHIDs()` on that bool throws an exception. Thus as a workaround, return an empty array instead in this case, like the code already does when $xscript->getObjectTranscript()->getAppliedTransactionPHIDs() === null. Note that it remains unclear whether/when the bool may have the value `true` hence this situation is not covered by this code change (the code would still throw an exception in this situation). Closes T15343 Test Plan: 1. Change system time to one year back (or optionally, use `./phorge/bin/garbage set-policy --collector herald.transcripts --days 1`, drop steps 9 and 10, and wait a day before performing step 11) 2. As an admin, go to http://phorge.localhost/herald/create/ 3. Select "Maniphest Tasks" 4. On http://phorge.localhost/herald/create/?adapter=HeraldManiphestTaskAdapter , select "Global Rule" 5. Under "Conditions", select `any of` and set the three conditions `Description | contains | Internet Archive`, `Description | contains | archive.org`, `Description | contains | Wayback Machine`. 6. Under "Action", select `Add projects | someProject` 7. Select "Save Rule" 8. Trigger the rule by creating a new task. 9. Change system time back to today 10. On the shell, run `./phorge/bin/phd restart` 11. Visit created task and select "View Herald Transcript" 12. Transcript page displays "Old Transcript - Details of this transcript have been garbage collected." and shows no exception anymore Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15343 Differential Revision: https://we.phorge.it/D25246
… tags Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=diffusionEmptyRepoPatternSearch, ref.master=97e163187418, ref.diffusionEmptyRepoPatternSearch=97e163187418), phorge(head=master, ref.master=108cbcd09bd3) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diffusion/controller/DiffusionTagListController.php:28] ``` Closes T15461 Test Plan: After applying this line, get the expected `CommandException: Command failed with error #128!` due to ` fatal: detected dubious ownership in repository at '/var/repo/1'` (plus a `RuntimeException: file_exists(): Passing null to parameter freebsd#1 ($filename) of type string is deprecated` as a bonus), instead of the previous RuntimeException about `strlen()`. Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15461 Differential Revision: https://we.phorge.it/D25285
Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` EXCEPTION: (RuntimeException) strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261] arcanist(head=master, ref.master=97e163187418), phorge(head=master, ref.master=7bbd657c19c4) #0 <freebsd#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/conduit/controller/PhabricatorConduitAPIController.php:686] ``` Closes T15469 Test Plan: After applying this change, `/api/user.whoami` correctly renders the "Method Call Result" Reviewers: O1 Blessed Committers, speck Reviewed By: O1 Blessed Committers, speck Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15469 Differential Revision: https://we.phorge.it/D25292
Summary:
When the user "Mrs. Kitten" adds or remove "Mrs. Kitten" as Subscriber,
usually these messages were generated:
Mrs. Kitten added a subscriber: Mrs. Kitten.
Mrs. Kitten removed a subscriber: Mrs. Kitten.
This was a bit like the Spiderman meme self-pointing Spiderman.
After this change, these beautiful messages are generated instead:
Mrs. Kitten subscribed.
Mrs. Kitten unsubscribed.
| Before | After |
| {F286215} | {F286216} |
Closes T15347
Test Plan:
- subscribe on something
- unsubscribe from something
- all other cases remain as-is
Reviewers: O1 Blessed Committers, bfs, speck
Reviewed By: O1 Blessed Committers, bfs, speck
Subscribers: bfs, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15347
Differential Revision: https://we.phorge.it/D25191
Summary: Add Phorge icon to auth/login PNG files. Closes T15403 Test Plan: Go to `/auth/config/new/` in browser and look at icons. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15403 Differential Revision: https://we.phorge.it/D25255
Summary: Closes T15479 Test Plan: Go to `/auth/config/new/` in browser and look at icons. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15479 Differential Revision: https://we.phorge.it/D25297
Summary: Allow humans to understand what happened when `phabricator.serious-business` mode is enabled: The administrator role was removed from a user account. Closes T15480 Test Plan: Enable `phabricator.serious-business` mode. Enable and disable an account as admin. Check that account's feed and the general feed. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15480 Differential Revision: https://we.phorge.it/D25298
Summary: This seems like a fairly obvious oversight with the column.search API. Knowing: 1. isHidden - boolean to indicate Active vs Archived 2. isDefaultColumn - the one that Tasks get dropped in by default, usually called "Backlog" 3. sequence - numerical order on the Workboard are all necessary for a lot of things that very sensible real-world API clients need to do when working with columns. Partial cherry-pick from: https://phabricator.wikimedia.org/rPHABebfe30890b52784d222ec4ed36c05462b2a57f92 Closes T15484 Test Plan: Tested on phabricator.wikimedia.org over many months and used by real client apps. To do additional tests, visit this page: /conduit/method/project.column.search/ Check that the new fields are returned correctly and nothing explodes. Reviewers: #blessed_committers, O1 Blessed Committers, 0, avivey, valerio.bozzolan Reviewed By: #blessed_committers, O1 Blessed Committers, 0, valerio.bozzolan Subscribers: Cigaryno, 0, speck, tobiaswiese, valerio.bozzolan, Matthew Maniphest Tasks: T15081, T15484 Differential Revision: https://we.phorge.it/D25038
Summary: Fix typo "pohrge" in "Adding New Classes" documentation Test Plan: Carefully read letter by letter. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D25541
Summary: As of February 2024, phurl.io (which is not run by Phorge.it) shows HTTP 503 errors instead of redirecting to the target URIs. Thus replace any URIs pointing to phurl.io with the corresponding target URIs, based on the mapping listed on https://secure.phabricator.com/phurl/. Closes T15746 Test Plan: Carefully read the diff. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15746 Differential Revision: https://we.phorge.it/D25543
Summary: If you are reading this commit message, it probably happened to you this situation: 1. Somebody published a comment that shows the file F123 2. You Submit another comment that shows F123 again After you click "Submit", a wild warning may appear: > Post as Comment > ---- > The action you are taking has no effect: > Transaction (of type "core:file") has no effect. > Do you want to post your comment anyway? > [Undo] [Continue] In this case you probably contact your favorite Phorge administrator to translate that stuff. This is an example unofficial translation: > Post as Comment > ---- > You mentioned a file. Phorge loves files. Phorge attach files. But somebody else already > mentioned that file, successfully. So, since Phorge attach Files, Phorge will result in > doing nothing special, since the file is already perfectly attached by Phorge. > Phorge will just post your comment. > Would you like me to just STFU and post your comment? > [OMG I'm scared] [Post comment and STFU] After this change you don't see anymore that warning. So it's even rarer in your Pokedex. Closes T15293 Test Plan: - Create a comment showing F123 (an existing file) - Create another comment showing F123 - No wild warnings Reviewers: O1 Blessed Committers, 20after4 Reviewed By: O1 Blessed Committers, 20after4 Subscribers: 20after4, speck, tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15293 Differential Revision: https://we.phorge.it/D25544
Summary:
Improve remarkup reference documentation.
* Added a heading for `Code Blocks`
* Added a sentence mentioning the ##```lang## syntax which was added in {T15481}
Test Plan:
* Run `bin/diviner generate` to regen diviner docs.
* Check out the code blocks section of [[/book/phorge/article/remarkup/#code-blocks|Remarkup Reference]]
Reviewers: valerio.bozzolan, O1 Blessed Committers
Reviewed By: valerio.bozzolan, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25547
Summary:
One preview is better than a thousand of words:
{F270656}
This Sound is probably universally recognized thanks to
videogames like Super Mario® or other popular platforms; but
this specific Coin Sound is not proprietary: it's released
with a Free as in Freedom of Speach license: Creative Commons 0.
As if that were not enough, this Coin sound, instead of
potentially creating frustration like many others - it positively
stimulates part of the human cerebral cortex involved in making
humans more addicted to Phorge. To be precise, this Coin sound is
particularly suitable to be associated with Trigger actions
such as:
- Mark as Resolved
- Move on to the next Milestone
- etc.
But the only limitation here, as usual in Phorge, is
your imagination.
Insert a Coin and have fun with Phorge.
Complete sound credits:
https://freesound.org/people/Jocabundus/sounds/678385/
2023, Jocabundus, Creative Commons Zero (public domain dedication)
This change also introduces a Credits page in Diviner to mentions this
kind of things. Future contributions in that page are encouraged.
Insert a Coin. Play again with Phorge.
Ref T15178
Ref T15248
Test Plan:
- Create a Project with a Workboard
- Create a Column
- Column > Gear > New Trigger...
- Action > Play Sound > "Coin"
- Create a Task on the Backlog column, and drop it on your new Column
- Turn on audio, have fun!
Reviewers: O1 Blessed Committers, Cigaryno, avivey, aklapper
Reviewed By: O1 Blessed Committers, Cigaryno, avivey, aklapper
Subscribers: avivey, aklapper, chris, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15178, T15248
Differential Revision: https://we.phorge.it/D25079
Summary:
PhabricatorDefaultSyntaxStyle works now with 'CSS-color-aliases' instead of fixed colors.
The aliases are translated in the corresponding themes CelerityPostprocessor.
| Before | After |
|-----------|-----------|
| {F709419} | {F709421} |
Ref T15056
Test Plan:
1) bin/celerity syntax
2) bin/celerity map
3) bin/cache purge --all
4) sign in
5) go to user's Settings > Display Preferences and select the Accessibility (user interface) "Dark Mode".
6) go to Phriction or Maniphest and add a codeblock.
Example:
``` lang=php
<?php
final class Example extends PhabricatorApplication {
}
```
7) verify that codeblock content is readable
8) also check that with/without this, nothing changes in "normal" theme
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15056
Differential Revision: https://we.phorge.it/D25509
Summary: `./bin/lipsum` allows to fill up a Phorge instance with bogus data for testing. When creating a project, the code creates a project title, project status, random subscribers/members and picks a random project creator, but does not set a project description. Make this code also set a project description (which is implemented as a custom field). Closes T15748 Test Plan: * On the CLI, use `./bin/lipsum generate projects` to create a bogus project. * In the web browser, go to `/project/query/all/` and open the project page of the recently created bogus project: See no project description. * Apply this change. * On the CLI, use `./bin/lipsum generate projects` to create another bogus project. * In the web browser, go to `/project/query/all/` and open the project page of the recently created bogus project: See a random project description. Reviewers: O1 Blessed Committers, 20after4 Reviewed By: O1 Blessed Committers, 20after4 Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15748 Differential Revision: https://we.phorge.it/D25545
Summary: To prevent spammers from abusing this feature on a public server, do not include query parameters in the generated filenames. See <mozilla-conduit@d8bb7d9>. Ref T15665. Test Plan: Download raw diff from a revision and check filename in URL. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15665 Differential Revision: https://we.phorge.it/D25478 Signed-off-by: Zero King <[email protected]>
Summary: This change is probably needed after 892f527. At that time, the unit test was successfully completed probably because of a lack of git rebase master. Test Plan: The unit test PhabricatorCelerityTestCase::testCelerityMaps is not screaming. Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: tobiaswiese, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D25553
…removing assignee Summary: Sometime you just want to flag something as Resolved and keep that task claimed by nobody. But, there are some task statuses that can auto-claim, and "Resolved" is one of these. So, if you "Resolved", Phorge tries to set yourself as claimer. Keeping that "claimed by nobody" is a bit tricky and also generates a confusing warning. In fact, after you "Resolved", you can override the defaults with: - Add Action > Assign / Claim > (nobody) The problem is, on saving, the above action causes this warning: > **Action With No Effect** > One of your actions has no effect: > The task already has the selected owner. > Apply remaining actions? > [ Cancel ] [ Apply Remaining Actions ] That warning "The task already has the selected owner" really means "The task is already claimed by nobody" and, indeed, that is exactly what the user wants. This patch intercepts the above action, and prevents the related confusing "non-effect" warning. Thanks to hard troubleshooting from user https://we.phorge.it/p/aklapper/ See also https://we.phorge.it/D25476 Closes T15164 Test Plan: Task 1 open unassigned: 1. Change Status to Resolved 2. Preview yourself as Claimer 3. Add Action > Assign / Claim, and **set <nothing>** 4. Save and, instead of any confusing warning, only the Status changes. Task 2 open unassigned: 1. Change Status to Resolved 2. Preview yourself as Claimer 3. Add Action > Assign / Claim, keep it as-is 4. Save and, it works as expected (just like before) Task 3 open unassigned: 1. Change Status to Resolved 2. Preview yourself as Claimer 3. Add Action > Assign / Claim, set to somebody else 4. Save and, it works as expected (just like before) Reviewers: O1 Blessed Committers, aklapper Reviewed By: O1 Blessed Committers, aklapper Subscribers: tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15164 Differential Revision: https://we.phorge.it/D25550
Summary: Example cases that were wrongly considered external URLs: - "#foo" - "/foo" Note that Phorge usually does not change stable things. In fact this thing was not documented even inline, and was not even in the unit tests. So this is a fix, and not a breaking change. Probably no one had ever tried it. Closes T15182 Test Plan: The already existing unit tests still work. The added ones make sense. There are no problems doing other random things. Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15182 Differential Revision: https://we.phorge.it/D25555
…nst modern field keys Summary: Sorting a Maniphest search query by custom fields throws a "Map returned omits required key" exception. The `isCustomFieldOrderKey()` check still tested against legacy field key format (for example `[custom:]std:maniphest:deadline.due`) while the code passes modern field key format (for example `custom.deadline.due`). After fixing this, `PhutilTypeSpec::checkMap()` throws an exception when a non-optional (extra) key `$column` is in `$columns` but not in the array of type parameters below to check against: `"Got unexpected parameters: customfield, customfield.index.table, customfield.index.key"` Thus add optional types for customfields in `buildPagingClauseFromMultipleColumns()` to allow them instead of throwing another exception. Closes T15631 Test Plan: * Have a custom field (with search: true) defined via http://phorge.localhost/config/edit/maniphest.custom-field-definitions/ * Have two tasks have the custom field set * Go to http://phorge.localhost/maniphest/query/ , select the custom field (e.g. "Due Date (Reversed)") in the "Order By" field, and click the "Search" button. Iterate through the results via the "Next" button if needed Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15631 Differential Revision: https://we.phorge.it/D25504
Summary:
This is supposed to render in plain text instead of crashing or giving an error, after D25415:
a {{{a}}}
The related unit test is updated accordingly.
We have unnoticed this, because of the well-known bug T15500.
Ref T15372
See D25415
Test Plan:
This makes sense with the introduced functionality in D25415.
Unit tests are happy again.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15372
Differential Revision: https://we.phorge.it/D25559
Summary:
Interestingly Phorge allows to paste a Phorge URL in the search bar to be redirected there...
Now this interesting feature was crashing the whole search engine for simple queries like "asdasd"
with the following exception message:
Refusing to redirect to local resource "asdasd". This URI is not formatted in a recognizable way.
You are affected by this crash after this commit:
328aee0
This specific commit is probably legitimate since "#asdasd" and "/asdasd" are indeed internal URIs,
plus, yes, there are some undocumented and untested cases like "asdasd" that may be considered
internal URIs as well. Or not. But this is just an undocumented corner-case.
In short:
PhabricatorDatasourceURIEngineExtension should not be built over undocumented corner-cases and
should require an absolute URI, with at least a domain.
Note that PhutilURI#getDomain() is always a string and never null, so we can do a strict check.
This fix also involves a micro-optimization to avoid duplicate URI parsing. Since the method was calling
the constructor PhutilURI(string) twice (line 13, line 24). Now just once.
Closes T15763
Test Plan:
Search "lol", no crash anymore.
Copy a random Phorge URL and paste that in your search bar. It still redirects there.
Reviewers: zhe, avivey, O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15763
Differential Revision: https://we.phorge.it/D25561
Summary: The file name is a sufficient source of information, if available. Closes T15729 Test Plan: The new unit test is green. Old unit tests are green. Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: avivey, tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15729 Differential Revision: https://we.phorge.it/D25560
Summary: Fix T15762. There's a complex interaction with project's AutoCapability requiring the membership info to be already loaded, and the Mention rule assuming any project is fully loaded before trying to check permissions. As far as I can tell, the "contextObject" is currently used only to make the mention handle gray if the mentioned user can't see it. Test Plan: Load a Project's hovercard that @mentions a user that can't see the project - no crash. Reviewers: aklapper, O1 Blessed Committers Reviewed By: aklapper, O1 Blessed Committers Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15762 Differential Revision: https://we.phorge.it/D25562
Summary:
Add a documentation link in a specific Config page, that is:
Config > Setting > projects.custom-field-definitions
Comparison example:
| User | People |
|-----------|------------|
|{F1633919} | {F1633920} |
This change is a boring follow-up of this one:
https://we.phorge.it/D25507
Test Plan:
Visit the page and click on the new link:
- /config/edit/projects.custom-field-definitions/
Visit this page that was the inspirational page:
- /config/edit/user.custom-field-definitions/
Check that no nuclear implosion is raised.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25552
Summary: This minimal changes seems the natural expansion of this search function, that "seems" designed to only search with a single value, but the backend is designed to receive an array of possible values. Look at the same method in PhabricatorStandardCustomFieldLink that already works also for array inputs. To get extra confidence look at the method withApplicationSearchContainsConstraint() that works perfectly with arrays (to be honest it only works with arrays in mind...) This feature allows to avoid crashes when extension developers tries to run "IN" queries because for example they try to follow our performance guidelines: https://we.phorge.it/book/contrib/article/n_plus_one/ Closes T15752 Test Plan: Have a nice custom integer field, like this one for Maniphest: { "mycompany.estimated-hours": { "name": "Estimated Hours", "type": "int", "caption": "Estimated number of hours this will take.", "search": true } } You can reproduce T15752 before the change. It just works after the change. Or, just trust your instincts by looking at the same method in PhabricatorStandardCustomFieldLink. Also, use the "normal" frontend search page. Still works as usual. Reviewers: O1 Blessed Committers, 20after4 Reviewed By: O1 Blessed Committers, 20after4 Subscribers: tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15752 Differential Revision: https://we.phorge.it/D25554
…eader Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` ERROR 8192: strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php:527] ``` Closes T15767 Test Plan: See T15767 Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15767 Differential Revision: https://we.phorge.it/D25564
…eMailParser Summary: Add `#[AllowDynamicProperties]` attribute to avoid the deprecation notice. See https://php.watch/versions/8.2/dynamic-properties-deprecated ``` # ERROR 8192: Creation of dynamic property MimeMailParser::$parts is deprecated at [/var/www/html/phorge/phorge/externals/mimemailparser/MimeMailParser.class.php:137] # #0 MimeMailParser::parse() called at [<phorge>/externals/mimemailparser/MimeMailParser.class.php:126] ``` Closes T15765 Test Plan: See T15765 Reviewers: O1 Blessed Committers, avivey Reviewed By: O1 Blessed Committers, avivey Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15765 Differential Revision: https://we.phorge.it/D25563
…mplete without additional hashtag slugs defined Summary: `strlen()` was used in Phabricator to check if a generic value is a non-empty string. This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement. Note: this may highlight other absurd input values that might be worth correcting instead of just ignoring. If phutil_nonempty_string() throws an exception in your instance, report it to Phorge to evaluate and fix that specific corner case. ``` ERROR 8192: strlen(): Passing null to parameter freebsd#1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/project/typeahead/PhabricatorProjectDatasource.php:99] ``` Closes T15761 Test Plan: Go to a task, enter `#` and start typing a project tag of a project which has no additional slugs defined. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15761 Differential Revision: https://we.phorge.it/D25556
Summary: The tab menu in a repository can be a bit confusing. The first tab will be rendered active for two distinctive states: - the home view in the currently selected branch - the browse view in the currently selected branch This diff will give the home view a dedicated extra tab. When switching the branches, we won't switch between home and browse view like it is done now. And I have changed the crumb text for the browse view to "Code" to match it up with the tab name. Additionally the readme is now the first content on the home view. Fixes T15728 Test Plan: Browse in your favorite repos and feel an UX elevation. Reviewers: O1 Blessed Committers, avivey, valerio.bozzolan Reviewed By: O1 Blessed Committers, avivey, valerio.bozzolan Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15728 Differential Revision: https://we.phorge.it/D25538
Summary: Certain web servers (or cache layers such as Varnish) strip the body of GET requests, resulting in the setup check failing when it would otherwise function normally if a request is sent as POST. Test Plan: Phorge will no-longer mark the test as failing due to the body of a GET request being dropped. As per RFC7231, certain implementations of HTTP/1.1 may reject the request or drop the payload due to a previous revision of the spec where the body was supposed to be ignored (RFC2616, section 4.3) Reviewers: O1 Blessed Committers, 20after4, speck, avivey Reviewed By: O1 Blessed Committers, 20after4, speck, avivey Subscribers: speck, 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D25566
Summary: When a column value to export to CSV is empty, `null` is passed to `preg_match()` which is deprecated behavior since PHP 8.1. Thus only call `preg_match()` when the value is set. ``` ERROR 8192: preg_match(): Passing null to parameter freebsd#2 ($subject) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/export/format/PhabricatorCSVExportFormat.php:51] ``` ``` ERROR 8192: preg_match(): Passing null to parameter freebsd#2 ($subject) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/export/format/PhabricatorCSVExportFormat.php:55] ``` Closes T15770 Test Plan: Export a Maniphest task list of query results to CSV. Reviewers: O1 Blessed Committers, 20after4 Reviewed By: O1 Blessed Committers, 20after4 Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15770 Differential Revision: https://we.phorge.it/D25567
Summary: * Properly handle when no mail headers at all can be parsed * Properly handle when mail headers can be parsed but no subject line can be found ``` EXCEPTION: (RuntimeException) Undefined index: subject ``` Closes T15769 Test Plan: See T15769 Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15769 Differential Revision: https://we.phorge.it/D25565
Summary: T15772. Test Plan: Add a `throw` statement in graph calculation, load a task that has a graph, see message. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D25570
Summary: The signature and return type of the `getAllTypesForApplication()` function differed from its docs. Test Plan: Carefully read the code of the function and compare it to the `getAllInstalledTypes()` function above from which the docs were incorrectly copied. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D25572
Summary: Signed-off-by: Yongmin Hong <[email protected]> Test Plan: Checkout https://we.phorge.it/book/phorge/article/arcanist and https://we.phorge.it/book/phorge/article/arcanist_quick_start/ and verify the new clone URL. Reviewers: O1 Blessed Committers, aklapper Reviewed By: O1 Blessed Committers, aklapper Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D25573
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since
maybe it's time to migrate from phabricator to phorge.
I merged the master branch of phorgeit/phorge, with
$fbsd_usernamefix for PHP 8.3Please read carefully phorge's migration guide and don't forget to upgrade storage and restart services.
If the information in clusteradm/phabricator is still valid, please upgrade PHP 8.0 to PHP8.3 and MySQL 5.7 to MySQL 8 or MariaDB due to their EOL state.
If there is any issue or discussion needed for this change, I'm ready to actively discuss and participate in this process.