Date: Mon, 29 Apr 2013 10:57:45 +0200
From: Natanael Copa <ncopa@alpinelinux.org>
To: Nathan Angelacos <nangel@alpinelinux.org>
Cc: alpine-devel@lists.alpinelinux.org
Subject: Re: [alpine-devel] [PATCH] main/kamailio:
Message-ID: <20130429105745.259b2b78@ncopa-desktop.alpinelinux.org>
In-Reply-To: <1366906389-30635-1-git-send-email-nangel@alpinelinux.org>
References: <1366906389-30635-1-git-send-email-nangel@alpinelinux.org>
Precedence: list
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

I should have reviewed this earlier...

On Thu, 25 Apr 2013 16:13:09 +0000
Nathan Angelacos <nangel@alpinelinux.org> wrote:

> - upgrade to 4.0.1
> - add kamailio.groups file to build packages similar to other distros
> - add additional packages to the Alpine set (e.g. ldap, snmpstats)
> ---
>  main/kamailio/APKBUILD        | 215 +++++++++++++++--------------
>  main/kamailio/kamailio.groups | 305 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 423 insertions(+), 97 deletions(-)
>  create mode 100644 main/kamailio/kamailio.groups
> 
> diff --git a/main/kamailio/APKBUILD b/main/kamailio/APKBUILD
> index 797240c..13b03c6 100644
> --- a/main/kamailio/APKBUILD
> +++ b/main/kamailio/APKBUILD
> @@ -1,9 +1,9 @@

....

> +# Source the kamailio.groups file for the definition of all the various modules
> +. ./kamailio.groups

I wonder if we could maintain this list inlined in the APKBUILD? eg, in vim, do
 
  :r kamailio.groups

There are some scripts (like abuild -R) that does things like:

# list all build .apk files
for i in */APKBUILD; do
	# source the apkbuild
	. ./$i
	for pkg in $pkgname $subpackages; do
		echo "${pkg}-${pkgver}-r${pkgrel}.apk"
	done
done

This will fail now because the APKBUILD currently requires that the working directory is same as the dir where the kamailio.groups is in.

You could do:

if [ -f ./kamailio.groups ]; then
	. ./kamailio.groups
fi

but then again, the above code snippet will break as the $subpackages will not be complete.

> +subpackages="$pkgname-doc"
> +_modules="$module_group_kstandard"
> +for i in db postgres sqlite dbtext mysql \

I will change this to "for _i ...". We need prefix variables in global scope with _. The above code snippet will break if it uses $i after sourcing the APKBUILD.

> +    cpl xml unixodbc snmpstats xmpp carrierroute \
> +    ldap utils tls presence lua ims outbound dbg \
> +    extra; do
> +   
> +   subpackages="$subpackages $pkgname-$i"
> +   _modules="$_modules `eval echo $\module_group_k$i`"

This I will change to:

  eval "_modules=\"\$_modules \$module_group_k$_i\""

For the simple reason to avoid forks in global scope. When there are many enough forks in the APKBUILDs, sourcing them all as in the loop example above will become significantly slower.

-nc


---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---