Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Start using stable branches tied to Nixpkgs releases #727

Open
emilazy opened this issue Jul 11, 2023 · 4 comments
Open

RFC: Start using stable branches tied to Nixpkgs releases #727

emilazy opened this issue Jul 11, 2023 · 4 comments

Comments

@emilazy
Copy link
Collaborator

emilazy commented Jul 11, 2023

The bottom (well, top) line: Not having release branches means that users who prefer stable Nixpkgs still get hit with unscheduled breaking changes, contributors have to code to a confusing polyglot of Nixpkgs versions, and nix-darwin improvements and clean-ups are held back. I have heard users directly wish that nix-darwin had stable releases. I believe that the status quo is more effort and pain for both users and maintainers, and that we should start cutting stable branches synchronized with their corresponding Nixpkgs versions, and tie master to nixpkgs-unstable.

Our support policy is very vague: because we don't have release branches, breaking support for a version that was only recently deprecated upstream is an invasive change for users in a way that simply no longer maintaining a deprecated release branch would not be. That means maintainers have to do additional work just to prevent already-deprecated Nixpkgs releases from breaking.

Furthermore, writing code in a polyglot of all supported Nixpkgs versions is error-prone; there's a reason that NixOS releases are coupled to the corresponding Nixpkgs. Code has to be written to a superposition of all supported lib versions and package sets, even in the face of upstream breaking changes. We have CI tests for the stable branch now, but those don't cover unsupported-but-not-yet-ancient Nixpkgs releases, and aren't terribly comprehensive besides.

It often requires more total effort, and more code, to write something that works with multiple Nixpkgs releases rather than writing code to handle each release individually, and in the case of nix-darwin changes that are breaking or otherwise not the kind of thing you'd want to backport to a stable release branch it's strictly more maintainer effort than having separate branches and only making the change on master. (Just cherry-picking examples I've had to deal with recently that would have been easier in a branching model: one, two, three.)

It's also hard to improve nix-darwin in (potentially-)breaking ways; every user is hit with breaking changes on their next update, even if they deliberately chose to use stable Nixpkgs branches, and there's no coordinated update point where they would have a change to read changelogs and prepare for it. That means that any changes that have even a chance of breaking things are scary to make, and we risk procrastinating on big improvements and fixes.

In contrast, the additional effort imposed by having release branches is low in the presence of adequate automation: it's no big deal if changes other than important bug fixes aren't backported immediately or at all unless people are particularly clamouring for a change, and as Nixpkgs shows, in many cases backporting PRs can be almost completely automated.

Proposed solution

We should branch off new release-YY.MM branches at the same time as Nixpkgs.

I believe that we should maintain support for the same versions Nixpkgs upstream does (i.e., the latest stable release, the beta branch when applicable, and unstable). The stable branches are not guaranteed to get any backports other than important bug fixes, but it is totally fine to backport things that are not breaking changes (e.g., new functionality, compatible bug fixes) or that are particularly important (e.g., backwards-incompatible security fixes). We should have a GitHub Action to allow automatically creating backport PRs like Nixpkgs does. The release branches for versions that are deprecated upstream will stick around, but are not expected to receive any changes beyond perhaps the most critical (unavoidable evaluation errors, security bugs, etc.).

That brings us to master. We can't keep our unstable branch as reliable as Nixpkgs': even if every bump of nixpkgs-unstable is green and we never break master ourselves, they can perform tree-wide breaking changes atomically, and we can't expect to stay in perfect sync with that even if we proactively work on keeping up with their upcoming changes. Therefore, there may be times when nix-darwin master does not work with the current nixpkgs-unstable. That said, we should still try to make sure master is always in what we expect to be a working state: no merging half-done changes, and any breaking changes should have reasonable error messages and/or migration paths. The message should be: you can use this on your systems, it's not a broken experimental branch, we do our best, but there may be times when Nixpkgs divergence or mistakes on our end prevent you from updating immediately, and it's where we make breaking changes for the upcoming stable release. Probably most people who are not familiar with Nix and nix-darwin should stay on stable branches.

Plan of action

  • Move the documentation for installation, etc. into the manual, so that it can vary appropriately per release.
  • Set up automatic GitHub Actions to:
    • Create a release branch and bump the master version when Nixpkgs branches off.
    • Regularly bump nixpkgs in flake.lock, so that we track upstream as closely as possible and get CI notification of problems caused by upstream Nixpkgs changes.
    • Merge the PRs for the above when they pass CI.
    • Create stable branch backport PRs for pull requests with appropriate labels.
  • Make system evaluation error out when the nix-darwin release doesn't match the Nixpkgs release.
    • With migration instructions, of course.
    • We should make sure the master branch check always passes on Nixpkgs unstable even when they bump before we can react to it (i.e. the check should be nixpkgsRelease == nixDarwinRelease on stable branches but nixpkgsRelease >= nixDarwinRelease on master).
  • Manually cut release-22.11 (born deprecated, expected to receive basically no changes except vital fixes) and release-23.05 from master.
  • Remove 22.11 backwards compatibility from release-23.05 and 23.05 backwards compatibility from master.
  • Move at a moderate pace and break some things :)

I am happy to drive the process and do pretty much all of these.

RFC? What's that?

We don't have any formal process, obviously, and we're not big enough to warrant one. I'm just posting this to get feedback from other committers, contributors, and users, and hopefully form a consensus so that I can move forwards with making this happen. Please feel free to comment even if you've never contributed to nix-darwin; I want this change to benefit everyone in the community.

