Hello,

With the change in core that add a hash in config files. All the features are marked as changed.

I find that in #2636514: ConfigUpdateTest is failing (finding spurious differences in some config) there is a patch that prevent the diff on this key.

I think it will also be useful to not export this hash.

I will upload a patch.

CommentFileSizeAuthor
#10 features-do_not_export_hash-2668526-10.patch1.3 KBgrimreaper
#8 features-do_not_export_hash-2668526-8.patch1.43 KBgrimreaper
#2 features-do_not_export_hash-2668526-2.patch956 bytesgrimreaper

Comments

Grimreaper created an issue. See original summary.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Active » Needs review
StatusFileSize
new features-do_not_export_hash-2668526-2.patch956 bytes

Here is the patch.

Thanks for the review.

juanramonperez’s picture

juanramonperez commented

Is a good idea remove this?

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » grimreaper
Status: Needs review » Needs work

Hello juanramonperez,

I don't know if it is a good practice to remove the hash but in config_update (which Features depends on), there is a commit that skip the hash in the diff #2636514-3: ConfigUpdateTest is failing (finding spurious differences in some config).

So if we will not see diff on the hash I don't think it is needed to export it.

I change the status to Needs work as I need to test more because I think I still saw hash exported. Maybe the patch does not cover all export plugins.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
Related issues: +#2599278: Move export processing to a configurable plugin

@Grimreaper: thanks for filing this. Yep, looks like we should be stripping this out on export.

This issue is related: #2599278: Move export processing to a configurable plugin. Ideally I'd like to see these various types of export processing done via plugin, but meantime we can just add 

+++ b/src/FeaturesManager.php
@@ -875,6 +875,9 @@ class FeaturesManager implements FeaturesManagerInterface {
           unset($data['uuid']);
+          if (isset($data['_core'])) {
+            unset($data['_core']);
+          }

This might be clearer if we consolidated with the existing processing:

$keys = ['uuid', '_core'];
foreach ($keys as $key) {
  if (isset($data[$key])) {
    unset($data[$key]);
  }
}
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Hello nedjo,

Thanks for the review.

As said in comment 4, I will test further the export (drush, download archive and write).

I will remake the patch this weekend.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

I found why I still get _core in exported config files.

For theme settings. $this->configManager->getEntityTypeIdByName($name) is null.

So uuid and _core are not unset.

And another thing stranged is that in the theme settings file there is no uuid...

I will see why and provide a patch to manage those files.

EDIT: ok when I see the config files export with core CMI, it seems that the config entities have uuid and _core. But "simple" config have only _core.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Needs work » Needs review
StatusFileSize
new features-do_not_export_hash-2668526-8.patch1.43 KB

Here is the patch with your recommandation nedjo.

Thanks for the review.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented 
+++ b/src/FeaturesManager.php
@@ -871,12 +871,15 @@ class FeaturesManager implements FeaturesManagerInterface {
-        // The UUID is site-specfic, so don't export it.
-        if ($entity_type_id = $this->configManager->getEntityTypeIdByName($name)) {

In the original code here we're testing to ensure we have a config entity before unsetting the uuid, presumably on the assumption that otherwise a uuid could be valid. So we should retain that. TBD: is _core added to all config or only to config entities? If to all config, it should be removed outside the conditional.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
StatusFileSize
new features-do_not_export_hash-2668526-10.patch1.3 KB

Hello nedjo,

_core is added for both simple config and config_entities.

Here is a new patch based on your last comment.

Thanks for the review.

smaz’s picture

smaz
Primary language English
Location England, UK
commented
Status: Needs review » Reviewed & tested by the community

Just tested the patch in #10 with a full config export (i.e. all features) into an installation profile, and _core was not included.

Using config_update-8.x-1.1, it doesn't compare against the _core that is then in the database.

All looks good to me!

  • nedjo committed c766e79 on 8.x-3.x authored by Grimreaper 
    Issue #2668526 by Grimreaper: Do not export _core default_config_hash.
nedjo’s picture

nedjo
he/him/his
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks!

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Thanks you all!

Status: Fixed » Closed (fixed)

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

vdacosta@voidtek.com’s picture

vdacosta@voidtek.com commented

Hi,

The export configuration of "default_core_hash" and "UUID" is set in "alter" option for the bundle.
See more information: https://www.drupal.org/node/2856786