1) The changes to sync.c look big but there are mostly caused by
the indentation. Fix a bug where download_size == 0 because the packages and
deltas are already in the cache, but we still need to build the deltas list
and apply the deltas to create the final package.

2) Fix the gzip / md5sum issue by switching to xdelta3, disabling external
recompression and using gzip -n in pacman, and disable bsdtar compression
and using gzip -n in makepkg.

Signed-off-by: Xavier Chantry <shiningxc@gmail.com>
---
doc/pacman.conf.5.txt | 2 +-
lib/libalpm/delta.c | 4 +-
lib/libalpm/sync.c | 98 +++++++++++++++++++++++--------------------------
scripts/makepkg.sh.in | 28 +++-----------
4 files changed, 55 insertions(+), 77 deletions(-)

diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt
index fa21294..2f1fe3d 100644
--- a/doc/pacman.conf.5.txt
+++ b/doc/pacman.conf.5.txt
@@ -144,7 +144,7 @@ Options

*UseDelta*::
Download delta files instead of complete packages if possible. Requires
- the xdelta program to be installed.
+ the xdelta3 program to be installed.

*TotalDownload*::
When downloading, display the amount downloaded, download rate, ETA,
diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c
index ff77501..337eb66 100644
--- a/lib/libalpm/delta.c
+++ b/lib/libalpm/delta.c
@@ -232,13 +232,13 @@ off_t _alpm_shortest_delta_path(alpm_list_t *deltas,
return(bestsize);
}

- _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search
");
+ _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search for '%s'
", to);

vertices = delta_graph_init(deltas);

bestsize = delta_vert(vertices, to, to_md5, &bestpath);

- _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete
");
+ _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete : '%lld'
", (long long)bestsize);

alpm_list_free_inner(vertices, _alpm_graph_free);
alpm_list_free(vertices);
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 709a36d..71d8771 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -725,14 +725,9 @@ static int apply_deltas(pmtrans_t *trans)
CALLOC(to, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, 1));
snprintf(to, len, "%s/%s", cachedir, d->to);

- /* an example of the patch command: (using /cache for cachedir)
- * xdelta patch /path/to/pacman_3.0.0-1_to_3.0.1-1-i686.delta
- * /path/to/pacman-3.0.0-1-i686.pkg.tar.gz
- * /cache/pacman-3.0.1-1-i686.pkg.tar.gz
- */
-
/* build the patch command */
- snprintf(command, PATH_MAX, "xdelta patch %s %s %s", delta, from, to);
+ /* -R for disabling external recompression with gzip, -c for sending to stdout */
+ snprintf(command, PATH_MAX, "xdelta3 -d -q -R -c -s %s %s | gzip -n > %s", from, delta, to);

