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

nixVersions.stable: 2.15 -> 2.17 #247475

Merged
merged 5 commits into from
Aug 13, 2023
Merged

nixVersions.stable: 2.15 -> 2.17 #247475

merged 5 commits into from
Aug 13, 2023

Conversation

zowoq
Copy link
Contributor

@zowoq zowoq commented Aug 6, 2023

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Aug 6, 2023
@zowoq zowoq mentioned this pull request Aug 6, 2023
12 tasks
@zowoq
Copy link
Contributor Author

zowoq commented Aug 6, 2023

@lf-
Copy link
Member

lf- commented Aug 6, 2023

oh cool, thanks for the notification. I'll get to writing a patch soon.

@figsoda
Copy link
Member

figsoda commented Aug 6, 2023

briefly checked:

  • nurl - good, unpinned nix nix can be unpinned (nixVersions.unstable -> nix) because all the bugfix patches are now in stable
  • nix-init - good
  • nix-update - good
  • nixpkgs-review - good

@Mic92
Copy link
Member

Mic92 commented Aug 6, 2023

Harmonia bump: #247544

@Mic92
Copy link
Member

Mic92 commented Aug 6, 2023

nix-eval-jobs bump: #247546

@lf- lf- mentioned this pull request Aug 6, 2023
12 tasks
@lf-
Copy link
Member

lf- commented Aug 6, 2023

nix-doc fix (can remove pin after merging): #247581

@zowoq zowoq marked this pull request as draft August 6, 2023 23:04
@zowoq zowoq mentioned this pull request Aug 6, 2023
12 tasks
@zowoq
Copy link
Contributor Author

zowoq commented Aug 6, 2023

briefly checked:

Sorry, I was pinging you for nil as you're listed as a maintainer.

nurl - ... nix can be unpinned

Okay.

@figsoda
Copy link
Member

figsoda commented Aug 6, 2023

Sorry, I was pinging you for nil as you're listed as a maintainer.

Ahh, I do maintain the nixpkgs package, but I'm not super involved in the development, but I can check if we don't get a response from @oxalica

@zowoq zowoq marked this pull request as ready for review August 7, 2023 00:51
@zowoq zowoq mentioned this pull request Aug 7, 2023
12 tasks
@Ma27
Copy link
Member

Ma27 commented Aug 7, 2023

fwiw home-manager is currently having problems with nix 2.17: nix-community/home-manager#4298

@Mic92
Copy link
Member

Mic92 commented Aug 7, 2023

harmonia does now also build on darwin: #247698

@happysalada
Copy link
Contributor

Is this waiting for something else ?
on the plus side, nix copy with ssh control master gets fixed with this upgrade.

@Ma27
Copy link
Member

Ma27 commented Aug 9, 2023

I guess we don't want to merge this if this breaks commonly used tools such as home-manager, so we should await a fix on their side IMHO.

@SuperSandro2000
Copy link
Member

also hydra needs a patch apperantly NixOS/hydra#1296

@zowoq
Copy link
Contributor Author

zowoq commented Aug 10, 2023

so we should await a fix on their side IMHO.

Merging this PR may motivate people to submit a fix.

@happysalada
Copy link
Contributor

It looks like the creator of home-manager proposed a patch and just needs time to get back to a computer and test it.
How about we give it a couple of days, like until next Monday ?

@Ma27
Copy link
Member

Ma27 commented Aug 11, 2023

In both cases there are fixes in progress (in fact I use both of them personally with Nix 2.17). I don't see why we need to rush here, though.

@happysalada
Copy link
Contributor

The fix was just merged in home-manager, does anyone have any other concern ?

@zowoq
Copy link
Contributor Author

zowoq commented Aug 13, 2023

@ofborg eval

@lovesegfault lovesegfault merged commit 70dd76d into NixOS:master Aug 13, 2023
@zowoq zowoq deleted the nix217 branch August 13, 2023 23:36
@RaitoBezarius
Copy link
Member

so we should await a fix on their side IMHO.

Merging this PR may motivate people to submit a fix.

Let's not do that anymore, please. Stable versions of Nix even on unstable are important, otherwise, this creates more work for release managers to triage what's going on, especially with respect to support channels.

@zowoq
Copy link
Contributor Author

zowoq commented Aug 16, 2023

so we should await a fix on their side IMHO.

Merging this PR may motivate people to submit a fix.

