[Buildroot] [PATCH 0/6] Warn user on missing optional dependencies

[Buildroot] [PATCH 0/6] Warn user on missing optional dependencies

[Jérôme Pouiller]

Following series aim to help developers (maybe with help of autobuilders) to
find missing optional dependencies. The principle is to list shared libraries
needed for each ELF found in $TARGET_DIR and check if it does not missing in
dependency list.

 - Patches 1 and 2 implement this feature
 - Patch 3 is a proposal to make packages-file-list.txt compatible with top
   level parallelize
 - Last 3 patches are fixes for errors I found for my configuration

Notice, I also work on reproducible builds. I am going to send my work soon.
Using it and fixing errors found by current series, I am able to do
reproducible builds with TLP enabled (and without per package staging
directory).

J?r?me Pouiller (6):
  check-shlibs-deps: new script to check shared library dependencies
  pkg-generic: add check_shlibs_deps hooks
  infra: fix 'packages-file-list.txt' with TLP
  ntp: fix missing optional dependencies
  xterm: depend on libXinerama if appropriate
  xserver_xorg-server: fix dependency with dbus

 package/ntp/ntp.mk                                 |  14 ++
 package/pkg-generic.mk                             |  24 +++
 .../xserver_xorg-server/xserver_xorg-server.mk     |   4 +-
 package/xterm/xterm.mk                             |   7 +
 support/scripts/check-shlibs-deps                  | 172 +++++++++++++++++++++
 5 files changed, 219 insertions(+), 2 deletions(-)
 create mode 100755 support/scripts/check-shlibs-deps

-- 
2.9.3

[Buildroot] [PATCH 1/6] check-shlibs-deps: new script to check shared library dependencies

[Jérôme Pouiller]

Add a script that show or check binary dependencies based on linked shared
libraries.

To do that, it scan NEEDED entries in ELF header and get corresponding package
from packages-file-list.txt.

It have 3 modes:

    check-shlibs-deps -b OUTDIR
        Scan $TARGET_DIR and display found dependencies for all ELF files

    check-shlibs-deps -b OUTDIR -p PACKAGE
        Display found dependencies for PACKAGE

    check-shlibs-deps -b OUTDIR -p PACKAGE -d DEP1,DEP2,...
        Display missing dependencies for PACKAGE

