Problem/Motivation

The minimal install profile carries a dependency on the dblog module. However, sites built by advanced users might typically use a different logging solution than database logging.

The original post of this issue also reported this hypothetical bug:

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."

However, no one was able to reproduce that bug in the past 1.5 years, and the original poster has not responded confirming how to reproduce it.

Proposed resolution

Remove dblog as a dependency in the minimal install profile.

This proposal is considered a "won't fix" or "works as designed" because we want the minimal profile to include some logging solution by default, even if it is often replaced by a different logging solution. (syslog is not guaranteed to be available on every site.)

Remaining tasks

The test fix from this issue has been split off into #2900222: Remove coupling to minimal profile in ModuleHandlerTest::testUninstallProfileDependency() and use the testing profile instead.

User interface changes

Database Logging module will not be enabled when installing via Minimal install profile.

API changes

None

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

achton created an issue. See original summary.

achton’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-minimal-dblog-remove.patch, failed testing.

andypost’s picture

Issue tags: +Configuration system, +Novice

Makes sense, just needs to fix test

alvar0hurtad0’s picture

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

andypost’s picture

Version: 8.0.x-dev » 8.1.x-dev
Issue tags: +Needs change record

features goes to 8.1 and CR needed

andypost’s picture

Saphyel’s picture

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

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

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

alexpott’s picture

Component: configuration system » other
Category: Feature request » Task
Issue summary: View changes
Issue tags: -Configuration system

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.

andypost’s picture

Status: Needs review » Needs work

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

alvar0hurtad0’s picture

Status: Needs work » Needs review
FileSize
1.05 KB
738 bytes

Here the changes on #12

Status: Needs review » Needs work

The last submitted patch, 13: do_not_require_dblog_as-2658864-13.patch, failed testing.

aburrows’s picture

Assigned: Unassigned » aburrows

Working on this at Drupalcon New Orleans

aburrows’s picture

Assigned: aburrows » Unassigned
alexpott’s picture

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?

aburrows’s picture

Assigned: Unassigned » aburrows
aburrows’s picture

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

Pradnya Pingat’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 19: do_not_require_dblog_as-2658864-19.patch, failed testing.

rajeshwari10’s picture

Assigned: aburrows » rajeshwari10
rajeshwari10’s picture

Status: Needs work » Needs review
FileSize
1.02 KB

Deleted the dependency for db_log.

Status: Needs review » Needs work

The last submitted patch, 23: do_not_require_dblog_as-2658864-23.patch, failed testing.

rajeshwari10’s picture

Status: Needs work » Needs review
FileSize
1.06 KB
1.02 KB

Adding patch and interdiff.
Thanks!!

markdorison’s picture

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.

dagmar’s picture

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
markdorison’s picture

@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.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

gambry’s picture

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.

Noemi’s picture

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

#ddrome17 #hackaton

arturopanetta’s picture

Ok #ddrome17

dagmar’s picture

Status: Needs review » Needs work

The patch doesn't apply anymore.

markdorison’s picture

Status: Needs work » Needs review

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

alexpott’s picture

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

gambry’s picture

Assigned: rajeshwari10 » Unassigned
Status: Needs review » Postponed (maintainer needs more info)

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

ZeiP’s picture

Status: Postponed (maintainer needs more info) » Needs review

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.

dagmar’s picture

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.

andypost’s picture

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -Novice

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?

andypost’s picture

Status: Needs work » Reviewed & tested by the community

@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

xjm’s picture

Issue summary: View changes

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.

xjm’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs review

@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.

xjm’s picture

Issue summary: View changes
Status: Needs review » Closed (cannot reproduce)

#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".