Split out AutoAllocateUidSettings#15043
Merged
Ericson2314 merged 1 commit intoNixOS:masterfrom Jan 26, 2026
Merged
Conversation
6f98e29 to
cebc5c9
Compare
This was referenced Jan 22, 2026
amaanq
added a commit
to amaanq/nix
that referenced
this pull request
Jan 22, 2026
This PR follows the same approach as NixOS#15043 and the [`LogFileSettings`](NixOS#15051) extraction: - `GCSettings` struct inherits from virtual `Config` - `Settings` privately inherits from it - Accessed through `getGCSettings()`
amaanq
added a commit
to amaanq/nix
that referenced
this pull request
Jan 22, 2026
This PR follows the same approach as NixOS#15043 and the [`LogFileSettings`](NixOS#15051) extraction: - `GCSettings` struct inherits from virtual `Config` - `Settings` privately inherits from it - Accessed through `getGCSettings()` The new method on `LocalStoreConfig` anticipates on making these settings per-store. [This commit](NixOS@0b606aa) added both the autoGC and periodic wakeups.
amaanq
added a commit
to amaanq/nix
that referenced
this pull request
Jan 22, 2026
This PR follows the same approach as NixOS#15043 and the [`LogFileSettings`](NixOS#15051) extraction: - `GCSettings` struct inherits from virtual `Config` - `Settings` privately inherits from it - Accessed through `getGCSettings()` The new method on `LocalStoreConfig` anticipates on making these settings per-store. 0b606aa added both the autoGC and periodic wakeups, which is why we think they are related.
amaanq
added a commit
to amaanq/nix
that referenced
this pull request
Jan 22, 2026
This PR follows the same approach as NixOS#15043 and the [`LogFileSettings`](NixOS#15051) extraction: - `GCSettings` struct inherits from virtual `Config` - `Settings` privately inherits from it - Accessed through `getGCSettings()` The new method on `LocalStoreConfig` anticipates on making these settings per-store. 0b606aa added both the autoGC and periodic wakeups, which is why we think they are related.
artemist
reviewed
Jan 23, 2026
f132a73 to
77cb943
Compare
Mic92
approved these changes
Jan 26, 2026
Follows the same pattern as `GCSettings`: extract UID allocation settings into a dedicated struct that Settings inherits privately from. The current settings infrastructure prevents correct data modeling that would allow `autoAllocateUids` to be a `std::optional<AutoAllocateUidSettings>`. To compensate, the getter `getAutoAllocateUidSettings()` returns a pointer - nullptr when disabled, providing the optional-like semantics we want. Co-authored-by: Amaan Qureshi <git@amaanq.com>
77cb943 to
ab56ac4
Compare
amaanq
added a commit
to obsidiansystems/nix
that referenced
this pull request
Jan 26, 2026
This PR follows the same approach as NixOS#15043 and the extraction in NixOS#15051. The `DatabaseSettings` struct inherits from the virtual `Config` class, and `Settings` privately inherits from it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Progress on #5638
Context
This is a bit of weird process for a few reasons:
I would like to get the struct as in the pair of fields to move out of
globals.hh, but I don't want to move the settings descriptions to a new header, so I just did a weird split in place.Firstly because the setting descriptions (as we agreed) should not be in a header at all, and secondly, because I do want to group all these settings together somehow, not pass many unrelated, unnested arguments around.
The current settings infrastructure prevents correct data modeling that would allow
autoAllocateUidsto go from aboolto astd::optional<AutoAllocateUidSettings>. So we are stuck just having a pair of aboolandAutoAllocateUidSettingswhich is not correct.However, we can compensate for this making the inheritance private and making the getter that looks like the field that we want to have. This pattern should be repeated as we break up the structs further, so we can enforce correct access patterns on the users of the globals without first having to overhaul the settings system before making any progress.
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.