Problem/Motivation
Considering issue #2168733: locale.settings.yml uses absolute path which contradicts CMI effort, the LocalStreamWrapper needs to support recursive path resolution, e.g.:
transaltions://folder
can depend from public://translation
folder, but LocalStreamWrapper do not support recursive stream resolution.
Proposed resolution
Update the native PHP function realpath
and dirname
in function getLocalPath with Drupal wrappers.
Remaining tasks
Unit Testing, maybe using vfsStream library.
Comment | File | Size | Author |
---|---|---|---|
#9 | 2263995_reroll_5.patch | 3.88 KB | Mile23 |
#5 | streamwrapper-support-for-recursive-2263995-5.patch | 3.83 KB | mavimo |
#1 | recursive-localstream-2263995-1.patch | 967 bytes | steva1982 |
Comments
Comment #1
steva1982 CreditAttribution: steva1982 commentedComment #2
steva1982 CreditAttribution: steva1982 commentedComment #3
mavimo CreditAttribution: mavimo commentedI've verified #1, test are required but feature is OK, also in the related issue.
Comment #4
mgiffordComment #5
mavimo CreditAttribution: mavimo commentedComment #6
mavimo CreditAttribution: mavimo commentedComment #7
mgiffordThis looks fine, but just looking at this warning in the api docs.
There are no similar concerns listed with drupal_dirname.
Thanks for writing up the tests.
Comment #8
mavimo CreditAttribution: mavimo commented@mgifford yes, but in this case we have to fix the "wrong usage" of realpath() function, that must never used (do not support StreamWrapper).
NB: this also solve issue #2168733: locale.settings.yml uses absolute path which contradicts CMI effort that improve CMI.
Comment #9
Mile23Reroll of #5.
drupal_realpath()
doesn't exist anymore and is replaced by thefile_system
services'srealpath()
method.An obvious flaw in this patch is that I'm using
\Drupal
instead of rigging up injected services. We can change that if interest picks up here.Comment #12
clemens.tolboom@Mile23 its better to fix as much as possible ;-)
Replace t() function
Replace t() function
Comment #13
mavimo CreditAttribution: mavimo commented@clemens.tolboom why we need to replace t() function? The UI interface need translation, right? It is also used on PrivateStream class (and other implemented stream).
Comment #24
larowlanIs this still an issue?
I'm not sure how I go about reproducing it, can you please provide steps to reproduce starting from 'install drupal' thanks
Comment #25
clemens.tolboomI came up with these command but have no translations setup.
or (better readable)
vendor/bin/drush php:cli
My output
Comment #26
clemens.tolboomInstalling all translation modules http://drupal.d9/admin/config/media/file-system has a setting
and
stream_get_wrappers();
has translations available.Not sure how to continue further :-O
Comment #29
smustgrave CreditAttribution: smustgrave at Mobomo commentedSo should this bug be reopened?