Let's not do that anymore, please. Stable versions of Nix even on unstable are important, otherwise, this creates more work for release managers to triage what's going on, especially with respect to support channels.

At the time I made the comment there hadn't been any activity on the linked issue for almost a week. How long are we supposed to wait? Was I expected to fix it myself to unblock this PR?

@RaitoBezarius
Copy link
Member

so we should await a fix on their side IMHO.

Merging this PR may motivate people to submit a fix.

Let's not do that anymore, please. Stable versions of Nix even on unstable are important, otherwise, this creates more work for release managers to triage what's going on, especially with respect to support channels.

At the time I made the comment there hadn't been any activity on the linked issue for almost a week. How long are we supposed to wait?

I understand, but, like, people are on vacations (preparing for vacations, etc.), etc. Saying that it will motivate them to submit a fix is a bit rude to people in those contexts.

For packages as critical as Nix (see the last upgrade for reference), it is preferable to collect much more working samples and run extensive testing, for the nixVersions.stable, at least, as a RM, this is my policy and you are free to ignore it but then this may make my work harder if there are things I have to deal with, like Nix bugs, asking Nix core team for backports for released versions, etc. Case in point: #224830.

I am planning to write a proper policy on the subject and submit to the community. BTW, this is not specific to nixVersions.stable, there is a subset of packages which are critical to NixOS, e.g. bootloaders or systemd.

Was I expected to fix it myself to unblock this PR?

I'm not sure you are expected to fix it because you cannot know all the bugs as Nix release management itself did not prove itself to be very stable and that's fine: software have bugs. But this is my responsibility and the responsibility of every Nixpkgs contributor/maintainer who care about this working for everyone, especially for complicated things like the Nix package manager, which is the core of NixOS, to ensure that we don't actively go out there and break things if we are aware they may not work. So please understand my side on this.

@zowoq
Copy link
Contributor Author

zowoq commented Aug 16, 2023

I asked two questions, I don't think that you have answered either of them in a way that I can understand or consider to be actionable for future PRs.

How long are we supposed to wait?

Was I expected to fix it myself to unblock this PR?

@happysalada
Copy link
Contributor

happysalada commented Aug 16, 2023

You're both great contributors, so let me try to dissipate the tension.
I'm going to try to answer from my understanding.

How long are we supposed to wait?

One week could be fine in a non holiday period, but during summer or winter breaks, 3 weeks might be more appropriate for critical packages that the ecosystem depends on.
(note that this is entirely my own opinion and might not be representative).

Was I expected to fix it myself to unblock this PR?

You're not expected to fix it yourself, but that does mean that the PR stays blocked until there is a real fix.
(from my understanding of his answer).

The matter is complicated by the fact that 2.15 had (at least one) annoying bug that we wanted fixed by making this upgrade.

@RaitoBezarius
Copy link
Member

I asked two questions, I don't think that you have answered either of them in a way that I can understand or consider to be actionable for future PRs.

That's because this is something to figure out, I am on vacations right now and I cannot say things that I didn't determine yet, if that does not help you for future PRs, please consider not upgrading nixVersions.stable please.

How long are we supposed to wait?

If you want a number, I'd say, 1 month in general is a good amount of time for critical packages, for vacations periods like summer (and especially during large scale Chaos events such as C3Camp for example).

I would advise to wait 3 months after a Nix release.

Was I expected to fix it myself to unblock this PR?

No. As I explained in my previous message:

I'm not sure you are expected to fix it because you cannot know all the bugs as Nix release management itself did not prove itself to be very stable and that's fine: software have bugs. But this is my responsibility and the responsibility of every Nixpkgs contributor/maintainer who care about this working for everyone, especially for complicated things like the Nix package manager, which is the core of NixOS, to ensure that we don't actively go out there and break things if we are aware they may not work. So please understand my side on this.

Fixing it yourself or fixing it wouldn't have unblocked this PR anyway.

@zowoq
Copy link
Contributor Author

zowoq commented Aug 16, 2023

I'm a home manager user and I quite like it, I don't mind waiting a bit to not break it but it is an external project, I don't see how it can block updating nix here for weeks. In theory nix cuts a tag every six weeks or so and not keeping up with nix versions could come with it's own set of problems.

@RaitoBezarius
Copy link
Member

