Automated Drupal 10 compatibility fixes

Patch #4 works for me without any error in local with Drupal 9 (9.5.7) and Drupal 10 (10.0.3) and php 8.1.

Attaching images.

I created a new merge request.

hi @YahyaAlHamad, what is the reasoning of creating the MR when there is an already working patch available?

I think the composer.lock file does not look at patches. If you try to update to Drupal 10 with this patch then you get composer errors because it expects D8 or D9 in the requirements.

It would be nice if this can be merged in a version we can require with composer.

@Maurice M., you are right, and there is https://github.com/mglaman/composer-drupal-lenient as a workaround solution (instead of using a forked repo that might go away any day)

Ah good to know there is a workaround for forked repo's. Thanks for the info.

Is there a way to use your unmerged commit with composer, with something like: composer require drupal/file_download:dev-1.x#30f53853?

We need a developer of this module to merge your commit, I'm afraid that could take a while.

Well, there are two options:

1. Use the fork with patch already committed via custom repository setup, described here https://www.mediacurrent.com/blog/how-fix-catch-22-problem-drupal-9-fixe...
2. Use normal project from drupal.org + mglaman/composer-drupal-lenient + patch from this issue

@Taran2L Forked repos from issues might go away? So it is not recommended to use them?

Seems are they are not being deleted (at least for now), see this thread in Drupal Slack: https://drupal.slack.com/archives/C51GNJG91/p1683628973141679

drummThey mostly aren’t really using much resources (except for a regression in GitLab at the moment https://gitlab.com/gitlab-org/gitlab/-/issues/408746). We may need to set up automated deletion in the future, something like delete forks for closed issues or with merged MRs after some number of weeks

The patch in #5 does not apply for me against v1.5. There are two failures on the info.yml files. The patch to file_download_counter.info.yml fails because there is no '#' at the start of the version: VERSION line which there is in the repo. The second failure on file_download.info.yml seems to be caused by the patch changing the last line of the info file and presumably there is some difference in either the EOL or EOF markers that mean it fails to apply. Have not found a workaround for this yet.

For projects using composer workflow, this patch should work.

Patch in #17 is the same as #4, except a tiny info.yml change, setting back to RTBC

Hi Everyone , Can you please update when this MR can be merged.

This is an automated patch generated by Drupal Rector. Please see the issue summary for more details.

It is important that any automated tests available are run with this patch and that you manually test this patch.

Drupal 10 Compatibility

According to the Upgrade Status module → , even with this patch, this module is not yet compatible with Drupal 10.

Currently Drupal Rector, version 0.15.1, cannot fix all Drupal 10 compatibility problems.

Therefore this patch does not update the info.yml file for Drupal 10 compatibility.

Leaving this issue open, even after committing the current patch, will allow the Project Update Bot → to post additional Drupal 10 compatibility fixes as they become available in Drupal Rector.

Debug info

This patch was created using these packages:

1. mglaman/phpstan-drupal: 1.1.35
2. palantirnet/drupal-rector: 0.15.1

+1 to RTBC. Works fine for me in Drupal 10. Also updated the MR according to the latest Drupal Rector's changes.

I have applied patch from MR!1. It working fine.

fixed phpcs issue

Fixed drupal coder issue

There are some issues with the patches.

The patch from #26 seems to be missing the core_version_requirement line from the .info.files both in file_download.info.yml and modules/file_download_counter/file_download_counter.info.yml. This gives an error with Upgrade status.

It also gives another error:
Call to deprecated method mimeHeaderEncode() of class Drupal\Component\Utility\Unicode. Deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Use \Symfony\Component\Mime\Header\UnstructuredHeader instead.

I also tested the MR from git.drupalcode.org but that patch does not apply against the latest stable version. The patch from #17 applies fine and seems to be fine in Upgrade status.

Seems like using the 1.x-dev does apply the patch. I am marking this reviewed if the dev version is fine.

There was still another issue after patch #26 from @ajeet_kumar. Thie issue was described by HeikkiY in #28. The file FileDownloadDownloadController.php was using a depricated Unicode library to read Mime headers. The native Drupal File::getMimeType works just fine.

I've combined the numerous patches into a commit and added my fix to the mimeHeaderEncode() bug. I may have inadvertently created some extra branches in this issue fork. If someone is able to delete them, please do so. My commits are made against the main branch of file_download-3297221.

I'm requesting a review.

Here's one single patch that will combine all of the previously contributed Drupal 10 compatibility patches into one patch file. This patch will apply cleanly against the 8.x-1.x-dev branch (aka the 8.x-1.5 branch).

@hyperlinked The merge request was last updated 2 months ago. Are you able to commit your changes there for easier reviewing: https://git.drupalcode.org/project/file_download/-/merge_requests/1#a45f...?.

We are currently using the issue fork in our Drupal 10 testing with the following Composer repository:

{
            "type": "git",
            "url": "https://git.drupalcode.org/issue/file_download-3297221.git"
},

If all the relevant changes would be in that fork, it would make testing the patch easier.

hi everyone,

thanks for working on this, but everything has been fixed in #17, and any further work on this is just duplication of efforts

@HeikkiY, good idea. Done. Someone please review so we can get this merged and not end up accidentally duplicating more effort.

hyperlinked → closed merge request !2

I just became a co-maintainer. I have been reviewing these all changes this week and testing in preparation for that. Some of the patches included out of scope changes that will be addressed in another issue.

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