golang / dep

Go dependency management tool experiment (deprecated)
https://golang.github.io/dep/
BSD 3-Clause "New" or "Revised" License
12.85k stars 1.05k forks source link

enabled persistent source caching by default #2163

Closed jmank88 closed 4 years ago

jmank88 commented 5 years ago

What does this do / why do we need it?

Enables persistent source caching by default (24h). Documents environment variables in the help/usage text.

Dep is a tool for managing dependencies for Go projects

Usage: "dep [command]"

Commands:
  init     Set up a new Go project, or migrate an existing one
  status   Report the status of the project's dependencies
  ensure   Ensure a dependency is safely vendored in the project
  version  Show the dep version information
  check    Check if imports, Gopkg.toml, and Gopkg.lock are in sync

Examples:
  dep init                               set up a new project
  dep ensure                             install the project's dependencies
  dep ensure -update                     update the locked versions of all dependencies
  dep ensure -add github.com/pkg/errors  add a dependency to the project

+Environment Variables:
+  DEPCACHEAGE     Maximum age of cached source metadata to use. Default 24h. Set <= 0 to disable.
+  DEPCACHEDIR     Directory for local cache. Defaults to $GOPATH/pkg/dep.
+  DEPPROJECTROOT  Override the GOPATH inferred project root.
+  DEPNOLOCK       Bypass local cache lock file creation.
+
Use "dep help [command]" for more information about a command.

What should your reviewer look out for in this PR?

Is 24h an appropriate default value? Are the env var descriptions clear?

jmank88 commented 5 years ago

I do wonder about what happens when a new version comes out and they have run dep recently though. Do I understand it right that dep won't fetch the newer version (because the version list is cached) and they would need to tweak the DEPCACHEAGE to get latest in this scenario?

If they want to implicitly fetch a newer version, yes. The version list is potentially as stale as the cache age (as are version<->revision mappings). However, if they explicitly set a newer version that is not in the cache, then it will still be looked for upstream, so the whole world view isn't stale.

With this in mind, do you think that a potentially much shorter cache age would be more appropriate (e.g. 5m)? Or should some of these things never be cached, for any length of time?

Something related to consider would be a way to disable reading from the cache, but to still write any new information to the cache (setting <= 0 currently bypasses it completely). Something like a -nocache flag could trigger this behavior, and then it's usage documentation would be a highly visible disclaimer about the potential foot gun it is intended to work around.

kevinburke commented 5 years ago

I'm not going to have time to review/merge this until tomorrow night at earliest, if you feel confident in it and can merge please go ahead!

kevinburke commented 5 years ago

Tests failed because Launchpad was down, I think. I am going to try to close and reopen the PR to trigger the build.

kevinburke commented 5 years ago

I'm curious about how the implicit fetching works though. My understanding is that if you have eg master as the version specifier in your Gopkg.toml and a commit has been chosen in Gopkg.lock we won't go fetch a new version when you run dep ensure. Maybe I don't have a good understanding of the scenario by which you'd expect to get a new version and fail?

I guess if you ran dep ensure -update github.com/foo/bar@master, it would seem like users would expect us to go over the network in that case.

If you are checking out a new project and doing dep init you could get a cached commit if a dependency had been cached by a different project on the same machine I think. Which... seems OK.

jmank88 commented 5 years ago

I guess if you ran dep ensure -update github.com/foo/bar@master, it would seem like users would expect us to go over the network in that case.

If you are checking out a new project and doing dep init you could get a cached commit if a dependency had been cached by a different project on the same machine I think. Which... seems OK.

Yes, one surprising case is when the user knows a new version has been released, and expects it to be found without specifying it explicitly - this isn't particularly difficult to work around, even without understanding the cache, since they know exactly what they want. I wonder if a more diabolical case is possible, where the solver would not be able to find a solution due to the cached version list being stale - this would be more difficult to work around, since it may not be immediately clear that the cache is the problem.

In either case, some sort of -nocache/-skipcache flag could serve as an easy workaround, but the user would have to know about it. We could also consider logging about the cache age as a disclaimer/warning, particularly for a 'no solution' case or even just in general when the cache is enabled.

mvdan commented 4 years ago

Dep was officially deprecated earlier this year, and the proposal to archive this repository was accepted. As such, I'm closing outstanding issues before archiving the repository. For any further comments, please use the proposal thread on the Go issue tracker. Thanks!