Migrate some go builds from buildGoPackage to buildGoModule

[c00w]

Motivation for this change

Now that everything is on go 1.14, go modules are officially suported + the suggested packaging solution.

https://github.com/golang/go/wiki/Modules#go-modules

Most of the go ecosystem is migrating over, and a number of nix packages already have module support but are still being built in the legacy buildGoPackage mode.

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.

[zowoq]

Why switch to modules when upstream has a /vendor?

[c00w]

https://github.com/golang/go/wiki/Modules#go-modules

In general, modules are the supported way forward. vendor directories are mostly used to provide backwards compatibility, as well as save time downloading code (i.e. go install -mod=vendor is default if there is a vendor directory). As long as a go.mod and go.sum file exist in the repo, we should flip over to using it.

[c00w]

Also updated the PR motivation.

[zowoq]

See #84826 for some discussion about this.

[Mic92]

In the long term we should also introduce a go2nix for buildGoModule.

[zowoq]

Could you format your commit messages please?

https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md#submitting-changes

[sveitser]

The package builds and runs with the changes. I was a bit confused by the new version tag at first. I noticed that some packages in this PR have an "unstable" tag in the version or package name.

[c00w]

Upstream for this is just pretty bad at releasing packages - 1.0.6 is over a year out of date and no releases have been tagged since then.

[sveitser]

OK. Thanks for updating the package. LGTM

[nlewo]

Please see #84826 for a discussion about this kind of change;)

[c00w]

Please see #84826 for a discussion about this kind of change;)

Please see #86282 (comment) for previous mention in the is PR :) I think I also dropped 50% of the comments on that issue because I got excited a few hours ago.

[Ma27]

I agree with @nlewo here.

@c00w reading your comment in #84826 I got the impression that you primarily argued about the traits of dependency-management in Go, but the original motivation why buildGoModule is discouraged isn't related to the Go-ecosystem itself. The problem is that we're actually "abusing" the concept of fixed-output derivations here: NixOS/nix#2270

I acknowledge that Go is moving towards the module-based approach, but as long as packages have e.g. a vendor/-directory, I don't see a reason to switch and if we want to, we should rather consider adding at least an alternative implementation for modules first.

[marsam]

Let's keep using buildGoPackage in packages with vendorized dependencies (vendor/ directory)

[c00w]

@c00w reading your comment in #84826 I got the impression that you primarily argued about the traits of dependency-management in Go, but the original motivation why buildGoModule is discouraged isn't related to the Go-ecosystem itself. The problem is that we're actually "abusing" the concept of fixed-output derivations here: NixOS/nix#2270

This discussion sounds like it belongs in nix#2270 or #84826. However citing nix#2270 as a justification for getting rid of buildGoModules doesn't seem to track logically. The core message in nix#2270 is that doing complicated downloading isn't reproducible. However with buildGoModule it is (and in fact it's a little more reproducible than normal nix downloads since it's validated across the universe of downloads). Additionally, citing a 2 year old discussion that hasn't seemed to have progressed or been accepted as a plan seems to be moving before concensus has been reached. If we can get #84826 merged, then I'll just nuke buildGoModule everywhere. I expect instead we'll end up in a state where we are still using buildGoModule, but may have an alternate strategy for specifying inputs.

I acknowledge that Go is moving towards the module-based approach, but as long as packages have e.g. a vendor/-directory, I don't see a reason to switch and if we want to, we should rather consider adding at least an alternative implementation for modules first.

So there are two bits - you can have modules enabled or disabled, and you can have a vendor folder (enabled or disabled).

with buildGoPackage, my understanding is that modules are disabled.
with buildGoModule, my understanding is that modules are enabled.

If you have no vendor folder this discussion is irrelevent so well ignore that case.

If you have a vendor folder, then buildGoPackage will use the vendor folder (even if you put other dependencies in your go deps, since vendor always wins). So the deps files are a lie for anything vendored :)

NixOS/nix#2270

If you have a vendor folder, then buildGoModule will also use the vendor folder automatically for builds. Just like deps, it's ignored completely if there is a vendor override. I just played with it, and it looks like we're downloading the modules, then ignoring the results - likely a patch to go send to go for the go mod download command. In fact, there is a maybe an argument that we should use go mod vendor instead of go mod download instead of buildGoModule + skip it if the vendor folder already exists.

https://golang.org/cmd/go/#hdr-Modules_and_vendoring

So the conclusion is that we're still building from the vendor folder in buildGoModule, we're just building in the modern mode, rather than the legacy go mode.

[Mic92]

We can also add an option to ignore modSha256 in case we want to build solely from vendor/ for buildGoModule. Than I don't see any reason in this case why we would not want to use buildGoModule.

[Ma27]

We can also add an option to ignore modSha256 in case we want to build solely from vendor/ for buildGoModule. Than I don't see any reason in this case why we would not want to use buildGoModule.

I fully agree, this would be a totally reasonable solution. In case it wasn't clear: my concerns are not about Go's module system, but about the way we implemented this.

This discussion sounds like it belongs in nix#2270 or #84826.

I disagree: when we switch to the buildGoModule-approach in its current state, we should discuss about this first.

However citing nix#2270 as a justification for getting rid of buildGoModules doesn't seem to track logically.

It's not about getting rid of it, but about the fact that it's currently rather discouraged because of its current implementation. If we can go-modules without the fixed-output derivation for the download for the majority of packages, I'd be fine with this.