-
Notifications
You must be signed in to change notification settings - Fork 1k
Dep should error if toml/lock contains current package. #1690
Description
I think dep should error (and fail hard) on jobs if the toml and/or lock files have the current package listed.
It refuses to add the current package with dep ensure -add <pkg> throwing cannot add current project to itself, but if it's already there for some reason, terrible things can happen.
For example, I was recently bitten by this badly. Here's the rough breakdown:
- Working in package
gihub.com/fooPreviously v0.1.0, has a file with a constantBAR="123" - For whatever reason*, Gopkg.toml and Gopkg.lock included references to github.com/foo@0.1.0
- Upgrade to v0.2.0, incluing changing the constant to `BAR="ABC"
- When our build system runs
bin/dep ensure -v -vendor-onlyversion0.1.0andBAR=123gets checked out into the /vendor folder. - Finally, on go build, the vendor directory overrides the local file, and the built binary contains
123instead ofABC(confirmed from both running/failing and from grepping the file.)
This is a pretty subtle and disastrous bug: not only does it mean that /vendor takes precedence over top-level files (which was unexpected to me), it means that you end up with scary mixed-state bugs, where a new feature of the binary exists (so it appears to be the new code), but where some constants will be set to older values, causing unexpected failures and broken functionailty.
I can't think of any situation where you'd want to vendor yourself - and this is supported by the fact that dep ensure -add doesn't let you do it. My proposal is that dep should check for this possibility on normal dep ensure runs, and fail hard, alerting the user of the issue.
But I'd welcome any solution that would have helped me prevent this issue, rather than wasting a couple of hours trying to figure out why my application was trying to read from the wrong database table (the real use for BAR, which had been changed months ago.
Thoughts?
*Why were these values in the toml/lock files, you may ask? I suspect it was from a bad copy/paste when it was initially created and/or migrated to dep. But either way, I think dep should help protect users from shooting themselves in the foot like this.