nix icon indicating copy to clipboard operation
nix copied to clipboard

Get rid of `impureOutputHash`; fix possible bug

Open Ericson2314 opened this issue 3 years ago • 5 comments

Motivation

I do not believe there is any problem with computing hashDerivationModulo the normal way with impure derivations.

Conversely, the way this used to work is very suspicious because two almost-equal derivations that only differ in depending on different impure derivations could have the same drv hash modulo. That is very suspicious because there is no reason to think those two different impure derivations will end up producing the same content-addressed data!

Context

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • [ ] agreed on idea
  • [ ] agreed on implementation strategy
  • [ ] tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests
  • [ ] documentation in the manual
  • [ ] code and comments are self-explanatory
  • [ ] commit message explains why the change was made
  • [ ] new feature or bug fix: updated release notes

Ericson2314 avatar Mar 31 '22 23:03 Ericson2314

The point of making DerivationType::ContentAddressed a record (and derivation type no longer an enum) was that we already handled this case and didn't need a new variant.

I don't agree that this already handles that case. An impure derivation is not conceptually the same as a derivation that isn't sandboxed and not fixed-output. You could imagine derivations that are not sandboxed and not fixed-output, but are still expected to have a reproducible result (e.g. a derivation that uses ccache to build something remotely).

edolstra avatar Apr 01 '22 07:04 edolstra

We conversely, one might want to cache actually impure derivations more normally, in a "per transaction" substitution map.

Still, I will split this up so the second commit doesn't hold back the first.

Ericson2314 avatar Apr 01 '22 13:04 Ericson2314

In the meeting today @edolstra mentioned that, as we discussed in the past, we don't necessary want to make impure derivations work "just like" CA derivations.

A thing I forgot to point out was that floating CA derivations are already not handled specially. The only special case in hashDerivationModulo is for fixed CA derivations -- input address and floating CA are handled the same way.

Impure derivations are certainly not fixed output, so I don't think we are going to run into any problems here.

Ericson2314 avatar Feb 03 '23 14:02 Ericson2314

  • Needs update.
  • @Ericson2314 is this still an issue?

tomberek avatar Oct 09 '24 19:10 tomberek

Yeah, but low priority.

Ericson2314 avatar Oct 09 '24 19:10 Ericson2314

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

https://discourse.nixos.org/t/2024-02-12-nix-team-meeting-minutes-212-5/60216/1

nixos-discourse avatar Feb 12 '25 21:02 nixos-discourse