Update: beyond near misses, it has actually happened three times now with the same duplicated libxml2 override.

Ah, that Edge PR.

Btw, this also reminded me I have to rewrite my Rofi PR (I have overrideAttrs somewhere there)

I think that if we introduce versioned attributes in python3Packages, we'll see many more uses of them in pkgs/development/python-modules (where they may only be used at build time) or outside of that in depencies (where packageOverrides should be used instead).
It would be nice to come up with an idea that prevents that.
Maybe put them under python3Packages.otherVersions? But i guess that wouldn't play well with packageOverrides?

I could consider dropping the mention of overridePythonAttrs for the time being if it's not clear what to do for it yet.

Hmm. I've always thought the security-related aspect of this should be addressed by a good security scanner that can descend into more than just attr-accessible derivations.

As far as accidental breakage goes due to a maintainer not realising that their package is being overridden elsewhere, that's what nixpkgs-review is supposed to expose, and a package that doesn't expose its breakage at build time doesn't have enough tests.

I think such a broad declaration is in danger of damaging something that I consider to be one of nix's superpowers - the ability to get awkward software with awkward dependencies packaged. Moving all variants of a package to be located with the package also surely means we'll have to expose it as an external attr, which means we'll end up with more fooBar_20, fooBar_noBaz attrs, which, other than being confusing and weird, may well encourage others to start depending on them. How are users going to know which attrs they're supposed to reference and which ones only exist to satisfy weirdPackage123?

As far as accidental breakage goes due to a maintainer not realising that their package is being overridden elsewhere, that's what nixpkgs-review is supposed to expose, and a package that doesn't expose its breakage at build time doesn't have enough tests.

That only works for packages with a low rebuild count.

That only works for packages with a low rebuild count.

Sure, but staging breakages eventually get thrown up by hydra. As far as the breakage argument goes though, I feel it's rather "caveat emptor" - the risk is with the depending package if they're going to use overrideAttrs.

the risk is with the depending package if they're going to use overrideAttrs.

Yes, that's what I like about overrideAttrs. It shouldn't be up to the maintainer of package A to make sure the overridden version of it that package B uses still works.

I also think it's a problem when you outlaw/discourage something that's so heavily used across nixpkgs, including by what I'd call "exemplar" packages. What are we then saying to people who are trying to learn from other, well regarded, packages? "We wouldn't accept this today"? Well then we wouldn't be accepting some of our core packages like LLVM either. Does the alternative we're proposing to people actually lead to a better situation, if it's even a sufficient replacement? If not, we're just leaving people in limbo.

the risk is with the depending package if they're going to use overrideAttrs.

Yes, that's what I like about overrideAttrs. It shouldn't be up to the maintainer of package A to make sure the overridden version of it that package B uses still works.

This is not how maintainers feel in practice. And maintainers (domain experts) also need to have a chance to say no to irresponsible overrides of packages, like how we have the "no new downstream kernels" rule. With overrideAttrs rather than proper abstractions, there's no possibility of quality control for these overrides.

How are users going to know which attrs they're supposed to reference and which ones only exist to satisfy weirdPackage123?

If a user has a need for the attr, they'll use it. The only difference from today will be that there'll be an incentive to reuse existing overrides rather than adding more mostly-duplicated closure-bloating overrides that could have been unified.

I also think it's a problem when you outlaw/discourage something that's so heavily used across nixpkgs, including by what I'd call "exemplar" packages. What are we then saying to people who are trying to learn from other, well regarded, packages? "We wouldn't accept this today"? Well then we wouldn't be accepting some of our core packages like LLVM either. Does the alternative we're proposing to people actually lead to a better situation, if it's even a sufficient replacement? If not, we're just leaving people in limbo.

When we were putting together this idea on Matrix, we discovered that there are actually very few unique occurrences of overrideAttrs in Nixpkgs when you take into account that something like 80% are overrides of generated packages that could be replaced in bulk with a better abstraction. Several of us had the intention of gradually working through all those and implementing better alternatives. But I opened the documentation PR first, explaining the rationale, because the alternative was explaining it separately on every one of those PRs. Nobody intends to have packages using overrideAttrs around in Nixpkgs contrary to the guidelines forever.

This is not how maintainers feel in practice.

I agree with this - it does not feel like you can do an update of a widely-used package without bearing some responsibility for altered versions of it throughout the tree. This leads to an unfortunate responsibility-without-authority situation where there is at least partial maintenance responsibility for overrides in downstream packages despite the fact that maintainers of dependencies don't really have a strong right to be super opinionated about what a downstream package does. override of feature flags is less problematic on this count because they are intended as public API (OTOH, overrides of dependencies are often essentially overrideAttrs).

