[policy] Patching dependencies

Created on 21 November 2024, 13 days ago

We need a coherent and documented patching policy for dependencies.

We cannot ship patched dependencies in stable releases because:

  • They can interfere with automatic updates
  • Because we don't pin dependencies, they can prevent those dependencies from being updated, which could result in security issues
  • Fixing a patch-related problem requires manual intervention by a developer, which contradicts Starshot's entire mission

Patches are, however, useful for development. I propose that cweagans/composer-patches be permanently added to our internal development dependencies, and we adopt the following patching policy:

  • Patching is allowed during active development periods (i.e., up until a beta)
  • All patches need to be removed before, or during, the beta period
  • RCs and stable releases can never have patched dependencies
  • Any dependencies which still need to be patched must be removed before an RC is tagged

Thoughts? Should we adopt this as official governance?

πŸ“Œ Task
Status

Active

Component

General

Created by

πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @phenaproxima
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts
  • πŸ‡¬πŸ‡§United Kingdom catch

    Thanks for opening this, clarifying sounds great.

    Patching is allowed during active development periods (i.e., up until a beta)
    All patches need to be removed before, or during, the beta period

    This might be a case for a main vs release branches. e.g. with core experimental modules, we commit them to e.g. 11.x, but remove them from the 11.1.x branch if they haven't reached beta stability yet.

    Feels like there are at least two kinds of patches for Drupal CMS:

    1. Patches that improve/fix something but don't require changes to the Drupal CMS codebase itself.

    2. Patches that add features or otherwise result in changes to recipes, installed modules etc.

    The first kind it would be good to leave in a 'main' branch so that there is early warning when those patches get committed or cease to apply.

    The second it would be a lot more complicated to leave in a main branch, since not only the patch but also the feature would need to be removed from the release branch.

    In terms of removing during or before beta, the main question for me would be whether someone should be able to install a beta release and then continue with their site from there - if so, no patches, if not, seems a bit more flexible.

  • πŸ‡¦πŸ‡ΊAustralia pameeela

    @phenaproxima should we update this to also say we won't use dev versions of modules for RCs and stables?

  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    IMHO that's not in scope; that's more about "what dependencies will we allow", regardless of whether we patch them or not.

  • πŸ‡¦πŸ‡ΊAustralia pameeela
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    > All patches need to be removed before, or during, the beta period

    How exactly would you ensure this? I obviously don't have the same weight, budget and time as the team working on Drupal CMS, but at the moment, it can take years for more complex core issues to be resolved, in some components more than others, for example multilingual is an especially complex aspect.

    Our solution to that is we do pin dependencies and ship patches in our internal distribution, but I also understand that this kind of contributes to the problem as we sometimes just do the minimal work to maintain a patch/MR without adressing blocking feedback that would allow it to be committed. But just as often, there also is no feedback.

    Drupal CMS is on a fairly tight schedule for the scope it has, and you can only have release blockers for you time windows if you have the means to resolve them if moving the time windows is not an option, which I assume is the case. (Anyone remember the critical issue queue blocking the 8.0 release that just did. not. want. to. end.)

    Related 🌱 [policy] Impact on contributed projects and its maintainers Active

  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    How exactly would you ensure this?

    One possible answer: before a patch is added, there needs to be an agreed-upon exit strategy for that patch if it doesn't get committed. A few ideas:

    • A guarantee from the maintainer that the patch will be committed, backed up by granting commit and release access to a Drupal CMS committer
    • A promise to remove the dependency in question, and possibly the entire feature, if the patch is not committed (this is similar to core's "we don't ship alpha experimental modules in stable releases" policy)
    • Some plan to polyfill the functionality
    • Some sort of agreed-upon fallback or workaround
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Sounds good, makes sense that that's part of the policy too then.

    > Some plan to polyfill the functionality

    That might also be rather version-dependant on the specific dependency, but has more control over that, than a fixed patch, e.g. altering a service, plugin or similar thing could attempt to check if the desired fix/functional is already there.

  • πŸ‡¬πŸ‡§United Kingdom catch

    Yes agreed the ideas in #8 sound good, obviously it will depend on the issue and project which of those might apply at any one time.

    Sort of like a polyfill, there's also the option to extract a new feature out to a new permanent contrib module with a dependency on the 'main' module. https://www.drupal.org/project/ggroup β†’ and https://www.drupal.org/project/grequest β†’ started as patches to group module, but the mutual decision was taken that they should just be independent contrib projects with their own maintainers. This obviously means different people are on the hook for maintaining those modules, and they need to be kept up-to-date with changes in group, but those problems don't necessarily go way for sub-modules or large parts of individual contrib projects.

  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

    Here's where I think some of the strategies I outlined in #8 could be useful:

    A guarantee from the maintainer that the patch will be committed, backed up by granting commit and release access to a Drupal CMS committer

    I think this would be the best fit for new features that do not have backwards compatibility implications. Example: adding config actions to some already-existing config entity provided by a given module.

    A promise to remove the dependency in question, and possibly the entire feature, if the patch is not committed (this is similar to core's "we don't ship alpha experimental modules in stable releases" policy)

    This strategy would be wise for recipes that rely on a particular bugfix in a dependency.

    Some plan to polyfill the functionality

    Best for complex new features, especially ones that might carry BC implications.

    Some sort of agreed-upon fallback or workaround

    Best for a "nice to have" kind of thing. Maybe there's some bug that is annoying but something we could grudgingly live with if there was no fix.

  • πŸ‡¦πŸ‡ΊAustralia pameeela

    FWIW we did have to remove some modules where patches weren't reviewed/committed. So we lost some features that we wanted to deliver. It's disappointing but just the way it goes. No one owes us anything, and considering how many modules we were adding it was bound to happen.

    One good thing to note is that in many cases, where we proposed features or found bugs, we were able to provide patches, and didn't rely on maintainers doing it for us. The community enthusiasm may not last forever, but it certainly has helped!

Production build 0.71.5 2024