I'm a home manager user and I quite like it, I don't mind waiting a bit to not break it but it is an external project, I don't see how it can block updating nix here for weeks. In theory nix cuts a tag every six weeks or so and not keeping up with nix versions could come with it's own set of problems.

Thank you for that, but Home-Manager is not the only external project here… So blocking Nix updates here for weeks is not only about giving Home-Manager the time needed to fix everything, it is to give time to all stakeholders, including the ones who are not mentioned in this PR, to pick Nix 2.17, verify that for example known regressions of the previous releases, are also retested over this version, etc.

Basically, the thing is that we need a "Nix stable upgrade" checklist in nixpkgs. Not keeping up with Nix versions should not be a problem because Nixpkgs is supposed to work with Nix ≥ 2.3, so…

And just to be clear: I am not opposed to bump the nixVersions.unstable and have people test it and do things, etc. I am just opposed to nixVersions.stable moving too fast without all the testing because people depending on the "stable contract" can have very broken Nix and as of recently, we didn't have easy ways for beginners to repair NixOS installations, makes all of this very unfriendly to everyone. Waiting has the advantage of riding on the fact that we have all the experience and people who went through this process to ensure that the current stable version is indeed stable and usable to a certain extent.

@zowoq
Copy link
Contributor Author

zowoq commented Aug 16, 2023

I don't think we moved to fast here, if anything we moved slower than usual. #188777, #204841, #211290

We can always revert and try again if necessary. #212010, #212769

2.17.0 was cut three weeks ago and added to nixpkgs the following day. This PR was opened two weeks after that and merged a week later.

I think waiting half of the six week nix tag cycle to bump the default in nixpkgs is sufficient, if it isn't then perhaps people should be testing against master of the nix repo and not waiting for a new tag.

@lovesegfault
Copy link
Member

@RaitoBezarius I think you're mostly right, but IMO your argument is backwards.

I think Nix needs an integration test gating releases that checks what happens when the release is used in nixpkgs, and what the broad impact will be.

The current cycle of:

  1. Nix releases 2.x
  2. We import it into nixpkgs
  3. A bunch of things break, we report and fix them upstream (i.e. in Nix)
  4. Nix releases 2.(x+1) with the fixes
  5. We import it into nixpkgs
  6. There were other things in the release, which break a bunch of things
  7. ...

Is not really sustainable.

@lovesegfault
Copy link
Member

With that said, I acknowledge I should have waited for the home-manager fix before landing this PR. I fat-fingered it and didn't want to revert, open another PR, etc. for an issue that would be resolved in a few days.

My bad :)

@zowoq
Copy link
Contributor Author

zowoq commented Aug 16, 2023

With that said, I acknowledge I should have waited for the home-manager fix before landing this PR.

It was fixed.

@RaitoBezarius
Copy link
Member

@RaitoBezarius I think you're mostly right, but IMO your argument is backwards.

I agree this is backward.

I think Nix needs an integration test gating releases that checks what happens when the release is used in nixpkgs, and what the broad impact will be.

I would love this and release candidates maybe.

The current cycle of:

  1. Nix releases 2.x
  2. We import it into nixpkgs
  3. A bunch of things break, we report and fix them upstream (i.e. in Nix)
  4. Nix releases 2.(x+1) with the fixes
  5. We import it into nixpkgs
  6. There were other things in the release, which break a bunch of things
  7. ...

Is not really sustainable.

I agree.

The problem is that I am not really in a position to say what to do to Nix developers (or Nix core team), I try to communicate what I feel is needed for NixOS and inside of nixpkgs and I adjust with the reality.
Currently, I do not feel like this is realistic to require this from the Nix core team, so the backward argument is the only thing I have right now. But if I am missing something in your proposal to make what the forward argument realistic, please let me know, we should document this instead :).

@RaitoBezarius
Copy link
Member

I don't think we moved to fast here, if anything we moved slower than usual. #188777, #204841, #211290

I don't understand what are you referring at. Those are initialization PRs, not stable bumps.

Just to make it clear : I am completely fine and I encourage packaging the newest versions of Nix as soon as they are tagged.

I am not comfortable with moving nixVersions.stable too quickly, what you cited is not relevant to that point.

We can always revert and try again if necessary. #212010, #212769

Yes, but reverting will not fix broken systems of people who cannot upgrade anymore because they do not have the knowledge to revert their Nix version.

This happens more often than you may believe and yes, people follows unstable closely and can get caught in the changes.

