On 04/15/16 at 09:54pm, Allan McRae wrote:
We can end up with .part files downloaded that are unable to be used by pacman in two ways: 1) a well timed ctrl+c right at the rename of the download file can leave a complete download with a .part extension, or 2) for some reason (bad mirror) a .part file can be larger than the file to be downloaded. In both cases, pacman fails with a very cryptic error message.
When these files are encountered by libalpm, they are now either renamed to remove the if .part suffix if the download is complete, or deleted if the download file is too large.
Fixes FS#35789 and FS#39257.
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/sync.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 00b68d0..95e34f8 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -337,7 +337,27 @@ static int compute_download_size(alpm_pkg_t *newpkg) /* subtract the size of the .part file */ _alpm_log(handle, ALPM_LOG_DEBUG, "using (package - .part) size\n"); size = newpkg->size - st.st_size; - size = size < 0 ? 0 : size; + if(size < 0) { + /* we have a bad .part file - delete it */ + _alpm_log(handle, ALPM_LOG_DEBUG, "deleting invalid .part file\n"); + unlink(fpath); + size = newpkg->size;
This is true, but only because our download code is broken elsewhere. Even though we count them here, .part files in any non-primary cache directories are completely ignored. We only ever download to our first writable cache even if there are partial downloads in a later cache.
+ goto finish; + } else if (size == 0) { + /* the .part file is complete, but needs renamed */ + char *newpath; + size_t fpathlen = strlen(fpath); + + _alpm_log(handle, ALPM_LOG_DEBUG, "renaming complete .part file\n"); + + CALLOC(newpath, fpathlen + 1, sizeof(char), return -1); + strcpy(newpath, fpath); + newpath[fpathlen - 5] = '\0'; + rename(fpath, newpath); + FREE(newpath); + + goto finish; + }
unlink and rename both need to be checked for success, otherwise the user can still get the cryptic error message. I don't really like having this in this function. This is a query function that's used any time alpm_pkg_download_size is called. I don't like having query functions make, or at least try to make, modifications to the filesystem.
}
/* tell the caller that we have a partial */ -- 2.7.4