Completely support modern go.mod #42

SuperSandro2000 · 2020-09-07T19:17:32Z

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

When vgo2nix was run in a repository with both a vendor directory and a go.mod, such as the result of `go mod vendor`, `go list` would fail with: go list -m: can't compute 'all' using the vendor directory (Use -mod=mod or -mod=readonly to bypass.) This is fixed as suggested, which ensures `go list` uses the go.mod file if it exists.

from 27b8b52

dsx · 2020-09-09T22:04:44Z

Just tried on k3s master:

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

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

SuperSandro2000 · 2020-09-09T22:52:36Z

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

which is created when running nix-build which I used to debug the build failures.

SuperSandro2000 · 2020-09-10T20:20:47Z

@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.

main.go

dsx · 2020-09-17T18:59:57Z

@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 · 2020-09-17T19:57:41Z

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 · 2020-09-17T20:38:04Z

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 · 2020-09-17T21:00:29Z

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).

Solution: - using vgo2nix (nix-community/vgo2nix#42)

dsx · 2020-09-17T22:10:42Z

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 · 2020-09-23T03:36:17Z

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 · 2020-09-24T00:43:50Z

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.

yihuang · 2020-09-24T09:33:28Z

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: NixOS/nixpkgs#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 · 2020-09-24T19:17:23Z

main.go

 func getModules() ([]*modEntry, error) {
 	var entries []*modEntry

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


vX.Y.Z-pre.0.yyyymmddhhmmss-abcdefxyz when most recent versioned commit before the target commit is vX.Y.Z-pre

I think the rc part should not be hardcoded.
https://github.com/golang/go/blob/master/src/cmd/go/internal/modfetch/pseudo.go#L14

You are right. What do you think about the following change?

Suggested change

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

commitShaRev := regexp.MustCompile(`^v\d+\.\d+\.\d+-(?:\w+\d+\.)?(?:\d+\.)?[0-9]{14}-(.*?)(?:\+incompatible)?$`)

I'm trying this patch, more similar to how golang internally do:

diff --git a/main.go b/main.go index b4bac00..b2bbe1b 100644 --- a/main.go +++ b/main.go @@ -13,6 +13,8 @@ import ( "sort" "strings" + "golang.org/x/mod/module" + "golang.org/x/mod/semver" "golang.org/x/tools/go/vcs" ) @@ -49,10 +51,6 @@ 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$`) - var stderr bytes.Buffer cmd := exec.Command("go", "list", "-mod", "mod", "-json", "-m", "all") cmd.Stderr = &stderr @@ -110,16 +108,22 @@ func getModules() ([]*modEntry, error) { return nil, err } - if commitShaRev.MatchString(rev) { - rev = commitShaRev.FindAllStringSubmatch(rev, -1)[0][1] - } else if commitRevV2.MatchString(rev) { - rev = commitRevV2.FindAllStringSubmatch(rev, -1)[0][1] - } else if commitRevV3.MatchString(rev) { - rev = commitRevV3.FindAllStringSubmatch(rev, -1)[0][1] - } else if mod.Path != url.Root { - subPath := strings.Split(mod.Path, url.Root+"/")[1] - if !versionNumber.MatchString(subPath) { - rev = subPath + "/" + rev + path, _, ok := module.SplitPathVersion(mod.Path) + if !ok { + return nil, fmt.Errorf("invalid mod path: %s", mod.Path) + } + + build := semver.Build(rev) // +incompatible + gitRef := strings.TrimSuffix(rev, build) + if strings.Count(gitRef, "-") >= 2 { + // pseudo-version, use the commit hash + gitRef = gitRef[strings.LastIndex(gitRef, "-")+1:] + } else { + // fix tag + subModule := strings.TrimPrefix(path, url.Root) + if len(subModule) > 0 { + // trim the leading "/" + gitRef = subModule[1:] + "/" + gitRef } } @@ -131,7 +135,7 @@ func getModules() ([]*modEntry, error) { entries = append(entries, &modEntry{ replacePath: replacePath, importPath: mod.Path, - rev: rev, + rev: gitRef, }) }

https://github.com/golang/go/blob/master/src/cmd/go/internal/modfetch/coderepo.go#L656
this is the go internal code for this part of logic.

Nice. I searched for a function for hours but couldn't find one.

yihuang · 2020-09-24T20:18:54Z

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 · 2020-09-25T04:06:51Z

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

Mic92 · 2020-09-25T04:07:46Z

@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 · 2020-09-25T06:11:40Z

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 · 2020-09-26T13:38:09Z

@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 · 2020-09-26T14:56:06Z

@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

tests/test_major_version/expected.nix

SuperSandro2000 · 2020-09-26T20:55:39Z

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 · 2020-09-27T00:36:03Z

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 · 2020-09-27T02:57:36Z

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

I also added gopkg.in in that testcase.

SuperSandro2000 · 2020-09-27T08:29:28Z

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

yihuang · 2020-09-27T08:30:03Z

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 · 2020-09-27T15:42:45Z

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.

golang/go#41652

SuperSandro2000 · 2020-11-05T20:03:28Z

@Mic92 bump

Mic92 · 2020-11-06T07:01:13Z

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

Mic92 · 2020-11-06T07:11:38Z

Now we can also bump in nixpkgs.

mikroskeem and others added 13 commits September 7, 2020 18:38

Fix misleading log message

973daeb

Handle module replacements

0c364a6

Add version replacement test

9cf5b83

Add test for URL replacements

946235e

Support URL replacements

6b8cbc7

Enabled shallow clones

d7a4f51

Cherry-pick fixed regex

2d1b609

from 27b8b52

Make rev logging clearer to read

2b055df

Fix go vanity urls and repos with multiple go modules

1811177

Update log message to include shallow clone

6612bf8

Fix wrapProgram

92859c2

Update dependencies

77cd1d9

SuperSandro2000 mentioned this pull request Sep 7, 2020

dnscontrol: 3.2.0 -> 3.3.0, add me as maintainer NixOS/nixpkgs#97394

Merged

10 tasks

Ignore result/ directory

dae68eb

which is created when running nix-build which I used to debug the build failures.

SuperSandro2000 mentioned this pull request Sep 17, 2020

fix commitShaRev regex #44

Closed

yihuang reviewed Sep 17, 2020

View reviewed changes

main.go Outdated Show resolved Hide resolved

yihuang reviewed Sep 17, 2020

View reviewed changes

main.go Outdated Show resolved Hide resolved

yihuang reviewed Sep 17, 2020

View reviewed changes

main.go Outdated Show resolved Hide resolved

Match version numbers which include rcX.X

407b622

yihuang added a commit to crypto-org-chain/chain-main that referenced this pull request Sep 17, 2020

Problem: chain-maind is not built with nix

3925293

Solution: - using vgo2nix (nix-community/vgo2nix#42)

yihuang mentioned this pull request Sep 17, 2020

Problem: chain-maind build may not be reproducible crypto-org-chain/chain-main#83

Merged

SuperSandro2000 mentioned this pull request Sep 22, 2020

Issue while trying to package seaweedfs #45

Closed

This was referenced Sep 23, 2020

Problem: no nix shell environment for development crypto-org-chain/chain-main#113

Closed

buildGoPackage don't support multiple major version of a package NixOS/nixpkgs#98604

Closed

yihuang reviewed Sep 24, 2020

View reviewed changes

Replace custom ref handler with a go internal one

5166eb0

yihuang reviewed Sep 26, 2020

View reviewed changes

tests/test_major_version/expected.nix Show resolved Hide resolved

Add major version test

1f3365a

yihuang and others added 2 commits September 27, 2020 10:22

fix submodules and major versions

5b6804e

Update to include different submodule version and gopkg.in

b8eedc9

SuperSandro2000 added 2 commits September 27, 2020 11:00

Remove go version from go.mods

628334c

Update tests with moduleDir

ee7fc0d

Mic92 merged commit bc8785d into nix-community:master Nov 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Completely support modern go.mod #42

Completely support modern go.mod #42

SuperSandro2000 commented Sep 7, 2020 •

edited

Loading

dsx commented Sep 9, 2020

SuperSandro2000 commented Sep 9, 2020

SuperSandro2000 commented Sep 10, 2020

dsx commented Sep 17, 2020

yihuang commented Sep 17, 2020 •

edited

Loading

SuperSandro2000 commented Sep 17, 2020 •

edited

Loading

yihuang commented Sep 17, 2020 •

edited

Loading

dsx commented Sep 17, 2020

yihuang commented Sep 23, 2020 •

edited

Loading

yihuang commented Sep 24, 2020

yihuang commented Sep 24, 2020

yihuang Sep 24, 2020 •

edited

Loading

SuperSandro2000 Sep 24, 2020

yihuang Sep 24, 2020 •

edited

Loading

yihuang Sep 25, 2020

SuperSandro2000 Sep 26, 2020

yihuang commented Sep 24, 2020 •

edited

Loading

Mic92 commented Sep 25, 2020 •

edited

Loading

Mic92 commented Sep 25, 2020

yihuang commented Sep 25, 2020 •

edited

Loading

SuperSandro2000 commented Sep 26, 2020

yihuang commented Sep 26, 2020

SuperSandro2000 commented Sep 26, 2020

yihuang commented Sep 27, 2020

yihuang commented Sep 27, 2020 •

edited

Loading

SuperSandro2000 commented Sep 27, 2020

yihuang commented Sep 27, 2020 •

edited

Loading

yihuang commented Sep 27, 2020

SuperSandro2000 commented Nov 5, 2020

Mic92 commented Nov 6, 2020

Mic92 commented Nov 6, 2020

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

Completely support modern go.mod #42

Completely support modern go.mod #42

Conversation

SuperSandro2000 commented Sep 7, 2020 • edited Loading

dsx commented Sep 9, 2020

SuperSandro2000 commented Sep 9, 2020

SuperSandro2000 commented Sep 10, 2020

dsx commented Sep 17, 2020

yihuang commented Sep 17, 2020 • edited Loading

SuperSandro2000 commented Sep 17, 2020 • edited Loading

yihuang commented Sep 17, 2020 • edited Loading

dsx commented Sep 17, 2020

yihuang commented Sep 23, 2020 • edited Loading

yihuang commented Sep 24, 2020

yihuang commented Sep 24, 2020

yihuang Sep 24, 2020 • edited Loading

Choose a reason for hiding this comment

SuperSandro2000 Sep 24, 2020

Choose a reason for hiding this comment

yihuang Sep 24, 2020 • edited Loading

Choose a reason for hiding this comment

yihuang Sep 25, 2020

Choose a reason for hiding this comment

SuperSandro2000 Sep 26, 2020

Choose a reason for hiding this comment

yihuang commented Sep 24, 2020 • edited Loading

Mic92 commented Sep 25, 2020 • edited Loading

Mic92 commented Sep 25, 2020

yihuang commented Sep 25, 2020 • edited Loading

SuperSandro2000 commented Sep 26, 2020

yihuang commented Sep 26, 2020

SuperSandro2000 commented Sep 26, 2020

yihuang commented Sep 27, 2020

yihuang commented Sep 27, 2020 • edited Loading

SuperSandro2000 commented Sep 27, 2020

yihuang commented Sep 27, 2020 • edited Loading

yihuang commented Sep 27, 2020

SuperSandro2000 commented Nov 5, 2020

Mic92 commented Nov 6, 2020

Mic92 commented Nov 6, 2020

SuperSandro2000 commented Sep 7, 2020 •

edited

Loading

yihuang commented Sep 17, 2020 •

edited

Loading

SuperSandro2000 commented Sep 17, 2020 •

edited

Loading

yihuang commented Sep 17, 2020 •

edited

Loading

yihuang commented Sep 23, 2020 •

edited

Loading

yihuang Sep 24, 2020 •

edited

Loading

yihuang Sep 24, 2020 •

edited

Loading

yihuang commented Sep 24, 2020 •

edited

Loading

Mic92 commented Sep 25, 2020 •

edited

Loading

yihuang commented Sep 25, 2020 •

edited

Loading

yihuang commented Sep 27, 2020 •

edited

Loading

yihuang commented Sep 27, 2020 •

edited

Loading