Travis CI tests on MySQL 8.0 #3372

BenMorel · 2018-12-01T21:49:07Z

Q	A
Type	improvement
BC Break	no
Fixed issues	N/A

Summary

MySQL 8.0 has been out for some time, yet Doctrine DBAL is not tested against this version right now.

Following the rules edicted in #3347 and #3365, PHP 7.1, ~~7.3~~① and nightly are now only tested against the latest version, MySQL 8.0.

① Edit: MySQL 8.0 requires Xenial, and Travis CI does not support PHP 7.3 on Xenial yet. PHP 7.3 tests are therefore kept on MySQL 5.7 for the time being.

morozov · 2018-12-01T22:05:33Z

Referencing #3183.

tests/travis/install-mysql-8.0.sh

BenMorel · 2018-12-01T22:28:18Z

Crap, sorry for the dup, once again.

Ocramius · 2018-12-01T23:55:30Z

Seems to fail to upgrade MySQL. What are your thoughts about running a tagged docker container instead?

BenMorel · 2018-12-01T23:57:54Z

I'm debugging it right now, it should work soon.

BenMorel · 2018-12-02T00:03:48Z

It runs MySQL 8.0 alright, there's just a ConnectionException not thrown because of a MySQL error 2054 (CR_NOT_IMPLEMENTED), which is due to this error:

The server requested authentication method unknown to the client

If I add code 2054 to AbstractMySQLDriver::convertException(), it works, but because this looks like a generic error code for any non-implemented feature, I don't think it's a good idea.

Instead I'm adding default-authentication-plugin=mysql_native_password to my.cnf to fix this error.

BenMorel · 2018-12-02T01:29:42Z

All good now 👍

MysqlPlatform::getListTableIndexesSQL() and getListTableColumnsSQL() are fixed for MySQL 8.0.

Ocramius · 2018-12-02T01:30:33Z

Give it a squash, please

BenMorel · 2018-12-02T01:34:31Z

Squashed!

morozov

@BenMorel please make sure Scrutinizer code coverage configuration reflects the changes.

Majkl578 · 2018-12-02T02:11:13Z

tests/travis/install-mysql-8.0.sh

+echo "Installing MySQL 8.0..."
+
+echo mysql-apt-config mysql-apt-config/select-server select mysql-8.0 | sudo debconf-set-selections
+wget https://dev.mysql.com/get/mysql-apt-config_0.8.10-1_all.deb


Please use Docker container instead of arbitrary 3rd-party debfile. As done in #3183.

I'm not familiar with Docker, however for the record this isn't an arbitrary debfile, this is MySQL's official APT repository.

Given the nature of APT repositories, it's not a good idea to depend on a debfile as debfiles may randomly disappear when they get superseded by newer version.
Also Docker is easier to setup, configure environment, maintain and in the end is more reliable.

Note that MySQL 5.7 is already installed using MySQL's APT repo. If you move to Docker, then this one should be updated as well for consistency.

Majkl578 · 2018-12-02T02:15:10Z

How is this PR better than #3183? Why not finish that one instead of duplicating changes here?

+ fixes issues in MysqlPlatform::getListTableIndexesSQL() and getListTableColumnsSQL() on MySQL 8.0

BenMorel · 2018-12-02T08:38:10Z

@morozov Updated Scrutinizer config and rebased on latest master.

@Majkl578 Sorry, I didn't mean to interfer with your PR, I didn't find it when I opened this one. I did check it afterwards, and saw that it was left in a failing state with debug information. Because I don't have write access, I can't commit to your PR, so I chose to finish mine.

Feel free to merge either PR, I really don't mind!

morozov · 2018-12-02T17:33:08Z

@Majkl578 I think it will be fair to merge this pull request. It gets the job done, and we didn't have any problems with deb-packaged MySQL in the past. We can replace deb packages with Docker images later if needed.

MySQL 8.0 requires Xenial, and Travis CI does not support PHP 7.3 on Xenial yet. PHP 7.3 tests are therefore kept on MySQL 5.7 for the time being.

UPD: if it will work with a Docker image, we should reimplement it. I doubt Travis will ever support PHP 7.3 on Xenial, so if we don't take care of it now, we'll have to rework it later.

morozov · 2018-12-02T17:44:48Z

I'm not familiar with Docker […]

@BenMorel you can try using the bits from #3183. Let us know if you need help. Maybe @Majkl578 or I could help you finish it.

Alternatively, if we rebase #3183 and tests no longer fail there, we can merge it instead. @Majkl578 let me know if you need help rebasing it.

No pressure, but this is the last open ticket for 2.9.0 😄

BenMorel · 2018-12-02T19:27:08Z

I won't be able to help with Docker, as I don't have any incentive right now to learn it.

I suggest that you either:

merge this PR, that at least fixes the issues revealed on MySQL 8.0; maybe @Majkl578 can open another PR to convert it to Docker and make it work with PHP 7.3?
finish @Majkl578's PR, and integrate my fix for getListTableIndexesSQL()

If you're in a hurry for 2.9.0, then maybe the first option could be a good compromise?

morozov · 2018-12-02T19:50:30Z

If you're in a hurry for 2.9.0, then maybe the first option could be a good compromise?

Right now it's not that much of a blocker, so I'd like to get input from @Majkl578 first. Both options look legit to me.

Ocramius · 2018-12-03T00:29:54Z

I'd say let's merge here and then we rebase the other patch, mostly due to this being ready to be shipped.

Based on #3372 (comment).

morozov · 2018-12-03T00:44:09Z

Thank you @BenMorel. Time to put together some release notes.

Majkl578 · 2018-12-03T00:56:49Z

mostly due to this being ready to be shipped

I disagree, Xenial is not acceptable - it's unstable, undocumented and doesn't support PHP 7.3.

Ocramius · 2018-12-03T02:08:14Z

@Majkl578 let's fix it up, then - for now we reached the goal of getting 8.0 tested

BenMorel force-pushed the travis_mysql8 branch from 1da1a9c to 8c030c9 Compare December 1, 2018 21:59

Ocramius suggested changes Dec 1, 2018

View reviewed changes

tests/travis/install-mysql-8.0.sh Show resolved Hide resolved

BenMorel force-pushed the travis_mysql8 branch 9 times, most recently from 9e270ab to 58d2653 Compare December 1, 2018 23:50

BenMorel force-pushed the travis_mysql8 branch from 58d2653 to d5a689b Compare December 1, 2018 23:56

BenMorel force-pushed the travis_mysql8 branch 6 times, most recently from 7b81f2e to e9664f6 Compare December 2, 2018 01:11

BenMorel force-pushed the travis_mysql8 branch from e9664f6 to 8018765 Compare December 2, 2018 01:33

Ocramius previously approved these changes Dec 2, 2018

View reviewed changes

morozov requested changes Dec 2, 2018

View reviewed changes

Majkl578 previously requested changes Dec 2, 2018

View reviewed changes

Majkl578 added CI MySQL labels Dec 2, 2018

BenMorel dismissed Ocramius’s stale review via 4000e07 December 2, 2018 08:27

BenMorel force-pushed the travis_mysql8 branch from 8018765 to 4000e07 Compare December 2, 2018 08:27

Travis CI tests on MySQL 8.0

f7b4177

+ fixes issues in MysqlPlatform::getListTableIndexesSQL() and getListTableColumnsSQL() on MySQL 8.0

BenMorel force-pushed the travis_mysql8 branch from 4000e07 to f7b4177 Compare December 2, 2018 08:33

morozov approved these changes Dec 2, 2018

View reviewed changes

morozov merged commit 8c0bf79 into doctrine:master Dec 3, 2018

BenMorel mentioned this pull request Dec 12, 2018

Travis CI: PHP 7.3 tests on MySQL 8.0 #3405

Merged

morozov self-assigned this Feb 27, 2019

morozov added this to the 2.9.0 milestone Nov 2, 2019

staabm mentioned this pull request Jan 30, 2020

Support mysql8 redaxo/redaxo#3253

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Travis CI tests on MySQL 8.0 #3372

Travis CI tests on MySQL 8.0 #3372

BenMorel commented Dec 1, 2018 •

edited

Loading

morozov commented Dec 1, 2018

BenMorel commented Dec 1, 2018

Ocramius commented Dec 1, 2018

BenMorel commented Dec 1, 2018

BenMorel commented Dec 2, 2018

BenMorel commented Dec 2, 2018

Ocramius commented Dec 2, 2018

BenMorel commented Dec 2, 2018

morozov left a comment

Majkl578 Dec 2, 2018

BenMorel Dec 2, 2018

Majkl578 Dec 2, 2018 •

edited

Loading

BenMorel Dec 2, 2018

Majkl578 Dec 2, 2018

Majkl578 commented Dec 2, 2018

BenMorel commented Dec 2, 2018

morozov commented Dec 2, 2018 •

edited

Loading

morozov commented Dec 2, 2018

BenMorel commented Dec 2, 2018

morozov commented Dec 2, 2018

Ocramius commented Dec 3, 2018

morozov commented Dec 3, 2018

Majkl578 commented Dec 3, 2018

Ocramius commented Dec 3, 2018

Travis CI tests on MySQL 8.0 #3372

Travis CI tests on MySQL 8.0 #3372

Conversation

BenMorel commented Dec 1, 2018 • edited Loading

Summary

morozov commented Dec 1, 2018

BenMorel commented Dec 1, 2018

Ocramius commented Dec 1, 2018

BenMorel commented Dec 1, 2018

BenMorel commented Dec 2, 2018

BenMorel commented Dec 2, 2018

Ocramius commented Dec 2, 2018

BenMorel commented Dec 2, 2018

morozov left a comment

Choose a reason for hiding this comment

Majkl578 Dec 2, 2018

Choose a reason for hiding this comment

BenMorel Dec 2, 2018

Choose a reason for hiding this comment

Majkl578 Dec 2, 2018 • edited Loading

Choose a reason for hiding this comment

BenMorel Dec 2, 2018

Choose a reason for hiding this comment

Majkl578 Dec 2, 2018

Choose a reason for hiding this comment

Majkl578 commented Dec 2, 2018

BenMorel commented Dec 2, 2018

morozov commented Dec 2, 2018 • edited Loading

morozov commented Dec 2, 2018

BenMorel commented Dec 2, 2018

morozov commented Dec 2, 2018

Ocramius commented Dec 3, 2018

morozov commented Dec 3, 2018

Majkl578 commented Dec 3, 2018

Ocramius commented Dec 3, 2018

BenMorel commented Dec 1, 2018 •

edited

Loading

Majkl578 Dec 2, 2018 •

edited

Loading

morozov commented Dec 2, 2018 •

edited

Loading