Sticking to dedicated issues per the existing ones.

Replaces calls to file_temp_directory() and file_scan_directory() as well as two method calls that moved away from FileSystem to StreamWrapperManager.

A few calls should be injected, but that would conflict with the constructor issue, and I need a patch that I can combine :)

CommentFileSizeAuthor
#15 3121005-15.patch891 bytesaspilicious
#11 3121005-11.patch13.91 KBjrockowitz
#10 webform-file-system-3121005-10.patch13.97 KBrecrit
#6 webform-file-system-3121005-6-interdiff.txt1.55 KBberdir
#6 webform-file-system-3121005-6.patch14.53 KBberdir
#2 webform-file-system-3121005-2.patch14.05 KBberdir

Comments

Berdir created an issue. See original summary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new webform-file-system-3121005-2.patch14.05 KB

Status: Needs review » Needs work

The last submitted patch, 2: webform-file-system-3121005-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Ah, that's a change in the API, it needs a valid directory as argument, so that apparently needs to be checked first.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: [Drupal 8.8] Replace use of deprecated file functiosn » [Drupal 8.8] Replace use of deprecated file functions
Status: Needs work » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: +Drupal 9 compatibility
StatusFileSize
new webform-file-system-3121005-6.patch14.53 KB
new webform-file-system-3121005-6-interdiff.txt1.55 KB

Rerolled and fixed the test fails.

jrockowitz’s picture

jrockowitz commented

Ideally, when possible, I would like to use dependency injection instead of \Drupal::service('file_system').

jrockowitz’s picture

jrockowitz commented
Status: Needs review » Postponed

After rereviewing the patch is seems fine but I need to postpone it until on Drupal 8.8.x is supported.

jrockowitz’s picture

jrockowitz commented
Title: [Drupal 8.8] Replace use of deprecated file functions » [Drupal 8.8.x] Replace use of deprecated file functions
recrit’s picture

recrit commented
StatusFileSize
new webform-file-system-3121005-10.patch13.97 KB

rerolled against the latest 5.x-dev.
The webform-file-system-3121005-6.patch would not apply for me so I could not get an interdiff.

jrockowitz’s picture

jrockowitz commented
Status: Postponed » Needs review
StatusFileSize
new 3121005-11.patch13.91 KB

  • jrockowitz authored 7dfa757 on 8.x-5.x 
    Issue #3121005 by Berdir, jrockowitz, recrit: [Drupal 8.8.x] Replace use...
jrockowitz’s picture

jrockowitz commented
Status: Needs review » Fixed
aspilicious’s picture

aspilicious commented
Status: Fixed » Needs work

My project update failed because there is a "file_scan_directory" left in "webform_update_8192".
I also found "file_unmanaged_delete" in the same function.

aspilicious’s picture

aspilicious commented
Status: Needs work » Needs review
StatusFileSize
new 3121005-15.patch891 bytes
aspilicious’s picture

aspilicious commented

The update still fails:

May 26 21:50:55 website-drupal-script-21-4xqp9 website-drupal-script-21 notice >  [notice] Update started: webform_update_8192
May 26 21:50:55 website-drupal-script-21-4xqp9 website-drupal-script-21 error >  [error]  public://webform is not a directory. 
May 26 21:50:55 website-drupal-script-21-4xqp9 website-drupal-script-21 error >  [error]  Update failed: webform_update_8192 
May 26 21:51:00 website-drupal-script-21-4xqp9 website-drupal-script-21 error  [error]  Update aborted by: webform_update_8192

But I think that's a different problem but it's an important one to fix before a stable release.

The update tries to fix signatures but I don't have those so the folder doesn't exist.

berdir’s picture

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

Oh, that was possibly added in one of the recent security updates?

  • aspilicious authored 75c47f9 on 8.x-5.x 
    Issue #3121005 by Berdir, jrockowitz, aspilicious, recrit: [Drupal 8.8.x...
jrockowitz’s picture

jrockowitz commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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