Drupal 8.8 moveds some stream/uri functions to the StreamWrapperManager #3035273: Several file uri/scheme functions deprecated and moved to \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface. The attached patch fixes these. This will allow for the tests to run without notices in 8.8.
1x: FileSystem::uriScheme() is deprecated in drupal:8.8.0. It will be removed from drupal:9.0.0. Use \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface::getScheme() instead. See https://www.drupal.org/node/3035273
1x in FetchManagerTest::testStyleOriginalPath from Drupal\Tests\stage_file_proxy\Kernel
1x: file_default_scheme() is deprecated in drupal:8.8.0. It will be removed from drupal:9.0.0. Use \Drupal::config('system.file')->get('default_scheme') instead. See https://www.drupal.org/node/3049030
1x in FetchManagerTest::testStyleOriginalPath from Drupal\Tests\stage_file_proxy\Kernel
I moved the constructor out of the FetchManagerInterface. It prevents implementing classes to inject extra services (.e.g config factory). This shouldn't break existing FetchManager implementations.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | stage_file_proxy-3091092-3-fix-deprecation-notices.patch | 4.76 KB | interx |
Comments
Comment #2
interx commentedComment #3
interx commentedUrgh, missed one. Here's a new patch.
Comment #4
gregglesThis makes sense to me so moving to RTBC. Will wait a bit in case anyone else has feedback.
Comment #6
gregglesThanks again! This is now committed.
Comment #7
tobiasbStreamWrapperManager::getScheme is new in 8.8. So this changes breaks all sites which do not use 8.8-xyz. Change record https://www.drupal.org/node/3035273.
Comment #8
gregglesAh, ok. So what is the right way to be ready for Drupal 9 and still support 8.7?
Comment #9
tobiasbRevert the patch. And only replace deprecations when the function/method is deprecated in officially released version so that we can say we require drupal >= XX.XX.
Or we can check if newFunction is defined use newFunction else use deprecatedFunction. Or check the drupal version.
Comment #10
gregglesYes, I agree on reverting this patch ASAP, I'm just wondering what the general advice is to module maintainers to fix deprecation warnings and still be compatible with existing versions of Drupal.
Comment #12
gregglesI've reverted the commit for now. We need to either save this commit for when 8.7 is EOL or find a way to support both at the same time. I'm putting the status as "Needs work" in hopes of something that can work for 8.7, 8.8 and beyond.
Comment #13
gregglesBTW, I created https://www.drupal.org/project/stage_file_proxy/releases/8.x-1.0-rc4 so people won't be affected by this.
Comment #14
kristen polPer a Slack discussion with Gábor Hojtsy regarding usage of D9 tags (Drupal 9, Drupal 9 compatibility, Drupal 9 readiness, etc.), "Drupal 9 compatibility" should be used for contributed projects that need updating and "Drupal 9" was the old tag for D8 issues before the D9 branch was ready. Doing tag cleanup here based on that discussion.
Comment #15
berdirI would recommend to just update the info.yml file to require 8.8 at this point and then commit it again. token/pathauto/redirect/paragraphs all require 8.8 now in their dev release.
You can still hold out on actually doing a release for a while, which would give you a way to do a security release with 8.7 support if you want to based on the previous release. IMHO that is the only reason to still support 8.7 now.
Comment #16
gregglesSetting back to needs review, but it seems possible these were fixed elsewhere.
Comment #17
spokjeSeems to me all of the code in this patch was already applied in the commit of issue #3139001
Comment #18
rob230 commentedYou are probably aware of this, but Stage File Proxy is broken on Drupal 8.7 due to this error:
Error: Call to undefined method Drupal\Core\StreamWrapper\StreamWrapperManager::getScheme() in Drupal\stage_file_proxy\FetchManager->styleOriginalPath() (line 121 of modules/contrib/stage_file_proxy/src/FetchManager.php)
Despite the release notes saying Drupal 8.7 is the minimum, it is not. Anyone on Drupal 8.7 has to use version stage_file_proxy 1.0-rc4.
Comment #19
berdirDrupal 8.7 is unsupported, you shouldn't use it with anything :)
as
It's unfortunate that the version constraint is wrong, a new issue could be opened to set that higher as I suggested already above, would a be trivial patch. However, it's not possible to fix the already created release, that can't be changed, composer would keep installing the incompatible 1.1 even after a correctly defined 1.2 is out. But again, 8.7 is outdated, and 8.8 will be as well soon.