[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
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
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
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
The create-xdelta script can be used instead.
Signed-off-by: Xavier Chantry
Xavier Chantry wrote:
The create-xdelta script can be used instead.
Signed-off-by: Xavier Chantry
---
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
Xavier Chantry wrote:
The create-xdelta script can be used instead.
Signed-off-by: Xavier Chantry
--- 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
---
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>
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
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
--- 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>
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
--- 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>
...\n")" + printf "$(gettext "Usage: repo-remove [-q] <path-to-db> ...\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
+# 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
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
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
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
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
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
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
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
Signed-off-by: Dan McGee
On Wed, Feb 25, 2009 at 12:43 PM, Xavier Chantry
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
--- 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
participants (5)
-
Allan McRae
-
Brendan Hide
-
Dan McGee
-
Xavier
-
Xavier Chantry