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

Have our cake and eat it too derivation metadata #10780

Open
roberth opened this issue May 26, 2024 · 5 comments
Open

Have our cake and eat it too derivation metadata #10780

roberth opened this issue May 26, 2024 · 5 comments
Labels
breaking Changes we can't make without breaking old expressions, changing hashes, etc derivation design Issues to consider for new versions of the derivation format (major or incremental) feature Feature request or proposal store Issues and pull requests concerning the Nix store

Comments

@roberth
Copy link
Member

roberth commented May 26, 2024

Is your feature request related to a problem? Please describe.

meta fields are not included in derivations because they would cause unnecessary rebuilds.

Yet, we want users to be able to analyze this information, which isn't exposed reliably.

This lets us solve the problem of lost meta.timeout values in the store layer.

Describe the solution you'd like

Change derivation output hashing so that a section of the .drv is excluded from the computation, as well as being made unobservable from the derivation's environment. This causes outputs of equivalent derivations to still happily collide onto the same output, so that output caching remains as effective as it is today.

This extra info only needs to be unobservable under what we might call "equivalence under validity", an idea that we already exploit in other areas. For example, it is fundamental to fixed output derivations. An example here is meta.timeout: if we don't pass it to the build environment, it can not cause successful builds to have different outputs. We might even call it "equivalence under weak validity", because if a different timeout causes one derivation to fail, that's also ok.

The idea of making output hashes not depend on the entire derivation is also not new; again fixed output derivations already apply this trick very effectively.

Describe alternatives you've considered

Store such information in EvalState, so that we don't have to change the output hashing.
This causes a problem when the build happens with nix eval + nix-store -r, so we'd have to persist this info.
However, then we can't discern between stale entries from previous evaluations and the entries we actually need.
Also it does not work without forwarding this info to remote builders somehow.

Additional context

Another output hashing change:

Priorities

Add 👍 to issues you find important.

@roberth roberth added feature Feature request or proposal store Issues and pull requests concerning the Nix store labels May 26, 2024
@fricklerhandwerk fricklerhandwerk moved this to ⚖ To discuss in Nix team May 29, 2024
@fricklerhandwerk
Copy link
Contributor

Triaged in Nix maintainer meeting:

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-05-29-nix-team-meeting-minutes-148/46195/1

@roberth roberth added derivation design Issues to consider for new versions of the derivation format (major or incremental) breaking Changes we can't make without breaking old expressions, changing hashes, etc labels Jun 9, 2024
@roberth
Copy link
Member Author

roberth commented Jun 9, 2024

Added breaking label, because this won't be hashed correctly by any existing Nix version, which is a big deal considering our limited forward compatibility mechanisms.
Best incremental change I can think of is to do something similar to what the dynamic derivations did: intentionally breaking the first token.

@flokli
Copy link
Contributor

flokli commented Jun 9, 2024

I'd still prefer to be this auxillary metadata of some sort, rather than changing the ATerm and hashing scheme just for the sake of adding metadata in there.

Sure, care needs to be taken to make sure it properly traverses the places it needs to (like remote building scenarios etc), but this feels less hacky than what's proposed here.

@roberth
Copy link
Member Author

roberth commented Jun 9, 2024

I don't think this is proposing any new "hackiness" that we don't already have in the form of fixed output derivations, which also excludes almost everything from the path hash.
I can understand that it feels that way though, because adding something feels safer than changing something.
The main benefit I see in this approach is that we don't complicate the data model, but rather double down on an aspect that's not new but underutilized (aforementioned FOD-inspired hashing).

I've started collecting derivation design issues in the derivation design label. I've called it "derivation v2" briefly, but renamed it not to bias it towards a solution that may well be too heavy handed, but I do have to note that we have a number of issues that require breaking changes. An overview of related issues should help inform such decisions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Changes we can't make without breaking old expressions, changing hashes, etc derivation design Issues to consider for new versions of the derivation format (major or incremental) feature Feature request or proposal store Issues and pull requests concerning the Nix store
Projects
Status: ⚖ To discuss
Development

No branches or pull requests

4 participants