Go: cmd/go: the vendor/ folder should be kept up to date if present

[FiloSottile]

Using go mod vendor is great for backwards compatibility (and for self-contained builds), but it requires re-running every time any dependency changes. Multiple times now I have committed changes that I tested with GO11MODULES=on, only to have them fail in CI where they rely on vendor/.

The go tool already updates go.mod and go.sum when making builds, I believe it should do the same with the vendor/ folder if it's present, to avoid getting in an inconsistent state.

An out of date vendor/ folder feels like an entirely inconsistent and confusing state to be in, causing builds to unexpectedly fail only for certain users, with no advantages, so the tooling should probably just not let it happen. If a developer doesn't want to keep the vendor/ folder up to date, it should be deleted.

/cc @rsc @bcmills

[FiloSottile]

This by the way feels like a requirement of #27227, but I think that even if using vendor/ is not the default, its presence suggests it's an option, so it shouldn't be broken.

[bcmills]

An out of date vendor/ folder feels like an entirely inconsistent and confusing state to be in, causing builds to unexpectedly fail only for certain users, with no advantages,

Conversely, one of the most consistent use-cases I have heard for the vendor directory is as a place to experiment with patches to dependencies.

We clearly do need to be able to verify the consistency of the vendor directory (see also #27348), especially as a pre-commit hook, and go release should ensure that vendor is up-to-date (https://github.com/golang/go/issues/26420#issuecomment-413298674), but I don't think we can reasonably update it without any intervention at all.

[FiloSottile]

I feel like the fact that vendor/ is used to experiment with patches to dependencies makes it even worse for it to be inconsistent: a dev will be running builds in modules mode (or this issue does not apply) with patches in vendor/, and they will silently not work, while they did for someone else who was running in GOPATH mode. At least if they get rewritten it will be clear what's happening.

I agree that existing packages should not be modified when using -mod=vendor, because then they are authoritative in themselves. (But new ones should be added as needed.) If vendor is being used to experiment with patches to dependencies, -mod=vendor should (and probably will) be used anyway.

(I am very skeptical of the adoption of pre-commit hooks.)

[bcmills]

If vendor is being used to experiment with patches to dependencies, -mod=vendor should (and probably will) be used anyway.

Ideally I would like the -mod=vendor flag to go away entirely. (The vendor directory should behave more like a set of implicit replace statements than a separate build mode.)

[FiloSottile]

(The vendor directory should behave more like a set of implicit replace statements than a separate build mode.)

+1 on that. (Which I guess is a solution to #27227?) In that case it's even more natural to update vendor/ to reflect the build, because all existing packages stay as they are, and missing ones are filled in, providing backwards compatibility.

[bcmills]

Given #27227, it's not clear to me when we would update the vendor directory at all. Perhaps during go get if the previous contents were still clean?

That seems pretty subtle, though.

[FiloSottile]

Is the use case of vendor/ as backwards compatibility for GOPATH mode not a supported goal?

Also, a lot of the community support for vendor/ was not as implicit replaces but to guarantee self-contained builds, and if it's not kept up to date that goal is defeated.

Personally I never liked using vendor/ patches as it's subtle and implicit, and I don't think it's something we should be encouraging.

[nomad-software]

Ideally I would like the -mod=vendor flag to go away entirely. (The vendor directory should behave more like a set of implicit replace statements than a separate build mode.)

@bcmills I completely disagree with this and think this is a really bad idea. This may be ok for Google's workflow but would break lots of other company's.

The vendor folder is currently the only way to guarantee reproducible builds on all machines even if offline. Also, if you remove it, how are you going to tackle the problem of remote dependencies disappearing from the internet? These problems need addressing before any talk on removing the vendor folder.

[bcmills]

@nomad-software I said I want to remove the flag, not the vendor directory. If we use vendor by default, then we don't need an explicit flag to enable it.

[nomad-software]

@bcmills Ok, I misunderstood but that still raises the question how do we build against the download cache _or_ the vendor folder? That flag is really handy. You shouldn't just remove it without a really good reason or an alternative to achieve the functionality of what we enjoy now.

[aarzilli]

Given #27227, it's not clear to me when we would update the vendor directory at all. Perhaps during go get if the previous contents were still clean?

Older dependency management programs (govendor, go dep, glide, etc) all overwrote the vendor directory unconditionally. This didn't prevent people from using it to experiment with changes to dependencies. Modules are a bit different because dependencies are updated implicitly as part of the build process, however I think that this simple rule: "if go.mod is changed run go mod vendor", would satisfy everyone. It would keep the vendor directory up to date with go.mod and let people make changes inside vendor.

[bcmills]

I've been looking into the details for #27227, and I tend to agree that it will make this proposal necessary as well.

[bcmills]

33848 explicitly checks for consistency.

For the reasons described in https://github.com/golang/go/issues/30240#issuecomment-524983085, we should not update the vendor directory automatically.

[bcmills]

Closing in favor of #33848.