[Buildroot] [PATCH v3 3/5] libsepol: bump to 2.7, setools: bump to 4.1.1

[Thomas Petazzoni]

Hello,

A few comments below.

On Thu, 12 Oct 2017 14:09:19 -0400, Adam Duskett wrote:

> Some setools.mk notes to get the package to compile:
> 
> - Convert the package mk from C to python.

We don't write mk files in C or Python. This should be "Convert the
package .mk to use python-package instead of autotools-package".
Perhaps you could mention that the host variant of setools is no longer
needed.

> - setup.py hard codes base_lib_dirs to point to several host directories.
>   To fix this, sed is used before compiling to point the base_lib_dirs to
>   the staging directory.
> 
> - setup.py also includes the "Werror" flag, however compilers before gcc6
>   cause a few autogenerated variables to not be initialized before use,
>   causing the build to fail.
>   To fix this, sed is used before compiling to remove the Werror flag.
> 
> - Remove sedta and seinfoflow from the target system after install.  These
>   packages rely on the package python-networkx which is not available in
>   buildroot.
> 
> - Remove the apol and the folder setoolsgui from the target filing system

"target filing system" ?

> diff --git a/package/setools/Config.in b/package/setools/Config.in
> index d7b119ffc6..320028d25d 100644
> --- a/package/setools/Config.in
> +++ b/package/setools/Config.in
> @@ -1,36 +1,24 @@
>  config BR2_PACKAGE_SETOOLS
>  	bool "setools"
> -	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	depends on !BR2_arc # arc: libselinux not available
>  	depends on !BR2_STATIC_LIBS
> -	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
>  	depends on BR2_USE_WCHAR
>  	depends on BR2_TOOLCHAIN_USES_GLIBC # libselinux
> -	# bfin: infamous _ symbol prefix issue
> -	# nios2: triggers some toolchain issue "No symbol version
> -	# section for versioned symbol"
> -	# arc: libselinux not available
> -	depends on !BR2_nios2 && !BR2_bfin && !BR2_arc
> +	select BR2_PACKAGE_PYTHON3 if !BR2_PACKAGE_PYTHON

You need a "depends on BR2_USE_MMU" because you're selecting Python.

> +	   * sedta - Perform domain transition analyses
> +	   * seinfo - List policy components.
> +	   * seinfoflow - Perofrm information flow analyses.

You said in your commit log that sedta and seinfoflow are never
installed, because they need python-networkx. So perhaps this should be
mentioned ?

Also, why isn't apol mentioned here, perhaps with a note saying that it
will be installed if python-qt5 is enabled ?

> +	   * sesearch - Search rules (allow, type_transition, etc.)
>  
> -	  https://github.com/TresysTechnology/setools3/wiki
> +	  https://github.com/TresysTechnology/setools
>  
>  comment "setools needs a glibc toolchain w/ threads, C++, wchar, dynamic library"
>  	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS \
> -		|| !BR2_USE_WCHAR || !BR2_INSTALL_LIBSTDCPP \
> -		|| !BR2_TOOLCHAIN_USES_GLIBC
> -	depends on !BR2_nios2 && !BR2_bfin && !BR2_arc
> +		|| !BR2_USE_WCHAR || !BR2_TOOLCHAIN_USES_GLIBC || BR2_arc

Not the correct way of expressing the dependency. We want:

comment "setools needs a glibc toolchain w/ threads, C++, wchar, dynamic library"
	depends on BR2_USE_MMU && !BR2_arc
	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS || \
		!BR2_USE_WCHAR || !BR2_TOOLCHAIN_USES_GLIBC || \
		!BR2_INSTALL_LIBSTDCPP


>  ifeq ($(BR2_PACKAGE_PYTHON3),y)
> -HOST_SETOOLS_PYTHON_VERSION=$(PYTHON3_VERSION_MAJOR)
> -HOST_SETOOLS_DEPENDENCIES += host-python3
> -HOST_SETOOLS_CONF_ENV += am_cv_python_version=$(PYTHON3_VERSION)
> +PYLIBVER = python$(PYTHON3_VERSION_MAJOR)

This variable should be named SETOOLS_PYLIBVER. You seem to forget that
the namespace of variables is global in Buildroot. So if you name a
variable PYLIBVER, remember it is visible from *all* packages. This is
why we always prefix variables with the package name.

> +define FIX_SETUP

Missing prefix in the variable.

> +	# By default, setup.py will look for libsepol.a in the host directory.  This
> +	# needs to be changed to the staging directory.
> +	$(SED) "s at base_lib_dirs =.*@base_lib_dirs = ['$(STAGING_DIR)/usr/lib']@g" \
> +		$(@D)/setup.py
> +
> +	# Compilers older than gcc6 will generate uninitialized variable warnings
> +	# which will cause compiling to fail.
> +	$(SED) "s at extra_compile_args=.*@extra_compile_args=['-Wextra', at g" \
> +		$(@D)/setup.py

Perhaps a patch instead ?

> +endef
> +SETOOLS_POST_EXTRACT_HOOKS += FIX_SETUP

Should be a post-patch hook instead.

> +# sedta and seinfoflow depends on the python-networkx. This package is not

"on the python-networkx" -> "on python-networkx"

> +# available in buildroot.
> +define REMOVE_BROKEN_SCRIPTS

Missing prefix to the variable name.

> +	$(RM) $(TARGET_DIR)/usr/bin/sedta
> +	$(RM) $(TARGET_DIR)/usr/bin/seinfoflow
> +endef
> +SETOOLS_POST_INSTALL_TARGET_HOOKS += REMOVE_BROKEN_SCRIPTS
> +
> +# apol requires pyqt5, but is installed by default.  Remove these scripts from
> +# the target it pyqt5 is not selected.
> +define REMOVE_QT_SCRIPTS

Missing prefix to the variable. Also, please enclose the definition of
the variable inside the conditional block testing
BR2_PACKAGE_PYTHON_QT5.

> +	$(RM) $(TARGET_DIR)/usr/bin/apol
> +	rmdir --ignore-fail-on-non-empty $(TARGET_DIR)/usr/lib/$(PYLIBVER)/site-packages/setoolsgui/
> +endef
> +
> +ifneq ($(BR2_PACKAGE_PYTHON_PYQT5),y)

ifeq ($(BR2_PACKAGE_PYTHON_PYQT5),)

Thanks!

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