Meeting will happen in #d9readiness on drupal.slack.com.
| Gábor Hojtsy (he/him) |
Gábor, core committer |
| mradcliffe |
Matthew, mentor. |
| wimleers (he/him) |
@wimleers (he/him), contributor on a bunch of D9 issues. (But may need to go AFK soon, late for Belgian dinner time!) |
| xjm |
xjm, core release manager |
| shaal |
Ofer Shaal, OOTB Umami |
| greg.1.anderson |
Greg Anderson, participating in Composer in Core Initiative meeting, observing. |
| Nick Wilde (he/him) - DC 2020 DevOps Session Team |
Nick Wilde he/him, :flag-ca: in a work meeting so lurking |
| Alona Oneill |
Alona, taking notes for weekly blog |
| mixologic |
Mixologic, Ryan Aslett, From the DA/Drupal Infra |
| mikelutz |
I’m here, I’m here, we can start now. :stuck_out_tongue: |
| larowlan |
Lee, |
| catch |
Nat, core committer, joining late. |
| Gábor Hojtsy (he/him) |
@wimleers (he/him) has done some research into all the various related issues and now we need review and additional feedback. |
| Gábor Hojtsy (he/him) |
#3043471: Replace the DiactorosFactory message factory in symfony/psr-http-message-bridge with a PSR-17 compliant message factory |
| Gábor Hojtsy (he/him) |
#3045483: Incompatibility between zend-diactoros and psr-http-message-bridge versions: require symfony/psr-http-message-bridge >=1.1.2 |
| Gábor Hojtsy (he/him) |
#3047294: Switch to symfony/http-client |
| Gábor Hojtsy (he/him) |
#3039047: Adopt php-http/guzzle6-adapter 2.x to get PSR-18 support without losing Guzzle's async support |
| wimleers (he/him) |
That second issue (3045483) landed. |
| wimleers (he/him) |
That third issue (3047294) was closed as a duplicate |
| xjm |
The TLDR is, it looks like Guzzle will provide the coverage we need in D9 |
| xjm |
So we plan to retain Guzzle as a Drupal 9 dependency |
| wimleers (he/him) |
The first (3043471) and fourth (3039047) are blocked on PHP 7.1 being required. |
| xjm |
But since Guzzle's PSR-7 implementation is not complete, we'll also keep Diactoros |
| xjm |
(And not adopt symfony/http-client at this time) |
| wimleers (he/him) |
(I’ll let Jess cover the rest, she has the more complete perspective!) |
| Gábor Hojtsy (he/him) |
if PHP 7.1 requirement is a blocker, when do we expect to land those? |
| xjm |
As for implementing PSR-18, that's not a requirement for D9's release, but if we did so in D9 we could/should probably use the existing Guzzle adapter if Guzzle did not release the feature addition or Guzzle 7 in time to do so |
| xjm |
8.9 may or may not require 7.1, but 9.0 definitely will |
| xjm |
But they're not required for 8.8 as there's nothing to deprecate (and changing our interfaces is a BC break) |
| wimleers (he/him) |
Correct, those two issues blocked on PHP 7.1 are for adopting PSR-17 and PSR-18 respectively. |
| wimleers (he/him) |
Adopting new PSRs = new features |
| wimleers (he/him) |
And it’s something entirely optional, that Drupal can choose to do in any minor |
| wimleers (he/him) |
(or major ofc) |
| wimleers (he/him) |
As @larowlan pointed out: Drupal does not need to adopt every PSR. |
| wimleers (he/him) |
We could even wait until PSR-18's successor with async support lands. We could choose not to wait. It’s definitely not urgent. |
| mikelutz |
No, maybe, yes, yes, yes and yes. :stuck_out_tongue: |
| mikelutz |
Kidding, reading up, I think Jess hit it all. Guzzle is good for D9, and if we want to implement -17 or -18, we will do it in D9, but D9 shouldn’t be blocked on either. |
| wimleers (he/him) |
Exactly :slightly_smiling_face: |
| andypost |
One more PSR-related is classloader [#2023325]probably it needs separate topic |
| Gábor Hojtsy (he/him) |
Drupal 9 will require at minimum 7.2 as per #3079791: Bump Drupal's minimum PHP version to 7.2 as soon as 9.0.x is branched (a higher version may be required later) |
| Gábor Hojtsy (he/him) |
discussion / data gathering ongoing in #2917655: [9.4.x only] Drop official PHP 7.3 support in Drupal 9.4 to raise that further and also about what to do in Drupal 8 (LTS) |
| xjm |
Or rather, 7.2 is the minimum minimum :slightly_smiling_face: |
| xjm |
It may require 7.3 or higher as well but TBD |
| wimleers (he/him) |
Ohhhh! Wow! |
| mixologic |
php 7.4 lands in November 2019 right? are we targeting 8.8.0 to be 7.4 compatible? (and thus 8.9/9.0) ? |
| xjm |
We would, yes |
| xjm |
But dependent on the 7.4 env being more than "build successful" I think? Does anyone know of a 7.4 issue being open as yet? I can't remember if I saw one or not |
| chx (he/him) |
Raising the visibility of #3079791: Bump Drupal's minimum PHP version to 7.2 as soon as 9.0.x is branched (a higher version may be required later)#comment-13259317 more. This is important. Bumping to 7.3 needs more testing (real life, not just automated because of existing data concerns) than usual. |
| chx (he/him) |
Again: I am not against 7.3, heck, I want it (because of https://www.php.net/manual/en/migration73.new-features.php#migration73.n...) but it scares me. |
| Gábor Hojtsy (he/him) |
@xjm the SF 4.4 patch was certainly raising PHP 7.4 deprecations /cc @mikelutz |
| xjm |
There are other complications with 7.3 as well; at least one of the distro LTSes IIRC |
| xjm |
Anyway the above issue's scope is more limited |
| mikelutz |
Last I knew, there were depreciation errors in 7.4 that we haven’t fixed yet. They aren’t SF4 specific though, as far as I know. |
| mikelutz |
These are E_DEPRECATED, not E_USER_DEPRECATED. |
| Gábor Hojtsy (he/him) |
yeah I’ve only seen them in #2976394: Allow Symfony 4.4 to be installed in Drupal 8, but looks like we need a dedicated issue for 7.4 yup |
| mikelutz |
I thought there was an issue open, but I don’t see it right now. |
| chx (he/him) |
while going 7.4 and using https://wiki.php.net/rfc/custom_object_serialization in theory could solve the 7.3 issue i believe that's a no-go because we want contrib modules to be compatible with d8 and d9 and so we can't have 7.4 only features even in d9 that can affect contrib |
| chx (he/him) |
really, how this will work for any php version? |
| chx (he/him) |
if d9 uses , say, an interface, not found in earlier php versions , that'll break the desired compatibility wouldn't it? |
| mixologic |
I dont believe the goal is to have contrib modules be compatible with d8 and d9 in perpetuity. The goal is to allow for those modules to work with both when it comes out. At some point in the future some contrib modules are going to be d9 only. |
| chx (he/him) |
So we can bump the php requirements but we need to be super duper careful about using new features there (edited) |
| mikelutz |
Depends on how it’s used. There are BC ways to manage the serialization that will work. We can’t use php 7.4 only features until we require at least php-7.4, but we can write code that runs under 7.2-7.4 easily enough. |
| mikelutz |
Right, but that’s the same as every new php version. |
| mixologic |
In any case the php7.4 container is unable to run because we're using a backported version of typo3/phar. That looks like its being addressed here: #3039611: Update core PHP dependencies for 8.8.x |
| alexpott |
Our current PHPunit version is not PHP 7.4 compatible and it is unlikely it ever will be. We need to get #2950132: Support PHPUnit 7 optionally in Drupal 8, while keeping support for ^6.5 in and then think. |
| Gábor Hojtsy (he/him) |
What should happen to these in Drupal 8 and 9? |
| Gábor Hojtsy (he/him) |
@chx (he/him) raised this in the issue |
| chx (he/him) |
Oh yes, I did. And the need to split the MySQL and MariaDB drivers eventually as they continue to diverge. |
| mradcliffe |
Related issues #2846994: Increase minimum version requirement for Postgres to 10 and require the pg_trgm extension (postgresql to at least 9.5 for JSONB support), #2846366: Improve Drupal's Database Abstraction Layer Extensibility and Capabilities (idea thread for dbtng improvements), #3080133: Add a way for a driver to detect features/capabilities supported in the connection or driver (for implementation of a pattern for capability/feature/platform detection) |
| mradcliffe |
Also some discussion on the latter pointed out by mondrake in [#12309523]. |
| mradcliffe |
@chx (he/him), if you had any thoughts about capability detection and whether to implement something similar to doctrine/dbal in that regard or something new, I think that would be key to being able to bump up versions in Drupal 9? |
| chx (he/him) |
the biggest deal in mysql 8 is transactional ddl imo and that's not feature detectable imo |
| mradcliffe |
Yeah, it seems like there are different levels of detection - initial version / driver initialization, capability detection after the connection (hopefully inexpensive). Things like postgresql extensions could be detected after the connection is open that might change the way the driver behaves with regard to case sensitivity |
| mradcliffe |
It's hard to start on this without thinking about refactoring DBTNG entirel. |
| chx (he/him) |
yeah but going full on doctrine or something would be a little bit more of a bc break than our current policies warrant, wouldn't it? Also, it would be a bit of an effort rewriting every query out there. Although by now there should be a bit less with entity api and all that goodness but still. |
| mradcliffe |
Agree. |
| mradcliffe |
Cleaning up the code to be a bit more readable. There's a lot of protected / undocumented methods in Connection implementations :disappointed: |
| mradcliffe |
But maybe implementing something similar to doctrine's platform detection. Maybe a driver resolver? |
| andypost |
JSONB is more related to https://www.drupal.org/project/drupal/issues/839444 |
| alexpott |
wrt to database version updates with really need to get #2986452: Database reserved keywords need to be quoted as per the ANSI standard done so new db versions can add new keywords without us having to rename tables and columns |
| andypost |
@alexpott but this one can cause a mysql split instead of feature detection |
| alexpott |
@andypost “mysql split”? i don’t grok that |
| andypost |
@alexpott yes, split for mariadb and mysql8, irc |
| alexpott |
I still don’t understand how the patch will cause a split - MySQL8 and MariaDB have already kind of split - there are different keywords and features. |
| andypost |
@alexpott I mean this #2985788: Add a separate MariaDB driver I mixed issues |
| alexpott |
Ok sure but this patch doesn’t make doing that more or less likely - right? Only makes it slightly simpler because we don;t have to worry about the different keywords |
| Gábor Hojtsy (he/him) |
Drupal 8.8 is the deadline for #2873705: Add change records to all @deprecated code |
| Gábor Hojtsy (he/him) |
and #2856744: [META] Add trigger_error(..., E_USER_DEPRECATED) to deprecated code |
| xjm |
Not just docummenting, also triggering errors |
| xjm |
I.e., tests will not raise fails for things deprecated before Dublin until they're patched (edited) |
| Gábor Hojtsy (he/him) |
(updated topic title) |
| xjm |
So given that we want our deprecations to be complete before 8.8.0-beta1, these issues are becoming kind of urgent |
| xjm |
They're also great novice contributor tasks |
| xjm |
And great for sprints since you can work on them well in groups |
| mradcliffe |
:+1: |
| mikelutz |
As far as triggering errors go, the 10 or so that are left really aren’t novice. |
| xjm |
We've been nibbling away at them for a couple years now but there's also still a lot unaddressed |
| mradcliffe |
I'll triage whatever is left for Amsterdam :fingerscrossed-skin-tone-3: |
| mikelutz |
but documentations and CRs certainly are. |
| xjm |
Noted :slightly_smiling_face: |
| mikelutz |
most of the ones that are left really are just code that needs to be undeprecated. |
| mikelutz |
But I can put it on my list to do a runthrough and bump what’s left. |
| xjm |
If anyone has ideas or energy to help with the docs issue, as well, that would be great. It has very specific instructions, but no summary of what's outstanding |
| xjm |
We could promote it heavily so long as we have reviewers to check the CRs linked are accurate |
| xjm |
(and not dupes) |
| mikelutz |
2856744 should probably be closed as a duplicate of #3038170: Drupal core's own deprecation testing results no? |
| xjm |
I don't think they're the same? |
| xjm |
One is about removing usages, the other is about triggering errors for things regardless of whether they're used by core or not |
| xjm |
Even if core has no deprecated usages left, we still need to trigger the errors for contrib and custom code (or undeprecate them as you've said) |
| Gábor Hojtsy (he/him) |
Also if its not triggered/documented, then we are not finding usages of it :wink: (edited) |
| mikelutz |
If core doesn’t use them, they won’t show up in phpstan, I’m not sure how we can find them all. |
| mikelutz |
still you are correct, it’s a separate issue for sure, as the issue I linked is only for deprecations caught by phpstan because we are using them. |
| xjm |
So going back, does that mean that "those left aren't novice" wasn't about adding missing @trigger_error()? So someone needs to investigate that as well |
| xjm |
(And it should be novice once they're identified since it's just a matter of putting the errors in the right codepath) |
| andypost |
I feel it like 8.9 target because 9 is about only removal, so Amsterdam is great on trying to remove remains (9.0.x) and separate task to trigger for 8.8... |
| andypost |
Required test to proof the deprecated is most of time totally not a newcomer's task, that's why db_* was so hard to get commited, I bet @mikelutz knows much more cases |
| mikelutz |
@xjm Adding @trigger_errors to @deprecated code that core doesn’t use is novice. Identifying such code is not particularly straightforward though. |
| xjm |
Right, I thought Mile_23 had some work on the issue for that (edited) |
| xjm |
But can be done with a search-ahead regex or similar |
| xjm |
Or a first pass is even just checking for files with deprecations but no trigger_error |
| xjm |
Novice doesn't necessarily mean trivial |
| mikelutz |
Yes, looks like he had a basic scan done. |
| Gábor Hojtsy (he/him) |
In the theme of 6️⃣ , #3038170: Drupal core's own deprecation testing results is also getting really close to done :slightly_smiling_face: (edited) |
| Gábor Hojtsy (he/him) |
3 of its 9 remaining issues in the issue summary landed :slightly_smiling_face: (edited) |
| Gábor Hojtsy (he/him) |
the remaining 6 need reviews :slightly_smiling_face: |
| Gábor Hojtsy (he/him) |
two of them are also undeprecation :wink: |
| Gábor Hojtsy (he/him) |
sent it for a run again to see where we are now |
| mikelutz |
Yes, this is the proper thread for what I was discussing in the other thread. |
| andypost |
@Gábor Hojtsy (he/him) please fix link |
| Gábor Hojtsy (he/him) |
done, sorry @andypost |
| mikelutz |
@Gábor Hojtsy (he/him) I updated this, 15 errors, 7 unique. One points to a fixed issue, but there is still an instance of the deprecated class constant being used in a non-legacy test, so I’ll have to spin off a new issue for it. |
| Gábor Hojtsy (he/him) |
Hah, little sneaky one :) |
| mikelutz |
yeah, nothing that phpunit could have caught, but easy enough to update. I’ll go ahead and blame @berdir :stuck_out_tongue: |
| mikelutz |
And I’ll leave this here in case he sees his name and comes looking later… https://git.drupalcode.org/project/drupal/blob/8.8.x/core/tests/Drupal/T... |
| catch |
Is there an issue to remove core's own usages of deprecated global constants? I know there are individual issues but a meta? |
| mikelutz |
I don’t believe so. I can’t find one. |
| andypost |
I bet it mostly every second issue moving some constant to class or interface |
| Gábor Hojtsy (he/him) |
There’s been lots of work recently by @amitgoyal, @joshi.rohit100, @Pranit et. al. to add Drush 9 support and make the transformations produce Drupal 9 compatible results |
| Gábor Hojtsy (he/him) |
I would say a new point release is close here that would be producing Drupal 9 ready code at least in the transformations it executes :smile: |
| Gábor Hojtsy (he/him) |
they also made the test suite green with lots of work :slightly_smiling_face: |
| Gábor Hojtsy (he/him) |
and are looking into the possibility to write new transformations with rector |
| Gábor Hojtsy (he/him) |
because the underlying transformation library Pharborist has been abandoned 5 years ago, right after DMU adopted it pretty much |
| Gábor Hojtsy (he/him) |
but it still works miraculously :smile: |
| Gábor Hojtsy (he/him) |
unfortunately due to conflicts of dependencies, rector cannot be added to a Drupal instance without #2950132: Support PHPUnit 7 optionally in Drupal 8, while keeping support for ^6.5 being resolved |
| larowlan |
@Gábor Hojtsy (he/him) grom358 (Author of pharborist) lives in same city as me, still active at tech meetups, if there's anything it needs, I can ask him to consider merge requests or ownership transfer |
Comments
Comment #2
gábor hojtsyComment #3
mradcliffeI added these to my calendar, and am going to try to be available for this to help with DrupalCon Amsterdam planning for novice issue triage.
Comment #4
xjmLet's discuss Guzzle, Diactoros,
symfony/http-client, and PSRs-7, -17, and -18. @wimleers has done some research into all the various related issues and now we need review and additional feedback. Here are the issues:Comment #5
xjmWe should also discuss these two still-outstanding issues in light of the upcoming deadline for 8.8.x:
Having these outstanding will negatively impact the continuous upgrade path, so we should investigate what's still outstanding in each and encourage work on them. We should discuss whether deprecations missing a
@trigger_error()as of 8.8.0-beta1 should still be removed in 9.0.x or whether they should be retained until D10.Comment #6
ghost of drupal pastMuch like PHP 7.2 ends support a few months after so does MySQL 5.7 and MariaDB 10.1 (Oct 2020). If we required MySQL 8 / MariaDB 10.2 then we could use window functions (YAY!) and introduce powerful storage options based on JSON which would be support by all of our databases. Or, is it possible to raise DB requirements in say 9.1 if such storage gets written only by then? Should we raise to MySQL 8 then we would need to discuss transactional DDL which is supported by PostgreSQL and MySQL 8 but not yet by MariaDB. As the two diverge we will need to eventually split the database drivers, the only question is when. Is there an issue where these things are discussed?
Comment #16
gábor hojtsySaved the meeting log. Thanks Slack for expanding non-profit's free accounts to standard.
Comment #17
mikelutz