Problem/Motivation

I really think we should not call the staging directory of CMI "staging". I spoke to a lot of people that already use Drupal 8 in production and they all agree that it's a bad name. Because it is confused a lot with actual staging environments. Some CMI Workflows actually have separate folders for different hosting environments (like "to_dev", "from_prod", "staging") and then it is even more confusing.

Proposed resolution

In #1741804: Implement a config import/staging directory the staging directory was introduced. There it was decided to call it "staging" as the proposed "import" name was not good enough, as we use that folder also for exporting of configuration.

So I would suggest to actually call it "import-export" or if we don't like the two name system, maybe "synchronization" or "sync"? (we call it anyway synchronization in the UI)
I just think we will start to confuse a lot of people, because when you tell somebody "staging" they immediately think about staging environments

Remaining tasks

+ Agree
+ Write Patch
+ Create change record
- Inform contrib modules to use CONFIG_SYNC_DIRECTORY instead of CONFIG_STAGING_DIRECTORY

User interface changes

-

API changes

- Constant name CONFIG_STAGING_DIRECTORY probably changes

Change record: https://www.drupal.org/node/2574957

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug
Issue priority Major because the earlier we fix this issue, the less people are already confused by it. Especially before the RC1
Prioritized changes The main goal of this issue is to make a better developer experience. 8 ships with CMI, which nobody ever knew before, let's make it as streamlined as possible, so that we don't scare people away.
Is BC? Yes, fully backwords compatible to modules that still depend on CONFIG_STAGING_DIRECTORY
Files: 
CommentFileSizeAuthor
#70 interdiff.2487588.69.70.txt6.87 KBYesCT
#70 2487588.70.patch113.29 KBYesCT
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2487588.70.patch. Unable to apply patch. See the log in the details link for more information. View
#69 interdiff.2487588.66.69.txt594 bytesYesCT
#69 2487588.69.patch112.79 KBYesCT
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2487588.69.patch. Unable to apply patch. See the log in the details link for more information. View
#67 interdiff.2487588.63.66.txt683 bytesYesCT
#66 interdiff.2487588.63.66.txt0 bytesYesCT
#66 2487588.66.patch112.74 KBYesCT
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2487588.66.patch. Unable to apply patch. See the log in the details link for more information. View
#63 interdiff.2487588.50.63.txt435 bytesYesCT
#63 2487588.63.patch112.66 KBYesCT
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,457 pass(es). View
#51 2487588-50.patch115.69 KBSchnitzel
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,066 pass(es). View
#51 interdiff-44-50.txt1.82 KBSchnitzel
#21 2487588-config_sync-21.patch111.79 KBSchnitzel
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,875 pass(es), 12 fail(s), and 1 exception(s). View
#14 2487588-interdiff-12-14.txt8.5 KBLeksat
#14 2487588-config_sync-14.patch109.08 KBLeksat
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2487588-config_sync-14.patch. Unable to apply patch. See the log in the details link for more information. View
#5 2487588-alternate-approach.5.patch591 bytesalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,492 pass(es). View
#4 2487588.4.patch23.35 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,487 pass(es). View

Comments

xjm’s picture

Thanks @Schnitzel.

So I see the case for this (and kind of agree now that I've thought about it), but this would be a massive BC/workflow/expectation break. Unfortunately my gut reaction is that we shouldn't do this during the beta.

xjm’s picture

Status: Needs work » Active
alexpott’s picture

It depends I think this is possible in a backwards compatible way.

alexpott’s picture

Status: Active » Needs review
FileSize
23.35 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,487 pass(es). View

If we want to change the code to match the name then we need to do something like this.

alexpott’s picture

FileSize
591 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,492 pass(es). View

Here is the smallest possible change.

Schnitzel’s picture

nice, thanks @alexpott for just doing that =)

I'm in favor of #4 as I think it's not only confusing for people to have a folder that is called "staging" but also during the backend development.

