Skip to content
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

New derivation-making builtin #9774

Open
Ericson2314 opened this issue Jan 15, 2024 · 7 comments
Open

New derivation-making builtin #9774

Ericson2314 opened this issue Jan 15, 2024 · 7 comments
Labels
derivation design Issues to consider for new versions of the derivation format (major or incremental) feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc store Issues and pull requests concerning the Nix store

Comments

@Ericson2314
Copy link
Member

Ericson2314 commented Jan 15, 2024

builtins.derivation (and builtins.derivationStrict) have a number of issues:

  • drvPath has a funky "DrvDeep" string context that needs to be stripped away with the scary-sounding builtins.unsafeDiscardOutputDependency.

  • Mixing together environment variable and magic flags is icky (this also applies to the on-disk .drv format)

  • Returning the arguments to the user provided encourages memory leaks (derivationStrict already improves on this over derivation)

  • Returning the boiled-down inputs (e.g. inputSrcs, inputDrvs) is sometimes useful, and we could just return it here.

  • The outputs are mixed in with the other attributes (separation of dicts and records; see JSON guideline)

  • Remembers meta attributes, including such attributes as

  • Possibly: arbitrary other meta attributes

TODO other things

Priorities

Add 👍 to issues you find important.

@Ericson2314 Ericson2314 added the feature Feature request or proposal label Jan 15, 2024
@Ericson2314 Ericson2314 changed the title New derivation builtin New derivation-making builtin Jan 15, 2024
@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-01-15-nix-team-meeting-minutes-115/38297/1

@roberth roberth added language The Nix expression language; parser, interpreter, primops, evaluation, etc store Issues and pull requests concerning the Nix store labels Jan 15, 2024
@roberth
Copy link
Member

roberth commented Jan 16, 2024

Outdated - possible pkg.derivation attr in Nixpkgs

We should align this with

Currently that means that the return type should have a path field containing the .drv, but other than that, I've proposed to keep it lean.

@roberth
Copy link
Member

roberth commented Jan 16, 2024

We should also consider whether we even want to return such attributes as inputSrcs, inputDrvs eagerly, as this will probably result in plenty of unused-but-referenced Values that we can't GC.
Technically we could work around that by adding them as thunks that call an (internal or not) primop for retrieving those from the .drv file. This is only a partial solution to the space leak though, because the attrs and thunks still take a bit of space, even if not evaluated.

@Ericson2314
Copy link
Member Author

Ericson2314 commented Jan 16, 2024

@roberth I was thinking the user would just drop them if they don't want them.

Outdated - possible pkg.derivation attr in Nixpkgs

But yes, this conflicts with derivation just being the result of the new primop.

@roberth
Copy link
Member

roberth commented Mar 17, 2024

Nix prints the position of the name attribute, but this is almost always useless:

whose name attribute is located at nixpkgs/pkgs/stdenv/generic/make-derivation.nix:331:7

Ideally, Nix would accept a __pos parameter with a user-provided position, such as mkDerivation's pos argument, which is already used for some Nixpkgs-specific error messages. Nix should use this too, but __pos must not be written to the actual derivation in the store, as that would affect the hash every time the position changes slightly.
Changes to derivation attribute hashing need to be forward compatible, and we have no good forward compatibility in the current derivation/derivationStrict primops.

In summary:

  • Implement measures for forward compatibility
  • Use __pos argument for error positions, but don't write it to the derivation.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-03-11-nix-team-meeting-132/42960/1

@roberth
Copy link
Member

roberth commented May 21, 2024

Added two examples of meta attributes that the store layer must be aware of.

@roberth roberth added the derivation design Issues to consider for new versions of the derivation format (major or incremental) label Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
derivation design Issues to consider for new versions of the derivation format (major or incremental) feature Feature request or proposal language The Nix expression language; parser, interpreter, primops, evaluation, etc store Issues and pull requests concerning the Nix store
Projects
None yet
Development

No branches or pull requests

3 participants