[pacman-dev] [PATCH 1/3] make compute_download_size consider .part files
Check for the existance of a partial download of a package file before jumping to delta calculations. Currently, if there were 10MiB remaining in a 100MiB the values passed to the front end do not reflect this. The frontend accounts for this change in the verbose target display by using alpm_pkg_download_size() instead of alpm_pkg_get_size(). We also change the label in the final column from a vague "Size" to a more descriptive "Download Size" Refactored from an old patch originally by Dan. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/sync.c | 29 ++++++++++++++++++++++++----- src/pacman/util.c | 4 ++-- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index e2562c0..471bbe7 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -284,7 +284,8 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t *dbs, */ static int compute_download_size(alpm_pkg_t *newpkg) { - char *fpath; + const char *fname; + char *fpath, *fnamepart = NULL; off_t size = 0; alpm_handle_t *handle = newpkg->handle; @@ -295,11 +296,26 @@ static int compute_download_size(alpm_pkg_t *newpkg) } ASSERT(newpkg->filename != NULL, RET_ERR(handle, ALPM_ERR_PKG_INVALID_NAME, -1)); - fpath = _alpm_filecache_find(handle, newpkg->filename); + fname = newpkg->filename; + fpath = _alpm_filecache_find(handle, fname); + /* downloaded file exists, so there's nothing to grab */ if(fpath) { - FREE(fpath); size = 0; + goto finish; + } + + CALLOC(fnamepart, strlen(fname) + 6, sizeof(char), return -1); + sprintf(fnamepart, "%s.part", fname); + fpath = _alpm_filecache_find(handle, fnamepart); + if(fpath) { + struct stat st; + if(stat(fpath, &st) == 0) { + /* 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; + } } else if(handle->usedelta) { off_t dltsize; @@ -315,15 +331,18 @@ static int compute_download_size(alpm_pkg_t *newpkg) alpm_list_free(newpkg->delta_path); newpkg->delta_path = NULL; } - } else { - size = newpkg->size; } +finish: _alpm_log(handle, ALPM_LOG_DEBUG, "setting download size %jd for pkg %s\n", (intmax_t)size, newpkg->name); newpkg->infolevel |= INFRQ_DSIZE; newpkg->download_size = size; + + FREE(fpath); + FREE(fnamepart); + return 0; } diff --git a/src/pacman/util.c b/src/pacman/util.c index 594186f..6963afb 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -748,7 +748,7 @@ static alpm_list_t *create_verbose_header(int install) pm_asprintf(&str, "%s", _("New Version")); res = alpm_list_add(res, str); } - pm_asprintf(&str, "%s", _("Size")); + pm_asprintf(&str, "%s", _("Download Size")); res = alpm_list_add(res, str); return res; @@ -779,7 +779,7 @@ static alpm_list_t *create_verbose_row(alpm_pkg_t *pkg, int install) ret = alpm_list_add(ret, str); /* and size */ - size = humanize_size(alpm_pkg_get_size(pkg), 'M', &label); + size = humanize_size(alpm_pkg_download_size(pkg), 'M', &label); pm_asprintf(&str, "%.2f %s", size, label); ret = alpm_list_add(ret, str); -- 1.7.6.1
Similar to an earlier commit which accounts for .part files for full packages, calculate the download_size for deltas keeping mind the possibility of a partial transfer. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/delta.c | 25 ++++++++++++++++++++----- 1 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c index 1ff4fde..da957bc 100644 --- a/lib/libalpm/delta.c +++ b/lib/libalpm/delta.c @@ -89,12 +89,27 @@ static void graph_init_size(alpm_handle_t *handle, alpm_list_t *vertices) /* determine whether the delta file already exists */ fpath = _alpm_filecache_find(handle, vdelta->delta); - md5sum = alpm_compute_md5sum(fpath); - if(fpath && md5sum && strcmp(md5sum, vdelta->delta_md5) == 0) { - vdelta->download_size = 0; + if(fpath) { + md5sum = alpm_compute_md5sum(fpath); + if(md5sum && strcmp(md5sum, vdelta->delta_md5) == 0) { + vdelta->download_size = 0; + FREE(md5sum); + } + FREE(fpath); + } else { + char *fnamepart; + CALLOC(fnamepart, strlen(vdelta->delta) + 6, sizeof(char), return); + sprintf(fnamepart, "%s.part", vdelta->delta); + fpath = _alpm_filecache_find(handle, fnamepart); + if(fpath) { + struct stat st; + if(stat(fpath, &st) == 0) { + vdelta->download_size = vdelta->delta_size - st.st_size; + } + FREE(fpath); + } + FREE(fnamepart); } - FREE(fpath); - FREE(md5sum); /* determine whether a base 'from' file exists */ fpath = _alpm_filecache_find(handle, vdelta->from); -- 1.7.6.1
On Wed, Sep 7, 2011 at 6:19 PM, Dave Reisner <d@falconindy.com> wrote:
Similar to an earlier commit which accounts for .part files for full packages, calculate the download_size for deltas keeping mind the possibility of a partial transfer.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/delta.c | 25 ++++++++++++++++++++----- 1 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c index 1ff4fde..da957bc 100644 --- a/lib/libalpm/delta.c +++ b/lib/libalpm/delta.c @@ -89,12 +89,27 @@ static void graph_init_size(alpm_handle_t *handle, alpm_list_t *vertices)
/* determine whether the delta file already exists */ fpath = _alpm_filecache_find(handle, vdelta->delta); - md5sum = alpm_compute_md5sum(fpath); - if(fpath && md5sum && strcmp(md5sum, vdelta->delta_md5) == 0) { - vdelta->download_size = 0; + if(fpath) { + md5sum = alpm_compute_md5sum(fpath); + if(md5sum && strcmp(md5sum, vdelta->delta_md5) == 0) { + vdelta->download_size = 0; + FREE(md5sum); + } strcmp() fails, md5sum leaks. + FREE(fpath); + } else { + char *fnamepart; + CALLOC(fnamepart, strlen(vdelta->delta) + 6, sizeof(char), return); + sprintf(fnamepart, "%s.part", vdelta->delta); + fpath = _alpm_filecache_find(handle, fnamepart); + if(fpath) { + struct stat st; + if(stat(fpath, &st) == 0) { + vdelta->download_size = vdelta->delta_size - st.st_size; You were careful to keep it > 0 in the other patch; no need to do so here? + } + FREE(fpath); + } + FREE(fnamepart); } - FREE(fpath); - FREE(md5sum);
/* determine whether a base 'from' file exists */ fpath = _alpm_filecache_find(handle, vdelta->from); -- 1.7.6.1
On Wed, Sep 7, 2011 at 6:19 PM, Dave Reisner <d@falconindy.com> wrote:
Check for the existance of a partial download of a package file before jumping to delta calculations. Currently, if there were 10MiB remaining in a 100MiB the values passed to the front end do not reflect this.
The frontend accounts for this change in the verbose target display by using alpm_pkg_download_size() instead of alpm_pkg_get_size(). We also change the label in the final column from a vague "Size" to a more descriptive "Download Size" No can do, or at least not in this same patch, and not this simply. Give it a spin with a -R or -U operation and you've made the column completely worthless. It should be a separate patch anyway.
Refactored from an old patch originally by Dan.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/sync.c | 29 ++++++++++++++++++++++++----- src/pacman/util.c | 4 ++-- 2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index e2562c0..471bbe7 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -284,7 +284,8 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t *dbs, */ static int compute_download_size(alpm_pkg_t *newpkg) { - char *fpath; + const char *fname; + char *fpath, *fnamepart = NULL; off_t size = 0; alpm_handle_t *handle = newpkg->handle;
@@ -295,11 +296,26 @@ static int compute_download_size(alpm_pkg_t *newpkg) }
ASSERT(newpkg->filename != NULL, RET_ERR(handle, ALPM_ERR_PKG_INVALID_NAME, -1)); - fpath = _alpm_filecache_find(handle, newpkg->filename); + fname = newpkg->filename; + fpath = _alpm_filecache_find(handle, fname);
+ /* downloaded file exists, so there's nothing to grab */ if(fpath) { - FREE(fpath); size = 0; + goto finish; + } + + CALLOC(fnamepart, strlen(fname) + 6, sizeof(char), return -1); + sprintf(fnamepart, "%s.part", fname); + fpath = _alpm_filecache_find(handle, fnamepart); + if(fpath) { + struct stat st; + if(stat(fpath, &st) == 0) { + /* 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; + } stat() shouldn't fail, but if it does, you've left size set to 0. This seems wrong. If you bump the struct stat def up to the beginning of the function, you could just run if(fpath && stat(xxx) == 0) { all in the same conditional.
} else if(handle->usedelta) { off_t dltsize;
@@ -315,15 +331,18 @@ static int compute_download_size(alpm_pkg_t *newpkg) alpm_list_free(newpkg->delta_path); newpkg->delta_path = NULL; } - } else { - size = newpkg->size; I'm confused- where do we now set the size to the full compressed size if nothing was found? Not sure why this disappears. }
+finish: _alpm_log(handle, ALPM_LOG_DEBUG, "setting download size %jd for pkg %s\n", (intmax_t)size, newpkg->name);
newpkg->infolevel |= INFRQ_DSIZE; newpkg->download_size = size; + + FREE(fpath); + FREE(fnamepart); + return 0; }
Everything below == different patch, as stated above.
diff --git a/src/pacman/util.c b/src/pacman/util.c index 594186f..6963afb 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -748,7 +748,7 @@ static alpm_list_t *create_verbose_header(int install) pm_asprintf(&str, "%s", _("New Version")); res = alpm_list_add(res, str); } - pm_asprintf(&str, "%s", _("Size")); + pm_asprintf(&str, "%s", _("Download Size")); res = alpm_list_add(res, str);
return res; @@ -779,7 +779,7 @@ static alpm_list_t *create_verbose_row(alpm_pkg_t *pkg, int install) ret = alpm_list_add(ret, str);
/* and size */ - size = humanize_size(alpm_pkg_get_size(pkg), 'M', &label); + size = humanize_size(alpm_pkg_download_size(pkg), 'M', &label); pm_asprintf(&str, "%.2f %s", size, label); ret = alpm_list_add(ret, str);
-- 1.7.6.1
participants (2)
-
Dan McGee
-
Dave Reisner