disallowedReferences/disallowedRequisites should not create a dependency on the disallowed derivations

[ajs124]

Describe the bug

Adding something to disallowedRequisites leads to it being built/substituted and the resulting derivation seemingly depending on it?

Take this code for example:

{ pkgs ? import <nixpkgs> {} }: pkgs.hello.overrideAttrs (oA: {
  disallowedReferences = [ pkgs.firefox ];
})

pkgs.hello obviously already does not depend on pkgs.firefox, but building this file leads to downloading firefox and nix-store --query --tree looks like this: https://gist.github.com/ajs124/1a36f6d55c28aa40c6451b5a49d16bfe#file-hello-tree-txt-L557

Expected behavior
I expected it not to do this, because it makes this feature much less useful.

nix-env --version output
nix-env (Nix) 2.3.10

Additional context

Maybe I'm having a fundamental misunderstanding of how this is supposed to work, but the documentation didn't mention any of what I observed above.

[knedlsepp]

I don't think there's really a bug here.
I think you would really only ever want to put something in disallowedReferences that is already somehow a buildInput.
The functionality here is that the build process can access Firefox, but it should be made sure that the output doesn't reference Firefox.
If you think about it it makes a lot of sense that this will need to download Firefox.

[ajs124]

Maybe you're reading different documentation from me and evidently you're thinking different things from me, but I really don't see how checking that there is no reference to something requires downloading said thing.

I admit that what you described is a use case, but I can also imagine more than enough use cases where you just don't want a derivation to depend on another derivation, no matter if you specify it as in input. Especially with disallowedRequisites.

[knedlsepp]

The nix manual states that "the outputs" of a derivation will not include references to whatever you specify.
There's no mention of what the ".drv" files themselves are supposed to reference.

I guess you might be right that if nix were completely lazy in this regard it wouldn't be necessary to realize Firefox in that case. In the special case of content-adressed-derivations that might no longer be true however.

[thufschmitt]

I think that proposition makes perfect sense, and it's a bit unfortunate that it doesn't work that way. We can't change that without breaking backwards-compat − that would mean changing the hash of every derivation that uses these attributes, but you can get this behavior by replacing firefox by builtins.unsafeDiscardStringContext firefox in your example. For example

pkgs.runCommand "foo" {
  disallowedReferences = [ (builtins.unsafeDiscardStringContext pkgs.firefox) ];
} "echo foo > $out"

doesn't depend on firefox, but

pkgs.runCommand "foo" {
  disallowedReferences = [ (builtins.unsafeDiscardStringContext pkgs.firefox) ];
} "echo ${pkgs.firefox} > $out"

will fail because of the disallowed reference

[ajs124]

builtins.unsafeDiscardStringContext

Hah, I could have come up with that myself. Thanks for reminding me that that exists.

We can't change that without breaking backwards-compat − that would mean changing the hash of every derivation that uses these attributes

That is also something that could have occurred to me. As nix does not have any versioning, this really can't be changed.

I guess we could still leave this open, but we could probably also close it.

[stale]

I marked this as stale due to inactivity. → More info

[r-vdp]

I ran into this issue when using disallowedRequisites to basically ensure that we would never accidentally introduce certain problematic dependencies in a closure. In large systems, it is not always easy to control what ends up in a closure, so it makes sense to me to include things in disallowedRequisites as a safe-guard so that we get a build failure if someone accidentally introduces a transitive dependency.

The trick with unsafeDiscardStringContext works, but I don't think we should put it on our users to know about this intricacy and use this builtin in their codebase. I think the current behaviour would definitely be unexpected for the majority of users.

I made a PR in nixpkgs for this as well, but fixing this in nix would obviously be preferable.

[infinisil]

Here's how I think this should be implemented in Nix: NixOS/nixpkgs#211783 (comment)

[picnoir]

The current Nixpkgs fix won't work for CA derivations: NixOS/nixpkgs#214044

It seems like this should be properly fixed in Nix instead.

[nyabinary]

Any update on this? It has been a little more than a year since the last comment.

[thufschmitt]

This is a very unfortunate behavior, but mostly a wontfix because we can't changing it would be a language breaking change.

Something more to add to the list of #9774 …