buildGoModule: warn for passthru.overrideModAttrs lost after overriding

[ShamrockLee]

This PR adds a warning to the workaround introduced in PR #339042, to remind users their dropping passthru attributes would result in the loss of buildGoModule's overridden overrideGoModule (added in PR #225051).

Errors shouldn't pass silently, and there will be more packages storing overriding information in passthru as more frameworks transit from custom overriders to <pkg>.overrideAttrs. Let's enforce this with a warning.

Things done

• Built on platform: (no rebuild)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

---

Add a 👍 reaction to pull requests you find important.

[ShamrockLee]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 434546
Commit: b3230442063e36c97dba762d0aef5ddabded9b34

---

x86_64-linux

⏩ 1 package blacklisted:

• tests.nixos-functions.nixos-test

✅ 1 package built:

• nixpkgs-manual

[philiptaron]

Can you share a test case where I can see this error where I wouldn't have previously? Something I can test in nix repl.

[ShamrockLee]

Can you share a test case where I can see this error where I wouldn't have previously? Something I can test in nix repl.

Sandro demonstrated a reproducer by overriding with { passthru = { }; }. A worked reproducer:

{ pet }:
pet.overrideAttrs {
  passthru = { };
}

We could compare the the error handling after #225051 (attribute missing error), #339042 (falling back to the initial value silently), and this PR (falling back to the inintial value with a warning message) with the following commands:

# After PR #225051 "buildGoModule: Fix overriding with overlay-style stdenv"
nix eval --impure --expr "((
  builtins.getFlake ''github:NixOS/nixpkgs/0d920a91a29ec8668ecbf967b485136000ad5d94''
).outputs.legacyPackages.\${builtins.currentSystem}.pet.overrideAttrs {
  passthru = { };
}).outPath"

# After PR #339042 "buildGoModule: be nicer when overrideAttrs clears passthru"
nix eval --impure --expr "((
  builtins.getFlake ''github:NixOS/nixpkgs/0e21bc6af9b313944aaca5f6e8ccbbf4a9c978f7''
).outputs.legacyPackages.\${builtins.currentSystem}.pet.overrideAttrs {
  passthru = { };
}).outPath"

# After this change
nix eval --impure --expr "((
  builtins.getFlake ''github:NixOS/nixpkgs/refs/pull/434546/head''
).outputs.legacyPackages.\${builtins.currentSystem}.pet.overrideAttrs {
  passthru = { };
}).outPath"

[ShamrockLee]

Warning message updated to include position information.

[philiptaron]

passthru = [ ];

Assuming this is actually passthru = { }; -- the other one throws an eval error.

[philiptaron]

Yeah, OK.

[acid-bong]

Hmm, I decided to make this passthru non-overridable in #457308 (by setting it on the right of //), I guess I'll need to revert this too?

[ShamrockLee]

Hmm, I decided to make this passthru non-overridable in #457308 (by setting it on the right of //), I guess I'll need to revert this too?

Nope.

1. The change proposed in buildGoModule: intersect given and available meta.platforms #457308 does not prevent passthru from being overridden with <pkg>.overrideAttrs, and this PR (buildGoModule: warn for passthru.overrideModAttrs lost after overriding #434546) is to prevent passthru.overrideModAttrs from being removed from passthru after <pkg>.overrideAttrs overriding.

2. Hacks to prevent <pkg>.overrideAttrs from changing passthru.overrideModAttrs is not what we want, as the existence of passthru.overrideModAttrs is mainly to allow goModule to be overridden while also allowing vendorHash to be overridden as null.