[pacman-dev] [PATCH] makepkg: rework libdepends

[Allan McRae]

On 19/05/12 23:44, Dave Reisner wrote:
> On Sat, May 19, 2012 at 11:22:36PM +1000, Allan McRae wrote:
>> Rewrite the handling of libdepends. The primary advantage are:
>>  - Moves functionality from write_pkginfo() to find_libdepends().
>>  - The order of the depends array in the PKGBUILD is kept in the package.
>>  - An unneeded libdepends is only a warning and not an error. This allows
>>    putting a libdepend on a library that is dlopened.
>>  - It is now modular so can be extended to library types other than
>>    ELF *.so.
>>  - Finding the list of libraries a package depends only occurs when a
>>    libdepend is specified in the depends array.
>>
>> Signed-off-by: Allan McRae <allan at archlinux.org>
>> ---
> 
> Without having tested this too thoroughly, this gets a +1 from me.
> There's two small nitpicks left inline.
> 
>>  scripts/makepkg.sh.in |  126 +++++++++++++++++++++++++++++--------------------
>>  1 file changed, 76 insertions(+), 50 deletions(-)
>>
>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>> index 8d018c0..8930ec4 100644
>> --- a/scripts/makepkg.sh.in
>> +++ b/scripts/makepkg.sh.in
>> @@ -1131,32 +1131,76 @@ tidy_install() {
>>  }
>>  
>>  find_libdepends() {
>> -	local libdepends
>> -	find "$pkgdir" -type f -perm -u+x | while read filename
>> -	do
>> -		# get architecture of the file; if soarch is empty it's not an ELF binary
>> -		soarch=$(LC_ALL=C readelf -h "$filename" 2>/dev/null | sed -n 's/.*Class.*ELF\(32\|64\)/\1/p')
>> -		[ -n "$soarch" ] || continue
>> -		# process all libraries needed by the binary
>> -		for sofile in $(LC_ALL=C readelf -d "$filename" 2>/dev/null | sed -nr 's/.*Shared library: \[(.*)\].*/\1/p')
>> -		do
>> -			# extract the library name: libfoo.so
>> -			soname="${sofile%.so?(+(.+([0-9])))}".so
>> -			# extract the major version: 1
>> -			soversion="${sofile##*\.so\.}"
>> -			if in_array "${soname}" ${depends[@]}; then
>> -				if ! in_array "${soname}=${soversion}-${soarch}" ${libdepends[@]}; then
>> -					# libfoo.so=1-64
>> -					printf "%s" "${soname}=${soversion}-${soarch}"
>> -					libdepends+=("${soname}=${soversion}-${soarch}")
>> +	local d sodepends;
>> +
>> +	sodepends=0;
>> +	for d in "${depends[@]}"; do
>> +		if [[ $d = *.so ]]; then
>> +			sodepends=1;
>> +			break;
>> +		fi
>> +	done
>> +
>> +	if (( sodepends == 0 )); then
>> +		printf '%s\n' "${depends[@]}"
>> +		return;
>> +	fi
>> +
>> +	local libdeps;
>> +	declare -A libdeps;
>> +
>> +	if (( sodepends == 1 )); then
> 
> You've already checked that $sodepends isn't 0. Can't you save a whole
> level of indenting here?

I was doing that test so allow the libdeps array to be filled by
different methods if someone steps up to implement this on an non-ELF
system.  Probably an absolute waste as that will likely never happen...
  Removed on my working branch.

>> +		local filename soarch sofile soname soversion
>> +
>> +		while read filename; do
>> +			# get architecture of the file; if soarch is empty it's not an ELF binary
>> +			soarch=$(LC_ALL=C readelf -h "$filename" 2>/dev/null | sed -n 's/.*Class.*ELF\(32\|64\)/\1/p')
>> +			[[ -n "$soarch" ]] || continue
>> +
>> +			# process all libraries needed by the binary
>> +			for sofile in $(LC_ALL=C readelf -d "$filename" 2>/dev/null | sed -nr 's/.*Shared library: \[(.*)\].*/\1/p')
>> +			do
>> +				# extract the library name: libfoo.so
>> +				soname="${sofile%.so?(+(.+([0-9])))}".so
>> +				# extract the major version: 1
>> +				soversion="${sofile##*\.so\.}"
>> +
>> +				if [[ ${libdeps[$soname]} ]]; then
>> +					if [[ ! ${libdeps[$soname]} = *${soversion}-${soarch}* ]]; then
> 
> nitpick: "$foo != $bar" instead of "! $foo = $bar"

Fixed.


Allan