To reproduce this:

- install d8
- copy sites/default/files/config_*/active/system.site.yml to sites/default/files/config_*/staging/system.site.yml, and, in the destination file, change the title so it reads: "name: NEW TITLE"
- create a new file, sites/default/files/config_*/staging/whatever.yml -- put anything you want in there.
- now go to admin/config/development/sync and "Import all"
- The title of the site gets changed, but the file whatever.yml does not get imported, and completely disappears.

We probably will want to define what happens to configuration that cannot be imported.

Files: 
CommentFileSizeAuthor
#3 1892682-2-d8core-dont-delete-all-files-on-config-import.patch3.88 KBalberto56
FAILED: [[SimpleTest]]: [MySQL] 50,646 pass(es), 3 fail(s), and 0 exception(s). View

Comments

alberto56’s picture

Issue tags: +Configuration system

tagging "Configuration system"

beejeebus’s picture

Status: Active » Closed (works as designed)

this is a feature, not a bug.

we don't support adding new non-entity config files via import. you'll have to use a hook_update_n() function instead.

alberto56’s picture

Status: Closed (works as designed) » Needs review
FileSize
3.88 KB
FAILED: [[SimpleTest]]: [MySQL] 50,646 pass(es), 3 fail(s), and 0 exception(s). View

Currently, this is what happens:

  • The form submit handler config_admin_import_form_submit() calls config_import()
  • config_import() itself returns TRUE if everything was successful (even if some files were not recognized) or FALSE on error.
  • The submit handler then deletes all the files.

This patch does the following:

  • First, the submit handler, it seems, should not be responsible for deleting the files; its role should only be that of submitting the form. Removing the deletion into another function allows cleaner api calls without going through the form. In this case, the patch adds a $delete_source parameter to config_import() and config_sync_changes().
  • Files are then deleted, in config_sync_changes(), only if they are processed. If not, they stay in the file system.
  • The submit handler then checks if any files are left over after config_import(), and if there are files left over, then the success message says that some files were not processed and left in the staging folder.
alberto56’s picture

Title: staging files which are not imported disappear without a trace » Don't delete staging files which could not be processed.
Category: bug » feature

@beejeebus sorry, I was editing the issue at the same time as you were.

I changed this to a feature request.

One situation in which this might be useful is probably this:

Right now new views are simply ignored by the import system and are deleted when staged.

Perhaps having a system in place where stuff which is not imported is left in the staging directory will allow us some time to debug problems with config files which were not imported.

Another situation may be this: if you try to stage something for a module which is temporarily disabled, isn't it better for the config file to stay in the staging directory rather than to be deleted? That way, when the module is re-enabled, the config file will be imported.

heyrocker’s picture

As beejeebus said, if what you are adding is not a config entity, then it should be installed when your module is installed by adding it to the module's /config directory. Deleting the config after import is important, because if the config is left around it can cause problems later on. For instance:

1) Some config is imported and the old config is not deleted.
2) One of the things that config affects (say a view) is changed.
3) Some new config is added to the staging directory.
4) The new config is imported.
5) The old config, which was still sitting there, gets reimported, overwriting your changes.

Some of this can begin to be addressed after #1515312: Add snapshots of last loaded config for tracking whether config has changed goes in, because then we can tell what config is 'overridden', however I still believe that deleting the config after an import is proper functionaity.

Right now new views are simply ignored by the import system and are deleted when staged.

This should not be happening, and if it is happening its a bug (although I have tested this a lot myself.) Note that for adding new views, you have to make sure and include the manifest.views.view.yml file in your staging directory.

Another situation may be this: if you try to stage something for a module which is temporarily disabled, isn't it better for the config file to stay in the staging directory rather than to be deleted? That way, when the module is re-enabled, the config file will be imported.

We don't do imports on install/uninstall. If the module is not enabled then the config actually should be ignored and removed, I see this as proper functionality.

As far as config that fails to be imported, I think this is better handled by #1842956: [Meta] Implement event listeners to validate imports, where if importing config fails then an error is thrown in the validate step and the import does not get attempted at all.

alberto56’s picture

Status: Needs review » Needs work

@heyrocker thanks for your response. It should be noted that I am testing this whole system as a site developer so I have less of an understanding of its intricacies.

Regarding the views file, in effect I did not import the manifest, which explains why it was failing.

What do you think of giving some sort of indication on the sync form that unprocessed files will be deleted, for example:

- 5 changes
- ...
- ...
- 3 deletions
- ...
- ...
- 3 new
- ...
- ...
- 1 unknown (will be deleted)
- ...
- ...

And then mentioning that the entire contents of the staging directory will be deleted.

It might be that certain site builders who will be using this system for deployment will do so via git, and will appreciate some indication that the destination directory files being deleted is normal behaviour.

Another option might be moving the ignored files to some sort of backup directory.

Thanks for your work,

Albert.

heyrocker’s picture

Thanks for trying out the system! I realize there aren't a lot of docs for it at the moment, which is somewhat by design as it is a bit of a moving target. One thing we have discussed as far as the deletion is offering a checkbox which allows you to selectively choose not to delete some configuration.

We might also be able to call out the config files that would be ignored in the validation step. beejeebus might know more about that, as he has been working on that patch.

alexpott’s picture

Status: Needs work » Closed (won't fix)

We only support importing full config trees

alexpott’s picture

Issue summary: View changes

Specify that anything can go in whatever.yml