Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Spin-off from #3346707: Add Alpha level Experimental Package Manager module.
Automatic updates allows sites to run both AU + composer patches at once.
Composer patches is incompatible with actually updating a site automatically, because as soon as a patch is committed upstream, or an upstream change causes a conflict, you need to manually determine the problem with the patch and either replace it with a newer one or remove it - both of which require manually editing JSON.
Additionally, applying patches that include update paths is very high risk and can leave a site in a state where it's not possible to update the database successfully - e.g. if a different update takes hook_update_N().
However, this is not a barrier to using project browser, or some advanced AU use-cases where a developer is prepared to manually intervene every so often and AU is only expected to handle what it can handle.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | package_manager_patch.patch | 4.98 KB | iamcorekhan |
Issue fork drupal-3474292
Show commands
Start 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:
- 3474292-package-manager-automatic
changes, plain diff MR !11903
Comments
Comment #2
quietone commentedI agree with not allowing composer patches by default, for the reasons explained in the issue summary.
Comment #3
catchTagging this as a package manager/automatic updates beta blocker, we could potentially move it to a stable blocker later but should at least discuss more before beta.
Comment #4
catchComment #5
larowlanComment #6
catchThis validation is done in
ComposerPatchesValidator, currently it checks if composer patches is a root composer dependency, and issues a validation error if it's a dependency anywhere else and not root.Probably what we'd need here is a settings.php flag to enable that behaviour, defaulting to FALSE, and then only allow composer patches as a root requirement when it's TRUE. This would force people to make an explicit choice to use package_manager with composer patches, rather than potentially doing it by accident/without thinking about the consequences.
My main concern here is that someone installing Drupal CMS (and eventually site templates) via the UI, never gets into a situation where they have to manually edit JSON to update their site. Requiring a settings.php flag would mean that only pre-release versions of site templates could ship with dependencies on composer patches, which is fine for testing, but they would need to be committed upstream before a stable release, and the composer dependency removed, in order to work with automatic updates/project browser.
I'm not sure how this interacts with #3355485: Dependencies should be 'unpacked' to the root composer.json and merged/resolved - e.g. if recipes specify dependencies on composer patches, and those then get unpacked to the root composer.json, will that trigger the validation or not? From reading it, it looks like it prevents that situation from happening. I think if the answer to this is that it's impossible for that to happen and it will produce a validation error, we could move this to a stable rather than beta blocker. As long as the settings.php flag is enough, and we're fine restricting the behaviour beyond what it allows now during beta, the actual code change should be pretty small.
Comment #7
iamcorekhan commentedComment #9
phenaproximaAdjusting credit, since the MR is a slimmed-down version of the patch in #7.
Comment #11
phenaproximaSeeing as how
ComposerPatchesValidatorTestfailed until I specifically enabled the setting in that test, I'd hazard to guess that there's probably no additional test coverage needed here. Crediting myself for the MR.Comment #12
phenaproximaComment #13
tedbowSee MR comment
Comment #14
phenaproximaI like that a lot better. We don't need to worry about versions, we support both majors in ComposerPatchesValidator.
Comment #15
catchThat looks simpler, but does it need new test coverage then?
edit: or wait - is the validator test change the test coverage? But if so, can we add an assertion above to show that it's really not allowed, or would that duplicate other test coverage elsewhere?
Comment #16
phenaproximaIt would duplicate other test coverage. Now the patcher is just treated like any other "untrusted" plugin, which will raise a validation error unless explicitly allowed in configuration.
Comment #17
catchThat was my only question, this is much simpler than the new setting.
Comment #19
larowlanCommitted to 11.x - thanks all
Comment #23
larowlanTake 2, committed the right thing this time. Been having grief with gitlab signing me out and looks like the signin button sent me to a previous MR I'd viewed. Still my fault for not doing git show before I pushed. Resolved now