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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC 0109] Nixpkgs Generated Code Policy #109
[RFC 0109] Nixpkgs Generated Code Policy #109
Changes from 9 commits
1391dac
71c4f28
9b343df
a43045b
183218c
a311978
a8e2f35
35df663
b93b2a9
da89a76
e38002d
d7a6f92
d38062a
e724981
d3b8f6f
f4d1e9b
1ab91b6
24078f4
47cb1a9
3876e77
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is more documentation needed we should rather write and improve documention IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SuperSandro2000 to me the documentation argument is like saying "what's the problem with saying what
apt-get
to run in the readme?" as is brought up in https://twitter.com/a_cowley/status/1463923361550159880.The answer is that no amount of a documentation is a substitute for automation. I don't want to read about a number of steps, I want to run 1 command that works as purely as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but that would also be possible without IFD if better automation would be written. Also we would still need to lock some input versions which makes it impossible to make fully reproducible but less which would probably make it easier to be reproducible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I agree with both of those statements. Vendoring generated code alone doesn't required IFD. IFD just allows the use of vendoring to be minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we still vendor all the files I don't see the benefit of IFD. Do we want to get the tooling there to better support IFD? That sounds like nixpkgs would be a playground which should be out of tree IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't vendor any more than today. And in fact we should vendor less: instead of vendoring all of hackage, we should just vendor enough to build
cabal2nix
, and bootstrap everything else wit IFD from that.But I don't want to put in the RFC what we should and shouldn't vendor, just establish the principles by which vendoring is allowed today.
Even if we kept all Hackage vendored as today, it would be still better if you could just update a
fetchgit
of the relevant from data from Nixpkgs and then regenerate everything purely.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but only when we can remove parts of the vendored files which would require the final nix user to have IFD enabled which is not going to happen anytime soon if I understood it correct. And that is my main point: this will only come in really useful if we don't need any vendor files anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SuperSandro2000 I'm a bit confused?
Yes, I do say I don't want to immediately crate a big dust-up making
haskellPackages
IFD only.What I rather do is go in the other direction. There are many would-be things like
haskellPackages
that don't get merged because we don't have an appetite for more generated code. Your response to @sternenseemann's wanted to add a second stable package set, NixOS/nixpkgs#138407 (comment), is in fact a prefect example of this! :)! Let's go those things out of limbo by making them use IFD instead of vendoring, and them pioneer the new IFD approach.I hope that well become a success, and we will then be open to using IFD for things that are vendored today, like the default
haskellPackages
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One prospect could in fact be to remove every haskell package that is marked as broken presently from
hackage-packages.nix
since those are never touched by CI anyways. We have been hesitant to do this so far, because some of those tend to fix themselves after a while or are really easy to unbreak, so removing them ruins the contributing experience.Requiring local users to enable ifd to (try) to build these could actually be a fairly decent option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could also cheat and only evaluate the
pkgs/top-level/release.nix
. The really bad one isnixos/release.nix
after all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't actually know that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One benefit of
allImportedDerivations
could really be that updating the generated code becomes easier because it is equivalent to updating some hard coded inputs (like the hackage data tarball) and then running anix-build -A allImportedDerivations.…
.For better or worse this would also put more scrutiny on the maintenance of
lang2nix
tools. E. g. we'd need to do something aboutemacs2nix
which currently is built against a reasonably ancient version ofhnix
we no longer have in nixpkgs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E. g. NixOS/nixpkgs#150941
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow? To comply with the nixpkgs conventions (overrideability,
meta
sets etc.) we need to generate proper nix files that can becallPackage
-ed, not derivation files.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that should be
I agree we probably won't be doing
builtins.readFile
very often, but strictly speaking, it doesn't mean violating our conventions: there are many legitimate uses ofbuiltins.fromJSON (builtins.readFile ...)
for example.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, agreed. I think IFD is the well known term for “evaluation depends on realizing derivations”, not sure if there's a more accurate, but catchy term to be had.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Evaluation-time build? The entire problem/idea is that we're building at evaluation-time, before we finish evaluation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I like that. My one quibble in the vein of "lazy" vs "non-strict" is that whether or not do any actually building or evaluating, the question of whether expressions use the contents of derivation outputs can be answered -- i.e. it is a static question.
Having that quibble I was still stuck with a name for 24 hours :), but now I have "build-dependent-expressions" or "output-dependent expressions" or "derivation-output-dependent-expressions" or most verbosely "content-of-derivation-outputs-dependent-expressions"
On a side note, It's very sneaky with input-addressing that we can do all sorts of eval-time inspection of the output path itself without IFD. With CA derivations, output paths are not known and so we instead use opaque placeholders. This brings the store path and the content of the store path back in line, as they are both equally opaque without IFD.
It can be an annoying restriction, but I think it also makes everything easier to understand :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EBEB = eval build eval build