Use dvm tmp for docker downloads #189
Conversation
|
Since more options are coming in here, perhaps there should be a front facing |
|
Yeah good call. The args list is getting unweildly. Also Docker broke how binaries are staged ... again... Just found that out when I reran the regression tests. They removed the files from test.docker.com and put up a download script there instead. Gonna see how small of a fix that I can do to keep things working for most people without doing massive surgery. So just the struct and a bit of cleanup, since we don't need to be running builds on go 1.7 anymore. 😀 (In separate PRs) |
carolynvs
force-pushed
the
use-dvm-tmp
branch
from
816b844 to
44268ed
Compare
Oh joy.
Ping me if you want me to review anything else! |
carolynvs
force-pushed
the
use-dvm-tmp
branch
from
44268ed to
87d23f1
Compare
carolynvs
changed the title
|
|
||
| func NewDvmOptions() DvmOptions { | ||
| return DvmOptions{ | ||
| Logger: log.New(ioutil.Discard, "", log.LstdFlags), |
There was a problem hiding this comment.
Should we set other defaults for the MirrorURL and others here too? Or just rely on "" being the default string and false being the default for bool?
There was a problem hiding this comment.
I don't want to go changing too much at once, especially stuff that is unrelated to the original bug (doing a file move across disk partitions).
I'm not terribly confident that I wouldn't cause regressions... 😊
This avoids cross partition move problems
Moved logger into DvmOptions as well since it's so ubiquitous
|
@rgbkrk I've rebased to get it passing. Let me know if this is okay as is or if it needs another pass. 😀 |
| func (version Version) Download(mirrorURL string, binaryPath string, l *log.Logger) error { | ||
| err := version.download(false, mirrorURL, binaryPath, l) | ||
| func (version Version) Download(opts config.DvmOptions, binaryPath string) error { | ||
| err := version.download(false, opts, binaryPath) |
There was a problem hiding this comment.
Something @captainsafia pointed out to me is that we should avoid the boolean trap. Since this was there before, I wouldn't bother changing it now. Might be good though, as I don't know if this means it's downloading or not.
Download docker binary to
~/.dvm/.tmpinstead of/tmp/dvm-*so that when we copy it into the dvm home directory afterwards we can always use a rename safely. This avoids this dreaded cross partition move errorinvalid cross-device link.Fixes #188