PHP8 upgrade follow-ups - #18
Conversation
timwoj
commented
Sep 25, 2025
timwoj
commented
- Fix a repeated deprecation warning in debug.log related to the last-minute upgrade to Cake 5.4.
- Bump the PHP container to PHP 8.4. We moved up to 8.1.33 in the original upgrade, but it's going to be end of life in December. Upgrading to 8.4 doesn't change anything about the site, so it's safe to just upgrade the containers.
- The Markdown canonifier was rewriting relative links on Github to be absolute links, but this doesn't work for images since it's linking to the page in the repo and not to the actual content. Fix this so it rewrites the links to raw.gtihubusercontent.com instead, which is where the content lives.
- Fix markdown conversion of tables, which I hadn't noticed on a couple of packages. This also sets the Boostrap class on generated tables so they're styled correctly.
This fixes a big block of warnings in the log when running in debug mode
According to https://endoflife.date/php, PHP 8.1 goes EOL in Dec 2025. Might as well jump to the latest version. Nothing seems to change code-wise because of the move.
ckreibich
left a comment
ckreibich
left a comment
There was a problem hiding this comment.
LGTM, just a few questions and perhaps nits re. comment wording.
| if (strcasecmp(parse_url($url, PHP_URL_HOST), "github.com") == 0) { | ||
| /* If the URL passed is to github.com, replace it with | ||
| * with the raw.githubusercontent.com hostname so that it can | ||
| * load the actual image instead of the repo page for the image. |
There was a problem hiding this comment.
This code runs for anything we render, not just images, right? I seem to recall coming in here to figure out how to grab the README.
There was a problem hiding this comment.
Hmmm, yes it does. I guess that could break if someone had a link to something on Github from their README and wanted to link to the GH repo view. It would require changes to the block below that too to keep the /blob/ part of the URL.
| function ($matches) use ($raw_url) { | ||
| if (empty(parse_url($matches[1], PHP_URL_SCHEME))) { | ||
| return "](" . $url . "/blob/main/" . $matches[1] . ")"; | ||
| return "](" . $raw_url . "/master/" . $matches[1] . ")"; |
There was a problem hiding this comment.
I'm just curious here how you came up with this? I can confirm it seems to work e.g. for https://raw.githubusercontent.com/zeek/hello-world/master/README.md (which has main as default branch). The default Github serves up when grabbing the raw page is https://raw.githubusercontent.com/zeek/hello-world/refs/heads/main/README.md.
There was a problem hiding this comment.
I was fighting this with loading one of the images, because it kept telling me it was a 404 when we were rewriting it with main, so I tried the other way around and rewrote it with master. That surprisingly worked instead. Looking at their documentation, they actually special-case master to redirect to whatever your default branch is defined as: https://github.blog/changelog/2020-07-17-links-to-deleted-branches-now-redirect-to-the-default-branch/.
I believe the same exists for refs/heads/master vs just master, but I don't have any documentation to prove that. As for the removal of the /blob/ part, that's only needed if you're looking at the image in the repo view on Github (e.g. https://github.com/esnet/zeek-exporter/blob/master/imgs/func_times.png). If you're looking at the image itself via raw.githubusercontent.com, that part of the URL isn't needed.
ckreibich
left a comment
ckreibich
left a comment
There was a problem hiding this comment.
Thanks Tim. Looks good to me, just a suggestion on the image filename extensions.
|
|
||
| if (strcasecmp($extension, "jpg") == 0 || | ||
| strcasecmp($extension, "jpeg") == 0 || | ||
| strcasecmp($extension, "png") == 0) { |
36090a6 to
fd84989
Compare
2 participants
