Problem/Motivation

Drupal 10 is almost on the table at 2022, or has been already in Drupal 9, they said.

Proposed resolution

Identify deprecation etc. Make sure it's compatible to D10 by checking it on upgrade status module.

Remaining tasks

CONTRIBUTED PROJECTS
--------------------------------------------------------------------------------
Crop API 8.x-2.2
Scanned on Wed, 03/02/2022 - 16:55.

4 errors found. 10 warnings found. Avoid some manual work by using drupal-rector
for fixing issues automatically or Upgrade Rector to generate patches.

C:\xampp\htdocs\d9\modules\contrib\crop\src\Events\AutomaticCrop.php:
┌──────────┬──────┬──────────────────────────────────────────────────────────────┐
│ STATUS │ LINE │ MESSAGE │
├──────────┼──────┼──────────────────────────────────────────────────────────────┤
│ Check │ 13 │ Class Drupal\crop\Events\AutomaticCrop extends deprecated │
│ manually │ │ class Symfony\Component\EventDispatcher\Event: since Symfony │
│ │ │ 4.3, use "Symfony\Contracts\EventDispatcher\Event" instead │
│ │ │ │
└──────────┴──────┴──────────────────────────────────────────────────────────────┘

C:\xampp\htdocs\d9\modules\contrib\crop\src\Events\AutomaticCropProviders.php:
┌──────────┬──────┬───────────────────────────────────────────────────────────┐
│ STATUS │ LINE │ MESSAGE │
├──────────┼──────┼───────────────────────────────────────────────────────────┤
│ Check │ 10 │ Class Drupal\crop\Events\AutomaticCropProviders extends │
│ manually │ │ deprecated class Symfony\Component\EventDispatcher\Event: │
│ │ │ since Symfony 4.3, use │
│ │ │ "Symfony\Contracts\EventDispatcher\Event" instead │
│ │ │ │
└──────────┴──────┴───────────────────────────────────────────────────────────┘

C:\xampp\htdocs\d9\modules\contrib\crop\tests\src\Functional\CropFunctionalTest.
php:
┌──────────┬──────┬──────────────────────────────────────────────────────────────┐
│ STATUS │ LINE │ MESSAGE │
├──────────┼──────┼──────────────────────────────────────────────────────────────┤
│ Fix with │ 100 │ Call to deprecated method drupalPostForm() of class │
│ rector │ │ Drupal\Tests\BrowserTestBase. Deprecated in drupal:9.1.0 and │
│ │ │ is removed from drupal:10.0.0. Use $this->submitForm() │
│ │ │ instead. │
│ │ │ │
│ Fix with │ 122 │ Call to deprecated method drupalPostForm() of class │
│ rector │ │ Drupal\Tests\BrowserTestBase. Deprecated in drupal:9.1.0 and │
│ │ │ is removed from drupal:10.0.0. Use $this->submitForm() │
│ │ │ instead. │
│ │ │ │
│ Fix with │ 141 │ Call to deprecated method drupalPostForm() of class │
│ rector │ │ Drupal\Tests\BrowserTestBase. Deprecated in drupal:9.1.0 and │
│ │ │ is removed from drupal:10.0.0. Use $this->submitForm() │
│ │ │ instead. │
│ │ │ │
│ Fix with │ 151 │ Call to deprecated method drupalPostForm() of class │
│ rector │ │ Drupal\Tests\BrowserTestBase. Deprecated in drupal:9.1.0 and │
│ │ │ is removed from drupal:10.0.0. Use $this->submitForm() │
│ │ │ instead. │
│ │ │ │
│ Fix │ 164 │ Call to deprecated function drupal_get_path(). Deprecated in │
│ later │ │ drupal:9.3.0 and is removed from drupal:10.0.0. Use │
│ │ │ Drupal\Core\Extension\ExtensionPathResolver::getPath() │
│ │ │ instead. │
│ │ │ │
│ Fix │ 166 │ Call to deprecated constant FILE_STATUS_PERMANENT: │
│ later │ │ Deprecated in drupal:9.3.0 and is removed from │
│ │ │ drupal:10.0.0. Use │
│ │ │ Drupal\file\FileInterface::STATUS_PERMANENT or │
│ │ │ \Drupal\file\FileInterface::setPermanent(). │
│ │ │ │
│ Fix │ 191 │ Call to deprecated function file_create_url(). Deprecated in │
│ later │ │ drupal:9.3.0 and is removed from drupal:10.0.0. Use the │
│ │ │ appropriate method on │
│ │ │ \Drupal\Core\File\FileUrlGeneratorInterface instead. │
│ │ │ │
└──────────┴──────┴──────────────────────────────────────────────────────────────┘

