When vendor dir is part complete, dep ensure doesn't add missing dependencies #883
Comments
If a the vendor directory already exists, and the lock file hasn't changed, even though a project may be missing from the vendor directory, dep ensure would not add it. This change checks if we're not already writing the vendor directory whether any of the projects are missing from the vendor directory and if they are, ensures the writer writes out the vendor directory. Fixes golang#883.
If the vendor directory already exists, and the lock file hasn't changed, even though a project may be missing from the vendor directory, dep ensure would not add it. This change checks if we're not already writing the vendor directory whether any of the projects are missing from the vendor directory and if they are, ensures the writer writes out the vendor directory. Fixes golang#883.
|
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 This issue is about the latter case - whether the Instead, we've thus far been relying on the idea that people aren't manually modifying |
|
I ran into this issue as well today, only because the README of If |
|
eek, yeah. that's an oversight, given the current blind spot we have on this 😢 |
|
Thanks @sdboyer, that does give context, and I see #489 is running with 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
} |
|
@bradleyfalzon yeah, that looks like a reasonable stopgap. |
If the vendor directory already exists, and the lock file hasn't changed, even though a project may be missing from the vendor directory, dep ensure would not add the dependency to the vendor folder. If the project is in the vendor folder, but it has been modified (no longer in sync with lock file), ensure would not replace the dependency in the vendor folder. This change causes dep ensure to run anytime there's a solution, regardless of the state vendor folder is in, erring on the side of replacing vendor without checking existing state which is the most correct behaviour given the ensure intention. Future optimisations may want to check and verify the contents of the vendor folder before blinding replacing it. Fixes golang#883.
If the vendor directory already exists, and the lock file hasn't changed, even though a project may be missing from the vendor directory, dep ensure would not add the dependency to the vendor folder. If the project is in the vendor folder, but it has been modified (no longer in sync with lock file), ensure would not replace the dependency in the vendor folder. This change causes dep ensure to run anytime there's a solution, regardless of the state vendor folder is in, erring on the side of replacing vendor without checking existing state which is the most correct behaviour given the ensure intention. Future optimisations may want to check and verify the contents of the vendor folder before blinding replacing it. Fixes golang#883.
What version of Go (
go version) anddep(git describe --tags) are you using?What
depcommand did you run?So far so good, we've proven the dependency
github.com/pkg/errorsdidn't initially exist, runningdep initthen 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.
This non-compilation behaviour is expected, the dependency has been removed. Now I want to add it back:
After running
dep ensurethe 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:
What did you expect to see?
I expect
depto 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 fromdep ensureordep 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
depsuccessfully for some time now, so cheers for the good work.The text was updated successfully, but these errors were encountered: