Re: [PATCH 1/4] lib/install.sh: fix shell adapt_packages()

[Ye Xiaolong <xiaolong.ye@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]

Reviewed-by: Ye Xiaolong <xiaolong.ye@intel.com>

Thanks,
Xiaolong

On 01/06, Luis R. Rodriguez wrote:
>There are only a few packages with architecture specific
>annotations right now, they all use a :i386 postfix. The
>current ruby adapt_packages() works well, however on shell,
>the distro_pkg=${mapping#$pkg:} does not work as well as
>Ruby's array use and find. Furthermore the bash implementation
>allows a final grep to end up giving you more than one package,
>so if you have libc-dev but a distro mapping file has a mapping
>for both libc-dev and libc-dev:i386 we'd end up with both
>mappings. Lastly the mapping for architectures simply does
>not work...
>
>Fix this for now by modifying the shell implementation of
>adapt_packages() to check if a package we are going to
>query is architecture specific or not, and as a general
>rule for now just require non-arch packages to go first,
>while arch packages go last. The awk query is much better
>and will do what we expected.
>
>Long term we should just modify the distro/adaptation/*
>files to use a delimiter which can never be part of a package
>name, for instance using the pipe "|". That would allow us to
>get rid of this small cruft on the bash implementation and
>enable both bash and ruby implementations to match much closely.
>
>Since we are sticking to a rule for now of having the arch
>packages go last modify the few archs which do have these
>mappings to match this rule.
>
>Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
>---
> distro/adaptation/archlinux |  2 +-
> distro/adaptation/fedora    |  2 +-
> distro/depends/lkp-dev      |  4 ++++
> lib/install.sh              | 10 ++++++++--
> 4 files changed, 14 insertions(+), 4 deletions(-)
>
>diff --git a/distro/adaptation/archlinux b/distro/adaptation/archlinux
>index dd8b4ab5967f..0a4ecd5c6d4d 100644
>--- a/distro/adaptation/archlinux
>+++ b/distro/adaptation/archlinux
>@@ -66,6 +66,6 @@ xfslibs-dev: xfsprogs
> libattr1-dev: attr
> libacl1-dev: acl
> libc-dev:i386:
>-linux-libc-dev:i386:
> linux-libc-dev:
>+linux-libc-dev:i386:
> libklibc-dev:
>diff --git a/distro/adaptation/fedora b/distro/adaptation/fedora
>index a56ed9d0b870..b3ad3fbcdf20 100644
>--- a/distro/adaptation/fedora
>+++ b/distro/adaptation/fedora
>@@ -69,6 +69,6 @@ libreadline5:
> libtool-bin:
> libnuma-dev: numactl-devel
> libc-dev:i386:
>-linux-libc-dev:i386:
> linux-libc-dev:
>+linux-libc-dev:i386:
> libklibc-dev:
>diff --git a/distro/depends/lkp-dev b/distro/depends/lkp-dev
>index 70f3c731787e..f3f27f73a965 100644
>--- a/distro/depends/lkp-dev
>+++ b/distro/depends/lkp-dev
>@@ -1,3 +1,7 @@
>+# Put architecture packages after the non architecture specific
>+# package. We can change this requiremet if we modify
>+# distro/adaptation/* files to use something uniq that is not
>+# part of real packages, like: |
> make
> automake
> gcc
>diff --git a/lib/install.sh b/lib/install.sh
>index d4f8c1feab37..0957f13c9937 100755
>--- a/lib/install.sh
>+++ b/lib/install.sh
>@@ -33,9 +33,15 @@ adapt_packages()
> 
> 	for pkg in $generic_packages
> 	do
>-		local mapping="$(grep "^$pkg:" $distro_file)"
>+		local mapping=""
>+		local is_arch_dep="$(echo $pkg | grep ":")"
>+		if [ -n "$is_arch_dep" ]; then
>+			mapping="$(grep "^$pkg:" $distro_file | tail -1)"
>+		else
>+			mapping="$(grep "^$pkg:" $distro_file | head -1)"
>+		fi
> 		if [ -n "$mapping" ]; then
>-			distro_pkg=${mapping#$pkg:}
>+			distro_pkg=$(echo $mapping | awk -F": " '{print $2}')
> 			[ -n "$distro_pkg" ] && echo $distro_pkg
> 		else
> 			echo $pkg
>-- 
>2.11.0
>