C:\xampp\htdocs\d9\modules\contrib\crop\tests\src\Kernel\CropUnitTestBase.php:
┌────────┬──────┬──────────────────────────────────────────────────────────────┐
│ STATUS │ LINE │ MESSAGE │
├────────┼──────┼──────────────────────────────────────────────────────────────┤
│ Fix │ 116 │ Call to deprecated function drupal_get_path(). Deprecated in │
│ later │ │ drupal:9.3.0 and is removed from drupal:10.0.0. Use │
│ │ │ Drupal\Core\Extension\ExtensionPathResolver::getPath() │
│ │ │ instead. │
│ │ │ │
│ Fix │ 119 │ Call to deprecated constant FILE_STATUS_PERMANENT: │
│ later │ │ Deprecated in drupal:9.3.0 and is removed from │
│ │ │ drupal:10.0.0. Use │
│ │ │ Drupal\file\FileInterface::STATUS_PERMANENT or │
│ │ │ \Drupal\file\FileInterface::setPermanent(). │
│ │ │ │
└────────┴──────┴──────────────────────────────────────────────────────────────┘

modules/contrib/crop/crop.info.yml:
┌──────────┬──────┬────────────────────────────────────────────────────────────┐
│ STATUS │ LINE │ MESSAGE │
├──────────┼──────┼────────────────────────────────────────────────────────────┤
│ Check │ 0 │ Value of core_version_requirement: ^8.8 || ^9 is not │
│ manually │ │ compatible with the next major version of Drupal core. See │
│ │ │ https://drupal.org/node/3070687. │
│ │ │ │
└──────────┴──────┴────────────────────────────────────────────────────────────┘

modules/contrib/crop/modules/crop_media_entity/crop_media_entity.info.yml:
┌──────────┬──────┬────────────────────────────────────────────────────────────┐
│ STATUS │ LINE │ MESSAGE │
├──────────┼──────┼────────────────────────────────────────────────────────────┤
│ Check │ 0 │ Value of core_version_requirement: ^8.7.7 || ^9 is not │
│ manually │ │ compatible with the next major version of Drupal core. See │
│ │ │ https://drupal.org/node/3070687. │
│ │ │ │
└──────────┴──────┴────────────────────────────────────────────────────────────┘

modules/contrib/crop/composer.json:
┌──────────┬──────┬──────────────────────────────────────────────────────────────┐
│ STATUS │ LINE │ MESSAGE │
├──────────┼──────┼──────────────────────────────────────────────────────────────┤
│ Check │ 0 │ The drupal/core requirement is not compatible with the next │
│ manually │ │ major version of Drupal. Either remove it or update it to be │
│ │ │ compatible. See │
│ │ │ https://drupal.org/node/2514612#s-drupal-9-compatibility. │
│ │ │ │
└──────────┴──────┴──────────────────────────────────────────────────────────────┘

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#18 interdiff_16-18.txt1.6 KBnkoporec
#18 3267287-18.patch93.58 KBnkoporec
#16 interdiff_14_16.txt808 bytess_bhandari
#16 3267287-16.patch91.86 KBs_bhandari
#14 interdiff_12-14.txt387 bytesnkoporec
#14 3267287-14.patch93.19 KBnkoporec
#12 3267287-12.patch93.19 KBnkoporec
#5 interdiff_2-4.txt3.71 KBheddn
#5 3267287-5.patch15.26 KBheddn
#5 interdiff_4-5.txt2.98 KBheddn
#4 3267287-4.patch12.27 KBheddn
#2 crop-api.PNG63.11 KBpradeepjha
#2 d10-compatibility-3267287-2.patch9.02 KBpradeepjha

Comments

pradeepjha created an issue. See original summary.

pradeepjha’s picture

pradeepjha commented
Status: Active » Needs review
StatusFileSize
new d10-compatibility-3267287-2.patch9.02 KB
new crop-api.PNG63.11 KB

Review this patch.
Upgrade status report after applying patch:
upgrade-status

daniel.bosen’s picture

daniel.bosen
he/him
commented 
+++ b/crop.info.yml
@@ -1,6 +1,6 @@
+core_version_requirement: '^8.8 || ^9 || ^10'

Since Drupal 8 is not supported anymore and we cannot run the tests anymore, we might drop the core_version_requirement for 8.x as well.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
StatusFileSize
new 3267287-4.patch12.27 KB
heddn’s picture

heddn
Primary language English
Location Illinois
commented
StatusFileSize
new interdiff_4-5.txt2.98 KB
new 3267287-5.patch15.26 KB
new interdiff_2-4.txt3.71 KB

I noticed I missed an interdiff in there. Here are some more fixes and the missing interdiff.

heddn’s picture

heddn
Primary language English
Location Illinois
commented

I'm not sure why the test failures. I did have to update to use the 9.3 bare fixture since the 8.8 and 9.0 fixture aren't available in drupal 10. Is there something related to that?

daniel.bosen’s picture

