zfs_unstable: 2.2.6 → 2.3.0-rc2

[toastal]

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.

¹

Footnotes

1. Please consider giving up MS GitHub or offering a non-proprietary, non-US-corporate-controlled mirror for this free software project. I wish to delete this Microsoft account in the future, but I need more projects like this to support alternative methods to send patches & contribute. ↩

[Titaniumtown]

@toastal if you rebase I can test it locally :)

[Titaniumtown]

On line 17, kernelCompatible = kernel: kernel.kernelOlder "6.11"; should be anything before 6.12 as 2.3.0-rc2 supports 6.11

[Titaniumtown]

Seems there are more changes when it comes to building:

Running phase: unpackPhase
@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking source archive /nix/store/zvii3zim2cry704j515j5m9nncag19g2-source
source root is source
Running phase: patchPhase
@nix { "action": "setPhase", "phase": "patchPhase" }
substituteStream() in derivation zfs-user-zfs-2.3.0-rc2: WARNING: '--replace' is deprecated, use --replace-{fail,warn,quiet}. (file './lib/libshare/os/linux/nfs.c')
substitute(): ERROR: file './etc/zfs/Makefile.am' does not exist

[toastal]

These older functions are an off-by-one problem in my head. Good catch. However it did build build & run locally for me on the other front which is a bit concerning. I will try to look when I am free again, but I may be busy til the weekend

[Titaniumtown]

These older functions are an off-by-one problem in my head. Good catch. However it did build build & run locally for me on the other front which is a bit concerning. I will try to look when I am free again, but I may be busy til the weekend

I rebased your patches on master locally to test, that is maybe why I got a different result.

[surfaceflinger]

I would wait with this for now openzfs/zfs#16631
I'm using the current version in nixpkgs with nixpkgs.config.allowBroken = true; and it works fine, I would just bump the compatible Linux version instead until upstream resolves the corruption issue

[toastal]

@surfaceflinger probably a decent call
@Titaniumtown I used --replace-warn instead of --replace

[toastal]

@surfaceflinger there could also be a broken check for ifLUKS as it seems that is the issue? …kinda weird choosing LUKS tho over the FS encryptions.

[surfaceflinger]

ZFS on LUKS is unfortunately quite common, idk why, maybe because ZFS native encryption is still too new for some people and also leaks metadata

[surfaceflinger]

also wouldn't --replace-fail be better? Personally I never used --replace-warn because I wouldn't like ryantm bot to open a PR with broken replaces for someone to then merge it blindly

[toastal]

I am not a maintainer nor did I write the code so I am not touching it. I just want it to not fail on build & be a reminder to the maintainer to fix the warn if they don’t like it.

[adamcstephens]

I am not a maintainer nor did I write the code so I am not touching it. I just want it to not fail on build & be a reminder to the maintainer to fix the warn if they don’t like it.

You did touch it though. It should indeed fail, not warn.

[toastal]

How should I know if or if it shouldn’t? What would I do if it failed? The only reason I am touching it is that `--replace` is deprecated & not building—& the maintainer hasn’t yet accounted for this change. It seems unreasonable to make contributors take on the burden of the maintainer in a separate file from the proposed upgrade. IMO this is what `--replace-warn` is for.

[adamcstephens]

It seems unreasonable to make contributors take on the burden of the maintainer in a separate file from the proposed upgrade

Part of being a contributor is taking that burden on yourself to do things the correct way, even if you're not a maintainer of the package. Sometimes that means touching files outside the simple change you wish to make. If that makes you uncomfortable, that's ok, you have the option of not making the contribution. I definitely walk away from contributions that grow past what I'm ready to commit to changing.

In this case, the file is a common include for multiple versions. As a maintainer of the stable zfs version, I disagree that warn is acceptable without more justification on individual calls, which should be documented in the code. That will require you to validate the affected versions and is the right way to address this automated check. It is indeed more work, but now is the time to fix it if you want to get this version updated. If you don't want to, maybe @amarshall will for you, or I will whenever the next stable release of zfs lands.

[adamcstephens]

warns should be fails.

[toastal]

If amarshall chimed in I would make that change. What is the point of deprecating replace or even having --replace-warn if all replaces are supposed to be failures?

[adamcstephens]

If amarshall chimed in I would make that change. What is the point of deprecating replace or even having --replace-warn if all replaces are supposed to be failures?

Why does my request not count? It affects a package I maintain (along with amarshall).

I didn't say they couldn't be warns. I said if they need to be warns (and some in this file even may!) that they should be correctly identified and documented. The default should be fail though, or upstream changes could be missed.

[toastal]

You are listed as a 2_2 maintainer (not in generic). I did not see this.

[toastal]

Seems there are failures that will require 2.3.x-specific features. My main motivator isn’t getting the latest ZFS features, but rather not getting left in the dust as kernels get deprecated & OpenZFS lags fairly regularly.

https://paste.sr.ht/~toastal/2cf1d54a473aa1ae6add02328020e7c2975040b0

[poscat0x04]

ZFS on LUKS is unfortunately quite common, idk why, maybe because ZFS native encryption is still too new for some people and also leaks metadata

For one zfs encryption does not support multiple key slots, which makes it hard/impossible to do stuff like using usb sticks and/or tpms to unlock your dataset, recovery keys etc.

[toastal]

I don’t have the energy to dig into the things that changed