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
Comment | File | Size | Author |
---|---|---|---|
#39 | 2658864-39.patch | 1.17 KB | dagmar |
Comments
Comment #2
achtonComment #4
andypostMakes sense, just needs to fix test
Comment #5
alvar0hurtad0In this patch the dependency module used in the test is chaged to "node"
Comment #6
andypostfeatures goes to 8.1 and CR needed
Comment #7
andypostComment #8
Saphyel CreditAttribution: Saphyel as a volunteer commented@andypost there is no documentation about which modules should be enabled in minimal atm. (In the code at least)
Comment #10
alexpottI 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
Comment #11
alexpottThis 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".
I've tested this and none of that occurs.
Comment #12
andypostStill needs change record
no reason to slowdown test with module that has a lot of dependencies
please use something slim like "action" module
Comment #13
alvar0hurtad0Here the changes on #12
Comment #15
aburrows CreditAttribution: aburrows as a volunteer commentedWorking on this at Drupalcon New Orleans
Comment #16
aburrows CreditAttribution: aburrows as a volunteer commentedComment #17
alexpottDependencies 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?
Comment #18
aburrows CreditAttribution: aburrows as a volunteer commentedComment #19
aburrows CreditAttribution: aburrows as a volunteer commentedPatch attached, removed the dblog from minimal.info.yml
Comment #20
Pradnya Pingat CreditAttribution: Pradnya Pingat commentedComment #22
rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commentedComment #23
rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commentedDeleted the dependency for db_log.
Comment #25
rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commentedAdding patch and interdiff.
Thanks!!
Comment #26
markdorisonThe patch removes
dblog
as a dependency but addsaction
, 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.Comment #27
dagmarThis was suggested on #12:
Comment #28
markdorison@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 usesblock
for the dependency test and does not add any additional dependencies to the minimal install profile.Comment #31
gambryI don't understand the additional values this issue is trying to bring.
Without applying the patch (using 8.2.6), I have:
$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.
Comment #32
Noemi CreditAttribution: Noemi commentedWe test it and it works. The minimal profile has been installed without dblog.
#ddrome17 #hackaton
Comment #33
arturopanettaOk #ddrome17
Comment #34
dagmarThe patch doesn't apply anymore.
Comment #35
markdorisonPatch in #26 applies cleanly for me against the
8.4.x
branch .Comment #36
alexpottNo one has answered @gambry's question in #31. Install profile "dependencies" are not the same as module dependencies.
Comment #37
gambryI'm not the maintainer, but it looks quite clear that due doubts raised on #31 and #36 we need more info here.
Comment #38
ZeiP CreditAttribution: ZeiP at Citrus Solutions Oy commentedAlthough 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.
Comment #39
dagmarIn 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.
Comment #40
andypostLet's get this in
Comment #41
catchProfile 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?
Comment #42
andypost@catch #31 was about using config_intaller profile
Manually tested it and that issue is not related here
drush (8.1.12) that still appends
settings.php
with with install profile nameMore detail in #2871896-7: 8.3.x "core.extension.profile" config prevents installation
Comment #43
xjmSeveral 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:
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.
Comment #44
xjm@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.
Comment #45
xjm#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".