PLEASE fix ALL packages which were broken by the Qt-wrapping-changes

I’m constantly stumbling over packages (in nixos-19.09), which are broken due to the Qt-wrapping-changes. This is an intolerable situation.

The Qt-wrapping-changes should have never been accepted, without fixing all affected packages first. But since it’s to late now to undo the Qt-wrapping-changes, PLEASE fix all packages affected to it now!

Doing nothing and waiting for individual bugreports for each package does not help.

It’s simply a matter of available resources.

It sucks when things don’t work but if you can open issues or even better PRs that fix things, we will definitely backport them to the latest stable release.

2 Likes

My problem here is that

  • changes are made, which obviously break many packages
  • nothing is done about this, not even a bug-report, nor a plan to fix these, nor a mark-all-such-packages-as-broken. Instead, the obviously broken packages are silently kept in the stable releases…

It’s not mainly a matter of available resources. It’s a matter of quality control, and of verifying and fixing bad effects of a major change. If a change breaks so many packages, but there are not enough resources to fix these, the package-breaking-change may not be merged at all. It’s as simple as this.

I’m willing to help fixing these problems. But only after there’s some quality control to prevent such situations in the future. I’m not willing to clean up the dirt others thoughlessly and ignorantly left.

No, it is not as simple as that. When a core package needs a change, then it cannot be expected that the maintainer of that core package fixes all reverse dependencies. That is a community effort, and in this case there are many packages that are affected that just don’t have a maintainer and apparently aren’t widely used either and thus the broken packages end up in stable. If you need the package, then you maintain it; don’t expect someone else to do that for you.

This is an important reason to reduce the size of Nixpkgs, or at least mark packages with quality levels, because this has happened often and will happen more often.

9 Likes

This is an important reason to reduce the size of Nixpkgs, or at least mark packages with quality levels, because this has happened often and will happen more often.

Quality levels might be a very good idea, because there are many packages that work for what the initial packager believes is the core usecase, and useful for that usecase, but don’t fully support some of the relatively well-known additional usecases. Improvements to the target usecase might even break some of the secondary features…

2 Likes

When a core package needs a change, then it cannot be expected that the maintainer of that core package fixes all reverse dependencies.

But it should be expected (and should be verified before the merge), that the maintainer marks all depending packages as may-need-upgrade/maybe-broken or something like this. Since that’s hard to do manually, some automatic tool should do this or at least help here.
And there needs to be a guideline how to do this.

Otherwise, you constantly silently break packages. And that’s intolerable for a usable distribution.

That’s another problem. We urgently need quality-control:

  • Before each stable release, all packages should be (if possible: automatically) tested, if possible. All non-tested packages should be marked as “maybe-broken”.
  • Before each stable release, for each package a maintainers should say, if he/she actively maintains the package. All non-actively-maintained packages should then be marked as “orphaned”/“missing-maintainer”.

All major Linux distributions do have something like this. I’m wondering why NixOS hasn’t.

1 Like

Please don’t take it the wrong way, but that is quite an entitled position. Nix is mostly maintained by volunteers, with varying levels of expertise and available time.

In my opinion, it isn’t very helpful - or polite - to request that certain features are implemented or procedures adapted, esp. without considering if it increases the already existing workload on regular maintainers. I’m also not sure what you want to accomplish with removing inactive maintainers after a rather short while, as there are various reasons why they could be not available at release time. (I.e. I was down with depression during the last release window, and honestly wouldn’t have had the energy to even check a button that I’m still there)

If you really need a package to work, imo you either have to pay someone to maintain it or do it yourself. That also applies mostly for other distros that are maintained by volunteers.

4 Likes

I don’t agree with @rkoe’s tone (which could be less accusatory and more constructive In my opinion) but I do agree that it would be nice to

  • test big changes
  • ping all maintainers of broken packages, give them reasonable time to fix breakages
  • mark remaining packages as broken

As @FRidh said it is not reasonable to expect a single person to fix all issues. Its also not a good idea to hold off on improvements because they might break things. But we should still do some testing and give maintainers some heads-up.

12 Likes

A first step in this direction would be to re-enable emails from hydra to the maintainer of a package when compilation used to work but now fails.

12 Likes

