[Buildroot] [PATCH v2 2/7] package/cups-filters: bump to version 1.27.5
Thomas Petazzoni
thomas.petazzoni at bootlin.com
Sat Jun 20 20:33:26 UTC 2020
On Fri, 19 Jun 2020 16:57:14 +0200
Angelo Compagnucci <angelo at 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 at amarulasolutions.com>
> Signed-off-by: Angelo Compagnucci <angelo at 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
More information about the buildroot
mailing list