When vendor dir is part complete, dep ensure doesn't add missing dependencies

[bradleyfalzon]

What version of Go (go version) and dep (git describe --tags) are you using?

root@d617216ea384:/go/src/vendtest# go version
go version go1.8.3 linux/amd64
root@d617216ea384:/go/src/github.com/golang/dep# git describe --tags
v0.1.0-282-g167adc2

What dep command did you run?

docker run -it --rm golang:1.8 bash
root@3650b93883fb:/go# go get -u github.com/golang/dep/cmd/dep
root@3650b93883fb:/go# mkdir src/vendtest && cd src/vendtest
root@3650b93883fb:/go/src/vendtest# cat > main.go <<EOF
package main

import "github.com/pkg/errors"

func main() {
        println(errors.New("foo error"))
}
EOF
root@fccf383a79f9:/go/src/vendtest# go run main.go
main.go:3:8: cannot find package "github.com/pkg/errors" in any of:
        /usr/local/go/src/github.com/pkg/errors (from $GOROOT)
        /go/src/github.com/pkg/errors (from $GOPATH)
root@3650b93883fb:/go/src/vendtest# dep init
Using ^0.8.0 as constraint for direct dep github.com/pkg/errors
Locking in v0.8.0 (645ef00) for direct dep github.com/pkg/errors
root@fccf383a79f9:/go/src/vendtest# go run main.go
(0x4e5140,0xc42000a160)
root@3650b93883fb:/go/src/vendtest# ls vendor/github.com/
pkg

So far so good, we've proven the dependency github.com/pkg/errors didn't initially exist, running dep init then added the dependency successfully and the program compiles.

The next step is a part of my workflow to switch between a vendored dependency, to that in my GOPATH, so I can iterate on a fork. This part isn't important (but appears to be the recommend workflow based on https://github.com/golang/dep/blob/master/README.md#testing-changes-to-a-dependency), just the fact that I've manually removed the dependency from the vendor dir.

root@3650b93883fb:/go/src/vendtest# rm -rf vendor/github.com/pkg
root@fccf383a79f9:/go/src/vendtest# go run main.go
main.go:3:8: cannot find package "github.com/pkg/errors" in any of:
        /go/src/vendtest/vendor/github.com/pkg/errors (vendor tree)
        /usr/local/go/src/github.com/pkg/errors (from $GOROOT)
        /go/src/github.com/pkg/errors (from $GOPATH)

This non-compilation behaviour is expected, the dependency has been removed. Now I want to add it back:

root@3650b93883fb:/go/src/vendtest# dep ensure
root@fccf383a79f9:/go/src/vendtest# go run main.go
main.go:3:8: cannot find package "github.com/pkg/errors" in any of:
        /go/src/vendtest/vendor/github.com/pkg/errors (vendor tree)
        /usr/local/go/src/github.com/pkg/errors (from $GOROOT)
        /go/src/github.com/pkg/errors (from $GOPATH)
root@fccf383a79f9:/go/src/vendtest# find .
.
./Gopkg.lock
./main.go
./Gopkg.toml
./vendor
./vendor/github.com

After running dep ensure the dependency wasn't added! I don't expect this. I expected the dependency to be added to ./vendor/github.com/pkg/errors.

But if I have an empty vendor directory, it works fine:

root@fccf383a79f9:/go/src/vendtest# rm -rf vendor/github.com
root@fccf383a79f9:/go/src/vendtest# dep ensure
root@fccf383a79f9:/go/src/vendtest# go run main.go
(0x4e5140,0xc42005c080)

What did you expect to see?

I expect dep to add missing dependencies to the vendor folder, even if the vendor folder is not empty. If this is not possible, I'd then expect some warning from dep ensure or dep status, but there are no errors or warnings.

What did you see instead?

I saw no errors or warnings, and I saw the vendor folder didn't get populated with the dependency.

Note, I've searched the issues and couldn't find an existing case, sorry if this a dupe or won't fix. There's been another case of this discussed in #vendor in Gophers Slack https://gophers.slack.com/archives/C0M5YP9LN/p1500757752167225. I was able to reproduce this back when we switched from json to toml file formats, so I believe it's been occurring for a while (but I only checked this briefly, I may have been wrong).

Thanks all, I've been using dep successfully for some time now, so cheers for the good work.

[sdboyer]

Yep, thanks for the detailed report. I'm happy to treat this as the canonical report for the problem, being the most detailed one I've seen yet.

The basic issue here comes back to the flow between states:

Each of the two steps can be thought of as functions, with knowable inputs. The function themselves are relatively expensive, but their inputs are generally stable, to the point where we can effectively precompute whether it is necessary to do all the work.

For the former case, that produces the lock, this is the role of the inputs-digest value in Gopkg.lock - it is the SHA256 hash digest of all the inputs to the solving process. (You can see the actual values for your project by running the hidden dep hash-inputs command, as the PR template advises). If the value in Gopkg.lock matches what's computed from the project's imports and Gopkg.toml, then we know they're in sync, and can skip the solving process.

This issue is about the latter case - whether the Gopkg.lock is in sync with the vendor/ directory. We're not good at this right now. Much of that is because of #121 - we simply don't have a method for doing fast verification right now. (Fortunately, @karrick has worked up a hashing implementation that is the first step to addressing this). That means we can't really tell if vendor/ is in sync with Gopkg.lock or not.

Instead, we've thus far been relying on the idea that people aren't manually modifying vendor/, given that dep WILL blow it away at times. But, the safest thing to do, prior to having verification, is just to always write vendor/ on dep ensure. Wasteful, but safe. That change is made as part of #489, which is the significant refactor of dep ensure's overall behaviors.

[dominikschulz]

I ran into this issue as well today, only because the README of dep suggest to manipulate vendor/ directly: https://github.com/golang/dep#testing-changes-to-a-dependency.

If dep can't handle this case (at the moment), it may be a good idea to not mention this workflow in the README (or make it clearer in case I got it wrong).

[sdboyer]

eek, yeah. that's an oversight, given the current blind spot we have on this 😢

[bradleyfalzon]

Thanks @sdboyer, that does give context, and I see #489 is running with VendorAlways more often.

What about in the meantime, we adopt similar behaviour like below, or in the proposed PR:

@@ -151,14 +149,8 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
                return errors.Wrap(err, "ensure Solve()")
        }

-       // check if vendor exists, because if the locks are the same but
-       // vendor does not exist we should write vendor
-       vendorExists, err := fs.IsNonEmptyDir(filepath.Join(p.AbsRoot, "vendor"))
-       if err != nil {
-               return errors.Wrap(err, "ensure vendor is a directory")
-       }
        writeV := dep.VendorOnChanged
-       if !vendorExists && solution != nil {
+       if solution != nil {
                writeV = dep.VendorAlways
        }

[sdboyer]

@bradleyfalzon yeah, that looks like a reasonable stopgap.