Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
base system
Priority:
Critical
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
13 Feb 2024 at 15:05 UTC
Updated:
20 Dec 2024 at 06:59 UTC
Jump to comment: Most recent
Comments
Comment #2
catchNot sure if it should be here or in its own issue, but we also need to update hook_requirements() for the various database drivers, and that in turn will require removing gitlab jobs for any newly-unsupported database drivers.
Comment #3
quietone commentedI made a item for 'database drivers' in the parent for the work mentioned in #2. I'd like to keep the database work separate from any generalized INSTALL or README files.
Comment #4
catchI think this is unblocked now.
Comment #5
catchLet's do the database hook_requirements() here. We could use a summary of the new requirements in the issue summary so tagging for that.
Comment #6
quietone commentedUpdate IS and also change component because this is about more than documentation.
Comment #8
quietone commentedComment #9
quietone commentedComment #10
smustgrave commentedComments and title mention a hook_requirements() change also.
INSTALL.txt looks correct though based on the database ticket.
Comment #11
quietone commentedI did not find any hook_requirements that needed a change.
Comment #12
smustgrave commentedIt's not a hook_requirements but believe Drupal\mysql\Driver\Database\mysql\Install\Tasks has to be updated too right?
Comment #13
quietone commentedThe issue for mysql did not discuss MYSQLND or libmysqlclient so those still need to be done.
Comment #14
catchWe haven't changed MYSQLND_MINIMUM_VERSION or LIBMYSQLCLIENT_MINIMUM_VERSION since 2015 when they were introduced afaict, probably worth a follow-up task to discuss whether to bump them, whether they're still needed at all etc. but don't need to touch here I think.
This looks good to me.
Comment #15
quietone commentedMade a followup to discuss the 2 constants in #14. #3437786: Remove MYSQLND_MINIMUM_VERSION and LIBMYSQLCLIENT_MINIMUM_VERSION checks
Comment #16
longwaveWe need to remove some GitLab CI jobs that are now incompatible:
Also, there is a build test failure:
Comment #17
quietone commentedI am pretty sure the failure is due to #3420972: Add testing wtih SQLite 3.45
Comment #18
xjmComment #19
quietone commentedComment #20
daffie commentedThe PR looks good to me.
All the minimum database versions have been correctly updated.
For me it is RTBC.
Comment #21
maks oleksyuk commentedIt might be good to change INSTALL.txt to INSTALL.md, which would improve the readability of the file from the repository page.
Comment #22
quietone commented@Maks Oleksyuk, that work it outside the scope of this issue and as far as I know there is no agreement yet in the community to convert the all .txt files. You can learn more about this topic in the scope guidelines in the Drupal wiki. Cheers.
Comment #23
alexpottAdded a comment to the MR - if it is just a case of applying the suggestion then we can set this back to RTBC.
Also how lovely is it to adjust the CI in the same issue that adjusted the requirements... gitlabci ftw!
Comment #24
alexpottWe need to update MySQL 5.7 documentation links in:
More tasks:
Comment #25
quietone commented\Drupal\mysql\Driver\Database\mysql\Connection::__construct : Changes made but not sure they are correct.
Todo:
\Drupal\Tests\mysql\Unit\ConnectionTest::providerVersionAndIsMariaDb : should also update the MariaDB strings?
We should have a look at the comment in \Drupal\path_alias\PathAliasStorageSchema::getEntitySchema() - maybe it is not specific to MySQL 5.7 - I think this probably deserves it's own followup.
Comment #26
gábor hojtsyComment #27
alexpottI ran the previous commit on the MR against MariaDB which didn't use the different properties to get the transaction isolation level - it was v broken... https://git.drupalcode.org/project/drupal/-/pipelines/163687
Gonna run the new MR against Maria too.
Comment #28
alexpottthis looks great - green on maria now. We need to create a follow up issue about 11.1.1 mariadb and the transaction isolation level query.
Comment #29
gábor hojtsyThe MR updates the docs on SQLite and Postgres but does not seem to make code changes for those? Is that correct?
Comment #30
catchsqlite already has
const SQLITE_MINIMUM_VERSION = '3.45';and pgsql already had16, it was just MySQL not updated (likely due to gitlab not having all the right versions when we made the decision).I made one commit to update a docs link for pgsql, moving back to RTBC.
Comment #31
gábor hojtsyAh I found #3420972: Add testing wtih SQLite 3.45 updated sqlite and PostgreSQL version requirement is changed in the MR. All right.
Comment #32
gábor hojtsyOpened #3445231: MariaDB is deprecating tx_isolation in 11.1.1.
Comment #33
alexpottCommitted and pushed c8e201a167 to 11.x and a86a0294fb to 11.0.x. Thanks!