Anyway, I think though, that import is not the right name, as we also use it for exporting *yay*.
How about "sync"?

tstoeckler’s picture

I can't find the issue right now, but it was suggested before to actually split the double meaning of the current staging directory into separate import and export directories. That would make a lot of sense IMO. And if you specify both to be the same the functionality would be the same as it currently is. We could even consider some sort of compatibility layer, i.e. if you have specified a staging directory, that will override both import and export.

Thoughts?

Schnitzel’s picture

@tstoeckler

but it was suggested before to actually split the double meaning of the current staging directory into separate import and export directories. That would make a lot of sense IMO

this all depends on which GIT workflows we as a community suggest in #1831818: Document git config import workflow.

My current understanding is, that you drush config export on the DEV site, add this code to GIT and on the PROD site you make a drush config import again. And per default drush would be configured to take the same directory for both actions.
But of course we could suggest to config export into the export directory on DEV, add to GIT, but then you would need to drush config import from the export directory? Which is a bit confusing. Or would we tell the people to copy from export to import directory?

In my understanding of having config in GIT: there should be one single directory that contains the config that matches the current code, and in there we should import and export to.

tstoeckler’s picture

Re @Schnitzel: Yes, that's certainly one workflow, and probably the most common (and preferred) one. It can totally be served by separating the import and export directories - as a concept - if you just point them to the same physical directory. That also makes the workflow very self-documenting: We don't need to explain what "staging" means, what happens is evident from the fact that import and export directory are identical. But this would a) allow a bunch of other use-cases, and it gets rid of this documentation/naming barrier. I.e. when people simply FTP the YAML files onto their Drupal sites, it's much more self-documenting to put them in a directory called "import" than "staging".

But that's just my 2 cents. I don't want to derail this, so feel free to ignore, if the suggestion is not as constructive as I hoped. I just thought it would be an elegant solution to this, while the added flexibility is a mere side-benefit.

moshe weitzman’s picture

