[pacman-dev] [PATCH 1/5] Fix several issues with xdelta
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\n"); + _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search for '%s'\n", to); vertices = delta_graph_init(deltas); bestsize = delta_vert(vertices, to, to_md5, &bestpath); - _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete\n"); + _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete : '%lld'\n", (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\n"), 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
The from_md5 and to_md5 fields were a nice extra safety, which would avoid trying to apply deltas on corrupted package files. However, they are not strictly necessary, since xdelta should be able to detect that on its own. The main problem is that it is impossible to compute these informations from the delta only. So repo-add would not be able to compute the delta entry based on just the delta file. Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- lib/libalpm/alpm.h | 2 - lib/libalpm/delta.c | 57 +++++++++++++------------------------------------- lib/libalpm/delta.h | 14 ++++-------- lib/libalpm/sync.c | 1 - 4 files changed, 20 insertions(+), 54 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 3836d60..43df31c 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -229,9 +229,7 @@ off_t alpm_pkg_download_size(pmpkg_t *newpkg); */ const char *alpm_delta_get_from(pmdelta_t *delta); -const char *alpm_delta_get_from_md5sum(pmdelta_t *delta); const char *alpm_delta_get_to(pmdelta_t *delta); -const char *alpm_delta_get_to_md5sum(pmdelta_t *delta); const char *alpm_delta_get_filename(pmdelta_t *delta); const char *alpm_delta_get_md5sum(pmdelta_t *delta); off_t alpm_delta_get_size(pmdelta_t *delta); diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c index 337eb66..6586626 100644 --- a/lib/libalpm/delta.c +++ b/lib/libalpm/delta.c @@ -43,24 +43,12 @@ const char SYMEXPORT *alpm_delta_get_from(pmdelta_t *delta) return(delta->from); } -const char SYMEXPORT *alpm_delta_get_from_md5sum(pmdelta_t *delta) -{ - ASSERT(delta != NULL, return(NULL)); - return(delta->from_md5); -} - const char SYMEXPORT *alpm_delta_get_to(pmdelta_t *delta) { ASSERT(delta != NULL, return(NULL)); return(delta->to); } -const char SYMEXPORT *alpm_delta_get_to_md5sum(pmdelta_t *delta) -{ - ASSERT(delta != NULL, return(NULL)); - return(delta->to_md5); -} - const char SYMEXPORT *alpm_delta_get_filename(pmdelta_t *delta) { ASSERT(delta != NULL, return(NULL)); @@ -104,12 +92,10 @@ static alpm_list_t *delta_graph_init(alpm_list_t *deltas) /* determine whether a base 'from' file exists */ fpath = _alpm_filecache_find(vdelta->from); - md5sum = alpm_compute_md5sum(fpath); - if(fpath && md5sum && strcmp(md5sum, vdelta->from_md5) == 0) { + if(fpath) { v->weight = vdelta->download_size; } FREE(fpath); - FREE(md5sum); v->data = vdelta; vertices = alpm_list_add(vertices, v); @@ -131,8 +117,7 @@ static alpm_list_t *delta_graph_init(alpm_list_t *deltas) * 3_to_4 * If J 'from' is equal to I 'to', then J is a child of I. * */ - if(strcmp(d_j->from, d_i->to) == 0 - && strcmp(d_j->from_md5, d_i->to_md5) == 0) { + if(strcmp(d_j->from, d_i->to) == 0) { v_i->children = alpm_list_add(v_i->children, v_j); } } @@ -142,7 +127,7 @@ static alpm_list_t *delta_graph_init(alpm_list_t *deltas) } static off_t delta_vert(alpm_list_t *vertices, - const char *to, const char *to_md5, alpm_list_t **path) { + const char *to, alpm_list_t **path) { alpm_list_t *i; pmgraph_t *v; while(1) { @@ -186,8 +171,7 @@ static off_t delta_vert(alpm_list_t *vertices, pmgraph_t *v_i = i->data; pmdelta_t *d_i = v_i->data; - if(strcmp(d_i->to, to) == 0 - || strcmp(d_i->to_md5, to_md5) == 0) { + if(strcmp(d_i->to, to) == 0) { if(v == NULL || v_i->weight < v->weight) { v = v_i; bestsize = v->weight; @@ -212,14 +196,13 @@ static off_t delta_vert(alpm_list_t *vertices, * size, not the length of the path. * @param deltas the list of pmdelta_t * objects that a file has * @param to the file to start the search at - * @param to_md5 the md5sum of the above named file * @param path the pointer to a list location where pmdelta_t * objects that * have the smallest size are placed. NULL is set if there is no path * possible with the files available. * @return the size of the path stored, or LONG_MAX if path is unfindable */ off_t _alpm_shortest_delta_path(alpm_list_t *deltas, - const char *to, const char *to_md5, alpm_list_t **path) + const char *to, alpm_list_t **path) { alpm_list_t *bestpath = NULL; alpm_list_t *vertices; @@ -236,7 +219,7 @@ off_t _alpm_shortest_delta_path(alpm_list_t *deltas, vertices = delta_graph_init(deltas); - bestsize = delta_vert(vertices, to, to_md5, &bestpath); + bestsize = delta_vert(vertices, to, &bestpath); _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete : '%lld'\n", (long long)bestsize); @@ -250,7 +233,7 @@ off_t _alpm_shortest_delta_path(alpm_list_t *deltas, /** Parses the string representation of a pmdelta_t object. * This function assumes that the string is in the correct format. * This format is as follows: - * $oldfile $oldmd5 $newfile $newmd5 $deltafile $deltamd5 $deltasize + * $deltafile $deltamd5 $deltasize $oldfile $newfile * @param line the string to parse * @return A pointer to the new pmdelta_t object */ @@ -262,9 +245,8 @@ pmdelta_t *_alpm_delta_parse(char *line) regex_t reg; regcomp(®, - "^[^[:space:]]* [[:xdigit:]]{32}" - " [^[:space:]]* [[:xdigit:]]{32}" - " [^[:space:]]* [[:xdigit:]]{32} [[:digit:]]*$", + "^[^[:space:]]* [[:xdigit:]]{32} [[:digit:]]*" + " [^[:space:]]* [^[:space:]]*$", REG_EXTENDED | REG_NOSUB | REG_NEWLINE); if(regexec(®, line, 0, 0, 0) != 0) { /* delta line is invalid, return NULL */ @@ -278,34 +260,27 @@ pmdelta_t *_alpm_delta_parse(char *line) tmp2 = tmp; tmp = strchr(tmp, ' '); *(tmp++) = '\0'; - STRDUP(delta->from, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); - - tmp2 = tmp; - tmp = strchr(tmp, ' '); - *(tmp++) = '\0'; - STRDUP(delta->from_md5, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + STRDUP(delta->delta, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); tmp2 = tmp; tmp = strchr(tmp, ' '); *(tmp++) = '\0'; - STRDUP(delta->to, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + STRDUP(delta->delta_md5, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); tmp2 = tmp; tmp = strchr(tmp, ' '); *(tmp++) = '\0'; - STRDUP(delta->to_md5, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + delta->delta_size = atol(tmp2); tmp2 = tmp; tmp = strchr(tmp, ' '); *(tmp++) = '\0'; - STRDUP(delta->delta, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + STRDUP(delta->from, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); tmp2 = tmp; - tmp = strchr(tmp, ' '); - *(tmp++) = '\0'; - STRDUP(delta->delta_md5, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + STRDUP(delta->to, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); - delta->delta_size = atol(tmp); + _alpm_log(PM_LOG_DEBUG, "delta : %s %s '%lld'\n", delta->from, delta->to, (long long)delta->delta_size); return(delta); } @@ -313,9 +288,7 @@ pmdelta_t *_alpm_delta_parse(char *line) void _alpm_delta_free(pmdelta_t *delta) { FREE(delta->from); - FREE(delta->from_md5); FREE(delta->to); - FREE(delta->to_md5); FREE(delta->delta); FREE(delta->delta_md5); FREE(delta); diff --git a/lib/libalpm/delta.h b/lib/libalpm/delta.h index 5bb86b9..29a2b0c 100644 --- a/lib/libalpm/delta.h +++ b/lib/libalpm/delta.h @@ -24,20 +24,16 @@ #include "alpm.h" struct __pmdelta_t { - /** filename of the 'before' file */ - char *from; - /** md5sum of the 'before' file */ - char *from_md5; - /** filename of the 'after' file */ - char *to; - /** md5sum of the 'after' file */ - char *to_md5; /** filename of the delta patch */ char *delta; /** md5sum of the delta file */ char *delta_md5; /** filesize of the delta file */ off_t delta_size; + /** filename of the 'before' file */ + char *from; + /** filename of the 'after' file */ + char *to; /** download filesize of the delta file */ off_t download_size; }; @@ -45,7 +41,7 @@ struct __pmdelta_t { pmdelta_t *_alpm_delta_parse(char *line); void _alpm_delta_free(pmdelta_t *delta); off_t _alpm_shortest_delta_path(alpm_list_t *deltas, - const char *to, const char *to_md5, alpm_list_t **path); + const char *to, alpm_list_t **path); #endif /* _ALPM_DELTA_H */ diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 71d8771..8777856 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -372,7 +372,6 @@ static int compute_download_size(pmpkg_t *newpkg) dltsize = _alpm_shortest_delta_path( alpm_pkg_get_deltas(newpkg), alpm_pkg_get_filename(newpkg), - alpm_pkg_get_md5sum(newpkg), &newpkg->delta_path); if(newpkg->delta_path && (dltsize < pkgsize * MAX_DELTA_RATIO)) { -- 1.6.1.3
Use the correct database format Instead of getting all the info from the filename, which was a bit ugly, use xdelta3 to get the source and destination files Instead of looking for .delta files in the current directory when adding a package, which was not very flexible, allow .delta files to be added with repo-add just like package files. delta files can also be removed with repo-remove. This is simply done by looking for a .delta extension in the arguments, and calling the appropriate db_write_delta or db_remove_delta functions. Example usage: repo-add repo/test.db.tar.gz repo/libx11-1.1.99.2-2-x86_64.pkg.tar.gz repo-add repo/test.db.tar.gz repo/libx11-1.1.5-2_to_1.1.99.2-2-x86_64.delta repo-remove repo/test.db.tar.gz libx11-1.1.5-2_to_1.1.99.2-2-x86_64.delta Note that db_write_entry had to be modified so that the deltas file is not lost on a package upgrade (remove + add). FS#13414 should be fixed in the same time, by printing a different message when the same package is added. Normal output: ==> Adding package 'repo/libx11-1.1.99.2-2-x86_64.pkg.tar.gz' -> Removing version 'libx11-1.1.5-2'... Warning: ==> Adding package 'repo/libx11-1.1.99.2-2-x86_64.pkg.tar.gz' ==> WARNING: An entry for 'libx11-1.1.99.2-2' already exists, overwriting... Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- scripts/repo-add.sh.in | 203 ++++++++++++++++++++++++++++-------------------- 1 files changed, 120 insertions(+), 83 deletions(-) diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index c6d25aa..95b7959 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -57,8 +57,8 @@ error() { # print usage instructions usage() { printf "repo-add, repo-remove (pacman) %s\n\n" "$myver" - printf "$(gettext "Usage: repo-add [-q] <path-to-db> <package> ...\n")" - printf "$(gettext "Usage: repo-remove [-q] <path-to-db> <packagename> ...\n\n")" + printf "$(gettext "Usage: repo-add [-q] <path-to-db> <package|delta> ...\n")" + printf "$(gettext "Usage: repo-remove [-q] <path-to-db> <packagename|delta> ...\n\n")" printf "$(gettext "\ repo-add will update a package database by reading a package file.\n\ Multiple packages to add can be specified on the command line.\n\n")" @@ -93,36 +93,84 @@ write_list_entry() { fi } -# write a delta entry to the pacman database -# arg1 - path to delta +# Get the package name from the delta filename +getpkgname() { + local tmp + + tmp=${1##*/} + echo ${tmp%-*-*_to*} +} + +find_pkgentry() +{ + local pkgname=$1 + local pkgentry + for pkgentry in $gstmpdir/$pkgname*; do + name=${pkgentry##*/} + if [ "${name%-*-*}" = "$pkgname" ]; then + echo $pkgentry + return 0 + fi + done + return 1 +} + +# write a delta entry +# arg1 - path to delta file db_write_delta() { - # blank out all variables - local deltafile="$1" - local filename=$(basename "$deltafile") - local deltavars pkgname fromver tover arch csize md5sum - - # format of the delta filename: - # (package)-(fromver)_to_(tover)-(arch).delta - deltavars=( $(echo "$filename" | sed -e 's/\(.*\)-\(.*-.*\)_to_\(.*-.*\)-\(.*\).delta/\1 \2 \3 \4/') ) - pkgname=${deltavars[0]} - fromver=${deltavars[1]} - tover=${deltavars[2]} - arch=${deltavars[3]} - - # get md5sum and size of delta + deltafile="$1" + pkgname="$(getpkgname $deltafile)" + + pkgentry=$(find_pkgentry $pkgname) + if [ -z "$pkgentry" ]; then + return 1 + fi + deltas="$pkgentry/deltas" + # create deltas file if it does not already exist + if [ ! -f "$deltas" ]; then + msg2 "$(gettext "Creating 'deltas' db entry...")" + echo -e "%DELTAS%" >>$deltas + fi + # get md5sum and compressed size of package md5sum="$(openssl dgst -md5 "$deltafile" | awk '{print $NF}')" csize=$(@SIZECMD@ "$deltafile") - # ensure variables were found - if [ -z "$pkgname" -o -z "$fromver" -o -z "$tover" -o -z "$arch" ]; then - return 1 + oldfile=$(xdelta3 printhdr $deltafile | grep "XDELTA filename (source)" | sed 's/.*: *//') + newfile=$(xdelta3 printhdr $deltafile | grep "XDELTA filename (output)" | sed 's/.*: *//') + + if grep -q "$oldfile.*$newfile" $deltas; then + warning "$(gettext "An entry for '%s' already exists, overwriting...")" "$deltafile" + sed -i "/$oldfile.*$newfile/d" $deltas fi + echo ${deltafile##*/} $md5sum $csize $oldfile $newfile >> $deltas - # add the entry for this delta file - echo -e "$fromver $tover $csize $filename $md5sum" >>deltas + return 0 } # end db_write_delta +# remove a delta entry +# arg1 - path to delta file +db_remove_delta() +{ + deltafile="$1" + filename=${deltafile##*/} + pkgname="$(getpkgname $deltafile)" + + pkgentry=$(find_pkgentry $pkgname) + if [ -z "$pkgentry" ]; then + return 1 + fi + deltas="$pkgentry/deltas" + if [ ! -f "$deltas" ]; then + return 1 + fi + if grep -q "$filename" $deltas; then + sed -i "/$filename/d" $deltas + return 0 + fi + + return 1 +} # end db_write_delta # write an entry to the pacman database # arg1 - path to package @@ -172,19 +220,34 @@ db_write_entry() return 1 fi - # remove an existing entry if it exists, ignore failures - db_remove_entry "$pkgname" - startdir=$(pwd) pushd "$gstmpdir" 2>&1 >/dev/null - # create package directory - mkdir "$pkgname-$pkgver" + # remove an eventual existing entry but keep the deltas file + if [ -d "$pkgname-$pkgver" ]; then + # special case : same version already exists + # package directory can be kept, just delete depends and desc + warning "$(gettext "An entry for '%s' already exists, overwriting...")" "$pkgname-$pkgver" + rm "$pkgname-$pkgver"/{depends,desc} + else + pkgentry=$(find_pkgentry $pkgname) + # create package directory + mkdir "$pkgname-$pkgver" + if [ -n "$pkgentry" ]; then + # keep the deltas file then delete the existing entry + if [ -f "$pkgentry/deltas" ]; then + cp "$pkgentry/deltas" "$pkgname-$pkgver" + fi + msg2 "$(gettext "Removing version '%s'...")" "${pkgentry##*/}" + rm -rf $pkgentry + fi + fi + cd "$pkgname-$pkgver" # create desc entry msg2 "$(gettext "Creating 'desc' db entry...")" - echo -e "%FILENAME%\n$(basename "$1")\n" >>desc + echo -e "%FILENAME%\n"${1##*/}"\n" >>desc echo -e "%NAME%\n$pkgname\n" >>desc echo -e "%VERSION%\n$pkgver\n" >>desc [ -n "$pkgdesc" ] && echo -e "%DESC%\n$pkgdesc\n" >>desc @@ -211,61 +274,16 @@ db_write_entry() write_list_entry "PROVIDES" "$_provides" "depends" write_list_entry "OPTDEPENDS" "$_optdepends" "depends" - # create deltas entry if there are delta files - # Xav : why should deltas be in $startdir? - for delta in $startdir/$pkgname-*-*_to_*-*-$arch.delta; do - # This for loop also pulls in all files that start with the current package - # name and are followed by a -whatever. For instance, running this loop for - # gcc would also grab gcc-libs. To guard against this, compare the package - # name of the delta to the current package name. - local filename=$(basename "$delta") - local dpkgname="$(echo "$filename" | sed -e 's/\(.*\)-.*-.*_to_.*-.*-.*.delta/\1/')" - if [ "$pkgname" = "$dpkgname" -a -f "$delta" ]; then - # create deltas file if it does not already exist - if [ ! -f "deltas" ]; then - msg2 "$(gettext "Creating 'deltas' db entry...")" - echo -e "%DELTAS%" >>deltas - fi - - # write this delta entry - if db_write_delta "$delta"; then - msg2 "$(gettext "Added delta '%s'")" "$(basename "$delta")" - else - warning "$(gettext "Could not add delta '%s'")" "$(basename "$delta")" - fi - fi - done - # add the final newline - [ -f "deltas" ] && echo -e "" >>deltas - popd 2>&1 >/dev/null # preserve the modification time # Xav : what for? pkgdir="$gstmpdir/$pkgname-$pkgver" touch -r "$pkgfile" "$pkgdir/desc" "$pkgdir/depends" - [ -f "$pkgdir/deltas" ] && touch -r "$pkgfile" "$pkgdir/deltas" return 0 } # end db_write_entry -# remove existing entries from the DB -# arg1 - package name -db_remove_entry() { - pushd "$gstmpdir" 2>&1 >/dev/null - - # remove any other package in the DB with same name - local existing - for existing in *; do - if [ "${existing%-*-*}" = "$1" ]; then - msg2 "$(gettext "Removing existing package '%s'...")" "$existing" - rm -rf "$existing" - fi - done - - popd 2>&1 >/dev/null -} # end db_remove_entry - # PROGRAM START # determine whether we have gettext; make it a no-op if we do not @@ -299,7 +317,7 @@ gstmpdir=$(mktemp -d /tmp/repo-tools.XXXXXXXXXX) || (\ exit 1) # figure out what program we are -cmd="$(basename $0)" +cmd=${0##*/} if [ "$cmd" != "repo-add" -a "$cmd" != "repo-remove" ]; then error "$(gettext "Invalid command name '%s' specified.")" "$cmd" exit 1 @@ -329,7 +347,14 @@ for arg in "$@"; do else if [ "$cmd" == "repo-add" ]; then if [ -f "$arg" ]; then - if ! bsdtar -tf "$arg" .PKGINFO 2>&1 >/dev/null; then + if [ "${arg##*.}" == "delta" ]; then + msg "$(gettext "Adding delta '%s'")" "$arg" + if db_write_delta "$arg"; then + success=1 + else + error "$(gettext "No package entry for '%s'.")" "$arg" + fi + elif ! bsdtar -tf "$arg" .PKGINFO 2>&1 >/dev/null; then error "$(gettext "'%s' is not a package file, skipping")" "$arg" else msg "$(gettext "Adding package '%s'")" "$arg" @@ -339,15 +364,27 @@ for arg in "$@"; do fi fi else - error "$(gettext "Package '%s' not found.")" "$arg" + error "$(gettext "File '%s' not found.")" "$arg" fi elif [ "$cmd" == "repo-remove" ]; then - msg "$(gettext "Searching for package '%s'...")" "$arg" - - if db_remove_entry "$arg"; then - success=1 + if [ "${arg##*.}" == "delta" ]; then + msg "$(gettext "Searching for delta '%s'...")" "$arg" + if db_remove_delta "$arg"; then + success=1 + else + error "$(gettext "Delta matching '%s' not found.")" "$arg" + fi else - error "$(gettext "Package matching '%s' not found.")" "$arg" + msg "$(gettext "Searching for package '%s'...")" "$arg" + + pkgentry=$(find_pkgentry $arg) + if [ -n "$pkgentry" ]; then + msg2 "$(gettext "Removing existing version '%s'...")" "${pkgentry##*/}" + rm -rf $pkgentry + success=1 + else + error "$(gettext "Package matching '%s' not found.")" "$arg" + fi fi fi fi @@ -364,7 +401,7 @@ if [ $success -eq 1 ]; then "$REPO_DB_FILE" ;; esac - filename=$(basename "$REPO_DB_FILE") + filename=${REPO_DB_FILE##*/} pushd "$gstmpdir" 2>&1 >/dev/null if [ -n "$(ls)" ]; then -- 1.6.1.3
This should obsolete the delta support in makepkg. Having a separate script should be more flexible. Example usage: $ create-xdelta repo/tzdata-2009a-1-x86_64.pkg.tar.gz repo/tzdata-2009b-1-x86_64.pkg.tar.gz ==> Generating delta from version 2009a-1 to version 2009b-1 ==> Generated delta : 'repo/tzdata-2009a-1_to_2009b-1-x86_64.delta' Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- scripts/.gitignore | 1 + scripts/Makefile.am | 7 ++- scripts/create-xdelta.sh.in | 143 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 scripts/create-xdelta.sh.in diff --git a/scripts/.gitignore b/scripts/.gitignore index f2f19fd..43ad30f 100644 --- a/scripts/.gitignore +++ b/scripts/.gitignore @@ -3,3 +3,4 @@ pacman-optimize rankmirrors repo-add repo-remove +create-xdelta diff --git a/scripts/Makefile.am b/scripts/Makefile.am index d6d9bb9..3ccf478 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -9,13 +9,15 @@ OURSCRIPTS = \ makepkg \ pacman-optimize \ rankmirrors \ - repo-add + repo-add \ + create-xdelta EXTRA_DIST = \ makepkg.sh.in \ pacman-optimize.sh.in \ rankmirrors.py.in \ - repo-add.sh.in + repo-add.sh.in \ + create-xdelta.sh.in # Files that should be removed, but which Automake does not know. MOSTLYCLEANFILES = $(bin_SCRIPTS) *.tmp @@ -63,5 +65,6 @@ repo-add: $(srcdir)/repo-add.sh.in repo-remove: $(srcdir)/repo-add.sh.in rm -f repo-remove $(LN_S) repo-add repo-remove +create-xdelta: $(srcdir)/create-xdelta.sh.in # vim:set ts=2 sw=2 noet: diff --git a/scripts/create-xdelta.sh.in b/scripts/create-xdelta.sh.in new file mode 100644 index 0000000..58ad11f --- /dev/null +++ b/scripts/create-xdelta.sh.in @@ -0,0 +1,143 @@ +#!/bin/bash -e +# +# create-xdelta - create delta files for use with pacman and repo-add +# @configure_input@ +# +# Copyright (c) 2009 Xavier Chantry <shiningxc@gmail.com> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +# gettext initialization +export TEXTDOMAIN='pacman' +export TEXTDOMAINDIR='@localedir@' + +myver='@PACKAGE_VERSION@' + +# ensure we have a sane umask set +umask 0022 + +msg() { + local mesg=$1; shift + printf "==> ${mesg}\n" "$@" >&1 +} + +warning() { + local mesg=$1; shift + printf "==> $(gettext "WARNING:") ${mesg}\n" "$@" >&2 +} + +error() { + local mesg=$1; shift + printf "==> $(gettext "ERROR:") ${mesg}\n" "$@" >&2 +} + +# print usage instructions +usage() { + printf "create-xdelta (pacman) %s\n\n" "$myver" + printf "$(gettext "Usage: create-xdelta <package1> <package2 ...\n")" + printf "$(gettext "\ + create-xdelta will create a delta file between two packages\n\ +This delta file can then be added to a database using repo-add.\n\n")" + echo "$(gettext "Example: create-xdelta pacman-3.0.0.pkg.tar.gz pacman-3.0.1.pkg.tar.gz")" +} + +version() { + printf "create-xdelta (pacman) %s\n\n" "$myver" + printf "$(gettext "\ +Copyright (c) 2009 Xavier Chantry <shiningxc@gmail.com>.\n\n\ +This is free software; see the source for copying conditions.\n\ +There is NO WARRANTY, to the extent permitted by law.\n")" +} + +read_pkginfo() +{ + unset pkgname pkgver arch + local OLDIFS="$IFS" + # IFS (field separator) is only the newline character + IFS=" +" + local line var val + for line in $(bsdtar -xOf "$1" .PKGINFO 2>/dev/null | + grep -v "^#" | sed 's|\(\w*\)\s*=\s*\(.*\)|\1="\2"|'); do + eval "$line" + done + IFS=$OLDIFS + if [ -z "$pkgname" ]; then + error "$(gettext "Invalid package '%s'")" "$1" + return 1 + fi + return 0 +} + +# $oldfile $oldmd5 $newfile $newmd5 $deltafile $deltamd5 $deltasize +create_xdelta() +{ + local oldfile=$1 + local newfile=$2 + local \ + oldname oldver oldarch \ + newname newver newarch \ + deltafile + + read_pkginfo "$oldfile" || return 1 + oldname="$pkgname" + oldver="$pkgver" + oldarch="$arch" + read_pkginfo "$newfile" || return 1 + newname="$pkgname" + newver="$pkgver" + newarch="$arch" + + if [ "$oldname" != "$newname" ]; then + error "$(gettext "The package names don't match : '%s' and '%s'")" "$oldname" "$newname" + return 1 + fi + + if [ "$oldarch" != "$newarch" ]; then + error "$(gettext "The package architectures don't match : '%s' and '%s'")" "$oldarch" "$newarch" + return 1 + fi + + if [ "$oldver" == "$newver" ]; then + error "$(gettext "Both packages have the same version : '%s'")" "$newver" + return 1 + fi + + msg "$(gettext "Generating delta from version %s to version %s")" "$oldver" "$newver" + deltafile="$(dirname $newfile)/$pkgname-${oldver}_to_${newver}-$arch.delta" + local ret=0 + + xdelta3 -q -f -s "$oldfile" "$newfile" "$deltafile" || ret=$? + if [ $ret -ne 0 ]; then + error "$(gettext "Delta could not be created.")" + return 1 + else + msg "$(gettext "Generated delta : '%s'")" "$deltafile" + fi + return 0 +} + +if [ $# -ne 2 ]; then + usage + exit 0 +fi + + +if [ ! "$(type -p xdelta3)" ]; then + error "$(gettext "Cannot find the xdelta3 binary! Is xdelta3 installed?")" + exit 1 +fi + +create_xdelta $1 $2 -- 1.6.1.3
The create-xdelta script can be used instead. Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- doc/makepkg.conf.5.txt | 7 +----- etc/makepkg.conf.in | 5 +-- scripts/makepkg.sh.in | 50 ------------------------------------------------ 3 files changed, 3 insertions(+), 59 deletions(-) diff --git a/doc/makepkg.conf.5.txt b/doc/makepkg.conf.5.txt index 04fe2bb..704dccc 100644 --- a/doc/makepkg.conf.5.txt +++ b/doc/makepkg.conf.5.txt @@ -70,7 +70,7 @@ Options This is often used to set the number of jobs used, for example, `-j2`. Other flags that make accepts can also be passed. -**BUILDENV=(**fakeroot !distcc color !ccache !xdelta**)**:: +**BUILDENV=(**fakeroot !distcc color !ccache**)**:: This array contains options that affect the build environment, the defaults are shown here. All options should always be left in the array; to enable or disable an option simply remove or place an ``!'' at the front of the @@ -93,11 +93,6 @@ Options be disabled for individual packages by placing `!ccache` in the PKGBUILD options array. - *xdelta*;; - Generate an xdelta binary patch from previous to current package. The - previous package must be available in the makepkg cache directory for - this to occur. - **DISTCC_HOSTS=**"host1 ...":: If using DistCC, this is used to specify a space-delimited list of hosts running in the DistCC cluster. In addition, you will want to modify your diff --git a/etc/makepkg.conf.in b/etc/makepkg.conf.in index d811867..76b9eca 100644 --- a/etc/makepkg.conf.in +++ b/etc/makepkg.conf.in @@ -39,16 +39,15 @@ CXXFLAGS="@CARCHFLAGS@-mtune=generic -O2 -pipe" # BUILD ENVIRONMENT ######################################################################### # -# Defaults: BUILDENV=(fakeroot !distcc color !ccache !xdelta) +# Defaults: BUILDENV=(fakeroot !distcc color !ccache) # A negated environment option will do the opposite of the comments below. # #-- fakeroot: Allow building packages as a non-root user #-- distcc: Use the Distributed C/C++/ObjC compiler #-- color: Colorize output messages #-- ccache: Use ccache to cache compilation -#-- xdelta: Generate delta patch from previous to current package # -BUILDENV=(fakeroot !distcc color !ccache !xdelta) +BUILDENV=(fakeroot !distcc color !ccache) # #-- If using DistCC, your MAKEFLAGS will also need modification. In addition, #-- specify a space-delimited list of hosts running in the DistCC cluster. diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 9e0f249..95a2f3e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -982,54 +982,6 @@ create_package() { shopt -u nullglob } -create_xdelta() { - if [ "$(check_buildenv xdelta)" != "y" ]; then - return - elif [ ! "$(type -p xdelta3)" ]; then - error "$(gettext "Cannot find the xdelta3 binary! Is xdelta3 installed?")" - return - fi - - local pkg_file=$1 - local cache_dir="/var/cache/pacman/pkg" # TODO: autoconf me - local pkginfo="$(mktemp "$startdir"/xdelta-pkginfo.XXXXXXXXX)" - - local old_file old_version - for old_file in $(ls {"$cache_dir","$PKGDEST"}/${pkgname}-*-*{,-$CARCH}$PKGEXT 2>/dev/null); do - bsdtar -xOf "$old_file" .PKGINFO > "$pkginfo" || continue - if [ "$(cat "$pkginfo" | grep '^pkgname = ')" != "pkgname = $pkgname" ]; then - continue # Package name does not match. - elif [ "$(cat "$pkginfo" | grep '^arch = ')" != "arch = $CARCH" ] ; then - continue # Not same arch. - fi - - old_version="$(cat "$pkginfo" | grep '^pkgver = ' | sed 's/^pkgver = //')" - - # old_version may include the target package, only use the old versions - local vercmp=$(vercmp "$old_version" "$latest_version") - if [ "$old_version" != "$pkgver-$pkgrel" -a $vercmp -gt 0 ]; then - local latest_version=$old_version - local base_file=$old_file - fi - done - - rm -f "$pkginfo" - - if [ -n "$base_file" ]; then - msg "$(gettext "Making delta from version %s...")" "$latest_version" - local delta_file="$PKGDEST/$pkgname-${latest_version}_to_$pkgver-$pkgrel-$CARCH.delta" - local ret=0 - - 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 - warning "$(gettext "No previous version found, skipping xdelta.")" - fi -} - create_srcpackage() { cd "$startdir" if [ "$SOURCEONLY" -eq 2 ]; then @@ -1794,8 +1746,6 @@ else fakeroot -- $0 -F $ARGLIST || exit $? fi fi - - create_xdelta "$PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}" fi msg "$(gettext "Finished making: %s")" "$pkgname $pkgver-$pkgrel $CARCH ($(date))" -- 1.6.1.3
Xavier Chantry wrote:
The create-xdelta script can be used instead.
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> ---
Signed-off-by me. I like the idea of moving the delta creation from makepkg. Allan
On Wed, Feb 25, 2009 at 8:51 PM, Allan McRae <allan@archlinux.org> wrote:
Xavier Chantry wrote:
The create-xdelta script can be used instead.
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> ---
Signed-off-by me. I like the idea of moving the delta creation from makepkg.
Good by me as well. -Dan
Xavier Chantry wrote:
This should obsolete the delta support in makepkg. Having a separate script should be more flexible.
Example usage: $ create-xdelta repo/tzdata-2009a-1-x86_64.pkg.tar.gz repo/tzdata-2009b-1-x86_64.pkg.tar.gz ==> Generating delta from version 2009a-1 to version 2009b-1 ==> Generated delta : 'repo/tzdata-2009a-1_to_2009b-1-x86_64.delta'
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> ---
Not sure about the name. I would prefer it mentioned pkg somehow as at the moment the is nothing in the name that indicates this is for pacman packages. Maybe pkgdelta?
<snip> + +# print usage instructions +usage() { + printf "create-xdelta (pacman) %s\n\n" "$myver" + printf "$(gettext "Usage: create-xdelta <package1> <package2 ...\n")"
Missing ">" after package 2. Why the "..." there? The script only uses two parameters. Otherwise, this is full of awesomeness and win. Allan
On Thu, Feb 26, 2009 at 3:55 AM, Allan McRae <allan@archlinux.org> wrote:
Xavier Chantry wrote:
This should obsolete the delta support in makepkg. Having a separate script should be more flexible.
Example usage: $ create-xdelta repo/tzdata-2009a-1-x86_64.pkg.tar.gz repo/tzdata-2009b-1-x86_64.pkg.tar.gz ==> Generating delta from version 2009a-1 to version 2009b-1 ==> Generated delta : 'repo/tzdata-2009a-1_to_2009b-1-x86_64.delta'
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> ---
Not sure about the name. I would prefer it mentioned pkg somehow as at the moment the is nothing in the name that indicates this is for pacman packages. Maybe pkgdelta?
Good point, I agree. pkgdelta sounds fine to me.
<snip> + +# print usage instructions +usage() { + printf "create-xdelta (pacman) %s\n\n" "$myver" + printf "$(gettext "Usage: create-xdelta <package1> <package2 ...\n")"
Missing ">" after package 2. Why the "..." there? The script only uses two parameters.
That's completely wrong indeed, it probably comes from the script I used as a base, repo-add probably.
Otherwise, this is full of awesomeness and win.
ahah
Xavier Chantry wrote:
Use the correct database format
Instead of getting all the info from the filename, which was a bit ugly, use xdelta3 to get the source and destination files
Instead of looking for .delta files in the current directory when adding a package, which was not very flexible, allow .delta files to be added with repo-add just like package files. delta files can also be removed with repo-remove. This is simply done by looking for a .delta extension in the arguments, and calling the appropriate db_write_delta or db_remove_delta functions.
Example usage: repo-add repo/test.db.tar.gz repo/libx11-1.1.99.2-2-x86_64.pkg.tar.gz repo-add repo/test.db.tar.gz repo/libx11-1.1.5-2_to_1.1.99.2-2-x86_64.delta repo-remove repo/test.db.tar.gz libx11-1.1.5-2_to_1.1.99.2-2-x86_64.delta
Note that db_write_entry had to be modified so that the deltas file is not lost on a package upgrade (remove + add). FS#13414 should be fixed in the same time, by printing a different message when the same package is added.
Normal output: ==> Adding package 'repo/libx11-1.1.99.2-2-x86_64.pkg.tar.gz' -> Removing version 'libx11-1.1.5-2'... Warning: ==> Adding package 'repo/libx11-1.1.99.2-2-x86_64.pkg.tar.gz' ==> WARNING: An entry for 'libx11-1.1.99.2-2' already exists, overwriting...
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- scripts/repo-add.sh.in | 203 ++++++++++++++++++++++++++++-------------------- 1 files changed, 120 insertions(+), 83 deletions(-)
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index c6d25aa..95b7959 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -57,8 +57,8 @@ error() { # print usage instructions usage() { printf "repo-add, repo-remove (pacman) %s\n\n" "$myver" - printf "$(gettext "Usage: repo-add [-q] <path-to-db> <package> ...\n")" - printf "$(gettext "Usage: repo-remove [-q] <path-to-db> <packagename> ...\n\n")" + printf "$(gettext "Usage: repo-add [-q] <path-to-db> <package|delta> ...\n")" + printf "$(gettext "Usage: repo-remove [-q] <path-to-db> <packagename|delta> ...\n\n")" printf "$(gettext "\ repo-add will update a package database by reading a package file.\n\ Multiple packages to add can be specified on the command line.\n\n")" @@ -93,36 +93,84 @@ write_list_entry() { fi }
-# write a delta entry to the pacman database -# arg1 - path to delta +# Get the package name from the delta filename +getpkgname() { + local tmp + + tmp=${1##*/} + echo ${tmp%-*-*_to*} +}
Rename this to getdeltapkgname to clarify it is only used for deltas? Self document function names make my head hurt less. Otherwise, a quick read of the other changes made sense to me, but I will need to do this in more detail. One comment that is moderately related to this. Should there be a mechanism to inspect the deltas are remove the ones that are no longer necessary? I.e. deltas with a broken chain, deltas who chain download will be bigger than the package. Should that be a job of repo-add when adding a new delta or a separate script? Allan
On Thu, Feb 26, 2009 at 4:11 AM, Allan McRae <allan@archlinux.org> wrote:
+# Get the package name from the delta filename +getpkgname() { + local tmp + + tmp=${1##*/} + echo ${tmp%-*-*_to*} +}
Rename this to getdeltapkgname to clarify it is only used for deltas? Self document function names make my head hurt less.
Sorry, I had exactly the same thought, I just didn't know where to add delta :) I will use your suggestion.
Otherwise, a quick read of the other changes made sense to me, but I will need to do this in more detail.
This is probably the patch that requires the most careful review (compared to the other 4), so it would be highly appreciated.
One comment that is moderately related to this. Should there be a mechanism to inspect the deltas are remove the ones that are no longer necessary? I.e. deltas with a broken chain, deltas who chain download will be bigger than the package. Should that be a job of repo-add when adding a new delta or a separate script?
I don't know, that seems neat but maybe too much complexity, I am not sure. Finding out how deltas could/should be managed has always been a big issue to me, how/when they are added, how/when they are removed.
Xavier wrote:
On Thu, Feb 26, 2009 at 4:11 AM, Allan McRae <allan@archlinux.org> wrote:
One comment that is moderately related to this. Should there be a mechanism to inspect the deltas are remove the ones that are no longer necessary? I.e. deltas with a broken chain, deltas who chain download will be bigger than the package. Should that be a job of repo-add when adding a new delta or a separate script?
I don't know, that seems neat but maybe too much complexity, I am not sure. Finding out how deltas could/should be managed has always been a big issue to me, how/when they are added, how/when they are removed.
Hi Guys I was just getting into understanding the code to see what I could do to get this going myself. Thank you, Xavier - I think it would have taken me a week or 2 to put all this together and likely not nearly good enough for Allan to be saying mostly good things about it. ;) Now that you mentioned it, other than broken chains and deltas bigger than "deltaless" downloads, what other criteria are there for delta removal? I can only think of disk space - though this is not likely to be an issue. Also on that note, other than this new code breaking the chain, how would a chain be broken?
Brendan Hide wrote:
Also on that note, other than this new code breaking the chain, how would a chain be broken?
By someone choosing not to or forgetting to add a delta. At the moment repo-add does not create deltas automatically, or at all... If it ever did create deltas, it would have to be through the use of a flag. Same with file list generation (http://bugs.archlinux.org/task/11302). Some people don't want their repo server dealing with the overhead. (I had to add >150 packages to the repo in one go for an ncurses rebuild). Allan
Xavier wrote:
On Thu, Feb 26, 2009 at 4:11 AM, Allan McRae <allan@archlinux.org> wrote:
+# Get the package name from the delta filename +getpkgname() { + local tmp + + tmp=${1##*/} + echo ${tmp%-*-*_to*} +}
Rename this to getdeltapkgname to clarify it is only used for deltas? Self document function names make my head hurt less.
Sorry, I had exactly the same thought, I just didn't know where to add delta :) I will use your suggestion.
Otherwise, a quick read of the other changes made sense to me, but I will need to do this in more detail.
This is probably the patch that requires the most careful review (compared to the other 4), so it would be highly appreciated.
OK. I will pull your patches into a git branch to remind myself to go through these in a lot of detail sometime in the next week or so. Do you have a git home now that Dan moved his server or do I need to apply them from the mailing list? shining.toofishes.net does not seem to exist anymore. Did you test whether creating a repo db from (e.g.) your cache gave the same result with the new and old version. I don't remember anything that should change just adding packages.
One comment that is moderately related to this. Should there be a mechanism to inspect the deltas are remove the ones that are no longer necessary? I.e. deltas with a broken chain, deltas who chain download will be bigger than the package. Should that be a job of repo-add when adding a new delta or a separate script?
I don't know, that seems neat but maybe too much complexity, I am not sure. Finding out how deltas could/should be managed has always been a big issue to me, how/when they are added, how/when they are removed.
I think deciding when they should be removed is the easiest part here: 1) when you can not get from a delta to the current package, it should be removed 2) when getting from a delta to the current package requires more download than just downloading the current package (or whatever criterion pacman uses - 80%?), it should be deleted. Perhaps this should be a separate script, or maybe a flag (repo-add --cleandelta)? Deciding when to add them is very political... Allan
On Thu, Feb 26, 2009 at 10:25 AM, Allan McRae <allan@archlinux.org> wrote:
Xavier wrote:
This is probably the patch that requires the most careful review (compared to the other 4), so it would be highly appreciated.
OK. I will pull your patches into a git branch to remind myself to go through these in a lot of detail sometime in the next week or so. Do you have a git home now that Dan moved his server or do I need to apply them from the mailing list? shining.toofishes.net does not seem to exist anymore.
I reworked and splitted this big repo-add patch in 4 patches : 0001-repo-add-print-warning-if-same-version-already-exis.patch 0004-repo-add.sh.in-repo-remove-improvements.patch 0005-repo-add-drop-delta-support-to-rewrite-it-from-scr.patch 0006-repo-add-rewrite-delta-support.patch I also added two patches : 0002-repo-add-fail-early-if-repo-can-not-be-created.patch 0003-repo-add-cleanup.patch These 6 patches are on my repoadd branch, I will send them to the list for review :)
Xavier wrote:
On Thu, Feb 26, 2009 at 10:25 AM, Allan McRae <allan@archlinux.org> wrote:
Xavier wrote:
This is probably the patch that requires the most careful review (compared to the other 4), so it would be highly appreciated.
OK. I will pull your patches into a git branch to remind myself to go through these in a lot of detail sometime in the next week or so. Do you have a git home now that Dan moved his server or do I need to apply them from the mailing list? shining.toofishes.net does not seem to exist anymore.
I reworked and splitted this big repo-add patch in 4 patches : 0001-repo-add-print-warning-if-same-version-already-exis.patch 0004-repo-add.sh.in-repo-remove-improvements.patch 0005-repo-add-drop-delta-support-to-rewrite-it-from-scr.patch 0006-repo-add-rewrite-delta-support.patch
I also added two patches : 0002-repo-add-fail-early-if-repo-can-not-be-created.patch 0003-repo-add-cleanup.patch
These 6 patches are on my repoadd branch, I will send them to the list for review :)
You could have branched the repoadd branch from your working branch so I could get all patches in one go. But combining the two has improved my git-fu so I graciously accept the lesson you were trying to provide me... :) Anyway, your patches are a lot easier for me to digest at this size. I will post any comments later. Allan
On Fri, Feb 27, 2009 at 12:00 AM, Allan McRae <allan@archlinux.org> wrote:
You could have branched the repoadd branch from your working branch so I could get all patches in one go. But combining the two has improved my git-fu so I graciously accept the lesson you were trying to provide me... :)
Hm indeed, it was useful to have a second branch temporarily, but when it was done, I could have merged everything together. Actually I did for my own use, so that I could test everything together :P I guess I will put back everything on the working branch tomorrow.
Anyway, your patches are a lot easier for me to digest at this size. I will post any comments later.
Cool :) I am also much happier about them.
On Thu, Feb 26, 2009 at 10:25 AM, Allan McRae <allan@archlinux.org> wrote:
I think deciding when they should be removed is the easiest part here: 1) when you can not get from a delta to the current package, it should be removed 2) when getting from a delta to the current package requires more download than just downloading the current package (or whatever criterion pacman uses - 80%?), it should be deleted. Perhaps this should be a separate script, or maybe a flag (repo-add --cleandelta)?
What is a bit disappointing is that this means reimplementing the logic we already have in pacman, but well. Btw, the delta sizes could not be more variable. The full range is obtained, just using my not so big cache : 0% - 100% (of the newest package) I stole and hacked a script from the forums to generate the numbers I wanted : http://bbs.archlinux.org/viewtopic.php?pid=433730#p433730 Thanks sabooky :) 0% : ghostscript-8.64-2_to_8.64-3-x86_64.delta 0% : ttf-bitstream-vera-1.10-5_to_1.10-6-x86_64.delta 0% : sound-theme-freedesktop-0.1-1_to_0.2-1-x86_64.delta 0% : libmad-0.15.1b-3_to_0.15.1b-4-x86_64.delta 0% : tdb-3.3.0-1_to_3.3.1-1-x86_64.delta 1% : libvorbis-1.2.1rc1-1_to_1.2.1rc1-2-x86_64.delta 2% : gnome-power-manager-2.24.2-2_to_2.24.4-1-x86_64.delta 2% : teeworlds-0.5.1-1_to_0.5.1-2-x86_64.delta 3% : ghostscript-8.64-1_to_8.64-2-x86_64.delta 3% : xextproto-7.0.4-1_to_7.0.5-1-x86_64.delta 4% : openjdk6-1.4-2_to_1.4.1-1-x86_64.delta 4% : xcb-proto-1.3-1_to_1.4-1-x86_64.delta 7% : tzdata-2009b-1_to_2009a-1-x86_64.delta 7% : alsa-utils-1.0.18-1_to_1.0.19-1-x86_64.delta 7% : groff-1.20.1-1_to_1.20.1-2-x86_64.delta 8% : xcb-util-0.3.2-1_to_0.3.3-1-x86_64.delta 8% : firefox-3.0.5-1_to_3.0.6-1-x86_64.delta 8% : pm-utils-1.2.3-4_to_1.2.4-1-x86_64.delta 9% : kernel26-2.6.28.3-1_to_2.6.28.4-1-x86_64.delta 9% : kernel26-2.6.28.5-1_to_2.6.28.6-1-x86_64.delta 9% : kernel26-2.6.28.6-1_to_2.6.28.7-1-x86_64.delta 10% : libxcb-1.1.93-1_to_1.2-1-x86_64.delta 10% : libxi-1.1.4-1_to_1.1.4-2-x86_64.delta 11% : kernel26-2.6.28.4-1_to_2.6.28.5-1-x86_64.delta 12% : xkeyboard-config-1.4-2_to_1.5-1-x86_64.delta 13% : xcb-proto-1.2-2_to_1.3-1-x86_64.delta 14% : libdrm-2.3.1-2_to_2.3.1-3-x86_64.delta 16% : gimp-2.6.4-2_to_2.6.5-1-x86_64.delta 16% : inputproto-1.4.4-1_to_1.5.0-1-x86_64.delta 18% : bluez-4.29-1_to_4.30-1-x86_64.delta 20% : evolution-data-server-2.24.3-1_to_2.24.4.1-1-x86_64.delta 23% : libxml2-2.7.3-1.1_to_2.7.2-1-x86_64.delta 24% : pciutils-3.0.3-1_to_3.1.2-1-x86_64.delta 27% : ntfs-3g-2009.1.1-1_to_2009.2.1-1-x86_64.delta 27% : flashplugin-10.0.d21.1-1_to_10.0.22.87-1-x86_64.delta 29% : libv4l-0.5.7-1_to_0.5.8-1-x86_64.delta 29% : gnutls-2.6.3-1_to_2.6.4-1-x86_64.delta 29% : man-pages-3.17-1_to_3.18-1-x86_64.delta 31% : ruby-1.8.7_p72-2_to_1.8.7_p72-3-x86_64.delta 33% : device-mapper-1.02.29-1_to_1.02.30-1-x86_64.delta 34% : libpng-1.2.34-1_to_1.2.35-1-x86_64.delta 37% : libx11-1.1.5-2_to_1.1.99.2-2-x86_64.delta 38% : gpm-1.20.5-2_to_1.20.6-1-x86_64.delta 40% : dhcpcd-4.0.7-1_to_4.0.10-1-x86_64.delta 41% : libx11-1.1.99.2-2_to_1.2-1-x86_64.delta 41% : xorg-xinit-1.1.0-1_to_1.1.1-1-x86_64.delta 42% : pixman-0.12.0-1_to_0.14.0-1-x86_64.delta 42% : hal-0.5.11-4_to_0.5.11-7-x86_64.delta 42% : tdb-3.2.7-1_to_3.3.0-1-x86_64.delta 46% : libxfont-1.3.4-1_to_1.4.0-1-x86_64.delta 46% : gsynaptics-0.9.14-2_to_0.9.15-1-x86_64.delta 49% : libmad-0.15.1b-2_to_0.15.1b-3-x86_64.delta 49% : xf86-input-evdev-2.1.0-1_to_2.1.2-1-x86_64.delta 49% : whois-4.7.28-2_to_4.7.30-1-x86_64.delta 49% : sqlite3-3.6.10-1_to_3.6.11-1-x86_64.delta 50% : gnome-mplayer-0.9.3-1_to_0.9.4-1-x86_64.delta 50% : perl-error-0.17011-1_to_0.17015-1-x86_64.delta 51% : xf86-input-synaptics-0.99.3-1_to_1.0.0-1-x86_64.delta 53% : man-db-2.5.2-2_to_2.5.4-1-x86_64.delta 54% : libxcb-1.1.90.1-1_to_1.1.93-1-x86_64.delta 56% : alsa-lib-1.0.18-1_to_1.0.19-1-x86_64.delta 56% : xf86-input-keyboard-1.3.1-1_to_1.3.2-1-x86_64.delta 58% : libtasn1-1.7-1_to_1.8-1-x86_64.delta 58% : lvm2-2.02.43-1_to_2.02.44-1-x86_64.delta 58% : xterm-239-1_to_241-1-x86_64.delta 59% : xfsprogs-2.10.2-1_to_3.0.0-1-x86_64.delta 60% : dnsutils-9.5.0.P2-1_to_9.6.0.P1-1-x86_64.delta 62% : libxi-1.1.4-2_to_1.2.0-1-x86_64.delta 65% : libxslt-1.1.24-1_to_1.1.24-2-x86_64.delta 65% : hdparm-9.6-1_to_9.10-1-x86_64.delta 67% : libcanberra-0.10-1_to_0.11-2-x86_64.delta 69% : libvorbis-1.2.1rc1-2_to_1.2.0-1-x86_64.delta 73% : smbclient-3.3.0-1_to_3.3.1-1-x86_64.delta 80% : grep-2.5.3-3_to_2.5.4-1-x86_64.delta 83% : smbclient-3.2.7-1_to_3.3.0-1-x86_64.delta 86% : libxext-1.0.4-1_to_1.0.5-1-x86_64.delta 88% : libice-1.0.4-1_to_1.0.5-1-x86_64.delta 89% : faac-1.26-1_to_1.28-1-x86_64.delta 96% : xulrunner-1.9.0.5-2_to_1.9.0.6-1-x86_64.delta 96% : gtk-engine-murrine-0.53.1-1_to_0.53.1-3-x86_64.delta 98% : libcroco-0.6.1-1_to_0.6.2-1-x86_64.delta 100% : pacman-git-20090226-1_to_20090227-2-x86_64.delta 101% : cabextract-1.2-1_to_1.2-2-x86_64.delta That's right, 101%, no error here. The delta is 32427 B while the package is 32045 B :D Since pacman indeed has a max ratio (#define MAX_DELTA_RATIO 0.7), it doesn't make any sense to put deltas bigger than 70% in a database, because pacman will never use them. Do we enforce this check somewhere? If yes where? In pkgdelta or in repo-add and/or in an external cleanup script?
On Wed, Feb 25, 2009 at 12:43 PM, Xavier Chantry <shiningxc@gmail.com> wrote:
The from_md5 and to_md5 fields were a nice extra safety, which would avoid trying to apply deltas on corrupted package files. However, they are not strictly necessary, since xdelta should be able to detect that on its own.
The main problem is that it is impossible to compute these informations from the delta only. So repo-add would not be able to compute the delta entry based on just the delta file.
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> Signed-off-by: Dan McGee <dan@archlinux.org>
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\n"); + _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search for '%s'\n", to);
vertices = delta_graph_init(deltas);
bestsize = delta_vert(vertices, to, to_md5, &bestpath);
- _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete\n"); + _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete : '%lld'\n", (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\n"), 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
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\n"); + _alpm_log(PM_LOG_DEBUG, "started delta shortest-path search for '%s'\n", to); vertices = delta_graph_init(deltas); bestsize = delta_vert(vertices, to, &bestpath); - _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete\n"); + _alpm_log(PM_LOG_DEBUG, "delta shortest-path search complete : '%jd'\n", (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\n", - (long long)size, alpm_pkg_get_name(newpkg)); + _alpm_log(PM_LOG_DEBUG, "setting download size %jd for pkg %s\n", + (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\n"), command); + _alpm_log(PM_LOG_DEBUG, "command: %s\n", 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
participants (5)
-
Allan McRae
-
Brendan Hide
-
Dan McGee
-
Xavier
-
Xavier Chantry