[pacman-dev] [PATCH] makepkg: fix check for previously built packages with package splitting

[Dan McGee]

On Wed, Jul 1, 2009 at 8:54 PM, Allan McRae<allan at archlinux.org> wrote:
> Dan McGee wrote:
>>
>> On Wed, Jul 1, 2009 at 2:36 AM, Allan McRae<allan at archlinux.org> wrote:
>>
>>>
>>> Checks if some or all packages are built before overwriting/installing.
>>> Adds two new translation strings
>>>
>>> Signed-off-by: Allan McRae <allan at archlinux.org>
>>> ---
>>>  scripts/makepkg.sh.in |   41 ++++++++++++++++++++++++++++++++---------
>>>  1 files changed, 32 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>>> index 2bb7eb9..61ae037 100644
>>> --- a/scripts/makepkg.sh.in
>>> +++ b/scripts/makepkg.sh.in
>>> @@ -1646,15 +1646,38 @@ if [ "${#pkgname[@]}" -gt "1" ]; then
>>>       SPLITPKG=1
>>>  fi
>>>
>>> -if [ -f "$PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}" \
>>> -     -a "$FORCE" -eq 0 -a "$SOURCEONLY" -eq 0 -a "$NOBUILD" -eq 0 ];
>>> then
>>> -       if [ "$INSTALL" -eq 1 ]; then
>>> -               warning "$(gettext "A package has already been built,
>>> installing existing package...")"
>>> -               install_package
>>> -               exit $?
>>> -       else
>>> -               error "$(gettext "A package has already been built. (use
>>> -f to overwrite)")"
>>> -               exit 1
>>> +if [ "$SPLITPKG" -eq 0 ]; then
>>> +       if [ -f
>>> "$PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}" \
>>> +                       -a "$FORCE" -eq 0 -a "$SOURCEONLY" -eq 0 -a
>>> "$NOBUILD" -eq 0 ]; then
>>> +               if [ "$INSTALL" -eq 1 ]; then
>>> +                       warning "$(gettext "A package has already been
>>> built, installing existing package...")"
>>> +                       install_package
>>> +                       exit $?
>>> +               else
>>> +                       error "$(gettext "A package has already been
>>> built. (use -f to overwrite)")"
>>> +                       exit 1
>>> +               fi
>>> +       fi
>>> +else
>>> +       local allpkgbuilt=1
>>> +       local somepkgbuilt=0
>>>
>>
>> Declared local which doesn't make sense outside of a bash function.
>> You probably want to just leave them as plain variables here and add
>> an "unset" at the end of the outer else/fi block.
>>
>>
>
> Crap, I removed the local and added the unset on the version of makepkg-git
> running on my system but not here...  Declaring local outside a function in
> bash does not actually work at all.
>>>
>>> +       for pkg in ${pkgname[@]}; do
>>> +               if [ -f
>>> "$PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}" ]; then
>>>
>>
>> $pkgver/$pkgrel can't be overridden in subpackages, correct? Otherwise
>> this line will not work right.
>>
>
> Correct.
>>
>>
>>>
>>> +                       somepkgbuilt=1
>>> +               else
>>> +                       allpkgbuilt=0
>>> +               fi
>>> +       done
>>> +       if [ -f "$FORCE" -eq 0 -a "$SOURCEONLY" -eq 0 -a "$NOBUILD" -eq 0
>>> ]; then
>>> +               if [ "$INSTALL" -eq 1 -a "$allpkgbuilt" -eq 1 ]; then
>>> +                       warning "$(gettext "The package group has already
>>> been built, installing existing packages...")"
>>> +                       install_package
>>> +                       exit $?
>>> +               fi
>>> +               if [ "$somepkgbuilt" -eq 1 ]; then
>>> +                       error "$(gettext "The package group has already
>>> been (partially) built. (use -f to overwrite)")"
>>>
>>
>> Awkward English text, trying to think of something better...I'd at
>> least drop the () around partially, maybe something like:
>> "Part of the package group has already been built. (use -f to overwrite)"
>>
>
> The problem is it could be all of the package group, or just part of it. I
> found this whole patch awkward...

You have somepkgbuilt and allpkgbuilt; do a different message for each case?

-Dan