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.

Sign up for GitHub

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

Jump to bottom

nix: revert default to 2.18 for now #343032

Merged
merged 3 commits into from
Sep 19, 2024
Merged

nix: revert default to 2.18 for now #343032

merged 3 commits into from
Sep 19, 2024
+9 −13

Conversation

K900
Copy link
Contributor

@K900 K900 commented Sep 19, 2024 •
edited
Loading

Description of changes

The segfault bug (see #335342) keeps hitting Hydra on unstable-small, so it's likely going to hit a lot of people once it rolls out widely. Let's revert for now, and investigate without time pressure.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

Add a 👍 reaction to pull requests you find important.

@K900 K900 requested review from RaitoBezarius and a team as code owners September 19, 2024 13:48
@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 Sep 19, 2024
@K900
Copy link
Contributor Author

K900 commented Sep 19, 2024

Running nixpkgs-review to see if there's anything else that needs reverting.

@emilazy
Copy link
Member

emilazy commented Sep 19, 2024

Seems like the simpleUefiSystemdBoot test is another thing that should go in the Nix passthru pile (though given that it’s flaky it might not help that much).

@Mic92
Copy link
Member

Mic92 commented Sep 19, 2024 •
edited
Loading

Seems like the simpleUefiSystemdBoot test is another thing that should go in the Nix passthru pile (though given that it’s flaky it might not help that much).

I am not so sure about that. It could be any nixos configuration causing this really.

@K900
Copy link
Contributor Author

K900 commented Sep 19, 2024

Yeah, maybe if someone finds an expression that reproduces this consistently, we can add it to the upstream test suite, but this feels way too flaky.

@alyssais
Copy link
Member

Please include reasons for reverts in commit messages.

@K900
Copy link
Contributor Author

K900 commented Sep 19, 2024

Updated commit messages, added nix-plugins revert.

@emilazy
Copy link
Member

emilazy commented Sep 19, 2024 •
edited
Loading

6c0fc2b45e0e3ed4bcaebe22b0b7ecb7e8303da8 isn’t right. (Or rather it’s insufficient.)

Edit: Never mind; it’s in the next commit.

Mic92
Mic92 reviewed Sep 19, 2024
View reviewed changes
Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the nix maintainer team should have consulted for that. Also there is no nix issue for this bug.

@K900
Copy link
Contributor Author

K900 commented Sep 19, 2024

Result of nixpkgs-review pr 343032 run on x86_64-linux 1

1 package blacklisted:
  • nixos-install-tools
94 packages built:
  • appvm
  • attic-client
  • attic-server
  • bundix
  • cabal2nix
  • cached-nix-shell
  • common-updater-scripts
  • crate2nix
  • crystal2nix
  • disko
  • dub-to-nix
  • dydisnix
  • fusionInventory
  • libnixxml
  • lua51Packages.luarocks-nix
  • lua52Packages.luarocks-nix
  • lua53Packages.luarocks-nix
  • lua54Packages.luarocks-nix
  • luajitPackages.luarocks-nix
  • luarocks-packages-updater
  • nim_lk
  • niv (niv.bin ,niv.data)
  • nix
  • nix-binary-cache
  • nix-bundle
  • nix-direnv
  • nix-du
  • nix-index
  • nix-init
  • nix-pin
  • nix-plugin-pijul
  • nix-plugins
  • nix-prefetch
  • nix-prefetch-bzr
  • nix-prefetch-cvs
  • nix-prefetch-docker
  • nix-prefetch-git
  • nix-prefetch-hg
  • nix-prefetch-scripts
  • nix-prefetch-svn
  • nix-required-mounts
  • nix-required-mounts.dist
  • nix-template
  • nix-update
  • nix-update-source
  • nix-update-source.dist
  • nix-update.dist
  • nix-visualize
  • nix-visualize.dist
  • nix.debug
  • nix.dev
  • nix.doc
  • nix.man
  • nixStatic
  • nixStatic.dev
  • nixStatic.doc
  • nixStatic.man
  • nixci
  • nixos-anywhere
  • nixos-generators
  • nixos-rebuild
  • nixos-shell
  • nixpkgs-hammering
  • nixpkgs-manual
  • nixpkgs-review
  • nixpkgs-review.dist
  • nixtract
  • node2nix
  • npins
  • nuget-to-nix
  • nurl
  • nvfetcher
  • outline
  • prefetch-yarn-deps
  • python311Packages.nix-kernel
  • python311Packages.nix-kernel.dist
  • python312Packages.nix-kernel
  • python312Packages.nix-kernel.dist
  • sbomnix
  • sbomnix.dist
  • sonarr
  • swiftPackages.swiftpm2nix
  • terranix
  • typescript-language-server
  • update-nix-fetchgit
  • update-python-libraries
  • vimPluginsUpdater
  • vulnix
  • vulnix.dist
  • vulnix.doc
  • vulnix.man
  • wp4nix
  • yarn2nix
  • zon2nix

@emilazy
Copy link
Member

emilazy commented Sep 19, 2024

I think we arrived at the consensus to merge the bump as long as we’re willing to revert if problems crop up again, given the reaction to #335342 (review) and discussion with @fabianhjr on Matrix when they were making the decision to merge. If there are channel‐blocking eval crash issues cropping up even on the small channel I don’t think we should let a delay risk this shipping to the channels before further assessment.

@alyssais alyssais added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Sep 19, 2024
@K900 K900 mentioned this pull request Sep 19, 2024
Closed
@K900
Copy link
Contributor Author

K900 commented Sep 19, 2024

Filed a proper bug to contain discussion: NixOS/nix#11547

@patka-123
Copy link
Contributor

I think the nix maintainer team should have consulted for that. Also there is no nix issue for this bug.

First and foremost we shouldn't break users systems, then do the proper communication and triaging. Reverting seems the most important to me for now, no?

@K900
Revert "nixos/nix-fallback-paths: 2.24.2 -> 2.24.6"
fb38235 
This reverts commit ac849e5.

Nix 2.24 segfaults semi-randomly during evaluation on specific configs (?).
This commit prepares for another revert, putting the default back to 2.18.

Upstream issue: NixOS/nix#11547
@K900
Revert "nix: nix_2_18 -> nix_2_24"
b3b9bdd 
This reverts commit 2e702d0.

Segfaults during evaluation in yet-to-be-determined circumstances.
Investigation is ongoing, reverting for now to be safe.

Upstream issue: NixOS/nix#11547
@K900
Revert "nix-plugins: 14.0.0 -> 15.0.0"
7d10007 
This reverts commit 0291167.

To match Nix version revert.

See upstream issue: NixOS/nix#11547
@K900
Copy link
Contributor Author

K900 commented Sep 19, 2024

One last force push to add upstream issue reference to all commits.

@K900 K900 merged commit c04d565 into NixOS:master Sep 19, 2024
9 of 10 checks passed
@nyabinary

This comment was marked as off-topic.

Sign in to view
@Mic92 Mic92 mentioned this pull request Sep 19, 2024
Closed
13 tasks
@MisileLab MisileLab mentioned this pull request Sep 20, 2024
Closed
1 task
@Aleksanaa Aleksanaa mentioned this pull request Sep 20, 2024
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@winterqt winterqt winterqt approved these changes

@RaitoBezarius RaitoBezarius RaitoBezarius approved these changes

Assignees
No one assigned
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/` 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@K900 @emilazy @Mic92 @alyssais @patka-123 @nyabinary @RaitoBezarius @winterqt