Completely support modern go.mod

[SuperSandro2000]

This PR combines #36, parts of #40 and adds the final bits to support modern go.mods. I tested this successfully with github.com/StackExchange/dnscontrol and updated it to 3.3.0.

It also fixes the tests and updates dependencies.

Any feedback is appreciated as this is the first time I am touching any such tool in the nix and go world.

I plan to keep this tool runnable in the future.

@dsx @Lucus16

Closes #36, #40

[dsx]

Just tried on k3s master:

panic: unrecognized import path "vbom.ml/util"

goroutine 1 [running]:
main.main()
        […]/main.go:286 +0x6d2

[SuperSandro2000]

Vanity URLs are a great feature... I will take a look at this the next days.

[SuperSandro2000]

@dsx the error is unrelated to this tool and caused by vbom.ml being no longer available. k3s probably needs to add something like https://github.com/ceph/ceph-csi/commit/22158ebf178d09dc3c834fccdcc0f245ec39832d.

[dsx]

@dsx the error is unrelated to this tool and caused by vbom.ml being no longer available. k3s probably needs to add something like ceph/ceph-csi@22158eb.

K3S just keeps it in /vendor. I wonder if there is any way to take into the account vendored packages.

[yihuang]

With the regex modification, vgo2nix works for https://github.com/cosmos/cosmos-sdk, but build still failed:

go/src/github.com/cosmos/cosmos-sdk/cosmovisor/upgrade.go:14:2: cannot find package "github.com/hashicorp/go-getter" in any of:
        /nix/store/chaz1p7zgj57qm8z55gw3a1aj1c64v4r-go-1.14.6/share/go/src/github.com/hashicorp/go-getter (from $GOROOT)
        /private/var/folders/4p/d9t4y4tn1xzdw_bdjbbdv_m40000gq/T/nix-build-cosmos-sdk-0.0.1.drv-0/go/src/github.com/hashicorp/go-getter (from $GOPATH)
builder for '/nix/store/hzazf24c78ng122z1pxam7n447w84s08-cosmos-sdk-0.0.1.drv' failed with exit code 1
error: build of '/nix/store/hzazf24c78ng122z1pxam7n447w84s08-cosmos-sdk-0.0.1.drv' failed

Not sure if it's an issue of vgo2nix though, still new to both golang and nix. This directory has it's own go.mod file might be the reason? The default.nix I used:

pkgs.buildGoPackage rec {
  pname = "cosmos-sdk";
  version = "0.0.1";
  goPackagePath = "github.com/cosmos/cosmos-sdk";
  src = ./.;
  goDeps = ./deps.nix;
}

[SuperSandro2000]

K3S just keeps it in /vendor. I wonder if there is any way to take into the account vendored packages.

There is always a way but is it worth implementing?

It is probably easier to just use buildGoModule instead.

This directory has it's own go.mod file might be the reason?

I don't think we parse repulsively like this.

[yihuang]

A small test case that fails:

mkdir test-mod
cd test-mod
cat > go.mod << EOF
module test-mod

go 1.15

require github.com/cespare/xxhash/v2 v2.1.1

require github.com/cespare/xxhash v1.1.0
EOF
cat > main.go << EOF
package main

import (
    "fmt"

    xxhash "github.com/cespare/xxhash"
    xxhash_v2 "github.com/cespare/xxhash/v2"
)

func main() {
    fmt.Printf("trace %s %s", xxhash.Digest{}, xxhash_v2.Digest{})
}
EOF
cat > default.nix << EOF
{ pkgs ? import <nixpkgs> {} }:
pkgs.buildGoPackage rec {
  pname = "test-mod";
  version = "0.0.1";
  goPackagePath = "test-mod";
  goDeps = ./deps.nix;
  src = ./.;
}
EOF

Run:

vgo2nix
nix-build

Error output:

building
go/src/test-mod/main.go:7:2: cannot find package "github.com/cespare/xxhash/v2" in any of:
        /nix/store/chaz1p7zgj57qm8z55gw3a1aj1c64v4r-go-1.14.6/share/go/src/github.com/cespare/xxhash/v2 (from $GOROOT)
        /private/var/folders/4p/d9t4y4tn1xzdw_bdjbbdv_m40000gq/T/nix-build-test-mod-0.0.1.drv-0/go/src/github.com/cespare/xxhash/v2 (from $GOPATH)
builder for '/nix/store/5afbfgism9dq1ir998c3k7mcz4d0syij-test-mod-0.0.1.drv' failed with exit code 1

The package github.com/cespare/xxhash/v2 is not written into deps.nix (the major version part of package path is removed after processing).

[dsx]

K3S just keeps it in /vendor. I wonder if there is any way to take into the account vendored packages. There is always a way but is it worth implementing? It is probably easier to just use buildGoModule instead.

This is actually a very good point!

[yihuang]

diff --git a/main.go b/main.go
index 10176c9..b4bac00 100644
--- a/main.go
+++ b/main.go
@@ -44,13 +44,14 @@ const depNixFormat = `  {
     };
   }`

+var versionNumber = regexp.MustCompile(`^v\d+`)
+
 func getModules() ([]*modEntry, error) {
    var entries []*modEntry

    commitShaRev := regexp.MustCompile(`^v\d+\.\d+\.\d+-(?:rc\d+\.)?(?:\d+\.)?[0-9]{14}-(.*?)(?:\+incompatible)?$`)
    commitRevV2 := regexp.MustCompile("^v.*-(.{12})\\+incompatible$")
    commitRevV3 := regexp.MustCompile(`^(v\d+\.\d+\.\d+)\+incompatible$`)
-   versionNumber := regexp.MustCompile(`^v\d+`)

    var stderr bytes.Buffer
    cmd := exec.Command("go", "list", "-mod", "mod", "-json", "-m", "all")
@@ -162,6 +163,9 @@ func getPackages(keepGoing bool, numJobs int, prevDeps map[string]*Package) ([]*
            return nil, wrapError(err)
        }
        goPackagePath := importRoot.Root
+       if versionNumber.MatchString(strings.TrimPrefix(entry.importPath, importRoot.Root+"/")) {
+           goPackagePath = entry.importPath
+       }

        if prevPkg, ok := prevDeps[goPackagePath]; ok {
            if prevPkg.Rev == entry.rev {

This patch works for the above failed test case, support multiple major version exists together.

[yihuang]

When run nix-shell in the pervious test case:

ln: failed to create symbolic link '/var/folders/4p/d9t4y4tn1xzdw_bdjbbdv_m40000gq/T/tmp.eYNogYmXpS-test-mod-0.0.1/src/github.com/cespare/xxhash/v2': Permission denied

It's trying to create v2 symbol link inside the v1 package, i guess no easy solution, other than copy the sources which is slow.

[yihuang]

When run nix-shell in the pervious test case:

ln: failed to create symbolic link '/var/folders/4p/d9t4y4tn1xzdw_bdjbbdv_m40000gq/T/tmp.eYNogYmXpS-test-mod-0.0.1/src/github.com/cespare/xxhash/v2': Permission denied

It's trying to create v2 symbol link inside the v1 package, i guess no easy solution, other than copy the sources which is slow.

https://github.com/NixOS/nixpkgs/issues/98604 This issue can only be fixed in buildGoPackage I think.

[yihuang]

When run nix-shell in the pervious test case:

ln: failed to create symbolic link '/var/folders/4p/d9t4y4tn1xzdw_bdjbbdv_m40000gq/T/tmp.eYNogYmXpS-test-mod-0.0.1/src/github.com/cespare/xxhash/v2': Permission denied

It's trying to create v2 symbol link inside the v1 package, i guess no easy solution, other than copy the sources which is slow.

NixOS/nixpkgs#98604 This issue can only be fixed in buildGoPackage I think.

With this patch in nixpkgs: https://github.com/NixOS/nixpkgs/pull/98632 And patch the generated deps.nix like this, it works:

20,28c20,30
<   }
<   {
<     goPackagePath = "github.com/cespare/xxhash/v2";
<     fetch = {
<       type = "git";
<       url = "https://github.com/cespare/xxhash";
<       rev = "v2.1.1";
<       sha256 = "0rl5rs8546zj1vzggv38w93wx0b5dvav7yy5hzxa8kw7iikv1cgr";
<     };
---
>     otherMajorVersions = [
>       {
>         majorVersion = "v2";
>         fetch = {
>           type = "git";
>           url = "https://github.com/cespare/xxhash";
>           rev = "v2.1.1";
>           sha256 = "0rl5rs8546zj1vzggv38w93wx0b5dvav7yy5hzxa8kw7iikv1cgr";
>         };
>       }
>     ];

[yihuang]

https://gist.github.com/yihuang/7e9e3cfb5ab3f126f077c1e5b66002ad I made a test case to cover the tricky situations. There are still several places where we haven't get it right.

[Mic92]

So after https://github.com/NixOS/nixpkgs/pull/98632 and the changes proposed by @yihuang we can merge this?

[Mic92]

@dsx the error is unrelated to this tool and caused by vbom.ml being no longer available. k3s probably needs to add something like ceph/ceph-csi@22158eb.

K3S just keeps it in /vendor. I wonder if there is any way to take into the account vendored packages.

I think buildGoModule can build everything from vendor, if all dependencies are there.

[yihuang]

So after NixOS/nixpkgs#97603 and the changes proposed by @yihuang we can merge this?

we have not passed all the test cases yet, like the one I made above.

[SuperSandro2000]

@yihuang I have added your new test. I take suggestions for the name cause I am not to happy with it.

Also it currently fails with the following:

Wrote deps.nix
--- expected.nix
+++ deps.nix
@@ -14,8 +14,8 @@
     fetch = {
       type = "git";
       url = "https://github.com/yihuang/test-golang-module-major-version";
-      rev = "sub/v2.0.1";
-      sha256 = "026cfk2bpc7cyddabj8fcay3nysbj9zjam63jbq8psav4qwhwc2d";
+      rev = "sub/v1.0.1";
+      sha256 = "10gnzs3l8j6skyrfi2blvqx9b3l2bs2gdvrgjg7a9aiz6z7dkpvy";
     };
   }

[yihuang]

@yihuang I have added your new test. I take suggestions for the name cause I am not to happy with it.

Also it currently fails with the following:

Wrote deps.nix
--- expected.nix
+++ deps.nix
@@ -14,8 +14,8 @@
     fetch = {
       type = "git";
       url = "https://github.com/yihuang/test-golang-module-major-version";
-      rev = "sub/v2.0.1";
-      sha256 = "026cfk2bpc7cyddabj8fcay3nysbj9zjam63jbq8psav4qwhwc2d";
+      rev = "sub/v1.0.1";
+      sha256 = "10gnzs3l8j6skyrfi2blvqx9b3l2bs2gdvrgjg7a9aiz6z7dkpvy";
     };
   }

We can also make the test case a little bit tricker, to have different minor version between two packages in same repo, I think current buildGoPackage can't handle this:

require github.com/yihuang/test-golang-module-major-version v1.0.1
require github.com/yihuang/test-golang-module-major-version/sub v1.0.2

[SuperSandro2000]

We can also make the test case a little bit tricker, to have different minor version between two packages in same repo, I think current buildGoPackage can't handle this:

require github.com/yihuang/test-golang-module-major-version v1.0.1
require github.com/yihuang/test-golang-module-major-version/sub v1.0.2

Wrote deps.nix
--- expected.nix
+++ deps.nix
@@ -14,8 +14,8 @@
     fetch = {
       type = "git";
       url = "https://github.com/yihuang/test-golang-module-major-version";
-      rev = "sub/v1.0.2";
-      sha256 = "1gscby4zzidnj9fs52p437vz29gqd6qfw6akwk42rmm43kpcff41";
+      rev = "sub/v2.0.1";
+      sha256 = "026cfk2bpc7cyddabj8fcay3nysbj9zjam63jbq8psav4qwhwc2d";
     };
   }

With that change the tests look like. buildGoPackage can't handle it right?

[yihuang]

We can also make the test case a little bit tricker, to have different minor version between two packages in same repo, I think current buildGoPackage can't handle this:

require github.com/yihuang/test-golang-module-major-version v1.0.1
require github.com/yihuang/test-golang-module-major-version/sub v1.0.2

Wrote deps.nix
--- expected.nix
+++ deps.nix
@@ -14,8 +14,8 @@
     fetch = {
       type = "git";
       url = "https://github.com/yihuang/test-golang-module-major-version";
-      rev = "sub/v1.0.2";
-      sha256 = "1gscby4zzidnj9fs52p437vz29gqd6qfw6akwk42rmm43kpcff41";
+      rev = "sub/v2.0.1";
+      sha256 = "026cfk2bpc7cyddabj8fcay3nysbj9zjam63jbq8psav4qwhwc2d";
     };
   }

With that change the tests look like. buildGoPackage can't handle it right?

I mean you can only specify one version for the root module, can't specify a different minor version for a sub module.

[yihuang]

@SuperSandro2000 I made a PR to you, https://github.com/SuperSandro2000/vgo2nix/pull/1 This together with https://github.com/NixOS/nixpkgs/pull/98903 works well with my test case above.

I also added gopkg.in in that testcase.

[SuperSandro2000]

I have picked your PR into master and updated the test.

[yihuang]

I can still construct failed test case though, the reason is go list -mod mod -m all outputs more modules than go mod vendor, and those modules might conflict with each other.

[yihuang]

I can still construct failed test case though, the reason is go list -mod mod -m all outputs more modules than go mod vendor, and those modules might conflict with each other.

https://github.com/golang/go/issues/41652

[SuperSandro2000]

@Mic92 bump

[Mic92]

@SuperSandro2000 Please ping me also on the PRs that can be closed after that.

[Mic92]

Now we can also bump in nixpkgs.