I actually was one of the first people who noticed these issues, and I had a similar sentiment that we should fix all of them semi-automatically before we released. However, the qt wrappers were not a breaking change in the first place. If you read the threads it is because a huge majority of packages were written incorrectly, often not using the qt specific deriver and callPackage function. Because of this it would be entirely unreasonable to ask one single person to fix every single expression for the maintainer who didn’t package it correctly in the first place.
I talked with numerous people on IRC in #nixos-dev and Thomas Tuegel as well, and we got a change that would abort builds that don’t use the supported deriver Abort Qt builds that do not use a supported deriver by ttuegel · Pull Request #70691 · NixOS/nixpkgs · GitHub, making all applications that fail at runtime to fail at build time. Then you could easily run a jobset and do a sed operation on all the newly broken packages. The issue for me is I didn’t really have the time to really support this, so I’ve done my very best to leave a very good trail so people can help. In the meantime, on a case by case basis I’ve reviewed probably 100+ submissions to fix expressions and made sure they were backported to stable releases always.

We are going to actually mark every package that doesn’t build as broken at the time of 20.03 release. But things not working at runtime is something we don’t have the infrastructure to determine automatically.

6 Likes

I think this is a key thing and should be done (and puts little additional load on the person executing the top-level change).

Many nixpkgs maintainers are very eager to fix things; if they are notified of the problems ahead of time, they can help fix things before the next release.

7 Likes

Sorry I understand your frustration but please remember that Nix is a volunteer project and we use it for free. Sometimes we may send a few PRs but that’s voluntary too and nothing compared to what we get back. I’d urge you to kindly reflect upon your actions and not demand much of the team, most of whom are some of the most incredibly helpful people I’ve met. We don’t want another “actix” like situation.

Also I’ve often hit the issue and worked around by pinning versions. Maybe you could try that if it works for your situation

1 Like

IMHO the real problem is that many, way too many, packages are not really maintained. Maybe the maintainer i still around but stopped using the package, stopped using NixOS altogether or is on NixOS stable and isn’t aware the package is broken on unstable for some time.

A breaking change on unstable wouldn’t be a problem at all if every package were maintained. Maintainers would fix their small (hopefully) set of packages and the problem would be quickly gone.

Many nixpkgs maintainers are very eager to fix things; if they are notified of the problems ahead of time, they can help fix things before the next release.

A first step in this direction would be to re-enable emails from hydra to the maintainer of a package when compilation used to work but now fails.

Yes, please, reenable the notification! I don’t mind if it spam my inbox: I want to know if my packages are broken. Maybe send a notification only once a week but send it.

7 Likes

I feel “stateful” notifications could be a mild improvement: a single email per “failure”. If a package fails due to the same reason afterwards, no need to resend an email.

Or better yet, create a single github issue and “@” maintainers. If there exists one, don’t create another…

2 Likes

I agree with @rkoe that there is some amount of responsibility attached to doing structural changes to nixpkgs.

A good example of somebody that took this at heart was @bhipple who changed how Rust crates are being fetched. First he created a WIP RP to discuss the change, then wrote a plan on how to migrate all of nixpkgs to the new fetcher, then updated all the rust packages in a multi-step plan. He also kept an eye on failing packages in the meantime while nixpkgs kept moving forward. This to me is the ideal amount of care that structural changes should get.

I don’t know the details about the Qt-wrapping changes and I don’t think that assigning blame is especially useful. Sometimes it is not possible to fix everything by yourself as well but at least all the maintainers of the affected packages should be @ mentioned on the PR.

9 Likes

I believe it was handled the best it could be. Shortly after we realized things weren’t packaged correctly I asked that Tracking issue for wrapQtAppsHook · Issue #65399 · NixOS/nixpkgs · GitHub could be opened. All documentation was updated and the new section was even linked there. There were several plans on how things could have been fixed in a semi-automatic way, but I’m really under the impression that @ttuegel is time limited when it comes to maintenance tasks. So for that I’m pretty thankful in the first place that we have Qt wrappers at all. Hopefully we can get all these packages fixed in 20.09 by finishing Abort Qt builds that do not use a supported deriver by ttuegel · Pull Request #70691 · NixOS/nixpkgs · GitHub, and actually deprecating the qt specific deriver.

And on this note, it would be great if we could get more people to help with the QT framework in nixpkgs Qt needs update to 5.14 and more maintainers · Issue #74355 · NixOS/nixpkgs · GitHub.

5 Likes

I think this is the issue here. The ticket that you mention basically says that if one didn’t use this non-standard Qt5 version of mkDerivation that used to be a no-op, but now had become very-very important, then the fault is their. It’s no surprise that it rubs people wrong way when they are told that they didn’t read the 70 thousand word manual well enough. Meanwhile we can’t merge a sorting for our maintainer list without implementing a CI first Sort the maintainer-list by yorickvP · Pull Request #77784 · NixOS/nixpkgs · GitHub.

