diff mbox series

[v3,1/1] package/system-config-printer: new package

Message ID 20210731222752.64315-1-james.hilliard1@gmail.com
State Accepted
Headers show
Series [v3,1/1] package/system-config-printer: new package | expand

Commit Message

James Hilliard July 31, 2021, 10:27 p.m. UTC 
This package has some udev tools to assist with printer autodetection.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes v2 -> v3:
  - add autoreconf
  - add host-intltool for autoreconf

Changes v1 -> v2:
  - update patch to merged upstream version
---
 package/Config.in                             |  1 +
 ...n-to-disable-xmlto-manual-generation.patch | 73 +++++++++++++++++++
 package/system-config-printer/Config.in       |  7 ++
 .../system-config-printer.hash                |  3 +
 .../system-config-printer.mk                  | 33 +++++++++
 5 files changed, 117 insertions(+)
 create mode 100644 package/system-config-printer/0001-Add-option-to-disable-xmlto-manual-generation.patch
 create mode 100644 package/system-config-printer/Config.in
 create mode 100644 package/system-config-printer/system-config-printer.hash
 create mode 100644 package/system-config-printer/system-config-printer.mk

Comments

Yann E. MORIN Aug. 1, 2021, 12:03 p.m. UTC | #1 
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
James Hilliard Aug. 1, 2021, 12:20 p.m. UTC | #2 
On Sun, Aug 1, 2021 at 6:03 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> 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?

Pull request opened:
https://github.com/OpenPrinting/system-config-printer/pull/234

>
> 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.  |
> '------------------------------^-------^------------------^--------------------'
diff mbox series

Patch

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"
 	source "package/dante/Config.in"
 	source "package/darkhttpd/Config.in"
 	source "package/dehydrated/Config.in"
diff --git a/package/system-config-printer/0001-Add-option-to-disable-xmlto-manual-generation.patch b/package/system-config-printer/0001-Add-option-to-disable-xmlto-manual-generation.patch
new file mode 100644
index 0000000000..e2cf395811
--- /dev/null
+++ b/package/system-config-printer/0001-Add-option-to-disable-xmlto-manual-generation.patch
@@ -0,0 +1,73 @@ 
+From 731c8d7e1f9edb527785d61f74031a5e5846554b Mon Sep 17 00:00:00 2001
+From: James Hilliard <james.hilliard1@gmail.com>
+Date: Sun, 25 Jul 2021 16:30:59 -0600
+Subject: [PATCH] Add option to disable xmlto manual generation.
+
+Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
+[james.hilliard1@gmail.com: backport from upstream commit
+95e0b461f5a46c74b00287aca65732ea2fc9d69b]
+---
+ Makefile.am  | 7 ++++++-
+ configure.ac | 5 +++++
+ 2 files changed, 11 insertions(+), 1 deletion(-)
+
+diff --git a/Makefile.am b/Makefile.am
+index 843ba30d..7facab94 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -209,9 +209,11 @@ else
+ systemd_CLEANFILES=
+ endif
+ 
++if XMLTO
+ man_MANS=					\
+ 	man/system-config-printer.1		\
+ 	man/system-config-printer-applet.1
++endif
+ 
+ dbus_DATA =					\
+ 	dbus/com.redhat.NewPrinterNotification.conf		\
+@@ -274,7 +276,6 @@ EXTRA_DIST=\
+ 	$(bin_SCRIPTS) \
+ 	setup.py \
+ 	$(EXPORT_MODULES) \
+-	man/system-config-printer.xml \
+ 	$(dbus_DATA) \
+ 	$(dbusinterfaces_DATA) \
+ 	$(dbusservices_DATA) \
+@@ -297,12 +298,16 @@ EXTRA_DIST=\
+ 	test_PhysicalDevice.py \
+ 	$(appdata_in_files)
+ 
++if XMLTO
++EXTRA_DIST+=man/system-config-printer.xml
++
+ # The man pages are generated from DocBook XML.
+ .stamp-man-pages-built: $(top_srcdir)/man/system-config-printer.xml
+ 	xmlto man -o man $<
+ 	touch .stamp-man-pages-built
+ 
+ $(man_MANS): .stamp-man-pages-built
++endif
+ 
+ html:	$(EXPORT_MODULES) $(EXPORT_MODULES_GEN)
+ 	rm -rf html
+diff --git a/configure.ac b/configure.ac
+index 811f897a..a2a66889 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -75,6 +75,11 @@ if test "x$with_systemdsystemunitdir" != xno; then
+ fi
+ AM_CONDITIONAL(HAVE_SYSTEMD, [test -n "$with_systemdsystemunitdir" -a "x$with_systemdsystemunitdir" != xno ])
+ 
++AC_ARG_WITH(xmlto,
++        [AC_HELP_STRING([--with-xmlto],
++                [Enable xmlto manual generation @<:@default=yes@:>@])])
++AM_CONDITIONAL([XMLTO], [test x$with_xmlto != xno])
++
+ ALL_LINGUAS="ar as ast bg bn_IN bn br bs ca cs cy da de el en_GB es et fa fi fr fur gu he hi hr hu id is it ja kn ko lt lv mai ml mr ms nb nds nl nn oc or pa pl pt_BR pt ro ru si sk sl sr@latin sr sv ta te th tr uk vi zh_CN zh_TW"
+ AC_CONFIG_FILES([
+ Makefile
+-- 
+2.25.1
+
diff --git a/package/system-config-printer/Config.in b/package/system-config-printer/Config.in
new file mode 100644
index 0000000000..8ebc4d060c
--- /dev/null
+++ b/package/system-config-printer/Config.in
@@ -0,0 +1,7 @@ 
+config BR2_PACKAGE_SYSTEM_CONFIG_PRINTER
+	bool "system-config-printer"
+	depends on BR2_PACKAGE_CUPS
+	help
+	  Graphical user interface for CUPS administration
+
+	  https://github.com/OpenPrinting/system-config-printer
diff --git a/package/system-config-printer/system-config-printer.hash b/package/system-config-printer/system-config-printer.hash
new file mode 100644
index 0000000000..56b7e47650
--- /dev/null
+++ b/package/system-config-printer/system-config-printer.hash
@@ -0,0 +1,3 @@ 
+# Locally calculated:
+sha256  fbd226ce215bf014213dded81b43b9d4d90470524f5bf7cd21ecc71d4aefa689  system-config-printer-1.5.15.tar.xz
+sha256  8177f97513213526df2cf6184d8ff986c675afb514d4e68a404010521b880643  COPYING
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
+SYSTEM_CONFIG_PRINTER_LICENSE_FILES = COPYING
+SYSTEM_CONFIG_PRINTER_DEPENDENCIES = cups host-intltool
+SYSTEM_CONFIG_PRINTER_AUTORECONF = YES
+
+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
+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))