Page MenuHomePhorge

Update Figlet implementation to be PHP8 compatible

Authored by speck on Apr 28 2023, 19:38.
Referenced Files
Unknown Object (File)
Wed, Apr 10, 19:30
Unknown Object (File)
Sun, Apr 7, 13:09
Unknown Object (File)
Tue, Apr 2, 12:08
Unknown Object (File)
Mon, Apr 1, 10:10
Unknown Object (File)
Mon, Apr 1, 04:26
Unknown Object (File)
Mon, Apr 1, 01:19
Unknown Object (File)
Sun, Mar 31, 19:05
Unknown Object (File)
Sun, Mar 31, 15:25
"Love" token, awarded by valerio.bozzolan.



As of PHP ~v8 the zip_open and associated functions have been deprecated and
removed. The replacement is the ZipArchive API. This updates the figlet
implementation to use this API which has been present in PHP since 5.2.

Additionally in PHP 8 the use of squiggly brackets for indexing into arrays
is also deprecated. This updates to remove two uses of squiggly brackets and
replace with square brackets.

These two deprecations would result in being unable to load differential
revisions in which someone had commented using figlet remarkup.

Imported from:

Closes T15289

Test Plan

Applied these changes to an install and loaded a revision that had comments
where someone utilized figlet remarkup. The revision loaded properly and the
figlet comment rendered properly.

Diff Detail

rP Phorge
Lint Not Applicable
Tests Not Applicable

Event Timeline

Hi @speck :D Do you still love Phorge, isn't it? ihih

Thank you for your beautiful patch in upstream Phabricator. Hoping to be useful, here the same thing, imported in Phorge (yep it already have the fix for the $zip variable)

IMPORTANT: It seems ZipArchive::RDONLY was introduced since PHP 7.4.3


✅ The strict check is correct since it must return true on success

We have not migrated to using Phorge of Phabricator, so issues that crop up which are critical will be updates I submit upstream.


huh, good find, I didn't consider that the argument would not have also existed since ZipArchive was added in 5.2

Some things to consider

  1. PHP < 7.4 is out of support according to
  2. Debian bullseye+ and Ubuntu 20.04+ package manager ships PHP 7.4+
  3. Debian buster and Ubuntu 18.04 package manager ships PHP 7.2
  4. Red Hat 8+ have PHP 7.4+ available in shipped packages (Red Hat 7 does not ship any PHP it looks like)
  5. Fedora project's EPEL registry has packages for 8.2
  6. Rocky Linux appstream includes 7.4+

Uhmm I'm not bold enough to raise PHP 7.4 as minimum version just for this exact line, since probably the workaround here is reasonable readable and minimal. Here an example ↑

do not introduce issue for PHP before 7.4 related to flag ZipArchive::RDONLY

clarify in the documentation that the zip extension is useful (wow, don't know why it was not mentioned before)

Thank you so much again, your patch is really nice and useful.

Hoping to save some time to you I've added a small check to do not introduce any regression for PHP before 7.4, and also added a small note in the documentation that was probably a good-faith omission from our predecessors, and not something introduced with this change.


This revision is now accepted and ready to land.Apr 29 2023, 09:05

Thank you for making the update. I'll do some testing and likely submit this upstream as well.


Yeah that should be a separate discussion. I'll copy that comment into T15047.


Is zip a separate extension for PHP? It seems to be included in the default installation, and there are likely other default extensions that are depended on (maybe openssl)?


Nice question. It seems Debian and Ubuntu ship that component as a separated package installable by sudo apt install php-zip. I think it was installed by default also in my installation since probably it was required by lot of other packages I use.

This is what it currently installs:

Also it seems some people talk about that relationship between class ZipArchive and php-zip package:

Also it seems the same is in CentOS:

So since php-gd is mentioned is probably OK to also mention php-zip to avoid later surprises. At least in the optional section, since we have not a complete overview, seems reasonable

Do you have any practical tip about how to test this?

(Premising I already approved this, so green light from me)

For a basic test additional compressed figlet font files have to be installed. I think it’s under src/support/figlet or src/resources/figlet or similar - pretty sure there’s a readme file in the location indicating it goes there. Then in a comment use remarkup to use figlet and verify it’s using the font you installed. For the zip check presumably you’d need to update php.ini to disable the zip extension.

In D25142#6649, @speck wrote:

Then in a comment use remarkup to use figlet


(I've read this before asking ↓)

Oh I didn't realize it wasn't documented.

figlet (font=font_name) {{{

where font_name is the name of the figlet font you want to use, and can be omitted. results in:

_ _ _ | |__ ___| | | ___ | '_ \ / _ \ | |/ _ \ | | | | __/ | | (_) | |_| |_|\___|_|_|\___/

If it's OK for you, feel free to land :)


I'm not sure if this is necessary. See PhabricatorZipSetupCheck.php which will create a new setup issue if the zip extension is not present.


Uhm. Just to clarify: so if there is a setup check, this should be not documented. Right?

If your answer is "yep" I will remove that above change. Sorry for that.


I don’t feel strongly either way. The setup check guides the user to instruct examples of what doesn’t work if it’s not installed, but doesn’t indicate it’s required.


(Indeed. Just a small note: the mentioned edited line is about "Optional PHP extensions")

Hoping to be useful, without further feedback I will land this in 7 days :)

Just unclear to me if the "Optional PHP extensions: gd, zip" is OK to you. I think yes :) Feel free to land