_alpm_log(PM_LOG_DEBUG, _("command: %s
"), command);

@@ -848,29 +843,31 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)

fname = alpm_pkg_get_filename(spkg);
ASSERT(fname != NULL, RET_ERR(PM_ERR_PKG_INVALID_NAME, -1));
- if(spkg->download_size != 0) {
- alpm_list_t *delta_path = spkg->delta_path;
- if(delta_path) {
- alpm_list_t *dlts = NULL;
-
- for(dlts = delta_path; dlts; dlts = dlts->next) {
- pmdelta_t *d = dlts->data;
-
- if(d->download_size != 0) {
- /* add the delta filename to the download list if
- * it's not in the cache */
- files = alpm_list_add(files, strdup(d->delta));
- }
-
- /* keep a list of the delta files for md5sums */
- deltas = alpm_list_add(deltas, d);
+ alpm_list_t *delta_path = spkg->delta_path;
+ if(delta_path) {
+ /* using deltas */
+ alpm_list_t *dlts = NULL;
+
+ for(dlts = delta_path; dlts; dlts = dlts->next) {
+ pmdelta_t *d = dlts->data;
+
+ if(d->download_size != 0) {
+ /* add the delta filename to the download list if needed */
+ files = alpm_list_add(files, strdup(d->delta));
}

- } else {
- /* not using deltas, so add the file to the download list */
+ /* keep a list of all the delta files for md5sums */
+ deltas = alpm_list_add(deltas, d);
+ }
+
+ } else {
+ /* not using deltas */
+ if(spkg->download_size != 0) {
+ /* add the filename to the download list if needed */
files = alpm_list_add(files, strdup(fname));
}
}
+
}
}

@@ -891,37 +888,34 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
handle->totaldlcb(0);
}

- if(handle->usedelta) {
+ /* if we have deltas to work with */
+ if(handle->usedelta && deltas) {
int ret = 0;
-
- /* only output if there are deltas to work with */
- if(deltas) {
- errors = 0;
- /* Check integrity of deltas */
- EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_START, NULL, NULL);
-
- for(i = deltas; i; i = i->next) {
- pmdelta_t *d = alpm_list_getdata(i);
- const char *filename = alpm_delta_get_filename(d);
- const char *md5sum = alpm_delta_get_md5sum(d);
-
- if(test_md5sum(trans, filename, md5sum) != 0) {
- errors++;
- *data = alpm_list_add(*data, strdup(filename));
- }
- }
- if(errors) {
- pm_errno = PM_ERR_DLT_INVALID;
- goto error;
+ errors = 0;
+ /* Check integrity of deltas */
+ EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_START, NULL, NULL);
+
+ for(i = deltas; i; i = i->next) {
+ pmdelta_t *d = alpm_list_getdata(i);
+ const char *filename = alpm_delta_get_filename(d);
+ const char *md5sum = alpm_delta_get_md5sum(d);
+
+ if(test_md5sum(trans, filename, md5sum) != 0) {
+ errors++;
+ *data = alpm_list_add(*data, strdup(filename));
}
- EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_DONE, NULL, NULL);
+ }
+ if(errors) {
+ pm_errno = PM_ERR_DLT_INVALID;
+ goto error;
+ }
+ EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_DONE, NULL, NULL);

- /* Use the deltas to generate the packages */
- EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_START, NULL, NULL);
- ret = apply_deltas(trans);
- EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_DONE, NULL, NULL);
+ /* Use the deltas to generate the packages */
+ EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_START, NULL, NULL);
+ ret = apply_deltas(trans);
+ EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_DONE, NULL, NULL);

