Derivation untangle #10760
Derivation untangle #10760
Conversation
| @@ -289,6 +362,8 @@ struct BasicDerivation | |||
| StringPairs env; | |||
| std::string name; | |||
|
|
|||
| DerivationOptions options; | |||
There was a problem hiding this comment.
It seems that having this field in BasicDerivation "denormalizes" it, since information like the value of __impureEnvVars is now stored in both drv.env and drv.options.impureEnvVars, which could get out of sync and would therefore be error-prone to use. Wouldn't it be better to have methods like BasicDerivation::{get,set}ImpureEnvVars()?
There was a problem hiding this comment.
@edolstra the goal is, via the JSON format, to allow that stuff to not exist in the env. Then only the legacy decoding results looks like denormalization — but actually I prefer the perspective that there is no denormalization and rather the legacy format simply lacks the expressive power to set one without setting the other.
Does that make sense? It is some subtle philosophical perspective-shifting.
There was a problem hiding this comment.
Philisophical doesn't do it justice. It's anticipating a design improvement.
There was a problem hiding this comment.
Yeah philosophical before the new feature (better JSON format as input), practical with it.
let's see for how long ;)
add some descriptions
This means that the functionality of `ParsedDerivation` is now approaching zero (it still provides get*Attr/s).
create new utility function `nullableValueAt`
Ericson2314
force-pushed
the
derivation-untangle
branch
from
2b0ffc1 to
5d08553
Compare
We'll move out the parsing code following this.
fricklerhandwerk
added
contributor-experience
haenoe
force-pushed
the
derivation-untangle
branch
from
b06017d to
5fab050
Compare
github-actions
bot
added
the
with-tests
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-06-24-nix-team-meeting-minutes-155/47739/1 |
| @@ -2,11 +2,24 @@ | |||
| ///@file | |||
|
|
|||
| #include <cstdint> | |||
| #include <nlohmann/json.hpp> | |||
There was a problem hiding this comment.
There is json_fwd we should use in headers
| AdditionalAttributes attrs; | ||
|
|
There was a problem hiding this comment.
I don't think this should go here, but replacing env in BasicDerivation
There was a problem hiding this comment.
Ah yes oopsie, should it still be named env though?
And should I put it in derivations.{cc,hh}?
There was a problem hiding this comment.
name doesn't matter, since call sites need to be updated either way.
Yes probably should go there.
This is a first step towards PR NixOS#10760, and the issues it addresses. See the Doxygen for details. Thanks to these changes, we are able to drastically restrict how the rest of the code-base uses `ParseDerivation`. Co-Authored-By: HaeNoe <[email protected]>
This is a first step towards PR NixOS#10760, and the issues it addresses. See the Doxygen for details. Thanks to these changes, we are able to drastically restrict how the rest of the code-base uses `ParseDerivation`. Co-Authored-By: HaeNoe <[email protected]>
This is a first step towards PR NixOS#10760, and the issues it addresses. See the Doxygen for details. Thanks to these changes, we are able to drastically restrict how the rest of the code-base uses `ParseDerivation`. Co-Authored-By: HaeNoe <[email protected]>
This is a first step towards PR NixOS#10760, and the issues it addresses. See the Doxygen for details. Thanks to these changes, we are able to drastically restrict how the rest of the code-base uses `ParseDerivation`. Co-Authored-By: HaeNoe <[email protected]>
This is a first step towards PR NixOS#10760, and the issues it addresses. See the Doxygen for details. Thanks to these changes, we are able to drastically restrict how the rest of the code-base uses `ParseDerivation`. Co-Authored-By: HaeNoe <[email protected]>
|
Long time no see ^^' |
Motivation
This is a first effort to reduce the usage of
ParsedDerivationby putting the relevant configuration options intoBasicDerivation(seeDerivationOptions, link).Context
#9866 (regarding the "magic values"
envfield that are used as options) #9846 (regarding the reduction ofParsedDerivation)Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.