Skip to content

Add MariaDB to the Github actions #10062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grooverdan
Copy link
Contributor

@grooverdan grooverdan commented Dec 7, 2022

Last year I made https://mariadb.org/new-service-quay-io-mariadb-foundation-mariadb-devel/ for consumption by CI/test systems.

Recently this was updated with the tags

  • earliest - the MariaDB earliest supported version
  • latest - the MariaDB latest stable version
  • verylatest - the MariaDB last development version

Each instance test is ~2.5 minutes extra to the test runs.

Hopefully (or very soon) I'll have all test fixes submitted.

@grooverdan
Copy link
Contributor Author

Failures outstanding in MariaDB:

  • mysqli_store_result() [ext/mysqli/tests/mysqli_store_result_copy.phpt] also odd getting a NULL result without warnings from version() into a uservar via procedures.
  • MySQL PDOStatement->nextRowSet() [ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt] - having trouble wondering if the test is doing something meaningful as the server (very oddly) is casting up making the uservar a longblob (medium-blob in mysql), so the warning/null seems like an improvement.
MariaDB [test]> delimiter |
MariaDB [test]> CREATE PROCEDURE p(OUT ver_param VARCHAR(25)) BEGIN SELECT VERSION() INTO ver_param; END;|
Query OK, 0 rows affected (0.001 sec)

MariaDB [test]> delimiter ;
MariaDB [test]> CALL p(@VERSION);
Query OK, 1 row affected (0.000 sec)

MariaDB [test]> SELECT @VERSION as _version;
Field   1:  `_version`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       LONG_BLOB
Collation:  latin1_swedish_ci (8)
Length:     16777215
Max_length: 15
Decimals:   39
Flags:      


+-----------------+
| _version        |
+-----------------+
| 10.3.38-MariaDB |
+-----------------+
1 row in set (0.000 sec)

MariaDB [test]> select version();
Field   1:  `version()`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       VAR_STRING
Collation:  latin1_swedish_ci (8)
Length:     15
Max_length: 15
Decimals:   39
Flags:      NOT_NULL 


+-----------------+
| version()       |
+-----------------+
| 10.3.38-MariaDB |
+-----------------+
1 row in set (0.000 sec)

I did mean to put mariadb tests after the main test in the workflow for pulls.

@iluuu1994
Copy link
Member

What value does this really provide for us though?

@iluuu1994
Copy link
Member

@kamil-tekiela WDYT? Is there any benefit to testing MariaDB specifically? Testing 3 versions seems like overkill to me.

@grooverdan
Copy link
Contributor Author

What value does this really provide for us though?

Like the COMMUNITY builds in nightly, adding CI against MariaDB stable branches ensure that by the time the php and MariaDB releases happen, regardless of timing, that compatibility is there for our mutual user-bases.

This gives us the opportunity to fix accidental incompatibilities in product or test cases before users even notice.

Testing 3 versions seems like overkill to me.

Maybe, but its what the users are using or a soon going to and only covers the mysqli/pdo_mysql components. The MariaDB is also pre-built. I did try to keep it minimal, though more could be minimized for nightly only and not push hence proposing a draft, and seeing what everyone here thinks.

Compared with how WordPress is tested against many PHP versions 3 is quite low.

I do have a MariaDB ci against PHP but I lost attention and now I'm struggling to get mainly test case fixes back in. The lack of working test cases resulting in MariaDB developers ignoring which I'm trying to change.

Every now and then warning might be slightly differently worded or result in a different from between MariaDB branches or MySQL versions so looking at a few branches is prudent.

@kamil-tekiela
Copy link
Member

kamil-tekiela commented Dec 7, 2022

I think running tests against the latest MariaDB version makes a lot of sense. I don't think we need three versions though.
It's possible for MariaDB to have new optimizations or changes to the protocol and we need to be aware of.

@grooverdan
Copy link
Contributor Author

I think running tests against the latest MariaDB version makes a lot of sense. I don't think we need three versions though.

