Stop creating config during rewrites of optional or non existing config

This looks like a good solution. Can we get an additional test added?

Here's an updated patch that should address the issue. I've included a bit more logic for checking if the rewrite should be saved in addition to some more logging and added a step to the test that should pass...

This is an updated patch that changes a few minor things and accomodates the failing multilingual test.

We had been creating translated config overrides on the fly here too but now that existing translation overrides do not yet exist, the check shows they fail during import. Therefore role.test4 will import as its original version, but config rewrite does not allow for the translation overrides to be written. We actually do want that feature though, so logic had to be changed to accomodate it.

In any case, this should allow for not-yet-existing overrides to be created (yeah, they'll load an empty $original_data object but thats ok).

At the same time, the new logic should catch when we're dealing with a standard config rewrite being attempted on an empty config object (which is the use case described that we need to avoid in the OP).

My bad, left some comments in that should have been removed to pass the phplint.

@a.dmitriiev I am going to leave this as-is over the weekend and look to commit it next week if no objections.

Updated patch with a minor fix that adds the missing @langcode.

// Log a message indicating whether the config rewrite was saved or not.
          $log = $langcode ? '@config (@langcode) @result by @module' : '@config @result by @module';
          $this->logger->notice($log, [
            '@config' => $file->name,
            '@result' => $result,
            '@module' => $extension->getName(),
            '@langcode' => $langcode
          ]);

Added some better logging for when $langcode is being used.

This updated patch adds support for optional config rewrites as a part of a profile install, which is a tricky problem as optional config is actually installed after all dependencies of an install profile are installed.

This change requires #2924549: Invoke hook after a site install is complete but should not affect the existing behavior of config rewrite if you do not patch core. All install profiles using config rewrite to rewrite optional config will need to patch core, after this is committed.

#14 works for me, but as mentioned, it depends on #2924549: Invoke hook after a site install is complete that blocks the patch to be committed into config_rewrite.

Another approach to solve this with no dependency on additional core hooks could be act on config save event. Optional configuration that currently cannot be rewritten could be added to a delay queue and be applied by a config save event subscriber.

Still not handled. We recommend to use our fork: https://www.drupal.org/project/config_replace. There this issue is already tackled.

Wait... you forked config_rewrite as config_replace? Are you deliberately trying to confuse people?

We probably could've worked together. Heartbroken!

I agree its not ideal we need a core patch to make this fix, but seems like a pretty big deal to fork just for one issue? Were there other issues we could've helped on?

As a general practice, its not really good form to just fork a module then pimp your fork in their issue queue. Why not try to collaborate or co-maintain? Honestly disappointed.

I went ahead and checked the codebase for config_replace and it looks like aside from renaming a bunch of classes and adding some additional tests re: writing nonexiting config that everyone could've used, nothing has really changed.

I don't have time today, but honestly not much stopping us from just taking back what you "contributed" in your module to provide fixes for the larger user base. What a colossal waste of everyone's time.

Next time try my user contact form, filing an issue here, or even emailing my very obvious work email before spending TONS of cycles on forks that no one needs. Ugh...

I freed up some cycles to try this out. Here's the patch. Its really not too big once you undo all the renaming, most of which could be undone via perl or by using the rename utility.

Anyway, here is is, with Peter listed as a co-maintainer. And yes, if he comes back here and wants it, he can RTBC the patch and be added to the list of maintainers. No hard feelings here, I just want what's best for the community of 2000 installs for this module and the other one combined. (1700 here, 300 there).

Maybe someone can review?

I see that the issue linked was closed, works as designed by Peter and I kind of agree with him there. That said, I think the patch you proposed to gracefully fail in those cases is a good work-around, its probably best to not obfuscate these things, but adjust architecture accordingly.

If anyone has time to review/RTBC this patch, I will bring in these changes and we can move forward.

No, I don't agree with that and I don't appreciate you derailing this issue with that edge case. If you want to file a different issue to talk out that problem, we can do that there, but this is not the place.

You shouldn't be trying to rewrite configuration that's optional. You should only be rewriting config that you know is already installed.

Now, the issue here, really, is that we dont want to create new config, we want to throw an exception because the developer has tried to do something that will break things.

What you're asking for is to violate the terms of the API in random cases because you do not know if you'll have modules installed. That's only going to cause you problems.

Yes, right now, we want an exception thrown. No, we don't want to soft obfuscate the error in cases where people provide rewrites for optional config that's not there.

The use case you describe should only exist when one stubbornly tried to force in rewrites without declaring dependencies in a way that ensures the config is installed before it's attempted to be rewritten.

Patch fails to apply to config_rewrite 1.4.0. Will try the dev version.