@zero-24 i have been working on some layout changes for this page - specifically related to j4 but i could backport them if you want

What kind of layout changes, can you share a POC with us?

The branch is here https://github.com/brianteeman/joomla-cms/tree/sxm31-preupdate

but of course you cant see the a11y changes in the screenshot

hmm i'm not talking about the pre-update checker i'm talking here about the postinstall message in 3 that we include to tell the existing sites they run on a system that 4.0 can't be installed on. ;)

But for the pre-update checker I'm sure a backport would be great too but I think we should first finish the work for 4.0 and than we see what should and can be backported to 3.10 anyway everything is out of scope for this PR as this is against 3.9 where no pre-update checker is installed ;)

Sorry for my confusion. Too much sun today I think

no problem enjoy the sun and SXM :)

I have tested this item 🔴 unsuccessfully on 6a341c2

The Database-check on line 49 returns false which causes it to skip the php-check further down. So which means, if I have a correct database version, but I am running php 5.6 I am not getting a notification that my php version is not supported.

Thanks please check agian @datsepp

Drone still needs some care. :)

I have tested this item ✅ successfully on 40c45d6

Checked every step again and got no problems.

Thanks @datsepp 👍

I have tested this item 🔴 unsuccessfully on 40c45d6

The postinstall message is also shown if on a MySQL database which fulfills the requirement (in my case MySQL 5.7 with MySQLi driver, PHP 7.2 and so on, all requirements for 4.0 fulfilled).

Reason: The regex to check for MariaDB is good for extracting the version number when having a MariaDB, but it is not good for checking if the DB is a MariaDB or not, because the "(mariadb-)?" part of the regex may appear zero ore one time. It is better just to check if the dbversion string contains "Mariadb", like I did it here: https://github.com/joomla-framework/database/pull/166/files#diff-9ba2bd07bed1c63e753d9278ae5e1d31R160

I have tested this item 🔴 unsuccessfully on 40c45d6

The postinstall message is also shown if on a MySQL database which fulfills the requirement (in my case MySQL 5.7 with MySQLi driver, PHP 7.2 and so on, all requirements for 4.0 fulfilled).

Reason: The regex to check for MariaDB is good for extracting the version number when having a MariaDB, but it is not good for checking if the DB is a MariaDB or not, because the "(mariadb-)?" part of the regex may appear zero ore one time. It is better just to check if the dbversio string contains "Mariadb", like I did it here: https://github.com/joomla-framework/database/pull/166/files#diff-9ba2bd07bed1c63e753d9278ae5e1d31R160

@zero-24 Additional info about my test result: You can see in this issue #25245 that the version string coming from db can be "5.5.5-10.1.29-MariaDB".

The regex @mbabker has provided to you will properly extract the version numbers regardless if that mariadb part comes at beginning or end, but it will also match if the version string is the one of a MySQL database, so it can't be used for checking if MariaDB or not, it can really only be used to extract the version when already knowing it is a mariadb.

Therefore it will not help if you change it to using "+" instead of "?" for the "(mariadb-)". As I said, easiest solution is to add a call to instr() for checking if the version string really contains "mariadb" (case insensitive) somewhere.

@zero-24 P.P.S.: Here and in the 4 lines below you can see how it is done in the DB driver, and there it works: https://github.com/joomla-framework/database/pull/166/files#diff-9ba2bd07bed1c63e753d9278ae5e1d31R440.

I don't have time to find it at the moment but in the course of doctrine/dbal#2825 they eventually ended up at the regex used here instead of a more simplified solution of arbitrary stripping 5.5.5- then checking if what was left was a valid version string containing "MariaDB" (case insensitive) anywhere (doctrine/dbal@4f876ef is the diff for that specific change, couldn't find the part of the conversation leading to it). Considering Doctrine gets eyes on it by a wider range of PHP developers, and people I would say are vastly smarter than me, I'm going to assume there is a reason they went with a more in-depth solution than the simple one, which is why I advocate for using their solution.

And TBH looking fresh at the framework code again I'm not sure I 100% like the fact we made the driver strip the MariaDB extra version junk (I prefer giving back the info that the API gathered instead of trying to do arbitrary post-processing, and if consumers really need to they can post-process the data, otherwise you can argue all version strings are simplified down to nothing more than the "typical" <major>.<minor>.<patch>(-<stability>) string, which means stripping more cruft like the examples in #25281).

@mbabker I had checked the dbal code when I did the PR for the framework. They use the regex inside a db specific function for extracting the version string. The detection which db was done somewhere else, also with a stripos like I did.

I think I have posted the links to the dbal code lines in our discussion, but am not sure anymore. But I think we dicsussed it.

That regex works well for extracting the version number, but not for detecting if mariadb or not. You can test it with an online regex tool and the diverse mariadb version strings which can be found in google.

Maybe we should use the dbal regesx to make a safe and proper version number, this should work with any of our supported databases, but we should not use it not to detect the db type. This I still recommend the stripos.

@mbabker When testing the regex with a tool, test also if it matches to a MySQL version string and you will see it matches.

This is in part why I have always advocated for avoiding MySQL drop-in replacement (AKA fork) detection. It's flaky at best because it relies on information in the version string, that is never a reliable platform detection mechanism (and when you have platforms like Percona Server which don't put anything in the version string then it's 100% impossible to detect you're using them).

AFAIK the regex isn't intended to only match MariaDB version strings. It tries to extract a SemVer compliant string out of whatever you give it (and maybe the 5.5.5- and mariadb- prefixes are optional which would be why the regex extracts a version string out of a "pure" MySQL version).

AFAIK the regex isn't intended to only match MariaDB version strings. It tries to extract a SemVer compliant string out of whatever you give it (and maybe the 5.5.5- and mariadb- prefixes are optional which would be why the regex extracts a version string out of a "pure" MySQL version).

Exact. But @zero-24 used it in his 2 PRs to detect if mariadb (does match) or not (does not match), and that's wrong because it matches in both cases.

With everything else I agree with you.

As quick fix I suggest we do it here and in the other PR like in the framework.

But sooner or later we should extract proper major.minor.patch strings, but then also for PostgreSQL.

Merged thanks.

Will test now 😄

I have tested this item ✅ successfully on b77cb35

PostgreSQL (PDO) with too low version => Postinstall message shown.
MySQL with sufficient version => Postinstall message not shown.
Code review ok.

Thanks @richard67 👍

@zero-24 Maybe you should extend your testing instructions by a test that the postinstall message is not shown when all requirements for 4.0 are fulfilled. That is missing, and that explains previous successful test. The unsuccessful test was exactly that case, that it shall not show up when all is ready for 4.0.

updated the instructions. thanks @richard67

I have tested this item ✅ successfully on b77cb35

When the server requirements are not met the message is shown, otherwise it is not shown.

PHP version too low => Postinstall message shown.
PHP version above requirement => Postinstall message not shown.

Thanks @roland-d 👍

Status "Ready To Commit".

Thanks guys! Much appreciated!

Thanks 👍