[pacman-dev] Fwd: [PATCH] repo-add: fix eval and quote issues

[Xavier]

On Sun, Feb 8, 2009 at 7:31 PM, Dan McGee <dan at archlinux.org> wrote:
> Yes, I think I sent this to myself on accident. Looks like it is time
> for another cup of coffee.
>
> ---------- Forwarded message ----------
> From: Dan McGee <dan at archlinux.org>
> Date: Sun, Feb 8, 2009 at 12:30 PM
> Subject: Re: [PATCH] repo-add: fix eval and quote issues
> To: Dan McGee <dan at archlinux.org>
>
> On Sun, Feb 8, 2009 at 12:22 PM, Dan McGee <dan at archlinux.org> wrote:
>> eval was ugly and dirty, and bit us here. Instead, use a safer form of
>> variable declaration to ensure quotes don't foil us in pkgdesc or any other
>> fields.
>>
>> This fixes FS#10837.
>>
>> Signed-off-by: Dan McGee <dan at archlinux.org>
>> ---
> We apparently had never handled quotes in descriptions correctly, so
> this patch should fix these issues. I'm going to push this patch to
> maint if there are no objections, although I expect our next release
> will come from master.
>
>>  scripts/repo-add.sh.in |   25 +++++++++++++------------
>>  1 files changed, 13 insertions(+), 12 deletions(-)
>>
>> diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
>> index be0859e..93fdd52 100644
>> --- a/scripts/repo-add.sh.in
>> +++ b/scripts/repo-add.sh.in
>> @@ -152,18 +152,19 @@ db_write_entry()
>>
>>        # read info from the zipped package
>>        local line
> I did make one small fix here- added var and val to the local declaration.
>> -       for line in $(bsdtar -xOf "$pkgfile" .PKGINFO | \
>> -               grep -v "^#" | sed 's|\(\w*\)\s*=\s*\(.*\)|\1="\2"|'); do
>> -               eval "$line"
>> -               case "$line" in
>> -                       group=*)    _groups="$_groups$group\n" ;;
>> -                       depend=*)   _depends="$_depends$depend\n" ;;
>> -                       backup=*)   _backups="$_backups$backup\n" ;;
>> -                       license=*)  _licenses="$_licenses$license\n" ;;
>> -                       replaces=*) _replaces="$_replaces$replaces\n" ;;
>> -                       provides=*) _provides="$_provides$provides\n" ;;
>> -                       conflict=*) _conflicts="$_conflicts$conflict\n" ;;
>> -                       optdepend=*) _optdepends="$_optdepends$optdepend\n" ;;
>> +       for line in $(bsdtar -xOf "$pkgfile" .PKGINFO | grep -v '^#'); do
>> +               var="$(echo $line | sed 's|\(\w*\)\s*=\s*\(.*\)|\1|')"
>> +               val="$(echo $line | sed 's|\(\w*\)\s*=\s*\(.*\)|\2|')"
>> +               declare $var="$val"
>> +               case "$var" in
>> +                       group)    _groups="$_groups$group\n" ;;
>> +                       depend)   _depends="$_depends$depend\n" ;;
>> +                       backup)   _backups="$_backups$backup\n" ;;
>> +                       license)  _licenses="$_licenses$license\n" ;;
>> +                       replaces) _replaces="$_replaces$replaces\n" ;;
>> +                       provides) _provides="$_provides$provides\n" ;;
>> +                       conflict) _conflicts="$_conflicts$conflict\n" ;;
>> +                       optdepend) _optdepends="$_optdepends$optdepend\n" ;;
>>                esac
>>        done
>>

For testing, I added all the packages in my cache to a database with
and without this patch, and the resulting databases are identical so
we should be safe.
However, we have twice as many forks as before now, so the time went
from 1:36 to 3:14 (for adding 493 packages).
I don't know if we should care or not, I guess we usually add a small
numbers of packages. If we care, we should have a look at all the
slowest operations in repo-add.