Drupal CMS out of the box includes a large number of modules that are not enabled by the base recipe. These will be on the filesystem of sites that install Drupal CMS without being installed - until a recipe that depends on them is installed.
If these modules are installed later without being update in the meantime, there are two potential negative consequences:
1. If the module has had a security release, site owners will start getting a notification that the module is insecure after they've installed it (and made their site insecure), not before.
2. If the module has had new releases with database updates included, the site owner will get a notification that a newer version is available after they install it, and then the module's updates will need to be run. If they update it before installing, no updates to run which is a lot more reliable and less error prone.
I've opened an issue with more or less the same issue summary against Drupal CMS here: #3510973: Set 'Check for updates of uninstalled modules and themes' to on by default, but also think we should consider flipping the default in core.
If people find the notifications annoying, then that might prompt them to actually composer remove the module, or they can always flip the setting back.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !11401
Comments
Comment #2
longwave+1 for this. Dependabot and similar tools already flag uninstalled modules because they only look at composer.json.
Also, this would help if there was ever another SA-CONTRIB-2016-039.
Comment #3
dwwThe UI for when you turn this on isn't terrible. 😅 The uninstalled stuff appears in a whole separate table at the bottom of the report. So hopefully won't be too much confusing noise in the regular case.
In general, +1 to the change.
Comment #4
dwwComment #6
dwwComment #7
dwwUpdateManagerUpdateTestis failing now, since it's assuming the old default.However, that test is completely removed @ #3502973: Remove UI and routes for the ability to update modules and themes via update.module and authorize.php.
Should we bother fixing the test here and needing to re-roll one of these MRs whenever the other lands, or should we postpone this on committing #3502973 first?
Comment #8
smustgrave commented+1 for defaulting this true. Think we can postpone as this is a "nice to have" update but don't think breaking anything immediately. Will prioritize the blocker though in my next round of reviews.
Comment #9
dwwBlocker is in. Just need trivial rebase and those should be green pipelines
Comment #10
dwwRebased. Pipeline is green.
Comment #11
longwaveWell, that was easy!
Comment #12
poker10 commentedDo we need a CR for this (for example for distribution developers), since we are changing the behavior for new sites?
Comment #13
catchAdded a CR:
https://www.drupal.org/node/3512744
Comment #15
catchCommitted/pushed to 11.x, thanks!
Comment #17
dwwThanks for adding the CR and committing this. Hopefully this helps more than it confuses. 🤞
Comment #19
dwwPer a Slack thread in #core-development, tentatively tagging this as a change that could be quite visible to end users. I'm not attached that it's actually a "highlight", but potentially worth mentioning. I'll leave that decision to the release managers and highlight editor(s). 😅
Thanks,
-Derek