> On 28 Mar 2022, at 12:05, Thomas Bracht Laumann Jespersen <t...@laumann.xyz> > wrote: > > Hi! > > I've been working on a new section in the devmanual regarding conditional > patching. In a PR [0] Sam suggested adding a section to clarify that > conditional > patching should be avoided, because it can quickly become a maintenance and > testing burden. > > The devmanual PR is here: [1] > > Ulrich points out that conditional patching is actually suggested elsewhere, > and > that actively discouraging conditional patching is a policy change, which > should > be brought up on this list. So this is what I'm doing now. He also pointed > out a > comment in eutils.eclass re [2] (the comment now lives in epatch.eclass). >
Not convinced it is a policy change explicitly as we do soft-discourage it, but having the discussion is worthwhile. I suppose it's harder to argue with the odd example of conditional patching in the devmanual not being accompanied with a warning though. :) > The overall policy (proposal, I guess) is something like: > > * Patches should be written such that they affect behaviour correctly based on > e.g. build time definitions or config options, rather than USE flags > directly. > > * They should be applied unconditionally, so that, for version bumps and other > development happening with a package, any failure to apply the patch will be > caught by the developer. Yep. Although we're somewhat accepting of this in Prefix land, it does still happen with odd use combinations. I have less of an issue with this for prefix & kind of musl (although musl stuff should really be done in a standards-compliant way & upstreamed) because especially for prefix, sometimes there is no easy way to do this properly, and at least a patch for prefix users will fail hard rather than silently breaking (e.g. sed). > > * Patching is ideally only done to make the package in question build > properly, > and should not be done to modify the runtime behaviour of the package. (This > is what USE flags and configuration options of the package are for.) > I have another reason for why this is important: a conditional patch can be pretty much never be upstreamed. There might be some cases where we're conditionally applying it out of conservatism (which we should probably re-evaluate) even though it has e.g. appropriate configure options, but most of the time, a conditional patch is unconditionally applying functionality and therefore isn't upstreamable in its current state. > Sam made a specific point re musl: "for e.g. musl patches, we want a portable > fix, not a hack which is only applied for musl" Yep. I'd note that sometimes (as I say in my reply to grobian), it's okay, but we want to discourage it for sure. > > Feedback on this very welcome. I'm grappling a bit with the exact wording to > go > for, so input on that is also appreciated. > > I think my question to this list is: Should it be policy that conditional > patching is to be avoided? > Thanks for bringing this up. > All the best, > -- Thomas Best, sam
signature.asc
Description: Message signed with OpenPGP