overrideAttrs fundamentally breaks an abstraction boundary. It's a useful hack for out-of-Nixpkgs consumers, but makes essentially any change to a package's internals a breaking change.

Triaging stagjng regressions with Hydra is also difficult as the nature of the process and our existing tooling makes it hard to know which downstream packages are failing as a result of changes to a dependency you maintain in particular.

I do agree with @risicle that having alternate versions of a package instead of a local overrideAttrs will make people more likely to reach for those versions rather than doing better things (bumping their downstream package for compatibility, finding a reliable patch from upstream or otherwise, etc.), and that this is bad. However, my experience is they it is really hard to stop people adding and depending on old versions of things no matter what you do (and as soon as you have two versions you are likely to find you have five in a few years' time).

Having specific versioned packages at least makes it clear when this happening and what needs fixing, and it also gives the maintainers more of a chance to be notified of it and reject it if necessary. We already have an explicit guideline to avoid having multiple versions of packages when possible, which a separate version more clearly violates than a quick overrideAttrs hack.

There are cases where an old version of a package is strongly desired to not be readily accessible by maintainers of that package or its containing package set, but is unavoidable in a downstream context. For instance, ceph contains an old unmaintained version of the Python cryptography library but takes pains to not expose it so that it does not get used inside the Python package set.

These cases are just bad in general and we should do what we can to avoid them, but vendoring the old version as ceph does is in some ways superior to overrideAttrs: it is clear that the maintainers of the upstream package are not responsible for its quality or keeping it working, which can be clearly expressed in the meta.maintainers field, but by completely decoupling changes in the old and new version it also avoids the likelihood of even implied responsibility from doing things that would break it were it an overrideAttrs. It can also reduce total maintenance burden: maybe the one package that really needs the old version doesn't need half of the features added by dependencies, or maybe we have a patch to make it work on Darwin but the vendoring package is inherently Linux-only. Then the vendored package can be trimmed down to reduce the total maintenance surface area and attendant risk.

(It doesn't fix the case where changes in dependencies of that vendored package themselves break it, which indeed happened with ceph and caused it to vendor at least one additional Python library since. It also doesn't avoid security issues in cryptography requiring intervention on the vendored package by the security team or others. That's why these situations are still terrible and should be avoided whenever remotely possible. But that's why we already have the guideline against multiple package versions, and overrideAttrs would not really make it any better.)

Re: triage and finding issues in general, while I greatly appreciate that Nixpkgs is a place where any contributor is allowed to change whatever part of the stack they need to (given review and consensus), I want to strongly push back on the idea that maintainers of dependencies can reasonably be expected to be responsible for their downstream closures. This scales okay for a package with only a handful of users, but breaks down when there are thousands of downstream packages, especially if any of them happen to poke directly into internals in a way that the upstream maintainer may not even have been aware of.

Of course there is greater responsibility in maintaining a package that is widely used - it is proper that such maintainers are diligent about potential breaking changes and perform some kind of assessment of their likely scope. But there is a reason there are only maybe a dozen regular contributors at most to the staging cycle: it is somewhat of a burnout machine where you suddenly find yourself choosing between doing far too much work on a package that is receiving essentially no maintenance from the people listed for it, often ones that you strongly suspect nobody is even using, or risking complaints from users or downstream maintainers. People get scared yo make important improvements be use they know they will end up being held responsible for everything it breaks. And the reaction from downstream package maintainers when it's a version bump is often "okay, add the old version back please" - externalizing the maintenance cost on you indefinitely.

This is partly a tooling problem: we need to land staging-next cycles regularly because of security updates, we can't ship sufficiently broken ones to the channel, and we have no good way of proactively notifying downstream maintainers when a change breaks their package. But even if we did, many maintainers aren't going to respond quickly enough for it to be possible to avoid one of the same handful of people investigating it themselves yet again, or shipping the regression to the channel and making things worse for users.

I don't have a complete, readymade solution, be t I think every solution will have to involve acknowledging that upstream maintainers have only limited responsibilities to downstream packages, that it is reasonable to expect downstream maintainers to take responsibility for resolving those issues in a way they does not add undue burden to the upstream maintainer, and that we can't force every maintainer to maintain a version of their package both before and after a change without very good reason.

Of course, there can be no hard rules here, and in a project as interconnected as ours there is always negotiation both ups and down the stack. But the current situation where maintainers of leaf packages can do pretty much whatever and upstream maintainers just have to deal with it is not good. It's hard enough to maintain those core parts of the stack without it also implying responsibility for the rest of it too. I think less overrideAttrs would be a step in the right direction here, and addressing the problem by more thoroughly making them responsible when it breaks would go the other way.

I would like to merge this at NixCon this week. I encourage anyone who thinks that we should not merge it to place an ❌ with the reasons they think we shouldn't do so in the intervening time.