IMO, this is exactly the sort of change we should do now, in order to spare thousands of confused devs in the next 5 years. Right now, there are 20 something live d8 sites that would suffer "massive BC break". I think we should go further than the latest patch proposes and rename the constant to IMPORT_EXPORT (i'm ambivalent on that). I volunteer to update drush accordingly.

alexpott’s picture

In #2247291: Reorder tabs in configuration UI @Eli-T has pointed out how this exactly same confusion exists in the current UI.

Leksat’s picture

FileSize
109.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 96,802 pass(es), 13 fail(s), and 18 exception(s). View

I personally more like sync rather than import-export, because import-export could also be import_export and ImportExport while sync is always sync.

Attached patch changes staging to sync in all places, including comments and variable names. So, now the core contains only two occurrences of word staging, where it means staging server.

There is no backward compatibility layer in the patch. I believe an upgrade path could be provided in the head2head module.

Status: Needs review » Needs work

The last submitted patch, 12: 2487588-config_sync-12.patch, failed testing.

Leksat’s picture

Status: Needs work » Needs review
FileSize
109.08 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2487588-config_sync-14.patch. Unable to apply patch. See the log in the details link for more information. View
8.5 KB

I forgot to rename some files in the previous patch.

Status: Needs review » Needs work

The last submitted patch, 14: 2487588-config_sync-14.patch, failed testing.

anavarre’s picture

Issue tags: +Needs reroll
cilefen’s picture

This needs thoughtful beta evaluation as soon as possible.

anavarre’s picture

Schnitzel’s picture

Assigned: Unassigned » Schnitzel

gonna work on that in Barcelona

Schnitzel’s picture

Status: Needs work » Needs review
FileSize
111.79 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,875 pass(es), 12 fail(s), and 1 exception(s). View

reroll

Status: Needs review » Needs work

The last submitted patch, 21: 2487588-config_sync-21.patch, failed testing.

The last submitted patch, 21: 2487588-config_sync-21.patch, failed testing.

Schnitzel’s picture

Status: Needs work » Needs review
FileSize
116.48 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,912 pass(es). View

another reroll, there was a typo in there and some files missing

Schnitzel’s picture

Title: Rename CMI import/export directory "staging", as it is confused with staging environments » Move CMI import/export directory "staging" to "sync", as it is confused with staging environments
Leksat’s picture

Issue summary: View changes
FileSize
117.12 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2487588-26.patch. Unable to apply patch. See the log in the details link for more information. View
1.16 KB

Added staging-to-sync fallback to not break existing installations.
Also created D9 followup #2574943: Trigger E_USER_DEPRECATED when CONFIG_STAGING_DIRECTORY fallback is called

Leksat’s picture

Created a draft change record: https://www.drupal.org/node/2574957

Eli-T’s picture

The patch in #26 changes strings that have been changed following UX review in #2572537: Update the UI texts for the Configuration Manager module.

moshe weitzman’s picture

Have we discussed splitting into two folders - one for import and one for export? I actually think thats the clearest. It also helps security since you would usually make the import one read-only as it is git controlled, while the web server is allowed to write to export.

Schnitzel’s picture

Assigned: Schnitzel » Unassigned
Schnitzel’s picture

Have we discussed splitting into two folders - one for import and one for export? I

Yes, see #7 and #8.

I think my take and also the take from @alexpott (discussed with him at DrupalCon Barcelona) is, that we actually don't need the staging/sync/import/export directories at all. Because the UI could just create a temporary folder when it is importing the full configuration, exporting anyway send a tarball via the Browser.

And the big discussion we have right now about how the GIT/rsync/anyother workflow should work, is a lot depending on people thinking to need to use the staging directory, which is completely wrong. You need it if you wanna use rsync CMI workflows, but you don't need it if you have git and drush config-import workflows.

So it's great that we have a possibility for config files to be imported via the UI with rsync, but that does not mean that we need to create a folder by default. We could just explain people in the settings.default.php how to do it and make the code less depended on an actual folder and just create the folder if we really need it.

But what we also agreed to, that this change is much bigger and needs involvement of a lot more people.
So we say: if we cannot remove the folder completely, we wanna at least name it in a way that people are not confused about it.

And therefore I would keep 1 folder and call it sync, like the UI also calls it :)

Schnitzel’s picture

Status: Needs review » Needs work
+++ b/core/includes/bootstrap.inc
@@ -181,6 +191,11 @@ function conf_path($require_settings = TRUE, $reset = FALSE, Request $request =
+  // @todo: Remove fallback in Drupal 9. https://www.drupal.org/node/2574943
+  if ($type == CONFIG_SYNC_DIRECTORY && !isset($config_directories[CONFIG_SYNC_DIRECTORY]) && isset($config_directories[CONFIG_STAGING_DIRECTORY])) {
+    $type = CONFIG_STAGING_DIRECTORY;
+  }
+

we should also adapt the exception to throw that 'sync' does not exist, because currently it would throw that 'staging' does not exist even when we request the 'sync' directory, which would be confusing a lot :)

Leksat’s picture

Status: Needs work » Needs review

@Schnitzel, regarding #32, the fallback happens only if "staging" directory is set, so no exception is thrown in this case.

Schnitzel’s picture

Status: Needs review » Postponed

@Leksat

Oh! correct, thanks for clarification :)

I think this is good then! RTBC, but as it conflicts with #2572537: Update the UI texts for the Configuration Manager module, let's wait till that one got in and then we can adapt your patch to change these strings as well.

Schnitzel queued 26: 2487588-26.patch for re-testing.

Schnitzel’s picture

Status: Postponed » Needs review

Status: Needs review » Needs work

The last submitted patch, 26: 2487588-26.patch, failed testing.

anavarre’s picture

Status: Needs work » Needs review
FileSize
112.81 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,643 pass(es). View

Attempting a reroll.

anavarre’s picture

Issue tags: -Needs reroll
Schnitzel’s picture

Status: Needs review » Reviewed & tested by the community

nice! thanks :)

Schnitzel’s picture

Priority: Normal » Major
Issue summary: View changes
Schnitzel’s picture

Issue tags: -Needs beta evaluation

added beta evaluation

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 38: 2487588-38.patch, failed testing.

alexpott’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
112.8 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,047 pass(es), 1 fail(s), and 0 exception(s). View

Rerolled - conflict with #2520540: Enforced configuration dependencies shouldn't have to be repeated in the calculated dependencies

102c102
< index 4a2b41b..b12b660 100644
---
> index e9e1194..9a9a9e6 100644
105c105
< @@ -178,8 +178,8 @@ function drupal_get_database_types() {
---
> @@ -202,8 +202,8 @@ function drupal_get_database_types() {
116c116
< @@ -187,7 +187,7 @@ function drupal_get_database_types() {
---
> @@ -211,7 +211,7 @@ function drupal_get_database_types() {
125c125
< @@ -463,9 +463,9 @@ function drupal_install_config_directories() {
---
> @@ -487,9 +487,9 @@ function drupal_install_config_directories() {
138c138
< @@ -481,19 +481,19 @@ function drupal_install_config_directories() {
---
> @@ -505,19 +505,19 @@ function drupal_install_config_directories() {
162c162
< @@ -501,7 +501,7 @@ function drupal_install_config_directories() {
---
> @@ -525,7 +525,7 @@ function drupal_install_config_directories() {
196c196
< index a762de9..5266ff4 100644
---
> index 80180ce..7de5058 100644
206c206
<   * get the current dependencies without recalculation, you can use
---
>   * get the current dependencies (without recalculation), you can use
1727c1727
< similarity index 78%
---
> similarity index 79%
1730c1730
< index 5c67c7b..e47a615 100644
---
> index 394c83b..83c9aae 100644
1744c1744
< similarity index 77%
---
> similarity index 79%
1747c1747
< index 6113d7e..fee4cbe 100644
---
> index d035ef8..d692f78 100644
1912c1912
< index 0e2bf5c..f617201 100644
---
> index 394c767..f11b567 100644
1943c1943
< index 65edaeb..9c0f46b 100644
---
> index 7abfa2f..2270958 100644
1946c1946
< @@ -1513,7 +1513,7 @@ public function configImporter() {
---
> @@ -1518,7 +1518,7 @@ public function configImporter() {
2094c2094
< index e8a7c1b..6e43ab2 100644
---
> index 5a3f359..b98dea2 100644
2097,2098c2097,2098
< @@ -138,7 +138,7 @@ public function testCalculateDependencies() {
<        $dependencies = $view->calculateDependencies();
---
> @@ -138,7 +138,7 @@ public function testGetDependencies() {
>        $dependencies = $view->getDependencies();
2107c2107
< index 085009c..5271a35 100644
---
> index 6b95f4d..fd7b864 100644
2128c2128
< @@ -942,7 +942,7 @@ protected function configImporter() {
---
> @@ -943,7 +943,7 @@ protected function configImporter() {
2137c2137
< @@ -1087,7 +1087,7 @@ public function __get($name) {
---
> @@ -1088,7 +1088,7 @@ public function __get($name) {
2160c2160
< index 1ea5d72..d93cc09 100644
---
> index 7d3a3ae..6a45d25 100644

Diff of #38 to this patch... just a context difference.

xjm’s picture

Assigned: Unassigned » xjm

Reviewing this.

xjm’s picture

Issue tags: +Needs change record

(Partial review notes.)

  1. +++ b/core/core.services.yml
    @@ -290,9 +290,9 @@ services:
    -  config.storage.staging:
    +  config.storage.sync:
    
    +++ b/core/includes/bootstrap.inc
    @@ -109,9 +109,19 @@
    +const CONFIG_SYNC_DIRECTORY = 'sync';
    
    +++ b/core/lib/Drupal/Core/Config/FileStorageFactory.php
    @@ -25,12 +25,12 @@ static function getActive() {
    -  static function getStaging() {
    ...
    +  static function getSync() {
    

    For the CR.

  2. +++ b/core/includes/bootstrap.inc
    @@ -109,9 +109,19 @@
      * $config_directories key for staging directory.
      *
      * @see config_get_config_directory()
    + *
    + * @deprecated in Drupal 8.0.x and will be removed before 9.0.0. The staging
    + *   directory was renamed to sync.
    

    I think there should also be an @see to CONFIG_SYNC_DIRECTORY? Could be fixed on commit/followup/whatevs.

  3. +++ b/core/includes/bootstrap.inc
    @@ -140,6 +150,11 @@
    +  // @todo: Remove fallback in Drupal 9. https://www.drupal.org/node/2574943
    +  if ($type == CONFIG_SYNC_DIRECTORY && !isset($config_directories[CONFIG_SYNC_DIRECTORY]) && isset($config_directories[CONFIG_STAGING_DIRECTORY])) {
    +    $type = CONFIG_STAGING_DIRECTORY;
    +  }
    

    Happy BC.

  4. +++ b/core/includes/install.inc
    @@ -211,7 +211,7 @@ function drupal_get_database_types() {
      *   $config_directories['active'] = 'config_hash/active';
    
    @@ -525,7 +525,7 @@ function drupal_install_config_directories() {
      *   Type of config directory to return. Drupal core provides 'active' and
    

    Looks like this was missed in #2487592: CMI: don't ship with a default "active" directory that is empty in most Drupal installations I guess? Followup.

  5. +++ b/core/modules/config/config.module
    @@ -33,7 +33,7 @@ function config_help($route_name, RouteMatchInterface $route_match) {
    -      $output .= '<p>' . t('Compare the configuration uploaded to your staging directory with the active configuration before completing the import.') . '</p>';
    +      $output .= '<p>' . t('Import configuration that is placed in your sync directory. All changes, deletions, renames, and additions are listed below.') . '</p>';
    
    @@ -43,7 +43,7 @@ function config_help($route_name, RouteMatchInterface $route_match) {
    -      $output .= '<p>' . t('Upload a full site configuration archive to the staging directory. It can then be compared and imported on the Synchronize page.') . '</p>';
    +      $output .= '<p>' . t('The full import page can be used to import a full set of configuration for this site by uploading a gzipped or bzipped tar file consisting of all configuration YAML files. The results will be placed in a the sync directory, so they can be compared in the Synchronize tab. There the import can be finalized.') . '</p>';
    

    Are these string rewrites intentional or a merge artifact? I'll look back on the issue.

YesCT’s picture

I will work on the change record.

[edit: wait, I see one was made in #27, but it didn't have the issue linked, so it was not showing on the sidebar. editing that one.]

Leksat’s picture

There is a draft of a change record: https://www.drupal.org/node/2574957

Schnitzel’s picture

Status: Reviewed & tested by the community » Needs work

Are these string rewrites intentional or a merge artifact? I'll look back on the issue.

No they should not be changed that hard, we just created new strings in https://www.drupal.org/node/2572537 and looks like in the reroll that got wrong.

Will fix that.

The last submitted patch, 44: 2487588-44.patch, failed testing.

Schnitzel’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.82 KB
115.69 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,066 pass(es). View
xjm’s picture

+++ b/sites/default/default.settings.php
@@ -223,13 +223,13 @@
- * directories used for configuration data. On install, "active" and "staging"
...
+ * directories used for configuration data. On install, "active" and "sync"
...
- * The default location for the active and staging directories is inside a
+ * The default location for the active and sync directories is inside a

Also for that "active" followup.

Schnitzel’s picture

YesCT’s picture

Issue summary: View changes

change record is now showing up in the sidebar here, and needs review: https://www.drupal.org/node/2574957

[edit: I'm searching for staging in change records to see if there are some that should be updated https://www.drupal.org/list-changes/published/drupal?keywords_descriptio...

xjm’s picture

Reviewed with mad word diff regex skills and confirmed the new patch does just this one thing. :)

Also:

[mandelbrot:maintainer | Mon 13:46:04] $ grep -ri "staging" * | grep -v "~" | grep -v "vendor"
core/includes/bootstrap.inc: * $config_directories key for staging directory.
core/includes/bootstrap.inc: * @deprecated in Drupal 8.0.x and will be removed before 9.0.0. The staging
core/includes/bootstrap.inc:const CONFIG_STAGING_DIRECTORY = 'staging';
core/includes/bootstrap.inc:  if ($type == CONFIG_SYNC_DIRECTORY && !isset($config_directories[CONFIG_SYNC_DIRECTORY]) && isset($config_directories[CONFIG_STAGING_DIRECTORY])) {
core/includes/bootstrap.inc:    $type = CONFIG_STAGING_DIRECTORY;
core/modules/serialization/src/EntityResolver/EntityResolverInterface.php:   * on example.com and staging.example.com), it is generally unsuitable for use
sites/default/default.settings.php: * Use settings.local.php to override variables on secondary (staging,,

The first three are the deprecated constant itself; the next three are the BC fallback; and the last two are unrelated things that actually refer to a staging environment.

CR looks great.

Now I'm just going to test it a bit.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/core.services.yml
@@ -290,9 +290,9 @@ services:
-  config.storage.staging:
+  config.storage.sync:

We could leave the config.storage.staging in core.services and just alias it to config.storage.sync

I think renaming the method in the factory is fine since no one should be using that directly.

xjm’s picture

Assigned: xjm » Unassigned

Unassigning for someone to fix that while I muck about manually testing things. :)

xjm’s picture

I tested this manually with an existing (pre-beta 16) site: applied the patch, ran update.php, proceeded to sync config with another install of the same site with no problems. Yay APIs and yay happy BC. I also tested a newly installed site and confirmed config sync still functions as expected there too.

drush cex -y works fine for the existing site, but breaks for the new site. We will need a PR for drush for this change so that drush works with either. Anyone up to file that?

I was also going to test https://www.drupal.org/project/config_installer but that is broken since the conf_path() removal, it seems.

moshe weitzman’s picture

I will update Drush quickly if noone else does.

xjm’s picture

The CR exists now.

YesCT’s picture

I will try and do #56

YesCT’s picture

Status: Needs work » Needs review
FileSize
112.66 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,457 pass(es). View
435 bytes

here goes.

I think the difference in patch size, is my git diff settings to show a rename. but there could be something else. a diff of the patch files might be good to do on review.

[edit: it's fine, diffing my patch to the one in #44 shows the changes from the #51. so all good.]

alexpott’s picture

Status: Needs review » Needs work
+++ b/core/core.services.yml
@@ -290,6 +290,8 @@ services:
+  config.storage.staging:
+    alias: config.storage.sync
   config.storage.sync:
     class: Drupal\Core\Config\FileStorage
     factory: Drupal\Core\Config\FileStorageFactory::getSync

Let's do 2 things here... add a comment saying config.storage.staging is deprecated.

But also swap it to be like this:

  config.storage.staging:
     class: Drupal\Core\Config\FileStorage
     factory: Drupal\Core\Config\FileStorageFactory::getSync
   config.storage.sync:
     alias: config.storage.staging

Because then if any site is swapping out the config.storage.staging service everything will magically still work.

YesCT’s picture

on it.

YesCT’s picture

Status: Needs work » Needs review
FileSize
112.74 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2487588.66.patch. Unable to apply patch. See the log in the details link for more information. View
0 bytes

comments in config files are not parsed by api.drupal.org, but used that notation anyway as we are used to it.

YesCT’s picture

FileSize
683 bytes

actual interdiff.

alexpott’s picture

Status: Needs review » Needs work
+++ b/core/core.services.yml
@@ -291,8 +291,12 @@ services:
       - { name: backend_overridable }
   config.storage.staging:
+    # @deprecated staging is renamed to sync.
+    # @see CONFIG_STAGING_DIRECTORY
     class: Drupal\Core\Config\FileStorage
-    factory: Drupal\Core\Config\FileStorageFactory::getStaging
+    factory: Drupal\Core\Config\FileStorageFactory::getSync
+  config.storage.sync:
+    alias: config.storage.staging
   config.storage.snapshot:

The Yaml looks great. I think we should put the deprecation notice above the service definition and use the standard text... like so...

   # @deprecated in Drupal 8.0.x-dev, will be removed before Drupal 9.0.0.
   #   Use config.storage.sync instead.
   config.storage.staging:
     class: Drupal\Core\Config\FileStorage
     factory: Drupal\Core\Config\FileStorageFactory::getSync
YesCT’s picture

Status: Needs work » Needs review
FileSize
112.79 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2487588.69.patch. Unable to apply patch. See the log in the details link for more information. View
594 bytes

ok. :)

YesCT’s picture

FileSize
113.29 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch 2487588.70.patch. Unable to apply patch. See the log in the details link for more information. View
6.87 KB
  1. +++ b/core/includes/bootstrap.inc
    @@ -140,6 +151,11 @@
    +  // @todo: Remove fallback in Drupal 9. https://www.drupal.org/node/2574943
    

    todo syntax doesn't use :
    https://www.drupal.org/node/1354#todo

  2. +++ b/core/includes/install.inc
    @@ -505,19 +505,19 @@ function drupal_install_config_directories() {
    -  // Put a README.txt into the staging config directory. This is required so
    +  // Put a README.txt into the sync config directory. This is required so
       // that they can later be added to git. Since this directory is auto-
    

    since line shorter, can wrap closer to 80 chars.

went through the whole patch, wrapped some other things also.

another couple changes to add a period to the end of a sentence (in lines this patch was changing anyway.)

alexpott’s picture

Status: Needs review » Reviewed & tested by the community

This looks great.

xjm’s picture

Assigned: Unassigned » xjm

mkay

xjm’s picture

Assigned: xjm » Unassigned
Status: Reviewed & tested by the community » Fixed

I checked the word diff and the grep again. All is well. +1 for the BC with the service alias.

I'm glad we got this in! Committed and pushed to 8.0.x, adn updated and published the CR. Thanks, all!

  • xjm committed bfb117c on 8.0.x
    Issue #2487588 by YesCT, Leksat, Schnitzel, alexpott, anavarre, xjm:...

The last submitted patch, 66: 2487588.66.patch, failed testing.

The last submitted patch, 69: 2487588.69.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 70: 2487588.70.patch, failed testing.

alexpott’s picture

Status: Needs work » Fixed
moshe weitzman’s picture

Drush has been updated for this change. Nice work all.

k4’s picture

Upgrading core from beta15 to rc1:

/admin/reports/status:
Exception: The configuration directory type 'sync' does not exist in config_get_config_directory() (line 162 of core/includes/bootstrap.inc).

/update.php:
Not present
Your /settings.php file must define the $config_directories variable as an array containing the name of a directories in which configuration files can be written.

What is the recommend upgrade path?

creating /sites/default/files/config_xyz/sync by hand does not help

$config_directories = array(); -> is always an empty array, even when installing rc1 from scratch

anavarre’s picture

@k4 you don't only need to have the sync directory created but also the following in settings.php

$config_directories['sync'] = 'sites/default/files/config_HASH/sync';

Change config_HASH by your unique HASH and default by anything else you might have chosen to go with..

Status: Fixed » Closed (fixed)

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

joshuautley’s picture

#81 worked for me