Modify buildGoModules to use -mod=vendor instead of the go package cache.

[c00w]

Motivation for this change

A number of users sound concerned about ensuring we have readable source. Additionally, we have to do a hack to the pkg cache to make it reproducible, where as vendor is by design reproducible.

This is the default behaviour of go 1.14 for packages with a go version of 1.14.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[zimbatm]

👍 good idea.

Do you mind also handling the case where the vendor/ folder already exists in the source? We have a number of upstream sources that already include that folder. At the moment those need to use buildGoPackage with all the complicated source relocating business.

I think goModSha256 = null should mean that the vendor/ folder already exists and skip the go-modules generation entirely. And change goModSha256 = "impure" for the heretic case where the go-modules is built as a input derivation instead of fixed derivation.

[zimbatm]

Some related issues to consider:

• Recommend buildGoPackage instead of buildGoModule in the nixpkgs manual #84826
• Undocumented / impossible: override go modules #86349
• I also remember somewhere somebody wanted to add a deps.nix equivalent so that all the sources could be declaratively specified instead of having them calculated with a fixed-output.

Sorry for expanding the scope of the PR. I think there is a potential to handle all these scenarios once and for all. And I am willing to put my time into making this work.

[Mic92]

I also remember somewhere somebody wanted to add a deps.nix equivalent so that all the sources could be declaratively specified instead of having them calculated with a fixed-output.

That person was me. I am working on it. However the PR here should not block this. This feature can be added later.

[c00w]

Redoing deps is another PR - this is just changing the storage format.

[c00w]

vendorSha256=null means use the built in vendor folder. If it's not null we won't overwrite that folder if it exists and instead throw an error. If the upstream source is bad, we'll need to drop that folder when packaging in an intermediate derivation between og source and input src.

[flokli]

I fixed the broken manual in 6f4f37d.

@emilazy as this PR has already been merged, it'd probably need some fixup PR.

[zowoq]

Also, as a tiny nitpick, this PR removed the (Unix-conventional) trailing newline from various files.

We have an .editorconfig, what do we think about enforcing it in PRs via CI?

[Mic92]

Maybe this could be done via an github action?

[ajs124]

maybe this discussion belongs to a different issue ^^

but yes, considering some of the weird formatting I've seen around nixpkgs, something like this does sound reasonable.

[c00w]

@ajs124 Any chance you can add some automation? Both the github checks + nix-review pr didn't notice this. If you give me a specific way to check I can probably add them in to nix-review, but I honestly have no idea how to build the manual.

[ajs124]

You can build the manual through release.nix, e.g. nix-build nixos/release.nix -A manual.x86_64-linux.

Alternatively, to check more things than just the manual, you can also build a "dummy" nixos system with nix-build nixos/release.nix -A dummy. This builds a basically empty nixos system, but that way, you can make sure you're not breaking basic nixos evaluation.

[zowoq]

I've opened a draft where we can continue the discussion. #87853

[nh2]

Can anybody explain to me what this change does? The added docs say

The go-modules builder now uses vendorSha256 instead of modSha256

but I don't understand what the effective difference. When reading the changelog, it just looks like "we renamed something but don't say why". Same with

A number of users sound concerned about ensuring we have readable source

That somehow suggests that modSha256 did not have "readable source", but I don't understand what that even means.

Maybe an example of the shortcomings of modSha256 would be useful?

[Mic92]

Can anybody explain to me what this change does?

see #88258

[doronbehar]

So this doesn't address NixOS/nix#2270 right?

[Mic92]

Not yet. However it is now easier with a vendor directory to create a generator that creates separate source derivations than it was before with the go module proxy directory.

[Ma27]

After seeing this in the wild for a while I'm not sure if that's the way to move forward: while the old approaches certainly had their issues, we now seem to push the task of solving messed-up dependency situations in downstream sources to our package-maintainers.

Previously I was able to work around this by generating some sort of deps.nix or by using buildGoModule to let gomod do the job of fetching the proper sources. Now it seems to me that I have to search e.g. for proper git revisions of dependencies to override those if needed which is IMHO another source of possible packaging bugs.