Unfortunately, `packages-file-list.txt' is not properly filled when Top Level
Parallelization is enabled. Therefore, check-shlibs-deps does not (yet) work
with it.

Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
 support/scripts/check-shlibs-deps | 172 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100755 support/scripts/check-shlibs-deps

diff --git a/support/scripts/check-shlibs-deps b/support/scripts/check-shlibs-deps
new file mode 100755
index 0000000..5ce024c
--- /dev/null
+++ b/support/scripts/check-shlibs-deps
@@ -0,0 +1,172 @@
+#!/usr/bin/env python
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+# Copyright (C) 2016 by Jerome Pouiller <jerome.pouiller@sysmic.org>
+# Inspired from size-stats and check-host-rpath scripts
+
+import os
+import re
+import subprocess
+import argparse
+
+ignored_deps = [ "unknown", "glibc", "uclibc", "musl" ]
+
+# Add an entry in dictionnaries pkgsdict and filesdict
+def add_file(pkgsdict, filesdict, abspath, pkg):
+    if not os.path.exists(abspath):
+        return
+    #if abspath in filesdict and filesdict[abspath] != pkg:
+    #    print("WARNING: %s is owned by %s, but overwritten by %s" %
+    #       (abspath, filesdict[abspath], pkg))
+    filesdict[abspath] = pkg
+    if not pkg in pkgsdict:
+        pkgsdict[pkg] = set()
+    pkgsdict[pkg].add(abspath)
+
+# Build dictionnaries from "build/packages-file-list.txt"
+def build_dicts(builddir):
+    pkgsdict = {}
+    filesdict = {}
+    with open(os.path.join(builddir, "build", "packages-file-list.txt")) as filelistf:
+        for line in filelistf.readlines():
+            pkg, fpath = line.split(",", 1)
+            fpath = fpath.strip()
+            fpath = os.path.join(builddir, "target", fpath)
+            fpath = os.path.normpath(os.path.relpath(fpath))
+            add_file(pkgsdict, filesdict, fpath, pkg)
+    return filesdict, pkgsdict
+
+# Return package associated to a file
+def get_package(filesdict, fpath):
+    if not fpath in filesdict:
+        #print("WARNING: %s is not part of any package" % fpath)
+        # Do not flood user with warning messages. Especially host-gcc-final
+        # does not declare its files and produce many warnings.
+        filesdict[fpath] = "unknown"
+    return filesdict[fpath]
+
+# Return list of libraries linked with a binary
+def get_shared_libs(builddir, binary):
+    libs = set()
+    # Host readelf seems able to read foreign binaries (tested with arm/glibc and arm/uclibc)
+    pipe = subprocess.Popen([ "readelf", "-d", binary ], stdout=subprocess.PIPE)
+    for line in pipe.stdout:
+        match = re.match("^.* \(NEEDED\) .*Shared library: \[(.+)\]$", line)
+        if match:
+            libname = match.group(1)
+            # Support case where "/lib" s a symlink to "/usr/lib"
+            lpaths = set()
+            for dir in [ "usr/lib", "lib" ]:
+                lpaths.add(os.path.relpath(os.path.realpath(os.path.join(builddir, "target", dir, libname))))
+            found = 0
+            for file in lpaths:
+                if os.path.exists(file):
+                    found += 1
+                    libs.add(file)
+            #if found == 0:
+            #    # FIXME: Also take into account RPATH in order to find missed libraries
+            #    print("WARNING: %s depends on %s but it was not found on filesystem" % (binary, libname))
+            if found > 1:
+                print("BUG: %s depends on %s but it was found multiple time on filesystem" % (binary, libname))
+    return libs
+
+# Return a list a dependencies for a list of ELF files
+def build_package_deps(builddir, filesdict, bins):
+    pkgdeps = { }
+    for binary in bins:
+        shlibs = get_shared_libs(builddir, binary)
+        for sh in shlibs:
+            pkg = get_package(filesdict, binary)
+            if not pkg in pkgdeps:
+                pkgdeps[pkg] = set()
+            dep = get_package(filesdict, sh)
+            if not dep in ignored_deps and dep != pkg:
+                pkgdeps[pkg].add(dep)
+    return pkgdeps
+
+# Filter ELF files from a list of files
+def filter_elf(builddir, files):
+    bins = set()
+    pipe = subprocess.Popen([ "file" ] + list(files), stdout=subprocess.PIPE)
+    for line in pipe.stdout:
+        match = re.match("^([^:]+): +ELF ", line)
+        if match:
+            bins.add(match.group(1));
+    return bins
+
+# Return files found in "target/"
+def build_file_list(builddir):
+    files = set()
+    for dirpath, _, filelist in os.walk(os.path.join(builddir, "target")):
+        for f in filelist:
+            file = os.path.join(dirpath, f)
+            file = os.path.relpath(os.path.realpath(file))
+            if not os.path.islink(file):
+                files.add(file)
+    return files
+
+def main(builddir, package, deps):
+    filesdict, pkgsdict = build_dicts(builddir)
+    if package and not package in pkgsdict:
+        print("'%s' is an unkown package" % package)
+        exit(0)
+
+    if package:
+        file_list = pkgsdict[package]
+    else:
+        file_list = build_file_list(builddir)
+    # print("List of files to check:\n  %s" % "\n  ".join(sorted(file_list)))
+    bins = filter_elf(builddir, file_list)
+    # print("List of binaries to check:\n  %s" % "\n  ".join(sorted(bins)))
+    pkgdeps = build_package_deps(builddir, filesdict, sorted(bins))
+    error = 0
+    for p, pdeps in sorted(pkgdeps.items()):
+        if not deps == None:
+            for d in pdeps:
+                if not d in sorted(deps):
+                    print("%s: missed dependency to %s" % (p, d))
+                    error += 1
+        else:
+            print("%s: %s" % (p, " ".join(sorted(pdeps))))
+    return error
+
+
+parser = argparse.ArgumentParser(description='Show or check binary dependencies based on linked shared libraries')
+
+parser.add_argument("--builddir", '-b', metavar="BUILDDIR", required=True,
+        help="Buildroot output directory")
+parser.add_argument("--package", '-p', metavar="PACKAGE",
+        help="Check only PACKAGE (else, show dpendencies of all binairies)")
+parser.add_argument("--deps", '-d', metavar="DEP1,DEP2,...", nargs='?', default="",
+        help="Report errors if found dependencies are not a subset of '--deps'. '-p' is mandatory with this option")
+parser.add_argument('-w', action='store_true',
+        help="Do not return non zero when dependency is missing")
+args = parser.parse_args()
+if not args.package and args.deps:
+    print("ERROR: cannot use --deps wihout --package")
+    exit(1)
+
+if args.deps == "":
+    deps = None
+elif args.deps == None:
+    deps = []
+else:
+    deps = args.deps.split(",")
+
+ret = main(args.builddir, args.package, deps)
+if not args.w:
+    exit(ret)
+exit(0)
-- 
2.9.3

[Buildroot] [PATCH 2/6] pkg-generic: add check_shlibs_deps hooks

[Jérôme Pouiller]

Call check-shlibs-deps after each package. Errors reported by this hooks mean
the package could compile differently depending of build order (= missing
optional dependency).

Currently, missed dependencies are only logged to
`$(BUILD_DIR)/missing-dependencies.txt'. In future, we may remove `-w' in order
to make these errors fatal.

In order to get correct results from step_check_shlibs_deps, we need to compute
recursive dependencies for package. get_recursive_dependencies is brittle, don't
try to add spaces or line breaks inside its definition.

A good idea to find many missed optional dependencies would be to use this
script with LTP. Unfortunately, check-shlibs-deps does not (yet) work with it.

Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
 package/pkg-generic.mk | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 827de62..987efa6 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -110,6 +110,26 @@ define step_check_build_dir
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
 
+# Compute dependencies for a set of packages recursively
+# $(1): list of package to check
+# $(2): list of package already processed. Only used for internal recursion.
+#     Keep empty for normal usage.
+# Example usage:
+#     $(call get_recursive_dependencies,util-linux,)
+define get_recursive_dependencies
+$(sort $(foreach d,$(1),$(if $(filter $(d),$(2)),$(d),$(call get_recursive_dependencies,$(d) $($(call UPPERCASE,$(d))_DEPENDENCIES),$(d) $(2)))))
+endef
+
+define step_check_shlibs_deps
+	$(if $(filter install-target,$(2)),\
+		$(if $(filter end,$(1)),support/scripts/check-shlibs-deps \
+			-b $(CONFIG_DIR) -p $(3) -w \
+			-d $(subst $(space),$(comma),$(call get_recursive_dependencies,$(3),)) \
+			| tee -a $(BUILD_DIR)/missing-dependencies.txt
+		))
+endef
+GLOBAL_INSTRUMENTATION_HOOKS += step_check_shlibs_deps
+
 # User-supplied script
 ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
 define step_user
