treewide: remove empty go vendor hashes

[figsoda]

Description of changes

see #126682

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.05 Release Notes (or backporting 22.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[zowoq]

Guess we should actually fix this, I'll open a PR for it. Something like this should work?

diff --git a/pkgs/build-support/go/module.nix b/pkgs/build-support/go/module.nix
index 647f2a2f7ad..e4ca029a891 100644
--- a/pkgs/build-support/go/module.nix
+++ b/pkgs/build-support/go/module.nix
@@ -129,6 +129,11 @@ let
 
       mkdir -p vendor
 
+      if ! [ "$(ls -A vendor)" ]; then
+        echo "vendor folder is not needed, please set 'vendorHash = null;' or 'vendorSha256 = null;' in your expression"
+        exit 10
+      fi
+
       runHook postBuild
     '';
 

[figsoda]

I think we should warn at the eval level instead of throwing a hard error

[zowoq]

warn at the eval level

Don't think that should be go specific, treewide check maybe?

throwing a hard error

We already do for other vendor cases so it would be consistent.

[figsoda]

Don't think that should be go specific, treewide check maybe?

that sounds like a cool idea, but I'm just not sure how this can be implemented without breaking things like emptyDirectory

We already do for other vendor cases so it would be consistent.

This could be our end goal we should definitely start with a warning before committing to a hard error

[zowoq]

This could be our end goal we should definitely start with a warning before committing to a hard error

As it's a FOD it's only going to error when the hash changes which seems fine? Don't see that we should let people re-add empty hashes?

[figsoda]

As it's a FOD it's only going to error when the hash changes which seems fine? Don't see that we should let people re-add empty hashes?

This is the case for nixpkgs, but there might be out of tree setups that don't actively cache that might be broken by this

[zowoq]

out of tree setups that don't actively cache that might be broken by this

I wouldn't say uncached empty vendor hashes is a common enough problem that it matters, we can add a rel note and move on.

Also it already has a warning from go: no dependencies to vendor so we'd only be adding another (slightly louder) warning if we don't error.

[figsoda]

Also it already has a warning from go: no dependencies to vendor

Logs barely show up for successful nix builds with the new cli without -L, and It's less notable even if logs are being streamed without the color and bold of nix warnings

There was a brief conversation in the original pull request (#126682) and a nixpkgs-hammering lint was suggested (jtojnar/nixpkgs-hammering#116), though it has been a while since then and things could have changed, and the conversation was quite brief

That said, feel free to open a new pr and receive more comments since I doubt many people would see this conversation in a merged pull request. I don't have a strong opinion on this, I just personally prefer to have eval warnings first instead

[zowoq]

#212024