[pacman-dev] [PATCH 4/4] makepkg: Fixed sourceball creation of split packages

[Allan McRae]

Eric Bélanger wrote:
> On Fri, Nov 6, 2009 at 10:36 AM, Allan McRae <allan at archlinux.org> wrote:
>> Eric Bélanger wrote:
>>> In some case, the install script of split packages was not included in the
>>> sourceballs. This was due because split packages can specify their install
>>> script in the package functions.  They also can use several install scripts.
>>>
>> Please add newlines to the commit message to keep them ~80 characters wide.
> 
> OK.  I'll try to remember next time.
> 
>> Note that this will need done for changelogs too as they are now handled
>> like install files.
> 
> ChangeLog are not  included in sourceball   even for non split
> package: $changelog doesn't get defined.  This will need to be fixed.
> We can  test if there is a ChangeLog file in $startdir and if so we
> include it. I could make a patch.
> 

On the master branch, you are required to add "changelog=<file>" to 
include a changelog in your package.  That was added to allow split 
packages to have different changelogs in different packages.


>>> Signed-off-by: Eric Bélanger <snowmaniscool at gmail.com>
>>> ---
>>>  scripts/makepkg.sh.in |   25 ++++++++++++++++++-------
>>>  1 files changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>>> index 8fc7c82..b77cc68 100644
>>> --- a/scripts/makepkg.sh.in
>>> +++ b/scripts/makepkg.sh.in
>>> @@ -1060,14 +1060,25 @@ create_srcpackage() {
>>>        msg2 "$(gettext "Adding %s...")" "$BUILDSCRIPT"
>>>        ln -s "${BUILDFILE}" "${srclinks}/${pkgbase}/${BUILDSCRIPT}"
>>>  -       if [ -n "$install" ]; then
>>> -               if [ -f $install ]; then
>>> -                       msg2 "$(gettext "Adding install script...")"
>>> -                       ln -s "${startdir}/$install"
>>> "${srclinks}/${pkgbase}/"
>>> -               else
>>> -                       error "$(gettext "Install scriptlet (%s) does not
>>> exist.")" "$install"
>>> +       local install_files
>>> +       install_files=$(grep "install=" "$BUILDSCRIPT")
>>> +       for pkg in ${pkgname[@]}; do
>>> +               install_files+=' '
>>> +               install_files+=$(echo $install_files |sed
>>> "s/\$pkgname/$pkg/"|sed "s/\${pkgname}/$pkg/")
>>> +               install_files=$(eval echo $install_files |tr '[:blank:]'
>>> '\n'|sort |uniq)
>>> +       done
>> This is too inclusive.  If I have one package with an install file
>> $pkgname.install, then it will add an install file $pkg.install for every
>> $pkg in $pkgname.  Some of these may not exist which will lead to trouble
>> below when the files presence is tested for.
> 
> I copied this from makechrootpkg.  I just tested it when a package in
> a split package doesn't use the install script and it works.

So, some background to my comment.

With split packages (currently) $pkgname within the package_foo() 
functions is just the first element of the ${pkgname[@]} array.  This 
may be quite different than expected and I think there is a bug report 
to allow the use of $pkgname within those functions and have it actually 
provide the name of the package being packaged.

Currently, that means you can skip the whole "for pkg in" loop and just 
do a single sed as $pkgname will always be the first element in the array.

However, if having $pkgname equal to "foo" within the "package_foo()" 
functions is implemented, you could have:

pkgname=('foo' 'bar')

package_foo() {
install=$pkgname.install
}

package_bar() {
#install=none
}

With your patch you would end up with:
install_files=('foo.install' 'bar.install')
which is wrong and would error out on missing bar.install.

So, there is currently too much looping going on here but in the future 
this may break completely.

As I said, I am not sure what the fix is here.

Allan