-- 
2.9.3

[Buildroot] [PATCH 3/6] infra: fix 'packages-file-list.txt' with TLP

[Jérôme Pouiller]

Until now, `$(BUILD_DIR)/packages-file-list.txt' was not filled properly when
top level parallelization is enabled. Therefore, all scripts that rely on
packages-file-list.txt did not work.

In order to fix it,this patch place target installation task in a critical
section.

Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
 package/pkg-generic.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 987efa6..c5f70e0 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -62,6 +62,9 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
 # files currently installed in the target. Note that the MD5 is also
 # stored, in order to identify if the files are overwritten.
 define step_pkg_size_start
+	while ! flock $(BUILD_DIR) -c "[ ! -e $(BUILD_DIR)/.target_lock ] && touch $(BUILD_DIR)/.target_lock"; do \
+		sleep 0.5; \
+	done
 	(cd $(TARGET_DIR) ; find . -type f -print0 | xargs -0 md5sum) | sort > \
 		$($(PKG)_DIR)/.br_filelist_before
 endef
@@ -78,6 +81,7 @@ define step_pkg_size_end
 		while read hash file ; do \
 			echo "$(1),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
 		done
+	rm $(BUILD_DIR)/.target_lock
 endef
 
 define step_pkg_size
-- 
2.9.3

[Buildroot] [PATCH 4/6] ntp: fix missing optional dependencies

[Jérôme Pouiller]

ntpq and ntpdc may depends on libedit and libcap.

$ arm-linux-readelf -d ./usr/bin/ntpdc | grep NEEDED
 0x00000001 (NEEDED)                     Shared library: [libcap.so.2]
 0x00000001 (NEEDED)                     Shared library: [libm.so.6]
 0x00000001 (NEEDED)                     Shared library: [libedit.so.0]
 0x00000001 (NEEDED)                     Shared library: [libncursesw.so.6]
 0x00000001 (NEEDED)                     Shared library: [libssl.so.1.0.0]
 0x00000001 (NEEDED)                     Shared library: [libcrypto.so.1.0.0]
 0x00000001 (NEEDED)                     Shared library: [libpthread.so.0]
 0x00000001 (NEEDED)                     Shared library: [libc.so.6]

However, build order with these libraries is not defined.

In order to keep things simple, we enforce build order even if ntpq/ntpdc are
not selected.

Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
 package/ntp/ntp.mk | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/package/ntp/ntp.mk b/package/ntp/ntp.mk
index 0b6304b..11061f4 100644
--- a/package/ntp/ntp.mk
+++ b/package/ntp/ntp.mk
@@ -28,6 +28,20 @@ else
 NTP_CONF_OPTS += --without-crypto --disable-openssl-random
 endif
 
+ifeq ($(BR2_PACKAGE_LIBCAP),y)
+NTP_CONF_OPTS += --enable-linuxcaps
+NTP_DEPENDENCIES += libcap
+else
+NTP_CONF_OPTS += --disable-linuxcaps
+endif
+
+ifeq ($(BR2_PACKAGE_LIBEDIT),y)
+NTP_CONF_OPTS += --with-lineeditlibs=edit
+NTP_DEPENDENCIES += libedit
+else
+NTP_CONF_OPTS += --with-lineeditlibs=""
+endif
+
 ifeq ($(BR2_PACKAGE_NTP_NTPSNMPD),y)
 NTP_CONF_OPTS += \
 	--with-net-snmp-config=$(STAGING_DIR)/usr/bin/net-snmp-config
-- 
2.9.3

[Buildroot] [PATCH 5/6] xterm: depend on libXinerama if appropriate

[Jérôme Pouiller]

xterm may depends on libXinerama.so:

$ arm-linux-readelf -d ./usr/bin/xterm | grep NEEDED
 0x00000001 (NEEDED)                     Shared library: [libXft.so.2]
 0x00000001 (NEEDED)                     Shared library: [libfontconfig.so.1]
 0x00000001 (NEEDED)                     Shared library: [libXmu.so.6]
 0x00000001 (NEEDED)                     Shared library: [libXaw.so.7]
 0x00000001 (NEEDED)                     Shared library: [libXt.so.6]
 0x00000001 (NEEDED)                     Shared library: [libX11.so.6]
 0x00000001 (NEEDED)                     Shared library: [libXinerama.so.1]
 0x00000001 (NEEDED)                     Shared library: [libXpm.so.4]
 0x00000001 (NEEDED)                     Shared library: [libICE.so.6]
 0x00000001 (NEEDED)                     Shared library: [libncursesw.so.6]
 0x00000001 (NEEDED)                     Shared library: [libc.so.6]

However, build order with libXinerama is not defined.

Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
 package/xterm/xterm.mk | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/package/xterm/xterm.mk b/package/xterm/xterm.mk
index eba781f..645de92 100644
--- a/package/xterm/xterm.mk
+++ b/package/xterm/xterm.mk
@@ -22,4 +22,11 @@ else
 XTERM_CONF_OPTS += --disable-freetype
 endif
 
+ifeq ($(BR2_PACKAGE_XLIB_LIBXINERAMA),y)
+XTERM_DEPENDENCIES += xlib_libXinerama
+XTERM_CONF_OPTS += --with-xinerama
+else
+XTERM_CONF_OPTS += --without-xinerama
+endif
+
 $(eval $(autotools-package))
-- 
2.9.3

[Buildroot] [PATCH 6/6] xserver_xorg-server: fix dependency with dbus

[Jérôme Pouiller]

If dbus and udev are enabled. Xorg link with both:

$ arm-linux-readelf -d usr/bin/X | grep NEEDED
 0x00000001 (NEEDED)                     Shared library: [libdbus-1.so.3]
 0x00000001 (NEEDED)                     Shared library: [libpthread.so.0]
 0x00000001 (NEEDED)                     Shared library: [libudev.so.1]
[...]

Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
---
 package/x11r7/xserver_xorg-server/xserver_xorg-server.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/package/x11r7/xserver_xorg-server/xserver_xorg-server.mk b/package/x11r7/xserver_xorg-server/xserver_xorg-server.mk
index 51c3efc..69f95fd 100644
--- a/package/x11r7/xserver_xorg-server/xserver_xorg-server.mk
+++ b/package/x11r7/xserver_xorg-server/xserver_xorg-server.mk
@@ -169,12 +169,12 @@ XSERVER_XORG_SERVER_CONF_OPTS += --enable-config-udev-kms
 else
 XSERVER_XORG_SERVER_CONF_OPTS += --disable-config-udev-kms
 endif
-else
+endif
+
 ifeq ($(BR2_PACKAGE_DBUS),y)
 XSERVER_XORG_SERVER_DEPENDENCIES += dbus
 XSERVER_XORG_SERVER_CONF_OPTS += --enable-config-dbus
 endif
-endif
 
 ifeq ($(BR2_PACKAGE_FREETYPE),y)
 XSERVER_XORG_SERVER_DEPENDENCIES += freetype
-- 
2.9.3

[Buildroot] [PATCH 4/6] ntp: fix missing optional dependencies

[Thomas Petazzoni]

Hello,

On Mon, 14 Nov 2016 14:22:36 +0100, J?r?me Pouiller wrote:
> ntpq and ntpdc may depends on libedit and libcap.
> 
> $ arm-linux-readelf -d ./usr/bin/ntpdc | grep NEEDED
>  0x00000001 (NEEDED)                     Shared library: [libcap.so.2]
>  0x00000001 (NEEDED)                     Shared library: [libm.so.6]
>  0x00000001 (NEEDED)                     Shared library: [libedit.so.0]
>  0x00000001 (NEEDED)                     Shared library: [libncursesw.so.6]
>  0x00000001 (NEEDED)                     Shared library: [libssl.so.1.0.0]
>  0x00000001 (NEEDED)                     Shared library: [libcrypto.so.1.0.0]
>  0x00000001 (NEEDED)                     Shared library: [libpthread.so.0]
>  0x00000001 (NEEDED)                     Shared library: [libc.so.6]
> 
> However, build order with these libraries is not defined.
> 
> In order to keep things simple, we enforce build order even if ntpq/ntpdc are
> not selected.
> 
> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>

I've applied your patch, after doing one small change, see below.

> +ifeq ($(BR2_PACKAGE_LIBEDIT),y)
> +NTP_CONF_OPTS += --with-lineeditlibs=edit
> +NTP_DEPENDENCIES += libedit
> +else
> +NTP_CONF_OPTS += --with-lineeditlibs=""

I've replaced this by --without-lineeditlibs, which has the same
effect but is more commonly used in Buildroot. I did check that with
the libedit package installed, but forcing --without-lineeditlibs
really causes ntp to ignore libedit, which proves that it does work.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 6/6] xserver_xorg-server: fix dependency with dbus

[Thomas Petazzoni]

Hello,

On Mon, 14 Nov 2016 14:22:38 +0100, J?r?me Pouiller wrote:
> If dbus and udev are enabled. Xorg link with both:
> 
> $ arm-linux-readelf -d usr/bin/X | grep NEEDED
>  0x00000001 (NEEDED)                     Shared library: [libdbus-1.so.3]
>  0x00000001 (NEEDED)                     Shared library: [libpthread.so.0]
>  0x00000001 (NEEDED)                     Shared library: [libudev.so.1]
> [...]
> 
> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> ---
>  package/x11r7/xserver_xorg-server/xserver_xorg-server.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/6] check-shlibs-deps: new script to check shared library dependencies

[Samuel Martin]

Hi J?r?me, all,

On Mon, Nov 14, 2016 at 2:22 PM, J?r?me Pouiller <jezz@sysmic.org> wrote:
> Add a script that show or check binary dependencies based on linked shared
> libraries.
>
> To do that, it scan NEEDED entries in ELF header and get corresponding package
> from packages-file-list.txt.
>
> It have 3 modes:
>
>     check-shlibs-deps -b OUTDIR
>         Scan $TARGET_DIR and display found dependencies for all ELF files
>
>     check-shlibs-deps -b OUTDIR -p PACKAGE
>         Display found dependencies for PACKAGE
>
>     check-shlibs-deps -b OUTDIR -p PACKAGE -d DEP1,DEP2,...
>         Display missing dependencies for PACKAGE
>
> Unfortunately, `packages-file-list.txt' is not properly filled when Top Level
> Parallelization is enabled. Therefore, check-shlibs-deps does not (yet) work
> with it.
>
> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> ---
>  support/scripts/check-shlibs-deps | 172 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 172 insertions(+)
>  create mode 100755 support/scripts/check-shlibs-deps
>
> diff --git a/support/scripts/check-shlibs-deps b/support/scripts/check-shlibs-deps
> new file mode 100755
> index 0000000..5ce024c
> --- /dev/null
> +++ b/support/scripts/check-shlibs-deps
> @@ -0,0 +1,172 @@
> +#!/usr/bin/env python
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> +
> +# Copyright (C) 2016 by Jerome Pouiller <jerome.pouiller@sysmic.org>
> +# Inspired from size-stats and check-host-rpath scripts
> +
> +import os
> +import re
> +import subprocess
> +import argparse
> +
> +ignored_deps = [ "unknown", "glibc", "uclibc", "musl" ]
> +
> +# Add an entry in dictionnaries pkgsdict and filesdict
> +def add_file(pkgsdict, filesdict, abspath, pkg):
> +    if not os.path.exists(abspath):
> +        return
> +    #if abspath in filesdict and filesdict[abspath] != pkg:
> +    #    print("WARNING: %s is owned by %s, but overwritten by %s" %
> +    #       (abspath, filesdict[abspath], pkg))
> +    filesdict[abspath] = pkg
> +    if not pkg in pkgsdict:
> +        pkgsdict[pkg] = set()
> +    pkgsdict[pkg].add(abspath)
> +
> +# Build dictionnaries from "build/packages-file-list.txt"
> +def build_dicts(builddir):
> +    pkgsdict = {}
> +    filesdict = {}
> +    with open(os.path.join(builddir, "build", "packages-file-list.txt")) as filelistf:
> +        for line in filelistf.readlines():
> +            pkg, fpath = line.split(",", 1)
> +            fpath = fpath.strip()
> +            fpath = os.path.join(builddir, "target", fpath)
> +            fpath = os.path.normpath(os.path.relpath(fpath))