Also, regenerating huge patches of a vendor/ directory is at least from my PoV not really a benefit over having to maintain automagically generated Nix-expressions with dependencies.

I may have missed something, but can someone explain why this is even needed? And, am I wrong with this assumption? Or in other words, shouldn't we keep the original behavior for "problematic" packages?

[numkem]

I can't make this work with my repo.

expression looks like this:

{ lib, buildGoModule, modSha256 ? null, maintainers, version ? "0.0.0-dev" }:

buildGoModule rec {
  pname = "repeater";
  inherit version;

  src = ../.;

  vendorSha256 = modSha256;

  subPackages = [ "cmd/repeater" ];

  meta = with lib; {
    [...]
  };
} 

And I don't have a vendor directory at the root of my module. Without the directory I get: run 'go mod vendor' to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory. With it it builds just fine. The documentation states it would run go mod vendor on it's own. Yet it doesn't look like it's doing that.

[Ma27]

@zimbatm @Mic92 @c00w would you mind taking a look at the questions I asked above? :)

[c00w]

@numkem Just noticed this. Can you include a full repro (i.e. with no variable parameters that only depends on nixpkgs) so I could run it? A seperate issue may also help with me responding quickly, but I can keep debugging here if it is easier for you :)

My guess is that you're setting vendorSha256=null; which says use the builtin in vendor directory. This is super useful if you have a vendor directory (or your code has no external dependencies). If that's not your intention, I'd set vendorSha256.

Another option is you copied the modSha256 hash and just set vendorSha256 to it. You need to modify the hash, so a full rebuild happens, then it will error and provide you the new hash to set.

If you're not doing either of those things, then I'll need to be able to pull the code and poke it to figure out what is going on.

[c00w]

@Ma27

After seeing this in the wild for a while I'm not sure if that's the way to move forward: while the old approaches certainly had their issues, we now seem to push the task of solving messed-up dependency situations in downstream sources to our package-maintainers.

The sad but glorious life of package managers :). I think we mostly are in the same state we used to be with regards to messed up dependencies. There were some regressions around go mod vendor not working will.

golang/go#38820

The other indirect question is "why do this" and the answer was that people (not me) were unhappy about using a binary format as our dependency format, since it was hard to audit and modify. It's actually a strength of the new approach that you can just use plain text patches to fix issues :)

However I really just want to get rid of buildGoPackages as much as possible, so I'm doing this to try and get #86282 merged.

Previously I was able to work around this by generating some sort of deps.nix or by using buildGoModule to let gomod do the job of fetching the proper sources. Now it seems to me that I have to search e.g. for proper git revisions of dependencies to override those if needed which is IMHO another source of possible packaging bugs.

I'm confused - buildGoModule never supported deps.nix.

If you expand on your issue a bit more I can be more prescriptive, but in general if you're having packaging issues for your own source, run go mod vendor in your directory and set vendorSha256 = null :). Then your code can build entirely from source and you don't have to deal with anything.

If you're trying to fix an issue in an upstream vendor, generally just patching go.mod solves your problems pretty well. There is a fun corner case of people using go modules to distribute c code (which is super not recommended by the go team). Making sure each c folder has a go file in it may help, alternatively, cp the c source into the directory after the fetching via a postInstall command. There are a few examples of this in the repor.

Also, regenerating huge patches of a vendor/ directory is at least from my PoV not really a benefit over having to maintain automagically generated Nix-expressions with dependencies.

Why are you needing huge patches? What are you trying to package that's exploding?

I may have missed something, but can someone explain why this is even needed? And, am I wrong with this assumption? Or in other words, shouldn't we keep the original behavior for "problematic" packages?

This happened because a lot of people were screaming at me that the go module packaging in nix was bad and we shouldn't use them. Rather than delete buildGoModule and migrate everything to buildGoPackage, I tried to address some of their complaints - one complaint was that the binary files weren't human readable, so now we have vendor directories which are human readable :). Another complaint is that go downloads don't proxy super well (since they're not just raw http fetches). That's the next complaint to fix, either by using deps.nix to generate the vendor folder, or by writing a go module proxy which can be configured via deps.nix.

TL;DR I'm just trying to get #86282 merged.