[Buildroot] [PATCH v2 2/7] package/cups-filters: bump to version 1.27.5

[Thomas Petazzoni <thomas.petazzoni@bootlin.com>]

On Fri, 19 Jun 2020 16:57:14 +0200
Angelo Compagnucci <angelo@amarulasolutions.com> wrote:

> This patch bumps cups-filters to version 1.27.5.
> While bumping, fixing also the missing installation for the service files
> for cups-browsed.
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>

If Michael is the first SoB, then he is the author of the patch, and we
should have his From: line, like it appears for PATCH 1/7. Otherwise,
if you (Angelo) are the author, your SoB should appear first.

The next comment is: are you sure the service files / init script are
related to the version bump ? They seem to be unrelated to the version
bump. Could you clarify that ? Keep in mind that we want one logical
change per commit.

> diff --git a/package/cups-filters/Config.in b/package/cups-filters/Config.in
> index 9e4e37ca6b..26e8d4aa06 100644
> --- a/package/cups-filters/Config.in
> +++ b/package/cups-filters/Config.in
> @@ -8,6 +8,9 @@ config BR2_PACKAGE_CUPS_FILTERS
>  	depends on BR2_TOOLCHAIN_HAS_THREADS # libglib2
>  	depends on BR2_PACKAGE_CUPS
>  	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # C++11
> +	select BR2_PACKAGE_DEJAVU
> +	select BR2_PACKAGE_DEJAVU_SANS
> +	select BR2_PACKAGE_DEJAVU_SERIF

Why DejaVu specifically, and not any other font ?


> +case "$1" in
> +	start)
> +		printf "Starting cups-browsed: "
> +		start-stop-daemon -S -q -m -p /var/run/cups-browsed.pid \
> +			-b -x cups-browsed -- -c /etc/cups/cups-browsed.conf
> +		[ $? = 0 ] && echo "OK" || echo "FAIL"
> +		;;
> +	stop)
> +		printf "Stopping cups-browsed: "
> +		start-stop-daemon -K -q -p /var/run/cups-browsed.pid
> +		[ $? = 0 ] && echo "OK" || echo "FAIL"
> +		;;

Please use package/busybox/S01syslodg as a template for new init
script. We try to increase the consistency between init scripts.


> -CUPS_FILTERS_DEPENDENCIES = cups libglib2 lcms2 qpdf fontconfig freetype jpeg
> +CUPS_FILTERS_DEPENDENCIES = cups libglib2 lcms2 qpdf fontconfig freetype jpeg dejavu
>  
>  CUPS_FILTERS_CONF_OPTS = \
>  	--disable-mutool \
> @@ -19,7 +19,10 @@ CUPS_FILTERS_CONF_OPTS = \
>  	--with-cups-config=$(STAGING_DIR)/usr/bin/cups-config \
>  	--with-sysroot=$(STAGING_DIR) \
>  	--with-pdftops=pdftops \
> -	--with-jpeg
> +	--with-jpeg \
> +	--with-rcdir=no \

	--without-rcdir \

> +	--with-fontdir=$(STAGING_DIR)/usr/share/fonts/ \

How is this path used ? Only at build time ? Or also on the target (in
which case it would be wrong).

> +	--with-test-font-path=$(STAGING_DIR)/usr/share/fonts/dejavu/DejaVuSans.ttf

So, what is this used for exactly ?

>  
>  ifeq ($(BR2_PACKAGE_LIBPNG),y)
>  CUPS_FILTERS_CONF_OPTS += --with-png
> @@ -71,4 +74,15 @@ else
>  CUPS_FILTERS_CONF_OPTS += --disable-poppler
>  endif
>  
> +define CUPS_FILTERS_INSTALL_INIT_SYSV
> +	@$(RM) $(TARGET_DIR)/etc/init.d/cups-browsed

Shouldn't this be removed unconditionally, i.e even in the systemd case?

Also, drop the @ at the beginning of the line.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com