fix: check free disk space and allow to specify a different download/extraction directory #23
Conversation
Co-authored-by: Per Tillisch <accounts@perglass.com>
per1234
dismissed
their stale review
Requested changes have been made. Thanks!
Co-authored-by: Luca Rinaldi <lucarin@protonmail.com>
Co-authored-by: Luca Rinaldi <lucarin@protonmail.com>
| Run: func(cmd *cobra.Command, args []string) { | ||
| checkDriversInstalled() | ||
| runFlashCommand(cmd.Context(), args, forceYes) | ||
| runFlashCommand(cmd.Context(), args, forceYes, tempDir) |
There was a problem hiding this comment.
I would like to handle the tmpDir definition directly here. You can check if tempDir is set, and if not, you can use the .cache folder as the default one
| const DownloadDiskSpace = uint64(12) | ||
| const ExtractDiskSpace = uint64(10) | ||
|
|
||
| func Flash(ctx context.Context, imagePath *paths.Path, version string, forceYes bool, tempDir string) error { |
There was a problem hiding this comment.
I would expect that the flash function will just flash and not download the image, Can you move the
if !imagePath.Exist() Inside the function runFlashCommand
There was a problem hiding this comment.
The reason behind this is that
defer func() { _ = temp.RemoveAll() }()does not seem to work properly if executed directly into runFlashCommand. There were many cases in which the files were not correctly removed. I can either rename this Flash function or do some more refactoring, but I would keep the runFlashCommand as a wrapper that just handles feedback to the user.
| const DownloadDiskSpace = uint64(12) | ||
| const ExtractDiskSpace = uint64(10) |
There was a problem hiding this comment.
I would keep two values. One for only the estimated archive size and another for the extract size
| const DownloadDiskSpace = uint64(12) | |
| const ExtractDiskSpace = uint64(10) | |
| const EstimatedDownloadSize = uint64(2) * GiB | |
| const EstimateExtractionSize = uint64(10) * GiB |
There was a problem hiding this comment.
When the image is not local, it is always downloaded and extracted. I don't think there is a need to keep them as separated values, since we would need the sum of the two anyway
3 participants
Motivation
The default download/extraction directory might be on a disk with insufficient space.
Change description
Two changes have been introduced:
--temp-dirflag;Additional Notes
Reviewer checklist
main.