Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
daemon,overlord,snap,tests: download to .partial in final dir #2237
Conversation
mvo5
added some commits
Oct 31, 2016
| deltaInfo := &downloadInfo.Deltas[0] | ||
| - workingDir, err := ioutil.TempDir(dirs.SnapPartialBlobDir, "deltas-"+name) | ||
| + // use /var/tmp here because it is not on a tmpfs | ||
| + workingDir, err := ioutil.TempDir("/var/tmp", "deltas-"+name) |
niemeyer
Nov 1, 2016
Contributor
Indeed. It'll also be better to not even create this directory. Using the same logic as above, we can use targetFn + ".delta.partial" in the final directory. If this ends up being a bit confusing, let's please just nuke the whole delta logic. This is experimental anyway and we can easily reintroduce on the next release.
| @@ -32,8 +32,8 @@ type Flags struct { | ||
| // Revert flags the SnapSetup as coming from a revert | ||
| Revert bool `json:"revert,omitempty"` | ||
| - // KeepSnapPath is used via InstallPath to flag that the file passed in is not temporary and should not be removed | ||
| - KeepSnapPath bool `json:"keep-snap-path,omitempty"` | ||
| + // RemoveSnapPath is used via InstallPath to flag that the file passed in is not temporary and should not be removed |
| + | ||
| + if downloadInfo.Sha3_384 != "" && sha3_384 != downloadInfo.Sha3_384 { | ||
| + // FIXME: find a better error message | ||
| + return fmt.Errorf("hashsum mismatch for %s: got %s but expected %s", w.Name(), sha3_384, downloadInfo.Sha3_384) |
niemeyer
Nov 1, 2016
Contributor
("sha3-384 mismatch downloading %s: got %s but expected %s", w.Name(), hash, downloadInfo.Sha3_384)
Note that this is different from the suggestion from the other PR, as here we can use the actual filename since it'll look nice (yay!).
| } | ||
| // 3 pₙ₊₁ ≥ 5 pₙ; last entry should be 0 -- the sleep is done at the end of the loop | ||
| var downloadBackoffs = []int{113, 191, 331, 557, 929, 0} | ||
| // download writes an http.Request showing a progress.Meter | ||
| -var download = func(name, downloadURL string, user *auth.UserState, s *Store, w io.Writer, pbar progress.Meter) error { | ||
| +var download = func(name, downloadURL string, user *auth.UserState, s *Store, w io.Writer, pbar progress.Meter) (sha3_384 string, err error) { |
niemeyer
Nov 1, 2016
Contributor
Same as the comment in the other PR, can we please revert the logic so we return the error above from within download, and pass the sha3-384 into download so it can do the comparison internally and report the error consistently for all cases.
mvo5
Nov 1, 2016
Collaborator
I moved the hash check into download(), the only small downside is that we can no longer show the real filename because we are only passing a io.Writer
| deltaInfo := &downloadInfo.Deltas[0] | ||
| - workingDir, err := ioutil.TempDir(dirs.SnapPartialBlobDir, "deltas-"+name) | ||
| + // use /var/tmp here because it is not on a tmpfs | ||
| + workingDir, err := ioutil.TempDir("/var/tmp", "deltas-"+name) |
niemeyer
Nov 1, 2016
Contributor
Indeed. It'll also be better to not even create this directory. Using the same logic as above, we can use targetFn + ".delta.partial" in the final directory. If this ends up being a bit confusing, let's please just nuke the whole delta logic. This is experimental anyway and we can easily reintroduce on the next release.
niemeyer
added
the
Critical
label
Nov 1, 2016
This was referenced Nov 1, 2016
mvo5
added some commits
Nov 1, 2016
niemeyer
changed the title from
bugfix: use a .partial file in download to unbreak `snap download` as user
to
daemon,overlord,snap,tests: download to .partial in final dir
Nov 1, 2016
|
2016/11/02 01:06:54 Successful tasks: 273 |
|
Travis was broken tonight, so I ran: |
mvo5 commentedNov 1, 2016
This branch makes
snap downloadandsnap prepare-imagework as a user again. While the goal is similar to #2205 this approach uses a "$final.partial" approach instead of going into a "partial" directory.The branch is relatively big, mostly because:
a) we reverse the way download works, before Download() would tells us where stuff got download, now we tell Download() where to download to
b) the downloaded files end up in the final place, so we don't need to copy/rename the downloaded files.
Slightly risky because of this. #2205 might be safer.