I wonder why relpath is needed here? since it is resloving relative
path from the current location, i.e. BR's top directory.
BTW, is it really the BR's top dir. you want to use here as start path
to compute the relative path, not $(O)? How does this behave with
out-of-tree build?

> +            add_file(pkgsdict, filesdict, fpath, pkg)
> +    return filesdict, pkgsdict
> +
> +# Return package associated to a file
> +def get_package(filesdict, fpath):
> +    if not fpath in filesdict:
> +        #print("WARNING: %s is not part of any package" % fpath)
> +        # Do not flood user with warning messages. Especially host-gcc-final
> +        # does not declare its files and produce many warnings.
> +        filesdict[fpath] = "unknown"
> +    return filesdict[fpath]
> +
> +# Return list of libraries linked with a binary
> +def get_shared_libs(builddir, binary):
> +    libs = set()
> +    # Host readelf seems able to read foreign binaries (tested with arm/glibc and arm/uclibc)
> +    pipe = subprocess.Popen([ "readelf", "-d", binary ], stdout=subprocess.PIPE)

Side note:
There is this project [1] out-there that looks nice, but I wonder if
it is worthwhile to add/embedded another python modules dependency to
BR.

> +    for line in pipe.stdout:
> +        match = re.match("^.* \(NEEDED\) .*Shared library: \[(.+)\]$", line)
> +        if match:
> +            libname = match.group(1)
> +            # Support case where "/lib" s a symlink to "/usr/lib"
> +            lpaths = set()
> +            for dir in [ "usr/lib", "lib" ]:
> +                lpaths.add(os.path.relpath(os.path.realpath(os.path.join(builddir, "target", dir, libname))))

ditto for relpath/start path.

> +            found = 0
> +            for file in lpaths:
> +                if os.path.exists(file):
> +                    found += 1
> +                    libs.add(file)
> +            #if found == 0:
> +            #    # FIXME: Also take into account RPATH in order to find missed libraries
> +            #    print("WARNING: %s depends on %s but it was not found on filesystem" % (binary, libname))
> +            if found > 1:
> +                print("BUG: %s depends on %s but it was found multiple time on filesystem" % (binary, libname))
> +    return libs
> +
> +# Return a list a dependencies for a list of ELF files

s/a list a/a list of/

> +def build_package_deps(builddir, filesdict, bins):
> +    pkgdeps = { }

s/{ }/{}/

> +    for binary in bins:
> +        shlibs = get_shared_libs(builddir, binary)
> +        for sh in shlibs:
> +            pkg = get_package(filesdict, binary)
> +            if not pkg in pkgdeps:
> +                pkgdeps[pkg] = set()
> +            dep = get_package(filesdict, sh)
> +            if not dep in ignored_deps and dep != pkg:
> +                pkgdeps[pkg].add(dep)
> +    return pkgdeps
> +
> +# Filter ELF files from a list of files
> +def filter_elf(builddir, files):
> +    bins = set()
> +    pipe = subprocess.Popen([ "file" ] + list(files), stdout=subprocess.PIPE)
> +    for line in pipe.stdout:
> +        match = re.match("^([^:]+): +ELF ", line)

ditto pyelftools

> +        if match:
> +            bins.add(match.group(1));
> +    return bins
> +
> +# Return files found in "target/"
> +def build_file_list(builddir):
> +    files = set()
> +    for dirpath, _, filelist in os.walk(os.path.join(builddir, "target")):
> +        for f in filelist:
> +            file = os.path.join(dirpath, f)
> +            file = os.path.relpath(os.path.realpath(file))

ditto for relpath/start path.

> +            if not os.path.islink(file):
> +                files.add(file)
> +    return files
> +
> +def main(builddir, package, deps):
> +    filesdict, pkgsdict = build_dicts(builddir)
> +    if package and not package in pkgsdict:
> +        print("'%s' is an unkown package" % package)
> +        exit(0)
> +
> +    if package:
> +        file_list = pkgsdict[package]
> +    else:
> +        file_list = build_file_list(builddir)
> +    # print("List of files to check:\n  %s" % "\n  ".join(sorted(file_list)))
> +    bins = filter_elf(builddir, file_list)
> +    # print("List of binaries to check:\n  %s" % "\n  ".join(sorted(bins)))
> +    pkgdeps = build_package_deps(builddir, filesdict, sorted(bins))
> +    error = 0
> +    for p, pdeps in sorted(pkgdeps.items()):
> +        if not deps == None:

s/deps == None/deps is None/

or simply: if deps:

> +            for d in pdeps:
> +                if not d in sorted(deps):
> +                    print("%s: missed dependency to %s" % (p, d))
> +                    error += 1
> +        else:
> +            print("%s: %s" % (p, " ".join(sorted(pdeps))))
> +    return error
> +
> +
> +parser = argparse.ArgumentParser(description='Show or check binary dependencies based on linked shared libraries')
> +
> +parser.add_argument("--builddir", '-b', metavar="BUILDDIR", required=True,
> +        help="Buildroot output directory")
> +parser.add_argument("--package", '-p', metavar="PACKAGE",
> +        help="Check only PACKAGE (else, show dpendencies of all binairies)")
> +parser.add_argument("--deps", '-d', metavar="DEP1,DEP2,...", nargs='?', default="",
> +        help="Report errors if found dependencies are not a subset of '--deps'. '-p' is mandatory with this option")
> +parser.add_argument('-w', action='store_true',
> +        help="Do not return non zero when dependency is missing")
> +args = parser.parse_args()
> +if not args.package and args.deps:
> +    print("ERROR: cannot use --deps wihout --package")
> +    exit(1)
> +
> +if args.deps == "":
> +    deps = None
> +elif args.deps == None:
> +    deps = []

What is the difference between "deps = None" and "deps = []"?

> +else:
> +    deps = args.deps.split(",")
> +
> +ret = main(args.builddir, args.package, deps)
> +if not args.w:
> +    exit(ret)
> +exit(0)

Please, put all the main code under a 'if __name__ == "__main__":' block.

> --
> 2.9.3
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[1] https://github.com/eliben/pyelftools


Regards,

-- 
Samuel

[Buildroot] [PATCH 2/6] pkg-generic: add check_shlibs_deps hooks

[Samuel Martin]

Hi J?r?me, all,

On Mon, Nov 14, 2016 at 2:22 PM, J?r?me Pouiller <jezz@sysmic.org> wrote:
> Call check-shlibs-deps after each package. Errors reported by this hooks mean
> the package could compile differently depending of build order (= missing
> optional dependency).
>
> Currently, missed dependencies are only logged to
> `$(BUILD_DIR)/missing-dependencies.txt'. In future, we may remove `-w' in order
> to make these errors fatal.
>
> In order to get correct results from step_check_shlibs_deps, we need to compute
> recursive dependencies for package. get_recursive_dependencies is brittle, don't
> try to add spaces or line breaks inside its definition.
>
> A good idea to find many missed optional dependencies would be to use this
> script with LTP. Unfortunately, check-shlibs-deps does not (yet) work with it.
>
> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> ---
>  package/pkg-generic.mk | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 827de62..987efa6 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -110,6 +110,26 @@ define step_check_build_dir
>  endef
>  GLOBAL_INSTRUMENTATION_HOOKS += step_check_build_dir
>
> +# Compute dependencies for a set of packages recursively
> +# $(1): list of package to check
> +# $(2): list of package already processed. Only used for internal recursion.
> +#     Keep empty for normal usage.
> +# Example usage:
> +#     $(call get_recursive_dependencies,util-linux,)

You could introduce a inner-get_recursive_dependencies that takes 2
arguments and keep the user API get_recursive_dependencies only taking
1 argument, which just calls inner-get_recursive_dependencies with an
empty 2nd argument.

> +define get_recursive_dependencies
> +$(sort $(foreach d,$(1),$(if $(filter $(d),$(2)),$(d),$(call get_recursive_dependencies,$(d) $($(call UPPERCASE,$(d))_DEPENDENCIES),$(d) $(2)))))
> +endef
> +
> +define step_check_shlibs_deps
> +       $(if $(filter install-target,$(2)),\
> +               $(if $(filter end,$(1)),support/scripts/check-shlibs-deps \
> +                       -b $(CONFIG_DIR) -p $(3) -w \

Here $(CONFIG_DIR) looks dubious... Why do you use it instead of $(O)?

> +                       -d $(subst $(space),$(comma),$(call get_recursive_dependencies,$(3),)) \
> +                       | tee -a $(BUILD_DIR)/missing-dependencies.txt
> +               ))
> +endef
> +GLOBAL_INSTRUMENTATION_HOOKS += step_check_shlibs_deps
> +
>  # User-supplied script
>  ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
>  define step_user
> --
> 2.9.3
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Regards,

-- 
Samuel

[Buildroot] [PATCH 1/6] check-shlibs-deps: new script to check shared library dependencies

[Thomas Petazzoni]

Hello J?r?me,

On Mon, 14 Nov 2016 14:22:33 +0100, J?r?me Pouiller wrote:
> Add a script that show or check binary dependencies based on linked shared
> libraries.

Since you now received some feedback, I've marked patches 1/6 and 2/6
as "Changes Requested" in patchwork. Could you address the comments and
submit an updated version?

I think this is a very useful thing, and we'd like to have it in
Buildroot.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 3/6] infra: fix 'packages-file-list.txt' with TLP

[Thomas Petazzoni]

Hello,

Adding Gustavo in Cc. Gustavo, you are working on TLP support. Could
you comment on the below patch?

See also my comments below.

On Mon, 14 Nov 2016 14:22:35 +0100, J?r?me Pouiller wrote:
> Until now, `$(BUILD_DIR)/packages-file-list.txt' was not filled properly when
> top level parallelization is enabled. Therefore, all scripts that rely on
> packages-file-list.txt did not work.
> 
> In order to fix it,this patch place target installation task in a critical
> section.
> 
> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> ---
>  package/pkg-generic.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 987efa6..c5f70e0 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -62,6 +62,9 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
>  # files currently installed in the target. Note that the MD5 is also
>  # stored, in order to identify if the files are overwritten.
>  define step_pkg_size_start
> +	while ! flock $(BUILD_DIR) -c "[ ! -e $(BUILD_DIR)/.target_lock ] && touch $(BUILD_DIR)/.target_lock"; do \
> +		sleep 0.5; \
> +	done

I personally don't really like this retry loop around flock, but since
package-file-list.txt is a global file, I don't really see how to do
otherwise. Unless storing a per-package file with just the time/date of
the start/end of each step for this package, and then have a
post-processing logic at the very end of the build that regroups all
those per-package files into a single global file, ordering the entries
by their timestamp. Don't know if it's really better.


>  	(cd $(TARGET_DIR) ; find . -type f -print0 | xargs -0 md5sum) | sort > \
>  		$($(PKG)_DIR)/.br_filelist_before
>  endef
> @@ -78,6 +81,7 @@ define step_pkg_size_end
>  		while read hash file ; do \
>  			echo "$(1),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
>  		done
> +	rm $(BUILD_DIR)/.target_lock
>  endef
>  
>  define step_pkg_size

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 1/6] check-shlibs-deps: new script to check shared library dependencies

[Jérôme Pouiller]

Hello Samuel,

On Monday 6 February 2017 22:04:23 CET Samuel Martin wrote:
> On Mon, Nov 14, 2016 at 2:22 PM, J?r?me Pouiller <jezz@sysmic.org> wrote:
[...]
> > +# Build dictionnaries from "build/packages-file-list.txt"
> > +def build_dicts(builddir):
> > +    pkgsdict = {}
> > +    filesdict = {}
> > +    with open(os.path.join(builddir, "build", "packages-file-list.txt")) as filelistf:
> > +        for line in filelistf.readlines():
> > +            pkg, fpath = line.split(",", 1)
> > +            fpath = fpath.strip()
> > +            fpath = os.path.join(builddir, "target", fpath)
> > +            fpath = os.path.normpath(os.path.relpath(fpath))
> 
> I wonder why relpath is needed here? since it is resloving relative
> path from the current location, i.e. BR's top directory.
> BTW, is it really the BR's top dir. you want to use here as start path
> to compute the relative path, not $(O)? How does this behave with
> out-of-tree build?

In subsequent case, I need to call realpath() in order to resolve
symlinks. realpath() return an absolute paths, but I prefer to convert
them in relative paths because they are smaller in error messages.

In case above, relpath is necessary because 'builddir' may be absolute.


> > +            add_file(pkgsdict, filesdict, fpath, pkg)
> > +    return filesdict, pkgsdict
> > +
> > +# Return package associated to a file
> > +def get_package(filesdict, fpath):
> > +    if not fpath in filesdict:
> > +        #print("WARNING: %s is not part of any package" % fpath)
> > +        # Do not flood user with warning messages. Especially host-gcc-final
> > +        # does not declare its files and produce many warnings.
> > +        filesdict[fpath] = "unknown"
> > +    return filesdict[fpath]
> > +
> > +# Return list of libraries linked with a binary
> > +def get_shared_libs(builddir, binary):
> > +    libs = set()
> > +    # Host readelf seems able to read foreign binaries (tested with arm/glibc and arm/uclibc)
> > +    pipe = subprocess.Popen([ "readelf", "-d", binary ], stdout=subprocess.PIPE)
> 
> Side note:
> There is this project [1] out-there that looks nice, but I wonder if
> it is worthwhile to add/embedded another python modules dependency to
> BR.

Interesting. (But not enough interesting to rewrite this script :) ) 

> > +    for line in pipe.stdout:
> > +        match = re.match("^.* \(NEEDED\) .*Shared library: \[(.+)\]$",
> > line)
> > +        if match:
> > +            libname = match.group(1)
> > +            # Support case where "/lib" s a symlink to "/usr/lib"
> > +            lpaths = set()
> > +            for dir in [ "usr/lib", "lib" ]:
> > +               
> > lpaths.add(os.path.relpath(os.path.realpath(os.path.join(builddir,
> > "target", dir, libname))))> 
> ditto for relpath/start path.\0

[...]

> > +# Return files found in "target/"
> > +def build_file_list(builddir):
> > +    files = set()
> > +    for dirpath, _, filelist in os.walk(os.path.join(builddir,
> > "target")):
> > +        for f in filelist:
> > +            file = os.path.join(dirpath, f)
> > +            file = os.path.relpath(os.path.realpath(file))
> 
> ditto for relpath/start path.

[...]

> > +    for p, pdeps in sorted(pkgdeps.items()):
> > +        if not deps == None:
> 
> s/deps == None/deps is None/

ok

> or simply: if deps:

I do want to make a difference between [] and None (see below). So I
think "if deps:" does not work.

[...]
> > +parser = argparse.ArgumentParser(description='Show or check binary dependencies based on linked shared libraries')
> > +
> > +parser.add_argument("--builddir", '-b', metavar="BUILDDIR", required=True,
> > +        help="Buildroot output directory")
> > +parser.add_argument("--package", '-p', metavar="PACKAGE",
> > +        help="Check only PACKAGE (else, show dpendencies of all binairies)")
> > +parser.add_argument("--deps", '-d', metavar="DEP1,DEP2,...", nargs='?', default="",
> > +        help="Report errors if found dependencies are not a subset of '--deps'. Pass "". '-p' is mandatory with this option")
> > +parser.add_argument('-w', action='store_true',
> > +        help="Do not return non zero when dependency is missing")
> > +args = parser.parse_args()
> > +if not args.package and args.deps:
> > +    print("ERROR: cannot use --deps wihout --package")
> > +    exit(1)
> > +
> > +if args.deps == "":
> > +    deps = None
> > +elif args.deps == None:
> > +    deps = []
> 
> What is the difference between "deps = None" and "deps = []"?

'None' means user don't to check dependencies (-d is not used) while '[]'
means package shouldn't have dependencies.

However, my current implementation is broken. Following command does not
work as expected:

  check-shlibs-deps -b OUTDIR -p PACKAGE -d ""

I will change that.

> > +else:
> > +    deps = args.deps.split(",")
> > +
[...]

-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr

[Buildroot] [PATCH 3/6] infra: fix 'packages-file-list.txt' with TLP

[Jérôme Pouiller]

Hello Thomas,

On Thursday 9 February 2017 23:24:29 CET Thomas Petazzoni wrote:
> Hello,
> 
> Adding Gustavo in Cc. Gustavo, you are working on TLP support. Could
> you comment on the below patch?
> 
> See also my comments below.
> 
> On Mon, 14 Nov 2016 14:22:35 +0100, J?r?me Pouiller wrote:
> > Until now, `$(BUILD_DIR)/packages-file-list.txt' was not filled properly
> > when top level parallelization is enabled. Therefore, all scripts that
> > rely on packages-file-list.txt did not work.
> > 
> > In order to fix it,this patch place target installation task in a critical
> > section.
> > 
> > Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>
> > ---
> > 
> >  package/pkg-generic.mk | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 987efa6..c5f70e0 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -62,6 +62,9 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
> > 
> >  # files currently installed in the target. Note that the MD5 is also
> >  # stored, in order to identify if the files are overwritten.
> >  define step_pkg_size_start
> > 
> > +	while ! flock $(BUILD_DIR) -c "[ ! -e $(BUILD_DIR)/.target_lock ] &&
> > touch $(BUILD_DIR)/.target_lock"; do \ +		sleep 0.5; \
> > +	done
> 
> I personally don't really like this retry loop around flock, but since
> package-file-list.txt is a global file, I don't really see how to do
> otherwise. Unless storing a per-package file with just the time/date of
> the start/end of each step for this package, and then have a
> post-processing logic at the very end of the build that regroups all
> those per-package files into a single global file, ordering the entries
> by their timestamp. Don't know if it's really better.

I think that any tool involving parallel processing need one day or another
to protect code inside a critical section. Even if we find another way to 
solve current problem, I am pretty sure we will need it later, anyway.

However, I think I am going to modify my patch in order to provide a
generic mutex implementation that would be enabled iff TLP is enable.

I wonder where I should place lock file. In add, tools like fakedate,
check-shlibs-deps and other QA tools could also generate log or
temporary files. Currently, we place all of them in build/ without any
specific rules. Maybe we should have a naming policy for these files?
(prefix them with "BR_"?) or place them elsewhere than in build/?

[...]

-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr

[Buildroot] [PATCH 3/6] infra: fix 'packages-file-list.txt' with TLP

[Thomas Petazzoni]

Hello,

On Mon, 14 Nov 2016 14:22:35 +0100, J?r?me Pouiller wrote:
> Until now, `$(BUILD_DIR)/packages-file-list.txt' was not filled properly when
> top level parallelization is enabled. Therefore, all scripts that rely on
> packages-file-list.txt did not work.
> 
> In order to fix it,this patch place target installation task in a critical
> section.
> 
> Signed-off-by: J?r?me Pouiller <jezz@sysmic.org>

Arnout said:

"""
 I don't think this is the proper approach. The proper approach is
map/reduce, i.e. generate the pieces in per-package files and collect
them together in a final gathering stage. Obviously, this requires a
per-package target first, but that's anyway the way we want to go.

 I thought someone already made that comment but I can't find it in
 that thread...
"""

So I've marked this patch as Changes Requested in patchwork.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com