Make sandbox-settings better typed, get globals.hh out of other headers#13799
Merged
Ericson2314 merged 6 commits intoNixOS:masterfrom Aug 20, 2025
Merged
Make sandbox-settings better typed, get globals.hh out of other headers#13799Ericson2314 merged 6 commits intoNixOS:masterfrom
sandbox-settings better typed, get globals.hh out of other headers#13799Ericson2314 merged 6 commits intoNixOS:masterfrom
Conversation
302b6ba to
13df322
Compare
sandbox-settings better typed, get globsals.hh out of other headerssandbox-settings better typed, get globals.hh out of other headers
edolstra
approved these changes
Aug 20, 2025
13df322 to
85672c1
Compare
I rather use designated initializers.
That is where it should be.
Will want these for settings in a moment.
We'll neeed some definitions elsewhere
This is needed to rearrange include order, but I also think it is a good thing anyways, as we seek to reduce the use of global settings variables over time.
Parsing logic is moved from `DerivationBuilder`, where is doesn't belong, to `Settings` itself, where it does.
85672c1 to
a712445
Compare
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
3 participants
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
The first part was my original goal: I saw there was some settings parsing ad-hoc included in
DerivationBuilder, and so I wanted to properly makeSettingsdo that instead, and usePathsInChrootonSettingsin order to do that (makingSettings::sandboxPathswell-typed).I did that by flipping the the header order so that
derivation-builder.hhwas used byglobals.hh, and then this caused a bunch of include order issues. I could have fixed this by making a new header, but I instead showed to do some "deferred maintenance" and and getglobals.hhonly directly included in*.ccfiles. I think this is good for eventually migrating away from global variable settings (make using globals more "pay if and only if you use" than "oh, you are already including them, so whatever"). So that is now part of this PR too.Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.