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

readTree discards data thanks to calling overrideAttrs when merging #1

Open
andrewhamon opened this issue Jun 30, 2024 · 3 comments
Open

Comments

@andrewhamon
Copy link

If a readTree node is a derivation, readTree will always call overrideAttrs on it, which will silently discard information.

I think I understand the rationale for why readTree prefers overrideAttrs when available - it avoids inadvertently changing a derivation by merging arbitrary values on top of it, while it is much safer to leverage passthru which avoids any inadvertent cache-busting.

Unfortunately, this ends up causing some very non-intuitive behavior. Anything that is merged on top of a derivation using simple tools like // gets silently discarded, which can be very surprising.

For a contrived example, consider a readTree node that looks like this:

# customCowsay.nix
{ pkgs, ...}:
pkgs.cowsay // { hello = "here is a value that I am very explicitly merging on top"; }

Under readTree, if I try to reference customCowsay.hello, I will get an error that the attribute hello doesn't exist. How can that be? I most certainly added it! It takes a person well versed in nix, and willing to go hunting, to understand that this problem is that our value is getting transformed by calling overrideAttrs.

For a more concrete example, consider lib.hiPrio in nixpkgs. It sets meta.priority on an attrset using simple merging. So if we try to do something such as:

# customCowsay.nix
{ pkgs, ...}:
pkgs.lib.hiPrio pkgs.cowsay

Then the priority will be lost.

@lukegb
Copy link
Contributor

lukegb commented Jun 30, 2024

This was originally added in https://cl.tvl.fyi/5186 - basically so that other people who call .overrideAttrs wouldn't cause problems for the readTree-added attributes. lol, lmao, etc.

@andrewhamon
Copy link
Author

lol. stuck between a rock and a hard place.

FWIW, i think the current behavior is the most correct behavior. My only aim by opening this was to have something I could link to :)

@andrewhamon
Copy link
Author

Opened up NixOS/nixpkgs#323624 over at nixpkgs, as on first glance it strikes me as reasonable that addMetaAttrs should try to use overrideAttrs if available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants