Updated: Comment #3

Problem/Motivation

If you installl via Drush, Drupal creates the files and the files/config_hash directories with permissions that doesn't allow the webserver to write them. Fun!

Proposed resolution

Go 0777, when installing via CLI. There are no changes when installing via Web or Simpletest non-interactive installer.

Remaining tasks

User interface changes

None.

API changes

None.

#1908440: Relax MTimeProtectedFileStorage permissions for DX, drush integration and world domination

Files: 
CommentFileSizeAuthor
#10 drupal8.other_.2099467-10.patch7.88 KBmoshe weitzman
PASSED: [[SimpleTest]]: [MySQL] 58,753 pass(es).
[ View ]
#5 2099467_5.patch7.5 KBchx
PASSED: [[SimpleTest]]: [MySQL] 58,908 pass(es).
[ View ]
#3 drupal8.other_.2099467-3.patch7.18 KBmoshe weitzman
PASSED: [[SimpleTest]]: [MySQL] 58,933 pass(es).
[ View ]
#2 clihappy.diff7.83 KBmoshe weitzman
FAILED: [[SimpleTest]]: [MySQL] 59,071 pass(es), 2 fail(s), and 1 exception(s).
[ View ]
drush.patch3.13 KBchx
PASSED: [[SimpleTest]]: [MySQL] 59,095 pass(es).
[ View ]

Comments

We spoke with alexpott. The agreement is to rework the patch so that the mode is passed into install_drupal as a param. Then drush will pass 777 when it does the install. The web installer will not change.

Status:Active» Needs review
StatusFileSize
new7.83 KB
FAILED: [[SimpleTest]]: [MySQL] 59,071 pass(es), 2 fail(s), and 1 exception(s).
[ View ]

Patch, as discussed at Drupalcon Friday sprint. I'm updating issue summary as well.

StatusFileSize
new7.18 KB
PASSED: [[SimpleTest]]: [MySQL] 58,933 pass(es).
[ View ]

Sorry, one unintended change snuck in.

Issue summary:View changes

Updated issue summary.

StatusFileSize
new7.5 KB
PASSED: [[SimpleTest]]: [MySQL] 58,908 pass(es).
[ View ]

Fixed a missing doxygen.

Title:Drush and Drupal 8: part 2Let non-interactive installers determine mode of files directory and its subdirectories
Status:Needs review» Reviewed & tested by the community

Thanks. Retitled for clarity.

Thanks. Better title.

Priority:Normal» Major

Drush and webserver can't both write to config dirs right now so escalating. After this, at least both can write after a drush install.

Status:Reviewed & tested by the community» Needs work

+++ b/core/includes/file.inc
@@ -481,12 +481,15 @@ function file_create_url($uri) {
+ *   Integer value for the permissions. Consult PHP chmod() documentation for

Can we do @param int $mode then say octal in the docs?

Status:Needs work» Needs review
StatusFileSize
new7.88 KB
PASSED: [[SimpleTest]]: [MySQL] 58,753 pass(es).
[ View ]

Rerolled with catch's Doxygen suggestion. No other changes.

Status:Needs review» Reviewed & tested by the community

Just a small question...

+++ b/core/modules/image/image.install
@@ -11,7 +11,8 @@
-  file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
+  $mode = isset($GLOBALS['install_state']['mode']) ? $GLOBALS['install_state']['mode'] : NULL;
+  file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS, $mode);
+++ b/core/modules/system/system.install
@@ -371,7 +371,8 @@ function system_requirements($phase) {
-      file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS);
+      $mode = isset($GLOBALS['install_state']['mode']) ? $GLOBALS['install_state']['mode'] : NULL;
+      file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS, $mode);

in general we are trying to remove global variables, not add them. Does $install_state need to be passed into these function? And/or is there an issue to move this to the Request object and pull it from there?

hook_install(). does not pass $install_state nor does it have the ability to do so today. see https://api.drupal.org/api/drupal/core%21modules%21system%21system.api.p... and its caller https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

So, addressing that would be a huge change. I don't know of efforts underway to improve this. Perhaps #1530756: Use a proper kernel for the installer would help. IMO, the installer should be a separate app, outside of Drupal (but bundled with it). Drupal should only focus on serving requests, not self-install.

Status:Reviewed & tested by the community» Fixed

Yeah, agreed with that.

I guess I was hoping we could replace those $GLOBALS calls with something like:

$mode = \Drupal::request()->get('install_state')->mode

...or something. But indeed, it sounds like this is #1530756: Use a proper kernel for the installer's job to clean up.

Committed and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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

Issue summary:View changes

Updated issue summary.