daniel.bosen
he/him
commented 
+++ b/tests/src/Functional/UpdatePathTest.php
@@ -13,14 +13,14 @@ class UpdatePathTest extends UpdatePathTestBase {
       __DIR__ . '/../../fixtures/crop-1.0-alpha2-installed.php',

This is propably the reason, why the update tests fail.

crop-1.0-alpha2 cannot be installed on Drupal 9.3, and the core update hooks, that change serial to integer are not run, when starting at Drupal 9.3.

We have to create a new fixture with crop-2.0 installed

heddn’s picture

heddn
Primary language English
Location Illinois
commented

Or remove testing for crop 1.0 alpha2 upgrade.

daniel.bosen’s picture

daniel.bosen
he/him
commented

I think, testing the upgrade path is a good idea, but we can only test from crop 2.0 onwards

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

All crop update functions are older than crop 8.x-2.0 (and most are older than 8.x-2.x-dev), so no, there is no point in keeping those update tests.

What should be done is to add a update last removed hook and then remove the update functions, test and the fixture. Nobody can be on Drupal 9 and haven't yet run those update functions.

Updating fixtures is a lot of work and should only be done if there's a good reason.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Issue tags: +Needs reroll

Patch no longer applies.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Needs work » Needs review
StatusFileSize
new 3267287-12.patch93.19 KB

As @berdir suggested, I removed the update hooks and implemented the hook_update_last_removed, I also removed the test and fixtures.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work
Issue tags: -Needs reroll 
+++ b/src/Events/AutomaticCrop.php
@@ -2,10 +2,10 @@
 
+use Symfony\Contracts\EventDispatcher\Event;
 use Drupal\Core\Image\ImageInterface;
 use Drupal\crop\CropInterface;
 use Drupal\crop\Entity\CropType;
-use Symfony\Component\EventDispatcher\Event;
 

+++ b/src/Events/AutomaticCropProviders.php
@@ -2,7 +2,7 @@
 namespace Drupal\crop\Events;
 
-use Symfony\Component\EventDispatcher\Event;
+use Symfony\Contracts\EventDispatcher\Event;
 
 /**
  * Collects "Automatic crop" providers.

this should use the drupal event classes, see change record for this: https://www.drupal.org/node/3159012

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Needs work » Needs review
StatusFileSize
new 3267287-14.patch93.19 KB
new interdiff_12-14.txt387 bytes

Updated the event classes.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

The last patch only fixes one of the two event classes.

s_bhandari’s picture

s_bhandari commented
Status: Needs work » Needs review
StatusFileSize
new 3267287-16.patch91.86 KB
new interdiff_14_16.txt808 bytes

Hi,

Added a patch for the same. Please review.

Thanks.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Added test runs (reminder to set up default test configuration for issues again), passed before so this will pass again RTBC, just have two minor remarks that could be fixed in an updated patch or fixed on commit (or ignored).

  1. +++ b/tests/src/Functional/CropFunctionalTest.php
@@ -163,11 +163,12 @@ class CropFunctionalTest extends BrowserTestBase {
+
     $file_uri = 'public://sarajevo.png';
-    $file = File::create(['uri' => $file_uri, 'status' => FILE_STATUS_PERMANENT]);
+    $file = File::create(['uri' => $file_uri, 'status' => \Drupal\file\FileInterface::STATUS_PERMANENT]);
     $file->save();

    this should probably be a use statement to not have the inline namespace. 

  2. +++ b/tests/src/Kernel/CropUnitTestBase.php
@@ -113,10 +113,10 @@ abstract class CropUnitTestBase extends KernelTestBase {
       'uri' => 'public://sarajevo.png',
-      'status' => FILE_STATUS_PERMANENT,
+      'status' => \Drupal\file\FileInterface::STATUS_PERMANENT,
     ]);
   }

    same here.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
StatusFileSize
new 3267287-18.patch93.58 KB
new interdiff_16-18.txt1.6 KB

I think its good to do it in the same patch, so attaching an updated patch with fixes from #17.

lisa.rae’s picture

lisa.rae commented
Issue tags: +Drupal 10 compatibility
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Crediting folks.

  • phenaproxima committed ffd9082 on 8.x-2.x 
    Issue #3267287 by nkoporec, heddn, pradeepjha, S_Bhandari, Berdir,...
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Fixed

Reviewed the patch and I didn't see anything objectionable. Sure feels good to remove those update paths, eh! :)

Committed and pushed to 8.x-2.x. Thanks!

solideogloria’s picture

solideogloria commented

Just a note, isn't this the sort of thing that should be done in a new branch, due to being backwards incompatible?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Raising the required core version is not a BC break. And the removed update functions are old enough that it's not possible for anyone to be on an older version when updating to Drupal 10, if someone is still on D8 they can update to 2.2 first.

Status: Fixed » Closed (fixed)

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