The worse about it is that NixOS is an operating system built around Nix, you can imagine that assisting someone with a broken Nix can become much harder than a lot of community support requests we have to deal on a daily basis.

So reverting is clearly not a solution here, nixVersions.unstable should ideally catch those or we should give enough time to explore the whole surface.

2.17.0 was cut three weeks ago and added to nixpkgs the following day. This PR was opened two weeks after that and merged a week later.

I think waiting half of the six week nix tag cycle to bump the default in nixpkgs is sufficient, if it isn't then perhaps people should be testing against master of the nix repo and not waiting for a new tag.

Who are "people" you are mentioning in "should be testing against master of the Nix repo", are you suggesting to push the maintenance burden on every consumers? That's not realistic given the nature of Nix breakages, it's not necessarily only external projects, it is sometimes Nix features in conjunction with trivial external projects, etc.

This does not really make sense to me. As long as Nix, the package manager, cannot really have releases which can be trusted right away because the integration / regression testing story is still being developed (and to be clear: I know how super hard the work can be for Nix developers and I am grateful for what is there right now), we have to take into account those facts in a realistic way and provide a sensible path for all the users out there.

For people who wants to live on the bleeding bleeding edge, nixVersions.unstable exist for this. nixVersions.stable should aim an extremely stable Nix version and change should be given a lot of time for testing.

That's my only point. 3 weeks is clearly not enough, people cannot switch immediately what they are doing to work on the newest Nix release, they cannot keep track of master all the time, etc. If we had release candidates, that would be another story, but release candidates are not a thing again, so let's not suggest more maintenance burden to people doing all their best.

Let's just be patient and provide slower paths even it ends up in everyone not getting the newest shiny feature that the newest Nix release has, IMHO, this is important because then we show mindfulness to our users.

@happysalada
Copy link
Contributor

Just want to add that the difficult point here is that 2.15 was already broken (ssh with control master didn't work).

I think your point makes sense when 2.15 is stable and working. The hard thing is that when it's not working so well in the first place, do we risk a bug with a newer version or just leave everyone with the known bug. I think that is why this version was pushed so fast.

@RaitoBezarius
Copy link
Member

Just want to add that the difficult point here is that 2.15 was already broken (ssh with control master didn't work).

Let's maybe define "broken" properly also. SSH with ControlMaster is an optimization for multiplexing SSH connections, this is not something that should be actively preventing from switching your configurations or doing things, though, I agree it create a very annoying behavior and not really fun.

But it is another level of impact, it's not like, e.g. not being able to handle symlinks at all.

I think your point makes sense when 2.15 is stable and working. The hard thing is that when it's not working so well in the first place, do we risk a bug with a newer version or just leave everyone with the known bug. I think that is why this version was pushed so fast.

This is a fallacy, backporting is a thing, we do it for 23.05 because 23.05 ships 2.13 anyway. I don't want to push more maintenance burden on the Nix core team, obviously, but I try to actively track & pin "world breaking" bugs, e.g. nixos-enter, "no valid substituter" confusing error messages, symlink handling, etc. and we ask for backports for them and they get backported at some point.

I do not think we can do more than this, as @lovesegfault put it, the mechanism where we move from 2.x to 2.(x + 1) to fix bugs is not really sustainable.

So, here what I am reading and understanding from what you are saying, after all this discussion where I reiterated my point, is that you don't care about potentially worse bugs if you can get the bugs you care about to get fixed even if they are not necessarily on the same impact level of the potentially other bugs.

So really, please no, let's not rush things, is ultimately my point. If you want something different while keeping the reasonable guarantees for stability, I'm all ears.

@zowoq
Copy link
Contributor Author

zowoq commented Aug 17, 2023

I don't think we moved to fast here, if anything we moved slower than usual. #188777, #204841, #211290

I don't understand what are you referring at. Those are initialization PRs, not stable bumps.

I am not comfortable with moving nixVersions.stable too quickly, what you cited is not relevant to that point.

They are initialization and stable bumps, don't see how they are not relevant.

@RaitoBezarius
Copy link
Member

RaitoBezarius commented Aug 17, 2023 via email

@zowoq
Copy link
Contributor Author

zowoq commented Aug 17, 2023

If you need me to come forward an official policy on the matter, I can in the next weeks.

What does "official policy" mean? An RFC?

@RaitoBezarius
Copy link
Member

RaitoBezarius commented Aug 17, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants