Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Feedback: Existing project using gvt vs. dep #138

Closed
dcelasun opened this issue Jan 24, 2017 · 6 comments
Closed

Feedback: Existing project using gvt vs. dep #138

dcelasun opened this issue Jan 24, 2017 · 6 comments
Labels

Comments

@dcelasun
Copy link

dcelasun commented Jan 24, 2017

Just tried this on an existing project. I've only been playing with this for a few minutes, so it's quite possible I'm doing something wrong. That said, I'm really excited to see official progress on dependency management! Thank you for working on this!

Quick thoughts:

  • dep init is slow. It took more than 2 minutes to complete on a high-end machine with a 100Mbps connection. I didn't check what the bottleneck is and I understand if performance is not a high priority at this stage.
  • After dep init, manifest.json has an empty JSON object: {}
  • lock.json has entries, but it only found 26 "projects", compared to 44 dependencies with gvt. See below.
  • I've tried creating vendor/ with both dep ensure and gvt restore and the resulting project compiles fine in both cases, so I'm inclined to think gvt includes some unnecessary deps.

lock.json

{
    "memo": "ff8a6c4e978a493862dc6632fa6505b2aa5e2480f8a8b5db380b66b286305f72",
    "projects": [
        {
            "name": "git.apache.org/thrift.git",
            "version": "0.10.0",
            "revision": "b2a4d4ae21c789b689dd162deb819665567f481c",
            "packages": [
                "lib/go/thrift"
            ]
        },
        {
            "name": "github.com/Sirupsen/logrus",
            "version": "v0.11.0",
            "revision": "d26492970760ca5d33129d2d799e34be5c4782eb",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/armon/consul-api",
            "branch": "master",
            "revision": "dcfedd50ed5334f96adee43fc88518a4f095e15c",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/codegangsta/negroni",
            "version": "v0.2.0",
            "revision": "fde5e16d32adc7ad637e9cd9ad21d4ebc6192535",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/coreos/go-etcd",
            "version": "v2.0.0",
            "revision": "f02171fbd43c7b9b53ce8679b03235a1ef3c7b12",
            "packages": [
                "etcd"
            ]
        },
        {
            "name": "github.com/fsnotify/fsnotify",
            "version": "v1.4.2",
            "revision": "629574ca2a5df945712d3079857300b5e4da0236",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/gorilla/context",
            "version": "v1.1",
            "revision": "1ea25387ff6f684839d82767c1733ff4d4d15d0a",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/hashicorp/hcl",
            "branch": "master",
            "revision": "39fa3a62ba92cf550eb0f9cfb84757ef79b8aa30",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/magiconair/properties",
            "version": "v1.7.0",
            "revision": "c265cfa48dda6474e208715ca93e987829f572f8",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/mediocregopher/radix.v2",
            "branch": "master",
            "revision": "1ac54a28f5934ea5e08f588647e734aba2383cb8",
            "packages": [
                "pool",
                "redis"
            ]
        },
        {
            "name": "github.com/mitchellh/mapstructure",
            "branch": "master",
            "revision": "ed105d635dfa9ea7133f7c79f1eb36203fc3a156",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/pelletier/go-buffruneio",
            "branch": "master",
            "revision": "df1e16fde7fc330a0ca68167c23bf7ed6ac31d6d",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/pelletier/go-toml",
            "version": "v0.4.0",
            "revision": "d464759235b8fd59f8ab4c44807d7c36e1678add",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/spf13/afero",
            "branch": "master",
            "revision": "72b31426848c6ef12a7a8e216708cb0d1530f074",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/spf13/cast",
            "branch": "master",
            "revision": "56a7ecbeb18dde53c6db4bd96b541fd9741b8d44",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/spf13/jwalterweatherman",
            "branch": "master",
            "revision": "fa7ca7e836cf3a8bb4ebf799f472c12d7e903d66",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/spf13/pflag",
            "branch": "master",
            "revision": "a232f6d9f87afaaa08bafaff5da685f974b83313",
            "packages": [
                "."
            ]
        },
        {
            "name": "github.com/spf13/viper",
            "branch": "master",
            "revision": "5ed0fc31f7f453625df314d8e66b9791e8d13003",
            "packages": [
                ".",
                "remote"
            ]
        },
        {
            "name": "github.com/xordataexchange/crypt",
            "branch": "master",
            "revision": "749e360c8f236773f28fc6d3ddfce4a470795227",
            "packages": [
                "config"
            ]
        },
        {
            "name": "goji.io",
            "version": "v1.0",
            "revision": "e1741450ad80d2837134e32ac7b1d0a7a2b11c51",
            "packages": [
                ".",
                "pat"
            ]
        },
        {
            "name": "golang.org/x/crypto",
            "branch": "master",
            "revision": "41d678d1df78cd0410143162dff954e6dc09300f",
            "packages": [
                "openpgp"
            ]
        },
        {
            "name": "golang.org/x/net",
            "branch": "master",
            "revision": "f2499483f923065a842d38eb4c7f1927e6fc6e6d",
            "packages": [
                "context"
            ]
        },
        {
            "name": "golang.org/x/sys",
            "branch": "master",
            "revision": "d75a52659825e75fff6158388dddc6a5b04f9ba5",
            "packages": [
                "unix"
            ]
        },
        {
            "name": "golang.org/x/text",
            "branch": "master",
            "revision": "11dbc599981ccdf4fb18802a28392a8bcf7a9395",
            "packages": [
                "transform",
                "unicode/norm"
            ]
        },
        {
            "name": "gopkg.in/tylerb/graceful.v1",
            "version": "v1.2.14",
            "revision": "0e9129e9c6d47da90dc0c188b26bd7bb1dab53cd",
            "packages": [
                "."
            ]
        },
        {
            "name": "gopkg.in/yaml.v2",
            "branch": "v2",
            "revision": "14227de293ca979cf205cd88769fe71ed96a97e2",
            "packages": [
                "."
            ]
        }
    ]
}

manifest (from gvt)

{
	"version": 0,
	"dependencies": [
		{
			"importpath": "git.apache.org/thrift.git/lib/go/thrift",
			"repository": "https://git.apache.org/thrift.git",
			"vcs": "git",
			"revision": "55f976e0decefb284b0f0a459745dd57f038ab4f",
			"branch": "master",
			"path": "/lib/go/thrift",
			"notests": true
		},
		{
			"importpath": "github.com/Sirupsen/logrus",
			"repository": "https://github.com/Sirupsen/logrus",
			"vcs": "git",
			"revision": "61e43dc76f7ee59a82bdf3d71033dc12bea4c77d",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/armon/consul-api",
			"repository": "https://github.com/armon/consul-api",
			"vcs": "git",
			"revision": "dcfedd50ed5334f96adee43fc88518a4f095e15c",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/codegangsta/negroni",
			"repository": "https://github.com/codegangsta/negroni",
			"vcs": "git",
			"revision": "b0a6f4500e67c3e53f2d1563cca1563ab9777533",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/coreos/go-etcd/etcd",
			"repository": "https://github.com/coreos/go-etcd",
			"vcs": "git",
			"revision": "003851be7bb0694fe3cc457a49529a19388ee7cf",
			"branch": "master",
			"path": "/etcd",
			"notests": true
		},
		{
			"importpath": "github.com/fsnotify/fsnotify",
			"repository": "https://github.com/fsnotify/fsnotify",
			"vcs": "git",
			"revision": "fd9ec7deca8bf46ecd2a795baaacf2b3a9be1197",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/gorilla/context",
			"repository": "https://github.com/gorilla/context",
			"vcs": "git",
			"revision": "08b5f424b9271eedf6f9f0ce86cb9396ed337a42",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/hashicorp/hcl",
			"repository": "https://github.com/hashicorp/hcl",
			"vcs": "git",
			"revision": "39fa3a62ba92cf550eb0f9cfb84757ef79b8aa30",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/kr/fs",
			"repository": "https://github.com/kr/fs",
			"vcs": "git",
			"revision": "2788f0dbd16903de03cb8186e5c7d97b69ad387b",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/magiconair/properties",
			"repository": "https://github.com/magiconair/properties",
			"vcs": "git",
			"revision": "b3b15ef068fd0b17ddf408a23669f20811d194d2",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/mediocregopher/radix.v2/pool",
			"repository": "https://github.com/mediocregopher/radix.v2",
			"vcs": "git",
			"revision": "1ac54a28f5934ea5e08f588647e734aba2383cb8",
			"branch": "master",
			"path": "/pool",
			"notests": true
		},
		{
			"importpath": "github.com/mediocregopher/radix.v2/redis",
			"repository": "https://github.com/mediocregopher/radix.v2",
			"vcs": "git",
			"revision": "1ac54a28f5934ea5e08f588647e734aba2383cb8",
			"branch": "master",
			"path": "redis",
			"notests": true
		},
		{
			"importpath": "github.com/mitchellh/mapstructure",
			"repository": "https://github.com/mitchellh/mapstructure",
			"vcs": "git",
			"revision": "ed105d635dfa9ea7133f7c79f1eb36203fc3a156",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/pelletier/go-buffruneio",
			"repository": "https://github.com/pelletier/go-buffruneio",
			"vcs": "git",
			"revision": "df1e16fde7fc330a0ca68167c23bf7ed6ac31d6d",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/pelletier/go-toml",
			"repository": "https://github.com/pelletier/go-toml",
			"vcs": "git",
			"revision": "a1f048ba24490f9b0674a67e1ce995d685cddf4a",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/pkg/errors",
			"repository": "https://github.com/pkg/errors",
			"vcs": "git",
			"revision": "248dadf4e9068a0b3e79f02ed0a610d935de5302",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/pkg/sftp",
			"repository": "https://github.com/pkg/sftp",
			"vcs": "git",
			"revision": "ef9d88fbd130eb74c44ff86725c8bfdd94152ebe",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/spf13/afero",
			"repository": "https://github.com/spf13/afero",
			"vcs": "git",
			"revision": "72b31426848c6ef12a7a8e216708cb0d1530f074",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/spf13/cast",
			"repository": "https://github.com/spf13/cast",
			"vcs": "git",
			"revision": "56a7ecbeb18dde53c6db4bd96b541fd9741b8d44",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/spf13/jwalterweatherman",
			"repository": "https://github.com/spf13/jwalterweatherman",
			"vcs": "git",
			"revision": "fa7ca7e836cf3a8bb4ebf799f472c12d7e903d66",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/spf13/pflag",
			"repository": "https://github.com/spf13/pflag",
			"vcs": "git",
			"revision": "a232f6d9f87afaaa08bafaff5da685f974b83313",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/spf13/viper",
			"repository": "https://github.com/spf13/viper",
			"vcs": "git",
			"revision": "5ed0fc31f7f453625df314d8e66b9791e8d13003",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/ugorji/go/codec",
			"repository": "https://github.com/ugorji/go",
			"vcs": "git",
			"revision": "d23841a297e5489e787e72fceffabf9d2994b52a",
			"branch": "master",
			"path": "/codec",
			"notests": true
		},
		{
			"importpath": "github.com/urfave/negroni",
			"repository": "https://github.com/urfave/negroni",
			"vcs": "git",
			"revision": "b0a6f4500e67c3e53f2d1563cca1563ab9777533",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "github.com/xordataexchange/crypt/backend",
			"repository": "https://github.com/xordataexchange/crypt",
			"vcs": "git",
			"revision": "749e360c8f236773f28fc6d3ddfce4a470795227",
			"branch": "master",
			"path": "backend",
			"notests": true
		},
		{
			"importpath": "github.com/xordataexchange/crypt/config",
			"repository": "https://github.com/xordataexchange/crypt",
			"vcs": "git",
			"revision": "749e360c8f236773f28fc6d3ddfce4a470795227",
			"branch": "master",
			"path": "/config",
			"notests": true
		},
		{
			"importpath": "github.com/xordataexchange/crypt/encoding/secconf",
			"repository": "https://github.com/xordataexchange/crypt",
			"vcs": "git",
			"revision": "749e360c8f236773f28fc6d3ddfce4a470795227",
			"branch": "master",
			"path": "encoding/secconf",
			"notests": true
		},
		{
			"importpath": "goji.io",
			"repository": "https://github.com/goji/goji",
			"vcs": "git",
			"revision": "0d89ff54b2c18c9c4ba530e32496aef902d3c6cd",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "golang.org/x/crypto/cast5",
			"repository": "https://go.googlesource.com/crypto",
			"vcs": "git",
			"revision": "41d678d1df78cd0410143162dff954e6dc09300f",
			"branch": "master",
			"path": "cast5",
			"notests": true
		},
		{
			"importpath": "golang.org/x/crypto/curve25519",
			"repository": "https://go.googlesource.com/crypto",
			"vcs": "git",
			"revision": "41d678d1df78cd0410143162dff954e6dc09300f",
			"branch": "master",
			"path": "curve25519",
			"notests": true
		},
		{
			"importpath": "golang.org/x/crypto/ed25519",
			"repository": "https://go.googlesource.com/crypto",
			"vcs": "git",
			"revision": "41d678d1df78cd0410143162dff954e6dc09300f",
			"branch": "master",
			"path": "ed25519",
			"notests": true
		},
		{
			"importpath": "golang.org/x/crypto/openpgp",
			"repository": "https://go.googlesource.com/crypto",
			"vcs": "git",
			"revision": "41d678d1df78cd0410143162dff954e6dc09300f",
			"branch": "master",
			"path": "/openpgp",
			"notests": true
		},
		{
			"importpath": "golang.org/x/crypto/ssh",
			"repository": "https://go.googlesource.com/crypto",
			"vcs": "git",
			"revision": "41d678d1df78cd0410143162dff954e6dc09300f",
			"branch": "master",
			"path": "ssh",
			"notests": true
		},
		{
			"importpath": "golang.org/x/sys/unix",
			"repository": "https://go.googlesource.com/sys",
			"vcs": "git",
			"revision": "d75a52659825e75fff6158388dddc6a5b04f9ba5",
			"branch": "master",
			"path": "/unix",
			"notests": true
		},
		{
			"importpath": "golang.org/x/text/internal/gen",
			"repository": "https://go.googlesource.com/text",
			"vcs": "git",
			"revision": "11dbc599981ccdf4fb18802a28392a8bcf7a9395",
			"branch": "master",
			"path": "internal/gen",
			"notests": true
		},
		{
			"importpath": "golang.org/x/text/internal/triegen",
			"repository": "https://go.googlesource.com/text",
			"vcs": "git",
			"revision": "11dbc599981ccdf4fb18802a28392a8bcf7a9395",
			"branch": "master",
			"path": "internal/triegen",
			"notests": true
		},
		{
			"importpath": "golang.org/x/text/internal/ucd",
			"repository": "https://go.googlesource.com/text",
			"vcs": "git",
			"revision": "11dbc599981ccdf4fb18802a28392a8bcf7a9395",
			"branch": "master",
			"path": "internal/ucd",
			"notests": true
		},
		{
			"importpath": "golang.org/x/text/transform",
			"repository": "https://go.googlesource.com/text",
			"vcs": "git",
			"revision": "11dbc599981ccdf4fb18802a28392a8bcf7a9395",
			"branch": "master",
			"path": "/transform",
			"notests": true
		},
		{
			"importpath": "golang.org/x/text/unicode/cldr",
			"repository": "https://go.googlesource.com/text",
			"vcs": "git",
			"revision": "11dbc599981ccdf4fb18802a28392a8bcf7a9395",
			"branch": "master",
			"path": "unicode/cldr",
			"notests": true
		},
		{
			"importpath": "golang.org/x/text/unicode/norm",
			"repository": "https://go.googlesource.com/text",
			"vcs": "git",
			"revision": "11dbc599981ccdf4fb18802a28392a8bcf7a9395",
			"branch": "master",
			"path": "unicode/norm",
			"notests": true
		},
		{
			"importpath": "gopkg.in/airbrake/gobrake.v2",
			"repository": "https://gopkg.in/airbrake/gobrake.v2",
			"vcs": "git",
			"revision": "668876711219e8b0206e2994bf0a59d889c775aa",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "gopkg.in/gemnasium/logrus-airbrake-hook.v2",
			"repository": "https://gopkg.in/gemnasium/logrus-airbrake-hook.v2",
			"vcs": "git",
			"revision": "bfee1239d796830ca346767650cce5ba90d58c57",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "gopkg.in/tylerb/graceful.v1",
			"repository": "https://gopkg.in/tylerb/graceful.v1",
			"vcs": "git",
			"revision": "0e9129e9c6d47da90dc0c188b26bd7bb1dab53cd",
			"branch": "master",
			"notests": true
		},
		{
			"importpath": "gopkg.in/yaml.v2",
			"repository": "https://gopkg.in/yaml.v2",
			"vcs": "git",
			"revision": "14227de293ca979cf205cd88769fe71ed96a97e2",
			"branch": "v2",
			"notests": true
		}
	]
}
@sdboyer
Copy link
Member

sdboyer commented Jan 24, 2017

Thanks for trying it out and providing feedback! 🎉 🎉

dep init is slow. It took more than 2 minutes to complete on a high-end machine with a 100Mbps connection. I didn't check what the bottleneck is and I understand if performance is not a high priority at this stage.

Some of that time is initial repository clones - a one-time cost. But yeah, we've not overly prioritized towards performance just yet. That said, there is a plan, and there's a lot of headroom here: #67

After dep init, manifest.json has an empty JSON object: {}

This is weird. Could you open an issue specifically for this, please, and provide a little more context about the state of your GOPATH when you ran dep init? Thanks!

lock.json has entries, but it only found 26 "projects", compared to 44 dependencies with gvt. See below.

I don't know gvt well, but I'm pretty sure that yeah, this is accounted for by the is the difference between "package" and "project." The latter is defined in the spec doc, but that's a bit loose, so the definition in gps might also be helpful.

I've tried creating vendor/ with both dep ensure and gvt restore and the resulting project compiles fine in both cases, so I'm inclined to think gvt includes some unnecessary deps.

Possible? Again, idk gvt that well - @jessfraz might be able to say more definitively. The question of what ultimately ends up in vendor is a bit of a thing itself, too - see #120.

@jessfraz
Copy link
Contributor

