Re: [Buildroot] [PATCH v3 1/1] package/system-config-printer: new package

["Yann E. MORIN" <yann.morin.1998@free.fr>]

James, All,

On 2021-07-31 16:27 -0600, James Hilliard spake thusly:
> This package has some udev tools to assist with printer autodetection.

Sorry, but this is still not enough:

    $ make check-package
    package/Config.in:2123: Packages in: menu "Networking applications",
                            are not alphabetically ordered;
                            correct order: '-', '_', digits, capitals, lowercase;
                            first incorrect package: dante

Also:

    $ make system-config-printer-configure
    [...]
    ./configure: line 7043: cups-config: command not found
    [...]

This is needed to set the path to the CUPS server executable (I guess):

    $ cat -n configure.ac
    [...]
       36 cupsserverbindir="`cups-config --serverbin`"
       37 AC_SUBST(cupsserverbindir)
    [...]

And thus:

    $ grep cupsserverbin config.status
    S["cupsserverbindir"]=""

which is later used to locate the CUPS filters:

    $ grep -nr cupsserverbindir .
    ./cupshelpers/cupshelpers.py:873: [...] config.cupsserverbindir + "/filter:"
    ./cupshelpers/cupshelpers.py:876: [...] add_missing (config.cupsserverbindir + "/filter/" + exe)
    ./cupshelpers/config.py.in:24:cupsserverbindir="@cupsserverbindir@"

So, maybe you have a cups-config in your PATH already, and that happens
to provide the correct setting...

[--SNIP--]
> diff --git a/package/Config.in b/package/Config.in
> index 5941e35c05..fa916370f0 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -2120,6 +2120,7 @@ menu "Networking applications"
>  	source "package/ctorrent/Config.in"
>  	source "package/cups/Config.in"
>  	source "package/cups-filters/Config.in"
> +	source "package/system-config-printer/Config.in"

Alphabetical order, please. Reported by:  make check-package

[--SNIP--]
> diff --git a/package/system-config-printer/system-config-printer.mk b/package/system-config-printer/system-config-printer.mk
> new file mode 100644
> index 0000000000..05dd2f5b46
> --- /dev/null
> +++ b/package/system-config-printer/system-config-printer.mk
> @@ -0,0 +1,33 @@
> +################################################################################
> +#
> +# system-config-printer
> +#
> +################################################################################
> +
> +SYSTEM_CONFIG_PRINTER_VERSION = 1.5.15
> +SYSTEM_CONFIG_PRINTER_SOURCE = system-config-printer-$(SYSTEM_CONFIG_PRINTER_VERSION).tar.xz
> +SYSTEM_CONFIG_PRINTER_SITE = https://github.com/OpenPrinting/system-config-printer/releases/download/v$(SYSTEM_CONFIG_PRINTER_VERSION)
> +SYSTEM_CONFIG_PRINTER_LICENSE = GPL-2.0

It really seems to be GPL-2.0-or-later, as specified in many files:

    In check-device-ids.py:

    ## 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.

At a cursory glance, I could not spot a file that was GPL-2.0-only...

> +SYSTEM_CONFIG_PRINTER_LICENSE_FILES = COPYING
> +SYSTEM_CONFIG_PRINTER_DEPENDENCIES = cups host-intltool

configure.ac also uses PKG_CHECK_MODULES(), so it needs to depend on
host-pkgconf.

> +SYSTEM_CONFIG_PRINTER_AUTORECONF = YES

This needs an explanation why autoreconf is needed. We usually just name
the patch that touches it:

    # 0001-Add-option-to-disable-xmlto-manual-generation.patch
    SYSTEM_CONFIG_PRINTER_AUTORECONF = YES

(but I had to add another patch, so I've also listed it).

> +
> +ifeq ($(BR2_PACKAGE_LIBGLIB2),y)
> +SYSTEM_CONFIG_PRINTER_DEPENDENCIES += libglib2
> +endif
> +
> +ifeq ($(BR2_PACKAGE_LIBGLIB2)$(BR2_PACKAGE_LIBUSB)$(BR2_PACKAGE_HAS_UDEV),yyy)
> +SYSTEM_CONFIG_PRINTER_CONF_OPTS = --with-udev-rules=yes

In conditional blocks, always use append-assignement:

    SYSTEM_CONFIG_PRINTER_CONF_OPTS += --with-udev-rules=yes

This caused issues as I had to add this, earlier on the file:

    SYSTEM_CONFIG_PRINTER_CONF_OPTS = --with-cups-config=...

And I add to add the asociated patch that adds --with-cups-config.

With all those changes: applied to master, thanks. Please review what I
did in case I really borked something...

Also, could you push that new patch 0002-configure-accept-non-system-cups-config.patch
I added, to upstream, please?

Regards,
Yann E. MORIN.

> +SYSTEM_CONFIG_PRINTER_DEPENDENCIES += libusb udev
> +else
> +SYSTEM_CONFIG_PRINTER_CONF_OPTS = --with-udev-rules=no
> +endif
> +
> +ifeq ($(BR2_PACKAGE_SYSTEMD),y)
> +SYSTEM_CONFIG_PRINTER_CONF_OPTS += --with-systemdsystemunitdir=/usr/lib/systemd/system
> +SYSTEM_CONFIG_PRINTER_DEPENDENCIES += systemd
> +else
> +SYSTEM_CONFIG_PRINTER_CONF_OPTS += --with-systemdsystemunitdir=no
> +endif
> +
> +$(eval $(autotools-package))
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot