[Buildroot] [PATCH 0/2] support/check-bin-arch: add support for excluding arbitrary locations

[Buildroot] [PATCH 0/2] support/check-bin-arch: add support for excluding arbitrary locations

[Yann E. MORIN]

Hello All!

Users have started reporting issues with this check, when they have
local packages (mostly out-of-tree, proprietary) that install binary
blobs (like co-processor firmwares) in arbitrary locations, like
/opt, which we currently do not exclude from the check.


Regards,
Yann E. MORIN.


The following changes since commit 74295b02d4b380e5267357be0ae1281c7410cdc2

  ipset: bump to version 6.36 (2018-03-06 15:44:18 +0100)


are available in the git repository at:

  git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to ce786b49c9bbe6a3e2ebde9ca28b56de86e68b2e

  support/check-bin-arch: exclude kernel modules for merged /usr (2018-03-07 21:17:11 +0100)


----------------------------------------------------------------
Yann E. MORIN (2):
      spport/check-bin-arch: accept arbitrary ignore paths
      support/check-bin-arch: exclude kernel modules for merged /usr

 docs/manual/adding-packages-generic.txt |  7 ++++
 package/pkg-generic.mk                  |  1 +
 support/scripts/check-bin-arch          | 59 +++++++++++++++++++++------------
 3 files changed, 45 insertions(+), 22 deletions(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/2] spport/check-bin-arch: accept arbitrary ignore paths

[Yann E. MORIN]

Some packages (mostly, out-of-tree) may want to install binary blobs for
another architecture,  outside the locations we currently exclude, like
in /opt or whatever...

Add support in check-bin-arch to accept any arbitrary location, that
individual package can each request to excude from the check, when they
are installed.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 docs/manual/adding-packages-generic.txt |  7 ++++
 package/pkg-generic.mk                  |  1 +
 support/scripts/check-bin-arch          | 58 ++++++++++++++++++++-------------
 3 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 63ea51bf89..1f5d92d2c9 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -453,6 +453,13 @@ information is (assuming the package name is +libfoo+) :
   FLAT binary format is only 4k bytes. If the application consumes more stack,
   append the required number here.
 
+* +LIBFOO_BIN_ARCH_EXCLUDE+ is a space-separated list of path (relative
+  to the target directory) to ignore when checking that the package
+  installs correctly cross-compiled binaries. You seldom need to set this
+  variable, unless the package installs installs binary blobs in the
+  non-default locations: `/lib/firmware`, `/usr/lib/firmware`,
+  `/lib/modules`, and `/usr/share`.
+
 The recommended way to define these variables is to use the following
 syntax:
 
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index a2a12e7b56..9eddaeee57 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -112,6 +112,7 @@ define check_bin_arch
 	$(if $(filter end-install-target,$(1)-$(2)),\
 		support/scripts/check-bin-arch -p $(3) \
 			-l $(BUILD_DIR)/packages-file-list.txt \
+			$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
 			-r $(TARGET_READELF) \
 			-a $(BR2_READELF_ARCH_NAME))
 endef
diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
index f6a4569c62..922791ab89 100755
--- a/support/scripts/check-bin-arch
+++ b/support/scripts/check-bin-arch
@@ -1,18 +1,47 @@
 #!/usr/bin/env bash
 
-while getopts p:l:r:a: OPT ; do
+# List of hardcoded paths that should be ignored, as they may
+# contain binaries for an architecture different from the
+# architecture of the target.
+declare -a IGNORES=(
+	# Skip firmware files, they could be ELF files for other
+	# architectures
+	"/lib/firmware"
+	"/usr/lib/firmware"
+
+	# Skip kernel modules
+	# When building a 32-bit userland on 64-bit architectures, the kernel
+	# and its modules may still be 64-bit. To keep the basic
+	# check-bin-arch logic simple, just skip this directory.
+	"/lib/modules"
+
+	# Skip files in /usr/share, several packages (qemu,
+	# pru-software-support) legitimately install ELF binaries that
+	# are not for the target architecture
+	"/usr/share"
+)
+
+while getopts p:l:r:a:i: OPT ; do
 	case "${OPT}" in
 	p) package="${OPTARG}";;
 	l) pkg_list="${OPTARG}";;
 	r) readelf="${OPTARG}";;
 	a) arch_name="${OPTARG}";;
+	i)
+		# Ensure we do have single '/' as separators,
+		# and that we have a leading and a trailing one.
+		IGNORES+=( "$(sed -r -e 's:/+:/:g; s:^/*:/:; s:/*$:/:;' \
+				  <<<"${OPTARG}"
+			     )"
+			 )
+		;;
 	:) error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
 	\?) error "unknown option '%s'\n" "${OPTARG}";;
 	esac
 done
 
 if test -z "${package}" -o -z "${pkg_list}" -o -z "${readelf}" -o -z "${arch_name}" ; then
-	echo "Usage: $0 -p <pkg> -l <pkg-file-list> -r <readelf> -a <arch name>"
+	echo "Usage: $0 -p <pkg> -l <pkg-file-list> -r <readelf> -a <arch name> [-i PATTERN ...]"
 	exit 1
 fi
 
@@ -23,26 +52,11 @@ IFS="
 "
 
 while read f; do
-	# Skip firmware files, they could be ELF files for other
-	# architectures
-	if [[ "${f}" =~ ^/(usr/)?lib/firmware/.* ]]; then
-		continue
-	fi
-
-	# Skip kernel modules
-	# When building a 32-bit userland on 64-bit architectures, the kernel
-	# and its modules may still be 64-bit. To keep the basic
-	# check-bin-arch logic simple, just skip this directory.
-	if [[ "${f}" =~ ^/lib/modules/.* ]]; then
-		continue
-	fi
-
-	# Skip files in /usr/share, several packages (qemu,
-	# pru-software-support) legitimately install ELF binaries that
-	# are not for the target architecture
-	if [[ "${f}" =~ ^/usr/share/.* ]]; then
-		continue
-	fi
+	for ignore in "${IGNORES[@]}"; do
+		if [[ "${f}" =~ ^"${ignore}" ]]; then
+			continue 2
+		fi
+	done
 
 	# Skip symlinks. Some symlinks may have absolute paths as
 	# target, pointing to host binaries while we're building.
-- 
2.14.1

[Buildroot] [PATCH 1/2] spport/check-bin-arch: accept arbitrary ignore paths

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Some packages (mostly, out-of-tree) may want to install binary blobs for
 > another architecture,  outside the locations we currently exclude, like
 > in /opt or whatever...

 > Add support in check-bin-arch to accept any arbitrary location, that
 > individual package can each request to excude from the check, when they
 > are installed.

 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > Cc: Peter Korsgaard <peter@korsgaard.com>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > ---
 >  docs/manual/adding-packages-generic.txt |  7 ++++
 >  package/pkg-generic.mk                  |  1 +
 >  support/scripts/check-bin-arch          | 58 ++++++++++++++++++++-------------
 >  3 files changed, 44 insertions(+), 22 deletions(-)

 > diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
 > index 63ea51bf89..1f5d92d2c9 100644
 > --- a/docs/manual/adding-packages-generic.txt
 > +++ b/docs/manual/adding-packages-generic.txt
 > @@ -453,6 +453,13 @@ information is (assuming the package name is +libfoo+) :
 >    FLAT binary format is only 4k bytes. If the application consumes more stack,
 >    append the required number here.
 
 > +* +LIBFOO_BIN_ARCH_EXCLUDE+ is a space-separated list of path (relative

s/path/paths/

> +  to the target directory) to ignore when checking that the package
 > +  installs correctly cross-compiled binaries. You seldom need to set this
 > +  variable, unless the package installs installs binary blobs in the

s/installs installs/installs/

 > +  non-default locations: `/lib/firmware`, `/usr/lib/firmware`,
 > +  `/lib/modules`, and `/usr/share`.

This sounds a bit confusing to me, E.G. like these are non default
locations.

Perhaps we should instead write:

variable, unless the package installs binary blobs outside the default
locations, which are `/lib/firmware`, `/usr/lib/firmware`,
`/lib/modules` and '/usr/share`, as these locations are automatically
excluded.


 >  if test -z "${package}" -o -z "${pkg_list}" -o -z "${readelf}" -o -z "${arch_name}" ; then
 > -	echo "Usage: $0 -p <pkg> -l <pkg-file-list> -r <readelf> -a <arch name>"
 > +	echo "Usage: $0 -p <pkg> -l <pkg-file-list> -r <readelf> -a <arch name> [-i PATTERN ...]"

We don't really document this as a pattern (even if it gets passed to ~=
in the end), so perhaps it would be clearer to say [ -i PATH ]?

Otherwise it looks good to me, thanks!

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] spport/check-bin-arch: accept arbitrary ignore paths

[Yann E. MORIN]

Peter, All,

On 2018-03-07 21:45 +0100, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
[--SNIP--]
>  > +* +LIBFOO_BIN_ARCH_EXCLUDE+ is a space-separated list of path (relative
> s/path/paths/
> > +  to the target directory) to ignore when checking that the package
>  > +  installs correctly cross-compiled binaries. You seldom need to set this
>  > +  variable, unless the package installs installs binary blobs in the
> s/installs installs/installs/

Typoes fixed.

>  > +  non-default locations: `/lib/firmware`, `/usr/lib/firmware`,
>  > +  `/lib/modules`, and `/usr/share`.
> 
> This sounds a bit confusing to me, E.G. like these are non default
> locations.
> 
> Perhaps we should instead write:
> 
> variable, unless the package installs binary blobs outside the default
> locations, which are `/lib/firmware`, `/usr/lib/firmware`,
> `/lib/modules` and '/usr/share`, as these locations are automatically
> excluded.

Indeed, it was not clear (except maybe just in my twisted head). I've
rephrased slightly differently from your suggestion, but if you don;t
like it, we can go with yours instead.

>  >  if test -z "${package}" -o -z "${pkg_list}" -o -z "${readelf}" -o -z "${arch_name}" ; then
>  > -	echo "Usage: $0 -p <pkg> -l <pkg-file-list> -r <readelf> -a <arch name>"
>  > +	echo "Usage: $0 -p <pkg> -l <pkg-file-list> -r <readelf> -a <arch name> [-i PATTERN ...]"
> 
> We don't really document this as a pattern (even if it gets passed to ~=
> in the end), so perhaps it would be clearer to say [ -i PATH ]?

ACK.

> Otherwise it looks good to me, thanks!

Thanks! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/2] support/check-bin-arch: exclude kernel modules for merged /usr

[Yann E. MORIN]

When using a merged /usr, the kernel module path is really
/usr/lib/modules, as /lib is a symlink to usr/lib .

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 support/scripts/check-bin-arch | 1 +
 1 file changed, 1 insertion(+)

diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
index 922791ab89..258014b98f 100755
--- a/support/scripts/check-bin-arch
+++ b/support/scripts/check-bin-arch
@@ -14,6 +14,7 @@ declare -a IGNORES=(
 	# and its modules may still be 64-bit. To keep the basic
 	# check-bin-arch logic simple, just skip this directory.
 	"/lib/modules"
+	"/usr/lib/modules"
 
 	# Skip files in /usr/share, several packages (qemu,
 	# pru-software-support) legitimately install ELF binaries that
-- 
2.14.1

[Buildroot] [PATCH 2/2] support/check-bin-arch: exclude kernel modules for merged /usr

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > When using a merged /usr, the kernel module path is really
 > /usr/lib/modules, as /lib is a symlink to usr/lib .

 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Reviewed-by: Peter Korsgaard <peter@korsgaard.com>

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 2/2] support/check-bin-arch: exclude kernel modules for merged /usr

[Yann E. MORIN]

Peter, All,

On 2018-03-07 21:45 +0100, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  > When using a merged /usr, the kernel module path is really
>  > /usr/lib/modules, as /lib is a symlink to usr/lib .
> 
>  > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> 
> Reviewed-by: Peter Korsgaard <peter@korsgaard.com>

But I forgot to update the manual with this new location! ;-)

Should I still cary your rev-tag in round 2?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/2] support/check-bin-arch: exclude kernel modules for merged /usr

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Peter, All,
 > On 2018-03-07 21:45 +0100, Peter Korsgaard spake thusly:
 >> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
 >> 
 >> > When using a merged /usr, the kernel module path is really
 >> > /usr/lib/modules, as /lib is a symlink to usr/lib .
 >> 
 >> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 >> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 >> 
 >> Reviewed-by: Peter Korsgaard <peter@korsgaard.com>

 > But I forgot to update the manual with this new location! ;-)

 > Should I still cary your rev-tag in round 2?

Sure!

-- 
Bye, Peter Korsgaard