Having said that, I do appreciate the great work that was put in by @ttuegel and others to fix the original issue with Qt in nixpkgs.

Yeah, that does come off very RTFM, but *shrug it is one of the first things in the qt section on how to package a qt app. It probably could have been populated with the relevant/helpful info.

I’m fairly new here, so forgive me if I’m off-topic and barging in an already-much-discussed topic, but personally I would much prefer “including some stale/broken packages” to “reducing the size of nixpkgs”. A stale derivation is a starting point for contributing a fix, and I’d rather look into that than having to hunt for other sources of derivations.

There is of course gray area here: while you can’t expect the core package maintainer to fix all downstream derivations, there’s some responsibility there to think about the downstream impact and not just randomly break things. I didn’t follow this particular change, but my impression is that that is exactly what happened. Of course it is always ‘better’ to ‘go even further’, but you have to stop somewhere.

I’m not sure more ‘formal’ quality control would have helped here, as the change appears to have been made pretty deliberately, understanding that it would break a certain class of downstream packages. Perhaps the impact was underestimated, but I don’t have the impression that it was made ‘thoughtlessly and ignorantly’.

Perhaps you could look into running Abort Qt builds that do not use a supported deriver by ttuegel · Pull Request #70691 · NixOS/nixpkgs · GitHub and taking inventory of what’s still broken?

On a more positive note, I’ve been tracking unstable since October, and only once encountered a package that did not work for which a fix wasn’t already underway. There even already was a github issue about it. From my experience NixOS is doing pretty well here.

3 Likes

Since my previous shorter post was well received, I’d like to expand on my view of the “ideal process”. My goal is to clearly define the expectations for unstable (or master) as follows:

  • failing builds are not acceptable
  • but things being marked broken is (though it is avoided when possible)
  • and runtime issues might happen

You could call this “perpetual ZHF”. I think that is desirable, and we could achieve it with the following process:

New Changes

  1. Test your change with nixpkgs-review and any manual tests that you deem necessary. In any case, nixpkgs-bisect should be the bare minimum requirement.

  2. If nixpkgs-bisect reports any breakage, notify the maintainers of the broken packages (this happens before merge). Optionally you can fix it yourself, but there is no expectation to do so. For this step it is essential that master is already in a “ZHF” state.

  3. From the time of notification, give people at least a week to fix the build of their packages. After that, mark the still-failing build as broken. A week might be a bit short here, but the goal is to give maintainers opportunity to fix things while also not delaying changes unnecessarily. The “broken” status is not permanent, maintainers can still come back after two weeks and fix their package.

  4. Merge.

Breakage Detected

Mistakes happen. Things might break at runtime.

  1. Identify a breakage, git-bisect if necessary.

  2. Revert the commit that caused it first. I think we need to normalize reverts more. If anything breaks anything, we should get master back to a working state ASAP and figure out a better solution afterwards. It should not be taken as any sort of offense or accusation of the original commit author, just part of the normal process. Mistakes happen.

  3. Create an issue, pinging the original commit author.

  4. Restart at step 1 of “New Changes”. The responsibility for the process goes back to the original commit author, not the one doing the revert.

Big Changes

The process would need to be amended a bit for large rebuilds, i.e. everything that goes through staging. Here we should test reasonably-many reverse dependencies, depending on the complexity of a change. If we’re adding a comment to the build script of gcc, not much testing is needed. If there’s any breakage after all. If any unexpected breakage is caught later in the process, follow “Breakage Detected”. On high-complexity, high-rebuild changes someone should create a dedicated hydra jobset. Some community effort might be needed to avoid too much strain on the current staging maintainers, but that’s a separate topic.

Qt-Example

I’m not familiar with the details here. But let’s assume the Qt example falls into the category where no breakage was expected, but breakage was discovered at runtime. Following my suggestion, we would have immediately reverted the change. In the newly created issue, we might have come up with Abort Qt builds that do not use a supported deriver by ttuegel · Pull Request #70691 · NixOS/nixpkgs · GitHub. That would have enabled us to identify the breaking packages, notify maintainers and continue with the “New Changes” process.

Let me clarify again that I’m not blaming anybody for acting differently here. We don’t have a consensus currently. But I think we’d be better of if we did.

4 Likes