Convert system file related variables to CMI

Should handle:
file_chmod_directory
file_chmod_file
file_default_scheme

In system.file.yml rather than system.site.yml

Issue now covers all these variables:

file_chmod_directory
file_chmod_file
file_default_scheme
file_public_path
file_private_path
file_temporary_path

Latest patch attached, but not complete. Todo:

-- convert file_*_path variables
-- update system_file_system_settings() to use system_config_form and provide a submit handler.

TODO:

1) Update system_file_system_settings() to use system_config_form() and provide a submit handler.

2) Some settings uses conf_path() to define the default. I do not know how to define this in the yml file. I am currently doing:

path:
  public: 'sites/default/files'
  private: ''
  temporary: 'sites/default/files/tmp'

I was apparently unable to do "file mode" as a octal in the YAML file. So I used integers.

# Directory and file modes.
# 0775 is 509, as converted from octal, and 664 is 436.
chmod:
  directory: 509
  file: 436

There are still errors in the installation.

Configuration should set temporary file directory as NULL

Fixed install hook conflict.

Yay, got past the installer. Working on the system form.

I should probably take ownership of the issue to avoid confusion.

The system settings form is done, not all tests are passing though.

My local tests against an 8.x branch are giving me fails so going to see what the testbot says.

Fixed up ->drupalGetTestFiles()

OK, 1 exception left. I do not know how to prevent it properly. It occurs in MTimeProtectedFileStorageTest.php in the testSecurity() method.

$expected_root_directory = DRUPAL_ROOT . '/' . config('system.file')->get('path.public') . '/php/simpletest';

I do not know why config('system.file')->get('path.public') is returning nothing when it is working fine in other tests. Also these PHPStorage tests are badly broken for me whereas the testbot shows only one exception.

Looking at the code, the old code uses a variable get and doesn't care much if it needs to grab a default value comprised of conf_path() . /files, so the new patch *might* be appropriate, and replaces the above line with:

$expected_root_directory = DRUPAL_ROOT . '/' . $this->originalFileDirectory . '/php/simpletest';

Otherwise, this is pretty close... unassigning myself, but will keep an eye on it.

Looks like #22 is the best effort so far.

This looks perfectly sane except for the removal of file_default_schema(), which could be changed to be a wrapper for config(), much like file_directory_temp(). Let's please revert those removals of file_default_schema() in the name of DX.

diff --git a/core/modules/image/image.field.inc b/core/modules/image/image.field.inc
index 09b07eb..3a8b0f9 100644
--- a/core/modules/image/image.field.inc
+++ b/core/modules/image/image.field.inc
@@ -14,7 +14,7 @@ function image_field_info() {
       'label' => t('Image'),
       'description' => t('This field stores the ID of an image file as an integer value.'),
       'settings' => array(
-        'uri_scheme' => variable_get('file_default_scheme', 'public'),
+        'uri_scheme' => config('system.file')->get('default_scheme'),
         'default_image' => 0,
       ),
       'instance_settings' => array(

This should use file_default_scheme() instead.

diff --git a/core/lib/Drupal/Core/StreamWrapper/TemporaryStream.php b/core/lib/Drupal/Core/StreamWrapper/TemporaryStream.php
index 52a6321..fd23619 100644
--- a/core/lib/Drupal/Core/StreamWrapper/TemporaryStream.php
+++ b/core/lib/Drupal/Core/StreamWrapper/TemporaryStream.php
@@ -19,7 +19,12 @@ class TemporaryStream extends LocalStream {
    * Implements Drupal\Core\StreamWrapper\LocalStream::getDirectoryPath()
    */
   public function getDirectoryPath() {
-    return variable_get('file_temporary_path', file_directory_temp());
+    $temporary_path = config('system.file')->get('path.temporary');
+    if (empty($temporary_path)) {
+      // Determine a temporary path. This will save the value to config for next time.
+      $temporary_path = file_directory_temp();
+    }
+    return $temporary_path;
   }
 
   /**

This should just be using return file_directory_temp() as a cleanup.

I guess from a security perspective, are the following comments preserved in the exported config files? Otherwise I could see people trying to manually change these values to '0775' and things not working as expected.

+# Directory and file modes.
+# 0775 is 509, as converted from octal, and 664 is 436.

heyrocker confirms comments are not preserved, so based on #32 alone I need to mark this as needs work.

A little confused why we can't actually use octal in YAML? http://trac.symfony-project.org/ticket/7067 seems it was fixed three years ago?

#43: drupal-system_file_settings-1799504-43.patch queued for re-testing.

Re-roll of patch in #43 comment.

Reroll for current head.

Changing octals back to strings after a discussion here at the London drop-in sprint.

#48: 1799504-convert_system_file_related_variables_to_CMI-48.patch queued for re-testing.

When I apply this patch and try to install Drupal using the Standard profile through the interface I get the following error:

The service definition "keyvalue" does not exist.

This is unfortunately a duplicate of #1496480: Convert file system settings to the new configuration system :(

I'll propose in the original issue that commit credit is given to the people that worked on it here so that the hard work done is not entirely lost.

Updated issue summary to include all variables.