@janvogt
Copy link

janvogt commented Jul 21, 2023

Certainly, let's modify your response accordingly:

While I'm presently navigating through compatibility issues between my nix-darwin channel and the current nixpkgs-23.05-darwin channel, I find your proposal to be most timely and pertinent.

Here are a couple of suggestions that I believe might help:

  • For keeping release-XX.YY branches in step with the upstream, it may be beneficial to test them against the most recent nixpkgs-XX.YY-darwin(!) channel. As far as I'm aware, this is the channel recommended by the upstream for Darwin users.
  • To aid intuitive understanding for users, consider aligning the naming convention of the release-XX.YY branches with the nixpkgs channel they correspond to.

I would like to express my gratitude for the remarkable work accomplished in this project.

EDIT: Upon further contemplation, I considered testing against the most recent upstream branch rather than the channel. This approach has its merits, such as earlier identification of potential problems. However, there are drawbacks, such as a higher incidence of false positives, given that changes breaking darwin are often rectified in the upstream CI before a channel update. Therefore, despite its benefits, I believe the cons may outweigh the pros in this scenario.

@emilazy
Copy link
Collaborator Author

emilazy commented Jul 21, 2023

Yes, we would track nixpkgs-YY.MM-darwin for release branches and nixpkgs-unstable for master.

The release-YY.MM naming matches the branches commits go to in Nixpkgs; the nix{pkgs,os}-* are separate because they are channel branches bumped by successful Hydra builds. We currently do all our testing before merge and have no post-merge CI, so we don't have a need for separate "tested" branches at present, which is a property I'd like to preserve if possible; I prefer the Rust/Bors model to NixOS's where broken commits can make it into the tree. I suppose that if our tests become extremely slow as we move to more comprehensive VM-based testing then we might feel a need for it in the future and people would need to adjust their flakes/channels, but hopefully the fact that a project as large and with as slow tests as Rust operates this way means that we can get away with it. release-YY.MM is also the convention Home Manager uses, with the same testing procedure. I personally feel like naming nix-darwin branches nixpkgs-YY.MM-darwin would be more confusing overall, but I'm open to disagreement on this.

I think testing against the channels is best, because nobody (who doesn't know exactly what they're doing) should be using the non-channel branches, and it would result in us duplicating a ton of CPU building things Hydra will get to soon or already failed on. This does mean that there could be times when nix-darwin will not work with the latest channel release of the corresponding Nixpkgs branch, but realistically this would be the case even tracking the non-channel branches if we don't get to it quickly enough, and anyway a stable Nixpkgs release branch should not be breaking anything if it doesn't absolutely have to; if we have the flake inputs bump automatically on CI success then flakes users can avoid this risk by simply not overriding nix-darwin's nixpkgs input. Taking nix-darwin as the source of truth for Nixpkgs also means that users would not have to interact with the nixpkgs-YY.MM-darwin naming in their configurations (though we would of course document the correspondence).

Sorry if something is broken on 23.05 right now; it shouldn't be! Please open a bug if you can.

@LnL7 I remember you saying you had thoughts on this proposal on Matrix; it'd be great to hear them when you can get around to it.

@janvogt
Copy link

janvogt commented Jul 22, 2023

Thank you for your prompt and comprehensive response! I now fully comprehend your rationale. The fact that the home-manager convention is being utilized here as well makes perfect sense to me. Thus, my previous suggestions can be viewed merely as a conversation starter :)

Regarding my issues with 23.05, I suspect they're related to my specific set-up. However, I still find troubleshooting in nix sometimes to be challenging. Rest assured, if I find a solution or begin to suspect it's a genuine nix-darwin problem, I will undoubtedly raise an issue or perhaps even contribute a pull request.

@LnL7
Copy link
Owner

LnL7 commented Jul 23, 2023

So far I haven't really seen the need for this, other than the occasional breaking change on nixpkgs master there have not really been problems for users as far as I am aware. But I see no problem with doing so as long as the branching, etc. happens somewhat timely. Other projects like home-manager also follow this model so totally makes sense.

That means maintainers have to do additional work just to prevent already-deprecated Nixpkgs releases from breaking.

I think this will always be the case, but this is reduced to the last release. And if nothing else it at least gives a nice point to do cleanup.

That brings us to master. We can't keep our unstable branch as reliable as Nixpkgs': even if every bump of nixpkgs-unstable is green and we never break master ourselves.

Indeed, I see these cases as upstream changes that have not gone through the proper deprecation path.

It's where we make breaking changes for the upcoming stable release. Probably most people who are not familiar with Nix and nix-darwin should stay on stable branches.

This is something I do want to avoid, regardless of the release branches. A proper breaking change should go through deprecation with warnings notifying the user before it's removed/changed even on master. But I see deprecations somewhat separate from this topic, regardless of how that's handled releases will only help to facilitate making those kind of changes.

Regularly bump nixpkgs in flake.lock, so that we track upstream as closely as possible and get CI notification of problems caused by upstream Nixpkgs changes.

The primary test target uses channels which is already pulling in the latest changes, but yeah would be nice to keep better track of the version used by the flake tests also.

We should make sure the master branch check always passes on Nixpkgs unstable even when they bump before we can react to it (i.e. the check should be nixpkgsRelease == nixDarwinRelease on stable branches but nixpkgsRelease >= nixDarwinRelease on master).

I'm not sure if a check like that is ideal on master as it could be rather annoying during the window where a new release is created, but for stable releases this definitely sounds like a good idea to at least show warnings about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants