Problem/Motivation
Per @Mixologic in #3109534: Raise the minimum MySQL version to 5.7.8 and MariaDB version to 10.2.7 in Drupal 9:
I've just finished building the mariadb containers, have them deployed, and you'll see two tests currently running on #94.
The regex in the commit needs some work:
There was 1 failure: 1) Drupal\KernelTests\Core\Action\EmailActionTest::testEmailAction Failed to run installer database tasks: The database server version <em class="placeholder">10.2.7-MariaDB-10.2.7+maria~jessie</em> is less than the minimum required version <em class="placeholder">10.2.7</em>.
So, the mariadb official docker container is where it gets
10.2.7-MariaDB-10.2.7+maria~jessie
So, not sure how many other variants we're going to see out there.
And in two comments later:
Okay, maybe its not the regex. 10.3.22 seems to be working: https://www.drupal.org/pift-ci-job/1608199
But 10.2.7 definitely fails: https://dispatcher.drupalci.org/job/drupal8_core_regression_tests/15035/
I see a lot of already installed exceptions, so, not sure where thats coming from, but I still think it might be the regex.
Proposed resolution
Investigate.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#12 | 3119017-12.patch | 2 KB | effulgentsia |
Comments
Comment #2
Gábor HojtsyComment #3
effulgentsia CreditAttribution: effulgentsia at Acquia commentedThe problem is PHP's version_compare(): "any string not found in this list < dev < alpha = a < beta = b < RC = rc < # < pl = p"
That means that
10.2.7-MariaDB
and10.2.7+maria~jessie
are both less than 10.2.7, because "MariaDB" and "maria" are not in the above list, so are treated as less than dev/alpha/etc.Ubuntu gets around this by prefixing their special builds with "0". For example,
10.1.44-0ubuntu0.18.04.1
The0
before the ubuntu causes that to be evaluated as a number.I don't think we should try to alter what the package gives us. But, we can change the
MARIADB_MINIMUM_VERSION
constant from10.2.7
to10.2.7-foo
, where "foo" can be replaced with any string that's not a number or one of the special strings like "dev", "alpha", etc.Comment #4
Gábor HojtsyWhy not include MariaDB in the version number then? :)
Comment #5
catchWon't that fail for
10.2.7+maria
?Comment #6
effulgentsia CreditAttribution: effulgentsia at Acquia commentedNope. All "any strings not found in this list" values seem to be equal (version_compare() returns 0).
However, because "any strings not found in this list" is < "dev" and "alpha", #4 will allow installation on "10.2.7-alpha1". I think that's ok though: you shouldn't deploy unstable releases of databases in production, and I don't think it has to be Drupal's job to tell you that.
Comment #7
xjmVery interested in seeing the above test results, lol.
Comment #8
Gábor HojtsyWell all passed :)
Comment #9
catchI think it's worth adding a comment explaining #6.
My other question is do we really want to show the meaningless suffix to people in the error message (which is what I assume will happen).
Comment #10
daffie CreditAttribution: daffie commentedFor me we should be fixing the string that we get from the MariaDB and removing the parts that we do not need. Changing the constant by adding the "-MariaDB" is something that happens to works, but is to me very brittle.
Comment #11
effulgentsia CreditAttribution: effulgentsia at Acquia commentedPerhaps another option is to replace the call to
version_compare()
with something better? Unfortunately, I don't know ifDrupal\Component\Version\Constraint
would give us the desired behavior, since that was written for a different purpose.Comment #12
effulgentsia CreditAttribution: effulgentsia at Acquia commentedHow about this? This avoids changing the constant, and also documents per #9.
Comment #13
daffie CreditAttribution: daffie commentedI am still to the opinion that we should remove the "-mariadb" part from the version string. I have stated that in #3109534: Raise the minimum MySQL version to 5.7.8 and MariaDB version to 10.2.7 in Drupal 9 and everybody else did not agreed with me. The solution that has been made in the current patch is to me the best alternative. The documentation of this solution is in order. For me it is RTBC.
Comment #14
catch+1 to #12 from me too.
Comment #15
alexpottCommitted 79cca07 and pushed to 9.0.x. Thanks!