- }
if(ret) {
pm_errno = PM_ERR_DLT_PATCHFAILED;
goto error;
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 52e80d1..9e0f249 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -975,7 +975,7 @@ create_package() {
# the null string rather than itself
shopt -s nullglob

- if ! bsdtar -c${TAR_OPT}f "$pkg_file" $comp_files *; then
+ if ! bsdtar -c${TAR_OPT}f - $comp_files * | gzip -n > "$pkg_file"; then
error "$(gettext "Failed to create package file.")"
exit 1 # TODO: error code
fi
@@ -985,8 +985,8 @@ create_package() {
create_xdelta() {
if [ "$(check_buildenv xdelta)" != "y" ]; then
return
- elif [ ! "$(type -p xdelta)" ]; then
- error "$(gettext "Cannot find the xdelta binary! Is xdelta installed?")"
+ elif [ ! "$(type -p xdelta3)" ]; then
+ error "$(gettext "Cannot find the xdelta3 binary! Is xdelta3 installed?")"
return
fi

@@ -1020,25 +1020,9 @@ create_xdelta() {
local delta_file="$PKGDEST/$pkgname-${latest_version}_to_$pkgver-$pkgrel-$CARCH.delta"
local ret=0

- # xdelta will decompress base_file & pkg_file into TMP_DIR (or /tmp if
- # TMP_DIR is unset) then perform the delta on the resulting tars
- xdelta delta "$base_file" "$pkg_file" "$delta_file" || ret=$?
-
- if [ $ret -eq 0 -o $ret -eq 1 ]; then
- # Generate the final gz using xdelta for compression. xdelta will be our
- # common denominator compression utility between the packager and the
- # users. makepkg and pacman must use the same compression algorithm or
- # the delta generated package may not match, producing md5 checksum
- # errors.
- msg2 "$(gettext "Recreating package tarball from delta to match md5 signatures")"
- msg2 "$(gettext "NOTE: the delta should ONLY be distributed with this tarball")"
- ret=0
- xdelta patch "$delta_file" "$base_file" "$pkg_file" || ret=$?
- if [ $ret -ne 0 ]; then
- error "$(gettext "Could not generate the package from the delta.")"
- exit 1
- fi
- else
+ xdelta3 -s "$base_file" "$pkg_file" "$delta_file" || ret=$?
+
+ if [ $ret -ne 0 ]; then
warning "$(gettext "Delta was not able to be created.")"
fi
else
--
1.6.1.3

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://www.archlinux.org/mailman/listinfo/pacman-dev

On Wed, Feb 25, 2009 at 12:43 PM, Xavier Chantry <shiningxc@gmail.com> wrote:
> 1) The changes to sync.c look big but there are mostly caused by
> the indentation. Fix a bug where download_size == 0 because the packages and
> deltas are already in the cache, but we still need to build the deltas list
> and apply the deltas to create the final package.
>
> 2) Fix the gzip / md5sum issue by switching to xdelta3, disabling external
> recompression and using gzip -n in pacman, and disable bsdtar compression
> and using gzip -n in makepkg.
>
> Signed-off-by: Xavier Chantry <shiningxc@gmail.com>
> ---
> *doc/pacman.conf.5.txt | * *2 +-
> *lib/libalpm/delta.c * | * *4 +-
> *lib/libalpm/sync.c * *| * 98 +++++++++++++++++++++++--------------------------
> *scripts/makepkg.sh.in | * 28 +++-----------
> *4 files changed, 55 insertions(+), 77 deletions(-)
>
> diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt
> index fa21294..2f1fe3d 100644
> --- a/doc/pacman.conf.5.txt
> +++ b/doc/pacman.conf.5.txt
> @@ -144,7 +144,7 @@ Options
>
> **UseDelta*::
> * * * *Download delta files instead of complete packages if possible. *Requires
> - * * * the xdelta program to be installed.
> + * * * the xdelta3 program to be installed.
>
> **TotalDownload*::
> * * * *When downloading, display the amount downloaded, download rate, ETA,
> diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c
> index ff77501..337eb66 100644
> --- a/lib/libalpm/delta.c
> +++ b/lib/libalpm/delta.c
> @@ -232,13 +232,13 @@ off_t _alpm_shortest_delta_path(alpm_list_t *deltas,
> * * * * * * * *return(bestsize);
> * * * *}
>
> - * * * _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search
");
> + * * * _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search for '%s'
", to);
>
> * * * *vertices = delta_graph_init(deltas);
>
> * * * *bestsize = delta_vert(vertices, to, to_md5, &bestpath);
>
> - * * * _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete
");
> + * * * _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete : '%lld'
", (long long)bestsize);
I'd rather do this the way we did it elsewhere, and use the %j
operator (see be_files.c):
... "%jd", (intmax_t)bestsize);

Remember that off_t is signed.

>
> * * * *alpm_list_free_inner(vertices, _alpm_graph_free);
> * * * *alpm_list_free(vertices);
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 709a36d..71d8771 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -725,14 +725,9 @@ static int apply_deltas(pmtrans_t *trans)
> * * * * * * * * * * * *CALLOC(to, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, 1));
> * * * * * * * * * * * *snprintf(to, len, "%s/%s", cachedir, d->to);
>
> - * * * * * * * * * * * /* an example of the patch command: (using /cache for cachedir)
> - * * * * * * * * * * * ** xdelta patch /path/to/pacman_3.0.0-1_to_3.0.1-1-i686.delta
> - * * * * * * * * * * * ** * * * * * * */path/to/pacman-3.0.0-1-i686.pkg.tar.gz * * *
> - * * * * * * * * * * * ** * * * * * * */cache/pacman-3.0.1-1-i686.pkg.tar.gz
> - * * * * * * * * * * * **/
> -
> * * * * * * * * * * * */* build the patch command */
> - * * * * * * * * * * * snprintf(command, PATH_MAX, "xdelta patch %s %s %s", delta, from, to);
> + * * * * * * * * * * * /* -R for disabling external recompression with gzip, -c for sending to stdout */
> + * * * * * * * * * * * snprintf(command, PATH_MAX, "xdelta3 -d -q -R -c -s %s %s | gzip -n > %s", from, delta, to);
>
> * * * * * * * * * * * *_alpm_log(PM_LOG_DEBUG, _("command: %s
"), command);
>
> @@ -848,29 +843,31 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
>
> * * * * * * * * * * * * * * * *fname = alpm_pkg_get_filename(spkg);
> * * * * * * * * * * * * * * * *ASSERT(fname != NULL, RET_ERR(PM_ERR_PKG_INVALID_NAME, -1));
> - * * * * * * * * * * * * * * * if(spkg->download_size != 0) {
> - * * * * * * * * * * * * * * * * * * * alpm_list_t *delta_path = spkg->delta_path;
> - * * * * * * * * * * * * * * * * * * * if(delta_path) {
> - * * * * * * * * * * * * * * * * * * * * * * * alpm_list_t *dlts = NULL;
> -
> - * * * * * * * * * * * * * * * * * * * * * * * for(dlts = delta_path; dlts; dlts = dlts->next) {
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * pmdelta_t *d = dlts->data;
> -
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * if(d->download_size != 0) {
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * /* add the delta filename to the download list if
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * ** it's not in the cache */
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * files = alpm_list_add(files, strdup(d->delta));
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * }
> -
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * /* keep a list of the delta files for md5sums */
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * deltas = alpm_list_add(deltas, d);
> + * * * * * * * * * * * * * * * alpm_list_t *delta_path = spkg->delta_path;
> + * * * * * * * * * * * * * * * if(delta_path) {
> + * * * * * * * * * * * * * * * * * * * /* using deltas */
> + * * * * * * * * * * * * * * * * * * * alpm_list_t *dlts = NULL;
> +
> + * * * * * * * * * * * * * * * * * * * for(dlts = delta_path; dlts; dlts = dlts->next) {
> + * * * * * * * * * * * * * * * * * * * * * * * pmdelta_t *d = dlts->data;
> +
> + * * * * * * * * * * * * * * * * * * * * * * * if(d->download_size != 0) {
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * /* add the delta filename to the download list if needed */
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * files = alpm_list_add(files, strdup(d->delta));
> * * * * * * * * * * * * * * * * * * * * * * * *}
>
> - * * * * * * * * * * * * * * * * * * * } else {
> - * * * * * * * * * * * * * * * * * * * * * * * /* not using deltas, so add the file to the download list */
> + * * * * * * * * * * * * * * * * * * * * * * * /* keep a list of all the delta files for md5sums */
> + * * * * * * * * * * * * * * * * * * * * * * * deltas = alpm_list_add(deltas, d);
> + * * * * * * * * * * * * * * * * * * * }
> +
> + * * * * * * * * * * * * * * * } else {
> + * * * * * * * * * * * * * * * * * * * /* not using deltas */
> + * * * * * * * * * * * * * * * * * * * if(spkg->download_size != 0) {
> + * * * * * * * * * * * * * * * * * * * * * * * /* add the filename to the download list if needed */
> * * * * * * * * * * * * * * * * * * * * * * * *files = alpm_list_add(files, strdup(fname));
> * * * * * * * * * * * * * * * * * * * *}
> * * * * * * * * * * * * * * * *}
> +
> * * * * * * * * * * * *}
> * * * * * * * *}
>
> @@ -891,37 +888,34 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
> * * * * * * * *handle->totaldlcb(0);
> * * * *}
>
> - * * * if(handle->usedelta) {
> + * * * /* if we have deltas to work with */
> + * * * if(handle->usedelta && deltas) {
> * * * * * * * *int ret = 0;
> -
> - * * * * * * * /* only output if there are deltas to work with */
> - * * * * * * * if(deltas) {
> - * * * * * * * * * * * errors = 0;
> - * * * * * * * * * * * /* Check integrity of deltas */
> - * * * * * * * * * * * EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_START, NULL, NULL);
> -
> - * * * * * * * * * * * for(i = deltas; i; i = i->next) {
> - * * * * * * * * * * * * * * * pmdelta_t *d = alpm_list_getdata(i);
> - * * * * * * * * * * * * * * * const char *filename = alpm_delta_get_filename(d);
> - * * * * * * * * * * * * * * * const char *md5sum = alpm_delta_get_md5sum(d);
> -
> - * * * * * * * * * * * * * * * if(test_md5sum(trans, filename, md5sum) != 0) {
> - * * * * * * * * * * * * * * * * * * * errors++;
> - * * * * * * * * * * * * * * * * * * * *data = alpm_list_add(*data, strdup(filename));
> - * * * * * * * * * * * * * * * }
> - * * * * * * * * * * * }
> - * * * * * * * * * * * if(errors) {
> - * * * * * * * * * * * * * * * pm_errno = PM_ERR_DLT_INVALID;
> - * * * * * * * * * * * * * * * goto error;
> + * * * * * * * errors = 0;
> + * * * * * * * /* Check integrity of deltas */
> + * * * * * * * EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_START, NULL, NULL);
> +
> + * * * * * * * for(i = deltas; i; i = i->next) {
> + * * * * * * * * * * * pmdelta_t *d = alpm_list_getdata(i);
> + * * * * * * * * * * * const char *filename = alpm_delta_get_filename(d);
> + * * * * * * * * * * * const char *md5sum = alpm_delta_get_md5sum(d);
> +
> + * * * * * * * * * * * if(test_md5sum(trans, filename, md5sum) != 0) {
> + * * * * * * * * * * * * * * * errors++;
> + * * * * * * * * * * * * * * * *data = alpm_list_add(*data, strdup(filename));
> * * * * * * * * * * * *}
> - * * * * * * * * * * * EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_DONE, NULL, NULL);
> + * * * * * * * }
> + * * * * * * * if(errors) {
> + * * * * * * * * * * * pm_errno = PM_ERR_DLT_INVALID;
> + * * * * * * * * * * * goto error;
> + * * * * * * * }
> + * * * * * * * EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_DONE, NULL, NULL);
>
> - * * * * * * * * * * * /* Use the deltas to generate the packages */
> - * * * * * * * * * * * EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_START, NULL, NULL);
> - * * * * * * * * * * * ret = apply_deltas(trans);
> - * * * * * * * * * * * EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_DONE, NULL, NULL);
> + * * * * * * * /* Use the deltas to generate the packages */
> + * * * * * * * EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_START, NULL, NULL);
> + * * * * * * * ret = apply_deltas(trans);
> + * * * * * * * EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_DONE, NULL, NULL);
>
> - * * * * * * * }
> * * * * * * * *if(ret) {
> * * * * * * * * * * * *pm_errno = PM_ERR_DLT_PATCHFAILED;
> * * * * * * * * * * * *goto error;
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 52e80d1..9e0f249 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -975,7 +975,7 @@ create_package() {
> * * * *# the null string rather than itself
> * * * *shopt -s nullglob
>
> - * * * if ! bsdtar -c${TAR_OPT}f "$pkg_file" $comp_files *; then
> + * * * if ! bsdtar -c${TAR_OPT}f - $comp_files * | gzip -n > "$pkg_file"; then
Doesn't this completely break the whole $TAR_OPT abstraction and no
longer allow bzip2 or lzma packages to be created? You also are going
to double gzip this, no?

I'm fine with having separate tar commands for each compression type-
instead of doing the switch statement to figure out TAR_OPT, just do
it for the entire tar command.

> * * * * * * * *error "$(gettext "Failed to create package file.")"
> * * * * * * * *exit 1 # TODO: error code
> * * * *fi
> @@ -985,8 +985,8 @@ create_package() {
> *create_xdelta() {
> * * * *if [ "$(check_buildenv xdelta)" != "y" ]; then
> * * * * * * * *return
> - * * * elif [ ! "$(type -p xdelta)" ]; then
> - * * * * * * * error "$(gettext "Cannot find the xdelta binary! Is xdelta installed?")"
> + * * * elif [ ! "$(type -p xdelta3)" ]; then
> + * * * * * * * error "$(gettext "Cannot find the xdelta3 binary! Is xdelta3 installed?")"
> * * * * * * * *return
> * * * *fi
>
> @@ -1020,25 +1020,9 @@ create_xdelta() {
> * * * * * * * *local delta_file="$PKGDEST/$pkgname-${latest_version}_to_$pkgver-$pkgrel-$CARCH.delta"
> * * * * * * * *local ret=0
>
> - * * * * * * * # xdelta will decompress base_file & pkg_file into TMP_DIR (or /tmp if
> - * * * * * * * # TMP_DIR is unset) then perform the delta on the resulting tars
> - * * * * * * * xdelta delta "$base_file" "$pkg_file" "$delta_file" || ret=$?
> -
> - * * * * * * * if [ $ret -eq 0 -o $ret -eq 1 ]; then
> - * * * * * * * * * * * # Generate the final gz using xdelta for compression. xdelta will be our
> - * * * * * * * * * * * # common denominator compression utility between the packager and the
> - * * * * * * * * * * * # users. makepkg and pacman must use the same compression algorithm or
> - * * * * * * * * * * * # the delta generated package may not match, producing md5 checksum
> - * * * * * * * * * * * # errors.
> - * * * * * * * * * * * msg2 "$(gettext "Recreating package tarball from delta to match md5 signatures")"
> - * * * * * * * * * * * msg2 "$(gettext "NOTE: the delta should ONLY be distributed with this tarball")"
> - * * * * * * * * * * * ret=0
> - * * * * * * * * * * * xdelta patch "$delta_file" "$base_file" "$pkg_file" || ret=$?
> - * * * * * * * * * * * if [ $ret -ne 0 ]; then
> - * * * * * * * * * * * * * * * error "$(gettext "Could not generate the package from the delta.")"
> - * * * * * * * * * * * * * * * exit 1
> - * * * * * * * * * * * fi
> - * * * * * * * else
> + * * * * * * * xdelta3 -s "$base_file" "$pkg_file" "$delta_file" || ret=$?
> +
> + * * * * * * * if [ $ret -ne 0 ]; then
> * * * * * * * * * * * *warning "$(gettext "Delta was not able to be created.")"
> * * * * * * * *fi
> * * * *else
> --
> 1.6.1.3
_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://www.archlinux.org/mailman/listinfo/pacman-dev

1) The changes to sync.c look big but there are mostly caused by
the indentation. Fix a bug where download_size == 0 because the packages and
deltas are already in the cache, but we still need to build the deltas list
and apply the deltas to create the final package.

2) Fix the gzip / md5sum issue by switching to xdelta3, disabling external
recompression and using gzip -n in pacman, and disable bsdtar compression
and using gzip -n in makepkg.

Signed-off-by: Xavier Chantry <shiningxc@gmail.com>
---
doc/pacman.conf.5.txt | 2 +-
lib/libalpm/delta.c | 5 +-
lib/libalpm/sync.c | 118 ++++++++++++++++++++++++++-----------------------
scripts/makepkg.sh.in | 18 +++++---
4 files changed, 78 insertions(+), 65 deletions(-)

diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt
index fa21294..2f1fe3d 100644
--- a/doc/pacman.conf.5.txt
+++ b/doc/pacman.conf.5.txt
@@ -144,7 +144,7 @@ Options

*UseDelta*::
Download delta files instead of complete packages if possible. Requires
- the xdelta program to be installed.
+ the xdelta3 program to be installed.

*TotalDownload*::
When downloading, display the amount downloaded, download rate, ETA,
diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c
index 9e4bcb4..de5dd60 100644
--- a/lib/libalpm/delta.c
+++ b/lib/libalpm/delta.c
@@ -21,6 +21,7 @@

#include <stdlib.h>
#include <string.h>
+#include <stdint.h> /* intmax_t */
#include <limits.h>
#include <sys/types.h>
#include <regex.h>
@@ -215,13 +216,13 @@ off_t _alpm_shortest_delta_path(alpm_list_t *deltas,
return(bestsize);
}

- _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search
");
+ _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search for '%s'
", to);

vertices = delta_graph_init(deltas);

bestsize = delta_vert(vertices, to, &bestpath);

- _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete
");
+ _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete : '%jd'
", (intmax_t)bestsize);

alpm_list_free_inner(vertices, _alpm_graph_free);
alpm_list_free(vertices);
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index fca96d8..49ea3c2 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -27,6 +27,7 @@
#include <stdio.h>
#include <fcntl.h>
#include <string.h>
+#include <stdint.h> /* intmax_t */
#include <unistd.h>
#include <time.h>
#include <dirent.h>
@@ -387,8 +388,8 @@ static int compute_download_size(pmpkg_t *newpkg)
size = alpm_pkg_get_size(newpkg);
}

- _alpm_log(PM_LOG_DEBUG, "setting download size %lld for pkg %s
",
- (long long)size, alpm_pkg_get_name(newpkg));
+ _alpm_log(PM_LOG_DEBUG, "setting download size %jd for pkg %s
",
+ (intmax_t)size, alpm_pkg_get_name(newpkg));

newpkg->download_size = size;
return(0);
@@ -679,6 +680,12 @@ off_t SYMEXPORT alpm_pkg_download_size(pmpkg_t *newpkg)
return(newpkg->download_size);
}

+static int endswith(char *filename, char *extension)
+{
+ char *s = filename + strlen(filename) - strlen(extension);
+ return (strcmp(s, extension) == 0);
+}
+
/** Applies delta files to create an upgraded package file.
*
* All intermediate files are deleted, leaving only the starting and
@@ -724,16 +731,18 @@ static int apply_deltas(pmtrans_t *trans)
CALLOC(to, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, 1));
snprintf(to, len, "%s/%s", cachedir, d->to);

- /* an example of the patch command: (using /cache for cachedir)
- * xdelta patch /path/to/pacman_3.0.0-1_to_3.0.1-1-i686.delta
- * /path/to/pacman-3.0.0-1-i686.pkg.tar.gz
- * /cache/pacman-3.0.1-1-i686.pkg.tar.gz
- */
-
/* build the patch command */
- snprintf(command, PATH_MAX, "xdelta patch %s %s %s", delta, from, to);
+ /* compression command */
+ char *compress = "cat";
+ if(endswith(to, ".gz")) {
+ compress = "gzip -n";
+ } else if(endswith(to, ".bz2")) {
+ compress = "bzip";
+ }
+ /* -R for disabling external recompression, -c for sending to stdout */
+ snprintf(command, PATH_MAX, "xdelta3 -d -q -R -c -s %s %s | %s > %s", from, delta, compress, to);

- _alpm_log(PM_LOG_DEBUG, _("command: %s
"), command);
+ _alpm_log(PM_LOG_DEBUG, "command: %s
", command);

EVENT(trans, PM_TRANS_EVT_DELTA_PATCH_START, d->to, d->delta);

@@ -847,29 +856,31 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)

fname = alpm_pkg_get_filename(spkg);
ASSERT(fname != NULL, RET_ERR(PM_ERR_PKG_INVALID_NAME, -1));
- if(spkg->download_size != 0) {
- alpm_list_t *delta_path = spkg->delta_path;
- if(delta_path) {
- alpm_list_t *dlts = NULL;
-
- for(dlts = delta_path; dlts; dlts = dlts->next) {
- pmdelta_t *d = dlts->data;
-
- if(d->download_size != 0) {
- /* add the delta filename to the download list if
- * it's not in the cache */
- files = alpm_list_add(files, strdup(d->delta));
- }
-
- /* keep a list of the delta files for md5sums */
- deltas = alpm_list_add(deltas, d);
+ alpm_list_t *delta_path = spkg->delta_path;
+ if(delta_path) {
+ /* using deltas */
+ alpm_list_t *dlts = NULL;
+
+ for(dlts = delta_path; dlts; dlts = dlts->next) {
+ pmdelta_t *d = dlts->data;
+
+ if(d->download_size != 0) {
+ /* add the delta filename to the download list if needed */
+ files = alpm_list_add(files, strdup(d->delta));
}

- } else {
- /* not using deltas, so add the file to the download list */
+ /* keep a list of all the delta files for md5sums */
+ deltas = alpm_list_add(deltas, d);
+ }
+
+ } else {
+ /* not using deltas */
+ if(spkg->download_size != 0) {
+ /* add the filename to the download list if needed */
files = alpm_list_add(files, strdup(fname));
}
}
+
}
}

@@ -890,37 +901,34 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
handle->totaldlcb(0);
}

- if(handle->usedelta) {
+ /* if we have deltas to work with */
+ if(handle->usedelta && deltas) {
int ret = 0;
-
- /* only output if there are deltas to work with */
- if(deltas) {
- errors = 0;
- /* Check integrity of deltas */
- EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_START, NULL, NULL);
-
- for(i = deltas; i; i = i->next) {
- pmdelta_t *d = alpm_list_getdata(i);
- const char *filename = alpm_delta_get_filename(d);
- const char *md5sum = alpm_delta_get_md5sum(d);
-
- if(test_md5sum(trans, filename, md5sum) != 0) {
- errors++;
- *data = alpm_list_add(*data, strdup(filename));
- }
+ errors = 0;
+ /* Check integrity of deltas */
+ EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_START, NULL, NULL);
+
+ for(i = deltas; i; i = i->next) {
+ pmdelta_t *d = alpm_list_getdata(i);
+ const char *filename = alpm_delta_get_filename(d);
+ const char *md5sum = alpm_delta_get_md5sum(d);
+
+ if(test_md5sum(trans, filename, md5sum) != 0) {
+ errors++;
+ *data = alpm_list_add(*data, strdup(filename));
}
- if(errors) {
- pm_errno = PM_ERR_DLT_INVALID;
- goto error;
- }
- EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_DONE, NULL, NULL);
+ }
+ if(errors) {
+ pm_errno = PM_ERR_DLT_INVALID;
+ goto error;
+ }
+ EVENT(trans, PM_TRANS_EVT_DELTA_INTEGRITY_DONE, NULL, NULL);

- /* Use the deltas to generate the packages */
- EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_START, NULL, NULL);
- ret = apply_deltas(trans);
- EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_DONE, NULL, NULL);
+ /* Use the deltas to generate the packages */
+ EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_START, NULL, NULL);
+ ret = apply_deltas(trans);
+ EVENT(trans, PM_TRANS_EVT_DELTA_PATCHES_DONE, NULL, NULL);

- }
if(ret) {
pm_errno = PM_ERR_DLT_PATCHFAILED;
goto error;
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index d2cf52e..fc072b6 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -961,25 +961,29 @@ create_package() {
# tar it up
msg2 "$(gettext "Compressing package...")"

- local TAR_OPT
case "$PKGEXT" in
- *tar.gz) TAR_OPT="z" ;;
- *tar.bz2) TAR_OPT="j" ;;
+ *tar.gz) EXT=${PKGEXT%.gz} ;;
+ *tar.bz2) EXT=${PKGEXT%.bz2} ;;
*) warning "$(gettext "'%s' is not a valid archive extension.")"
- "$PKGEXT" ;;
+ "$PKGEXT" ; EXT=$PKGEXT ;;
esac
-
- local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}"
+ local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${CARCH}${EXT}"

# when fileglobbing, we want * in an empty directory to expand to
# the null string rather than itself
shopt -s nullglob

- if ! bsdtar -c${TAR_OPT}f "$pkg_file" $comp_files *; then
+ if ! bsdtar -cf - $comp_files * > "$pkg_file"; then
error "$(gettext "Failed to create package file.")"
exit 1 # TODO: error code
fi
shopt -u nullglob
+
+ case "$PKGEXT" in
+ *tar.gz) gzip -f -n "$pkg_file" ;;
+ *tar.bz2) bzip2 -f "$pkg_file" ;;
+ esac
+
}

create_srcpackage() {
--
1.6.1.3

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://www.archlinux.org/mailman/listinfo/pacman-dev