jessfraz commented Jan 24, 2017 via email

@dcelasun
Copy link
Author

This is weird. Could you open an issue specifically for this, please, and provide a little more context about the state of your GOPATH when you ran dep init? Thanks!

Opened #149.

fwict i think gvt is just pulling in whole repos and their deps but idk

Yes, gvt is recursively vendoring all dependencies, so that probably explains the difference. I'm all for pruning though, so I'm not complaining :)

@nathany
Copy link
Contributor

nathany commented May 30, 2017

Yes, gvt pulls in whole repos (expect the tests) as @jessfraz stated. I ran into that exact issue in the past and had to manually vendor a subset of the files. Dep is much nicer in that regard 👍
https://github.com/gemnasium/migrate/issues/19#issuecomment-266878587

@dcelasun Can this ticket be closed or are there still outstanding issues? I opened up a new issues, #679, to address smoothing out the transition from gvt.

As far as speed, I just ran dep on a dual-core system with a fair sized project, which took 23s. There is a much better indication of progress these days, which goes a long ways.

Hopefully the strange JSON {} issue disappeared in the transition to .toml files.

Cheers!

@dcelasun
Copy link
Author

As far as speed, I just ran dep on a dual-core system with a fair sized project, which took 23s.

Yep, it looks quite a bit faster to me too.

Hopefully the strange JSON {} issue disappeared in the transition to .toml files.

The problem wasn't about the format, but it still got "fixed" (depending on how you look at it) in another way. As you also realized in #679, dep doesn't look into the contents of an existing vendor dir, only GOPATH.

Since I don't have any dependencies under $GOPATH/src, dep used to generate an empty JSON file. Now, it simply fetches from master for all those dependencies. That still feels wrong; changing a dependency tool shouldn't force me to change the versions of my dependencies.

But yes, I guess we can close this one since #679 has most of my concerns.

@sdboyer
Copy link
Member

sdboyer commented May 31, 2017

Now, it simply fetches from master for all those dependencies.

It's a little more sophisticated than that - https://github.com/golang/dep/blob/master/FAQ.md#how-does-dep-decide-what-version-of-a-dependency-to-use

It may just be that all the dependencies your project has have no tagged semver releases, so master is the best it can do.

That still feels wrong; changing a dependency tool shouldn't force me to change the versions of my dependencies.

Indeed, the solution here is writing a gvt adapter that knows how to translate from gvt's metadata into dep's. We should be able to start work on that shortly after #500 is merged. That should be this week. 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants