Skip to content

nixos/stalwart: add module-specific stateVersion, clean up and switch logging from stdout to journal#489143

Merged
happysalada merged 3 commits into
NixOS:masterfrom
axelkar:push-ylyrtstlxrnv
Apr 5, 2026
Merged

nixos/stalwart: add module-specific stateVersion, clean up and switch logging from stdout to journal#489143
happysalada merged 3 commits into
NixOS:masterfrom
axelkar:push-ylyrtstlxrnv

Conversation

@axelkar
Copy link
Copy Markdown
Member

@axelkar axelkar commented Feb 10, 2026

Sorry for a PR with multiple changes... Title basically says it.

journald's centralized logging has many advantages:

  • External centralized logging can simply source from journald
  • journald's auto-vacuum options (e.g. SystemMaxUse) can make sure logs don't take up too much space
  • Logs are in a single timeline, which is useful for analysis

This is also the first module-specific stateVersion in Nixpkgs. It has been planned before (ping @infinisil @KFearsoff). Basically it makes migrating easier/possible and also makes possible using newer state versions with an old system.stateVersion. The stateVersion doesn't have defaults (not just set to follow config.system.stateVersion by the module) so the user explicitly sets it to the version they first installed Stalwart on or the current NixOS version for new installs.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@axelkar
Copy link
Copy Markdown
Member Author

axelkar commented Feb 10, 2026

Module maintainer ping:
@happysalada
@pacien
@jonnynightingale
@norpl

@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 9.needs: reviewer This PR currently has no reviewers requested and needs attention. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: documentation This PR adds or changes documentation labels Feb 10, 2026
@axelkar axelkar force-pushed the push-ylyrtstlxrnv branch 2 times, most recently from eaf5291 to aa43df6 Compare February 10, 2026 17:56
@happysalada
Copy link
Copy Markdown
Contributor

This looks good, leaving this open to give others a chance to review

@axelkar
Copy link
Copy Markdown
Member Author

axelkar commented Feb 20, 2026

I'd like to include an example like "the current NixOS version" in this error message.

"The option `${showOption loc}' was accessed but has no value defined. Try setting the option.";

Comment thread nixos/tests/stalwart/stalwart-config.nix
Comment thread nixos/modules/services/mail/stalwart.nix
@axelkar axelkar requested a review from happysalada April 3, 2026 11:53
@nixpkgs-ci nixpkgs-ci Bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Apr 3, 2026
@nixpkgs-ci nixpkgs-ci Bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Apr 4, 2026
@happysalada
Copy link
Copy Markdown
Contributor

I dont have any qualms with this PR. I'm just noticing this would be breaking for existing users if they dont have the state version set. So if we merge this, we should do this earlier Rather than before we enter the freeze period for 26.05

axelkar added 3 commits April 4, 2026 20:02
journald's centralized logging has many advantages:
- External centralized logging can simply source from journald
- journald's auto-vacuum options (e.g. SystemMaxUse) can make sure logs
  don't take up too much space
- Logs are in a single timeline, which is useful for analysis
@axelkar axelkar force-pushed the push-ylyrtstlxrnv branch from 716436c to 28a9274 Compare April 4, 2026 17:11
@axelkar
Copy link
Copy Markdown
Member Author

axelkar commented Apr 4, 2026

I'd like to include an example like "the current NixOS version" in this error message.

This would require changing option value merging; not viable for this PR.

@nixpkgs-ci nixpkgs-ci Bot requested review from onny and pacien April 4, 2026 17:17
@happysalada happysalada added this pull request to the merge queue Apr 5, 2026
Merged via the queue into NixOS:master with commit a38303e Apr 5, 2026
27 of 30 checks passed
ilai-deutel pushed a commit to ilai-deutel/nixpkgs that referenced this pull request Apr 6, 2026
phanirithvij added a commit to ngi-nix/ngipkgs that referenced this pull request Apr 6, 2026
From NixOS/nixpkgs#489143

Fixed via switching to declarations instead of files

Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
phanirithvij added a commit to ngi-nix/ngipkgs that referenced this pull request Apr 6, 2026
From NixOS/nixpkgs#489143

Fixed via switching to declarations instead of files

Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
@amaanq
Copy link
Copy Markdown
Member

amaanq commented Apr 9, 2026

I'm not sure why the stalwart module has its own stateVersion, no other module does this and it is very bizarre to have users set this manually. I ran into this and was quite lost, especially so seeing that this is the only module to have this.

Users shouldn't be changing their stateVersion at all, and the module should track the system one instead. Updating the stateVersion can be done manually but typically the user needs to be very careful & thorough in ensuring that nothing breaks once they do so (e.g. they've performed any necessary db migrations).

Frankly I think 75742d2 should be reverted or reworked.

@axelkar
Copy link
Copy Markdown
Member Author

axelkar commented Apr 9, 2026

Thank you for the feedback. I agree that users shouldn't be updating their stateVersion, and this PR addresses exactly that by providing Stalwart with its own independent state version option so that it can be enabled on its own, separately from NixOS's state version. For example, one of my servers has the NixOS state version set to 24.05, but I'd want to use the latest Stalwart storage format without having to manually disable the 24.05 configuration applied by the module. The changes here begin a pattern where individual modules can have their own state versions, which provides NixOS users (and possibly users of other OSs with modular services) with more flexibility.

An easy change we could do to possibly improve UX would be to make this opt-in and have the option default to system.stateVersion. It'd still be discoverable even if it didn't have the error. It's important enough to be in the release notes but maybe it indeed should be opt-in?

@happysalada
Copy link
Copy Markdown
Contributor

@amaanq yes there is no prior example of this, but what is the fundamental problem with having this ? What do you propose instead ?

This is is just versioning the module in the end. The fact that there is no default is painful, but I dont see how this can be avoided.

@rhendric
Copy link
Copy Markdown
Member

If you haven't seen it, I'm campaigning for a different per-module stateVersion pattern as introduced here. Since as of this PR, it seems you don't depend at all on system.stateVersion, there's no actual conflict between the two patterns — I'm only going to try to migrate modules that use system.stateVersion. But since you're talking about how no other modules do this and considering using system.stateVersion to produce a default here, you might want to keep an eye on this.

phanirithvij added a commit to ngi-nix/ngipkgs that referenced this pull request Apr 20, 2026
From NixOS/nixpkgs#489143

Fixed via switching to declarations instead of files

Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
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: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants