Do not require dblog as part of the minimal install profile

Makes sense, just needs to fix test

In this patch the dependency module used in the test is chaged to "node"

features goes to 8.1 and CR needed

@andypost there is no documentation about which modules should be enabled in minimal atm. (In the code at least)

I have no issue with not installing dblog in minimal and as this doesn't effect existing sites I see no reason not to do this in 8..x.x

This is not an API change. Nor is it a feature. Nor is it part of the configuration system. There is no component that really fits this so going with "other".

This in turn means that disabling dblog in the future and importing an updated core.extensions will violate this dependency and issue an error like "Unable to install the Minimal module since it requires the Database Logging module."

I've tested this and none of that occurs.

Still needs change record

+++ b/core/modules/system/tests/src/Kernel/Extension/ModuleHandlerTest.php
@@ -187,7 +187,7 @@ function testDependencyResolution() {
-    $dependency = 'dblog';
+    $dependency = 'node';

no reason to slowdown test with module that has a lot of dependencies
please use something slim like "action" module

Here the changes on #12

Working on this at Drupalcon New Orleans

This in turn means that disabling dblog in the future and importing an updated core.extensions will violate this dependency and issue an error like "Unable to install the Minimal module since it requires the Database Logging module."

Dependencies of profile modules are not required so I'm not sure why we're seeing this. Can either the IS be updated or evidence of this behaviour provided?

Patch attached, removed the dblog from minimal.info.yml

Deleted the dependency for db_log.

Adding patch and interdiff.
Thanks!!

The patch removes dblog as a dependency but adds action, but I don't see any mention in the issue summary of why the latter is being done. +1 for an updated issue summary if I am missing something.

In the meantime, I am submitting a patch that depends on uninstalling block for the test and does not add a new dependency for the minimal profile.

The patch removes dblog as a dependency but adds action, but I don't see any mention in the issue summary of why the latter is being done. +1 for an updated issue summary if I am missing something.

This was suggested on #12:

no reason to slowdown test with module that has a lot of dependencies please use something slim like "action" module

@dagmar That makes sense with regard to the test, but then we are also adding a new dependency (action) for all site installs using the minimal profile. #26 uses block for the dependency test and does not add any additional dependencies to the minimal install profile.

I don't understand the additional values this issue is trying to bring.

Without applying the patch (using 8.2.6), I have:

1. Disabled dblog module
2. Exported the configuration. Now dblog is not listed on core.exstension and dblog.settings entity conf is deleted
3. Re-installed a Drupal instance, importing the configuration with config_installer with $settings['install_profile'] = 'minimal';

Minimal is the running profile, dblog is not installed, no error message during installation.

UPDATE: Same experience on importing the configuration without dblog into an instance running minimal, no errors.

Patch works and I'm happy to create the CR and RTBC but I'm not sure - as said above - what we are trying to achieve here.

We test it and it works. The minimal profile has been installed without dblog.

#ddrome17 #hackaton

Ok #ddrome17

The patch doesn't apply anymore.

Patch in #26 applies cleanly for me against the 8.4.x branch .

No one has answered @gambry's question in #31. Install profile "dependencies" are not the same as module dependencies.

I'm not the maintainer, but it looks quite clear that due doubts raised on #31 and #36 we need more info here.

Although the motivation in the issue summary is somewhat inaccurate, I absolutely agree with removing dblog from the minimal profile. As far as I understand the minimal profile should only depend on modules which are somewhat crucial for using Drupal. The dblog module is quite the opposite: I usually recommend disabling it pretty much right away after installation, and using syslog instead.

Enabling dblog by default makes sense in the standard profile, which is used by less experienced people. Minimal, however, is a profile which should only be used by skilled Drupal users anyway, and for them enabling dblog doesn't bring any added value.

Setting back to Needs review.

In my opinion the problem here is we are using minimal profile to check if a dependency can be uninstalled (See #26) If we use the testing profile instead we can verify the same functionality and keep removing modules form the minimal profile.

Added change record: https://www.drupal.org/node/2891197

The issue summary seems updated too.

Let's get this in

When you build your D8 site and export the core.extensions via CM, minimal is also a part of this export. This in turn means that disabling dblog in the future and importing an updated core.extensions will violate this dependency and issue an error like "Unable to install the Minimal module since it requires the Database Logging module."

Profile dependencies should only be counted as requirements by the installer, any other situation should ignore them. This is very confusing, and it's been discussed a lot in #820054: Add support for recommends[] and fix install profile dependency special casing but this looks like a generic bug report.

I'm also confused though why a config import would be trying to install a profile, shouldn't it already be installed?

@catch #31 was about using config_intaller profile

3. Re-installed a Drupal instance, importing the configuration with config_installer with $settings['install_profile'] = 'minimal';

Manually tested it and that issue is not related here
drush (8.1.12) that still appends settings.php with with install profile name

More detail in #2871896-7: 8.3.x "core.extension.profile" config prevents installation

Several people have asked how to reproduce the issue described in the summary, but it was only mentioned by the original poster 1.5 years ago, and that person has not commented on this issue since. People tried to reproduce it using both core and the config_installer module. It could be that the person just assumed this would happen and did not actually test.

If that bug existed, it would needs test coverage and so forth, but since the person has not responded in 1.5 years, we can just rescope the issue to address these two questions:

1. Is database logging really a "minimal" site usecase?
2. Is there any impact on existing sites by removing the dependency?

I'm pretty sure the answer to #2 is "No". As for #1, I think the answer there is "No" as well, for the reason described in #37. The purpose of minimal as far as I know is to save advanced site builders the annoyance of disabling many different modules, so we might as well save them the annoyance of disabling this one too.

I've updated the IS to remove the red herring of the hypothetical bug that no one has been able to reproduce and that the OP has not confirmed. Leaving at RTBC since catch was the one who bumped it back before.

@catch made the point that minimal should not "log to dev/null", i.e., some logging solution should be a part of minimal by default. Since we can't guarantee that syslog is available on every site environment, it makes sense for this logging solution to be dblog by default. I guess this is similar to the fact that minimal also ships with the internal page cache even if advanced users might configure different caching solutions.

So, that part of the issue is wontfix.

However, the OP also described a hypothetical bug that no one has been able to reproduce in 1.5 years, so for that, the issue is "cannot reproduce".

I've updated the IS to reflect this.

The test change in the current patch is worthwhile on its own, though, since we should use the testing profile for testing profiles rather than coupling tests to features that are not being tested. To avoid confusion, let's split that test change out into a separate issue.

#2900222: Remove coupling to minimal profile in ModuleHandlerTest::testUninstallProfileDependency() and use the testing profile instead is the new issue. Marking "Closed (cannot reproduce)" for the original, hypothetical bug report. The feature request to remove dblog from minimal is considered "Won't fix".