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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy created an issue. See original summary.

Gábor Hojtsy’s picture

effulgentsia’s picture

The 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 and 10.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 The 0 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 from 10.2.7 to 10.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.

Gábor Hojtsy’s picture

Why not include MariaDB in the version number then? :)

catch’s picture

Won't that fail for 10.2.7+maria?

effulgentsia’s picture

Won't that fail for 10.2.7+maria?

Nope. 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.

xjm’s picture

Priority: Normal » Critical
Issue tags: +Drupal9, +beta target

Very interested in seeing the above test results, lol.

Gábor Hojtsy’s picture

Well all passed :)

catch’s picture

I 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).

daffie’s picture

Status: Needs review » Needs work

For 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.

effulgentsia’s picture

Perhaps another option is to replace the call to version_compare() with something better? Unfortunately, I don't know if Drupal\Component\Version\Constraint would give us the desired behavior, since that was written for a different purpose.

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
2 KB

How about this? This avoids changing the constant, and also documents per #9.

daffie’s picture

Status: Needs review » Reviewed & tested by the community

I 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.

catch’s picture

+1 to #12 from me too.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 79cca07 and pushed to 9.0.x. Thanks!

  • alexpott committed 79cca07 on 9.0.x
    Issue #3119017 by Gábor Hojtsy, effulgentsia, catch, daffie: Tests fail...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.