Problem/Motivation
The page File system /admin/config/media/file-system
contains the info about the file system but has missing information regarding the application root folder.
Steps to reproduce
Install the Drupal project on the shared hosting.
Proposed resolution
Introduce a description of the application root and add the info for relative paths with resolved absolute paths.
Remaining tasks
Add an item at File system page which describes the application root.
Decide how to provide additional info for the absolute paths and implement it.
User interface changes
The content of the page File system will be updated.
API changes
-
Data model changes
-
Release notes snippet
Initial summary
It is not always obvious what directory should be used for the private file system path, especially for sites on shared hosting.
It would be helpful to show the full path to the Drupal root directory on the file system admin form (/admin/config/media/file-system)
Comment | File | Size | Author |
---|
Issue fork drupal-2704931
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
malcomio CreditAttribution: malcomio commentedHere's a patch against 8.0.x
Comment #3
malcomio CreditAttribution: malcomio commentedAnd here's a D7 patch.
Comment #5
mbayntonI think the best practice for private files is to not even locate them under the installation root (see https://www.drupal.org/documentation/modules/file, managing file locations and access.) If we give a hint to the installation root here, maybe it should also say you don't need to use it. Perhaps "You can use any absolute writable path, or the following path within this Drupal installation: @privpath", and append sites/default/files/private to the DRUPAL_ROOT for @privpath?
Comment #13
vikashsoni CreditAttribution: vikashsoni as a volunteer and at Zyxware Technologies commented@mailomio i try to applied #3 and #2 patches but giving error would you please suggest me that how i will check it sharing screenshot .....
Comment #14
Madhu kumar CreditAttribution: Madhu kumar as a volunteer and at Zyxware Technologies commented@mailomio even i am also facing same issue unable to apply patch , If am doing wrong let me know how to apply it properly.
Comment #15
volegerNo feature request allowed over 8.9.x branch, updating target branch to 9.3.x
Comment #16
ankithashettyThe patch in #2 needed a reroll. Attaching a rerolled patch here, thanks!
Comment #17
Madhu kumar CreditAttribution: Madhu kumar as a volunteer and at Zyxware Technologies commentedPatch #16 applied cleanly and working as expected ,Provide Drupal root path on file system admin form. Sharing Screenshot for reference.
Comment #18
longwaveWhy is this buried in the "private file system path" description? Wouldn't it be better to show it as its own item?
Comment #20
vikashsoni CreditAttribution: vikashsoni as a volunteer and at Zyxware Technologies commentedApplied patch #16 applied successfully
After patch root directory description has been added in description
Thanks for the patch
Comment #21
longwaveAlternative approach that implements #18, with additional help text explaining what this means.
Comment #22
joachim CreditAttribution: joachim at Factorial GmbH commentedI think clearer than both approaches so far would be to add the DRUPAL_ROOT as a prefix to the displayed value.
Comment #23
longwave@joachim if we are to do that we should just resolve the full absolute path of each value, rather than trying to figure it out ourselves - we don't want to prefix with DRUPAL_ROOT if the path is already absolute, for example.
Comment #24
heni_deepak CreditAttribution: heni_deepak commentedApplied patch #21 applied successfully
Comment #25
joachim CreditAttribution: joachim at Factorial GmbH commented> if we are to do that we should just resolve the full absolute path of each value, rather than trying to figure it out ourselves
Maybe we should do that?
What's the actual purpose of this form? None of these paths can be edited in the form, can they? So, is the purpose here:
A. Show what is literally defined in settings.php
B. Show the location of the file folders
If it's B, then resolving it makes sense.
Comment #28
smustgrave CreditAttribution: smustgrave at Mobomo commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
This could use an issue summary update for the proposed solution. Seems there is still discussion going on so when the path forward is decided it should be added to the IS. Will help the committer.
Comment #30
volegerRerolled #21 in a MR. Added descriptions for items regarding absolute values if applicable.
Comment #31
smustgrave CreditAttribution: smustgrave at Mobomo commentedThanks!
Code wise I think it looks fine. Not sure if we will need a trigger_error for the new parameter or not.
This could use a simple change record though to announce the new parameter option.
Comment #32
volegerAdded CR https://www.drupal.org/node/3341696
Updated constructor to handle new parameter addition.
Comment #33
smustgrave CreditAttribution: smustgrave at Mobomo commentedThanks think this good for committer review now.
Comment #34
quietone CreditAttribution: quietone at PreviousNext commentedThis is changing the user interface, adding tag. As such this should have before and after screenshot available from the issue summary. This is also adding text to the public and private file descriptions, so I think this should have a usability review.
I read the change record, well several times, and I am still not sure what 'related' means in "public, private, and temporary paths are related". Tagging for a change record update. This is not a complicated change but a screenshot could be useful in the change record.
I skimmed the patch and made some comments. I also note this was set to RTBC with an unresolved comment.
Comment #35
volegerChanged the relative paths with absolute as this will show the actual location of specific stream wrappers.
Added check for path accessibility. The realpath() file_system method fails absolute path resolving if the directory not exists.
I have added screenshots before, after, and after with warning messages.
Comment #36
smustgrave CreditAttribution: smustgrave at Mobomo commentedBrought this up in #ux channel and it was briefly looked at but was mentioned
Comment #37
volegerI agree that IS needs to be updated, but please check the screenshots from #35.
The latest changes in MR convert paths to absolute if they are relative. And if the destination does not exist, the path value is printed as is (relative or absolute) with an appropriate warning message that reuses existing sentences and translations.
In case such behavior is acceptable I'll update IS as I can.
Comment #39
volegerrerolled
assets filesystem wrapper added since then
if #35 comparison approach is good to go with, I'm ready to create updated screenshots and attach them to the IS.