If I had to choose 1 I'd go for the latest stable. Any preferences around the push vs nightly?

It's possible for MariaDB to have new optimizations or changes to the protocol and we need to be aware of.

I've been meaning to keep more on top of this for all the external connectors however, as expected, we've aimed for nothing incompatible.

Capability headers defines to match to ext/mysqlnd/mysqlnd_enum_n_def.h.

What there is that might be useful is:

@grooverdan grooverdan force-pushed the ci_mariadb branch 2 times, most recently from 8bc8562 to a8c5eb9 Compare December 12, 2022 00:16
Run the mysqli and pdo_mysqli tests on MariaDB.

The :latest tag is the latest stable version of MariaDB and used
in the push workflow.

The :verylatest tag is the last in development version of MariaDB
tested in the nightly flow.

The container quay.io/mariadb-foundation/mariadb-devel are the completed
works on development on the major branches.

We are testing both to ensure that neither PHP or MariaDB are breaking
the protocol contract, and if we do, either MariaDB or PHP can fix this
before this change ends up in  a release.
@iluuu1994 iluuu1994 changed the base branch from PHP-8.1 to master March 8, 2024 11:35
@iluuu1994
Copy link
Member

@SakiTakamachi Can you have a look at this PR sometime? You have mentioned previously that adding a build for MariaDB may be useful. I don't think we need one on-push, but nightly is certainly sensible.

@grooverdan Sorry for the slight delay. ^^

@SakiTakamachi
Copy link
Member

@iluuu1994

of course. I have already used this PR as a reference and tested it in my repository.

As a result, there were some tests that were failing, so I'm working on addressing them.

@grooverdan
Copy link
Contributor Author

Hi @SakiTakamachi , found SakiTakamachi@9648d18 which is great, since I was having trouble getting to a minimal github actions configuration (trying too hard to not repeat a compile for MySQL and MariaDB).

Since github actions as best I could find out still don't support a command for their image containers, the following is what I used to get addition tests to run successfully.

https://github.com/MariaDB/buildbot/blob/8e0bdf006ef163330a95e15fdb00fed08ae9d73a/dockerfiles/ecofiles/test-php.sh#L23

# variable needed to make mysqli_expire_password test pass
# only exists on 10.4+
/usr/local/mariadb/bin/mysql -u root -e '/*M!100403 set global disconnect_on_expired_password=1 */' 

For pam tests:

Install pam MariaDB server module:

INSTALL SONAME 'auth_pam'

https://github.com/MariaDB/buildbot/blob/8e0bdf006ef163330a95e15fdb00fed08ae9d73a/dockerfiles/ecofiles/test-php.sh#L26

Create pam service:

for t in auth account; do echo "$t required pam_unix.so audit"; done >> /etc/pam.d/mysql
# password: pamtest, but needed to be passed in crypt format otherwise silently ignored
useradd -m pamtest --password '$6$HGAoutbdknZeXJOb$1sQ5xzaCC0KUmc10FeVUFZSS1LbhoI/1hEEhaqe7zLLINGfnq7tz1lqjbXenIiNwe5m9TKGs4Lx68tQ/lrO9A1'

https://github.com/MariaDB/buildbot/blob/main/dockerfiles/eco-php-ubuntu-2004.dockerfile#L51-L5

These could be done as a step before running the tests.

@SakiTakamachi
Copy link
Member

Thanks, indeed, that test was skipped. However, if that is the case, MySQL may also be skipped...
I'll check the status of the test.

@grooverdan
Copy link
Contributor Author

Thanks, indeed, that test was skipped. However, if that is the case, MySQL may also be skipped... I'll check the status of the test.

Yep, I added the skip criteria (which seems a long time ago now).

MySQL Pam is an enterprise product for them.

A way to enable SSL is:
https://github.com/blackbeam/mysql_async/blob/master/azure-pipelines.yml#L180-L183
using https://github.com/blackbeam/rust-mysql-simple/tree/master/tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants