| Message ID | 20200619145719.2352019-4-angelo@amarulasolutions.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Cups revamp | expand |
Hello Angelo, On Fri, 19 Jun 2020 16:57:15 +0200 Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > This patch bumps cups to version 2.3.3. > While bumping, fixing also the wrong installation of service files: > the rcdir was left to the default value, that means installing the > service files into the /etc/rcX.d directory. > Adding also a simplified systemv service file for loading cupsd. Again, this seems to be mixing up different changes together in the same commit. If I understand correctly what you're saying, you should have at least three different commits: - Fix the rcdir value, due to which the systemd service file was not installed at the right place. - Provide and install an init script - Bump the version. > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> Same issue as PATCH 2/7 about author/SoB. > diff --git a/package/cups/S81cupsd b/package/cups/S81cupsd > new file mode 100644 > index 0000000000..f527d55f1e > --- /dev/null > +++ b/package/cups/S81cupsd Please take inspiration from package/busybox/S01syslogd. > -CUPS_VERSION = 2.3.1 > +CUPS_VERSION = 2.3.3 > CUPS_SOURCE = cups-$(CUPS_VERSION)-source.tar.gz > CUPS_SITE = https://github.com/apple/cups/releases/download/v$(CUPS_VERSION) > CUPS_LICENSE = Apache-2.0 with GPL-2.0/LGPL-2.0 exception > @@ -21,7 +21,8 @@ CUPS_CONF_OPTS = \ > --with-docdir=/usr/share/cups/doc-root \ > --disable-gssapi \ > --disable-pam \ > - --libdir=/usr/lib > + --libdir=/usr/lib \ > + --with-rcdir=no --without-rcdir > +define CUPS_INSTALL_INIT_SYSV > + @$(RM) $(TARGET_DIR)/etc/init.d/cups This should also be done in the systemd case I believe, and should not have the @. Thanks! Thomas
On Sat, Jun 20, 2020 at 10:35 PM Thomas Petazzoni < thomas.petazzoni@bootlin.com> wrote: > Hello Angelo, > > On Fri, 19 Jun 2020 16:57:15 +0200 > Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > > > This patch bumps cups to version 2.3.3. > > While bumping, fixing also the wrong installation of service files: > > the rcdir was left to the default value, that means installing the > > service files into the /etc/rcX.d directory. > > Adding also a simplified systemv service file for loading cupsd. > > Again, this seems to be mixing up different changes together in the > same commit. If I understand correctly what you're saying, you should > have at least three different commits: > > - Fix the rcdir value, due to which the systemd service file was not > installed at the right place. > > - Provide and install an init script > > - Bump the version. > It's really that important to split it? No problem to do that, but this package installs a service and the way it is doing in the new version is the one proposed in the patch. The old version could have been different, I've not checked btw. Anyway, having this is required to have the service correctly installed, so I don't think the patch should be splitted because the bump really requires it. Probably the commit message could be better explaining this. > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> > > Same issue as PATCH 2/7 about author/SoB. > > > > diff --git a/package/cups/S81cupsd b/package/cups/S81cupsd > > new file mode 100644 > > index 0000000000..f527d55f1e > > --- /dev/null > > +++ b/package/cups/S81cupsd > > Please take inspiration from package/busybox/S01syslogd. > Will do. > > -CUPS_VERSION = 2.3.1 > > +CUPS_VERSION = 2.3.3 > > CUPS_SOURCE = cups-$(CUPS_VERSION)-source.tar.gz > > CUPS_SITE = > https://github.com/apple/cups/releases/download/v$(CUPS_VERSION) > > CUPS_LICENSE = Apache-2.0 with GPL-2.0/LGPL-2.0 exception > > @@ -21,7 +21,8 @@ CUPS_CONF_OPTS = \ > > --with-docdir=/usr/share/cups/doc-root \ > > --disable-gssapi \ > > --disable-pam \ > > - --libdir=/usr/lib > > + --libdir=/usr/lib \ > > + --with-rcdir=no > > --without-rcdir > > > +define CUPS_INSTALL_INIT_SYSV > > + @$(RM) $(TARGET_DIR)/etc/init.d/cups > > This should also be done in the systemd case I believe, and should not > have the @. > Not for systemd, cause the package installs the .service file and not the systemv related ones. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >
On Sat, 20 Jun 2020 22:48:55 +0200 Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > It's really that important to split it? No problem to do that, but this > package installs a service and the way it is doing in the new version is > the one proposed in the patch. The old version could have been different, > I've not checked btw. > Anyway, having this is required to have the service correctly installed, so > I don't think the patch should be splitted because the bump really requires > it. How is this related to the bump? Is it the version bump that adds the service file? Or did it already exist before the bump? > > > +define CUPS_INSTALL_INIT_SYSV > > > + @$(RM) $(TARGET_DIR)/etc/init.d/cups > > > > This should also be done in the systemd case I believe, and should not > > have the @. > > > > Not for systemd, cause the package installs the .service file and not the > systemv related ones. OK, I see: when --enable-systemd is used to build cups, it doesn't install its own init script. Makes sense. Thanks for explaining this aspect! Thomas
Il sab 20 giu 2020, 22:55 Thomas Petazzoni <thomas.petazzoni@bootlin.com> ha scritto: > On Sat, 20 Jun 2020 22:48:55 +0200 > Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > > > It's really that important to split it? No problem to do that, but this > > package installs a service and the way it is doing in the new version is > > the one proposed in the patch. The old version could have been different, > > I've not checked btw. > > Anyway, having this is required to have the service correctly installed, > so > > I don't think the patch should be splitted because the bump really > requires > > it. > > How is this related to the bump? Is it the version bump that adds the > service file? Or did it already exist before the bump? > Cups is a service and a service without an init script is useless. So, to have the new version working it needs the rcdir change and the systemv script. Systemd script is instead fine. If you are asking me to even fix the older version I can do that, but I'm doing a bump and fixing at the same time because either the bump will not work. > > > > +define CUPS_INSTALL_INIT_SYSV > > > > + @$(RM) $(TARGET_DIR)/etc/init.d/cups > > > > > > This should also be done in the systemd case I believe, and should not > > > have the @. > > > > > > > Not for systemd, cause the package installs the .service file and not the > > systemv related ones. > > OK, I see: when --enable-systemd is used to build cups, it doesn't > install its own init script. Makes sense. Thanks for explaining this > aspect! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot >
Hello, On Sat, 20 Jun 2020 23:53:12 +0200 Angelo Compagnucci <angelo.compagnucci@gmail.com> wrote: > > How is this related to the bump? Is it the version bump that adds the > > service file? Or did it already exist before the bump? > > Cups is a service and a service without an init script is useless. Absolutely, and this is already true today, independently from your version bump. > So, to have the new version working it needs the rcdir change and the > systemv script. Systemd script is instead fine. But I guess this is also true not just for the "new version", but also for the "current version". > If you are asking me to even fix the older version I can do that, but I'm > doing a bump and fixing at the same time because either the bump will not > work. The two things are independent, they should be addressed separately. Thomas
diff --git a/package/cups/0001-Remove-man-from-BUILDDIRS-in-configure.patch b/package/cups/0001-Remove-man-from-BUILDDIRS-in-configure.patch index b1ab7cbace..7fcf7133c8 100644 --- a/package/cups/0001-Remove-man-from-BUILDDIRS-in-configure.patch +++ b/package/cups/0001-Remove-man-from-BUILDDIRS-in-configure.patch @@ -6,15 +6,17 @@ Subject: [PATCH] Remove man from BUILDDIRS in configure Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de> [Fabrice: updated for 2.3.0] Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +[Michael: updated for 2.3.3] +Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> --- config-scripts/cups-common.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config-scripts/cups-common.m4 b/config-scripts/cups-common.m4 -index fbba715..77d0f5c 100644 +index a460a73..d427acb 100644 --- a/config-scripts/cups-common.m4 +++ b/config-scripts/cups-common.m4 -@@ -446,7 +446,7 @@ AC_ARG_WITH(components, [ --with-components set components to build: +@@ -434,7 +434,7 @@ LIBHEADERSPRIV="\$(COREHEADERSPRIV) \$(DRIVERHEADERSPRIV)" case "$COMPONENTS" in all) @@ -24,5 +26,5 @@ index fbba715..77d0f5c 100644 core) -- -2.8.1 +2.17.1 diff --git a/package/cups/0002-Do-not-use-genstrings.patch b/package/cups/0002-Do-not-use-genstrings.patch index b3566b8b15..c7d6735b5f 100644 --- a/package/cups/0002-Do-not-use-genstrings.patch +++ b/package/cups/0002-Do-not-use-genstrings.patch @@ -16,23 +16,25 @@ genstrings call.] Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> [Fabrice: updated for 2.3.0] Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +[Michael: updated for 2.3.3] +Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> --- ppdc/Makefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/ppdc/Makefile b/ppdc/Makefile -index 68bf6b2..d57a0c9 100644 +index 32e2e0b..7b18879 100644 --- a/ppdc/Makefile +++ b/ppdc/Makefile -@@ -242,8 +242,6 @@ genstrings: genstrings.o libcupsppdc.a ../cups/$(LIBCUPSSTATIC) \ - $(LD_CXX) $(ARCHFLAGS) $(ALL_LDFLAGS) -o genstrings genstrings.o \ - libcupsppdc.a $(LINKCUPSSTATIC) - $(CODE_SIGN) -s "$(CODE_SIGN_IDENTITY)" $@ +@@ -186,8 +186,6 @@ genstrings: genstrings.o libcupsppdc.a ../cups/$(LIBCUPSSTATIC) \ + $(LD_CXX) $(ARCHFLAGS) $(ALL_LDFLAGS) -o genstrings genstrings.o \ + libcupsppdc.a $(LINKCUPSSTATIC) + $(CODE_SIGN) -s "$(CODE_SIGN_IDENTITY)" $@ - echo Generating localization strings... - ./genstrings >sample.c # -- -2.6.4 +2.17.1 diff --git a/package/cups/0004-Remove-PIE-flags-from-the-build.patch b/package/cups/0004-Remove-PIE-flags-from-the-build.patch index 8401e133e9..c2765dff09 100644 --- a/package/cups/0004-Remove-PIE-flags-from-the-build.patch +++ b/package/cups/0004-Remove-PIE-flags-from-the-build.patch @@ -13,15 +13,17 @@ Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Signed-off-by: Olivier Schonken <olivier.schonken@gmail.com> [Fabrice: updated for 2.3.0] Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +[Michael: updated for 2.3.3] +Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> --- Makedefs.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makedefs.in b/Makedefs.in -index 3afef0a..299b297 100644 +index 5f1d32f..d669ea8 100644 --- a/Makedefs.in +++ b/Makedefs.in -@@ -148,7 +148,7 @@ IPPFIND_BIN = @IPPFIND_BIN@ +@@ -155,7 +155,7 @@ ALL_CXXFLAGS = -I.. -D_CUPS_SOURCE $(CXXFLAGS) \ $(ONDEMANDFLAGS) $(OPTIONS) ALL_DSOFLAGS = -L../cups @ARCHFLAGS@ @RELROFLAGS@ $(DSOFLAGS) $(OPTIM) ALL_LDFLAGS = -L../cups @LDARCHFLAGS@ @RELROFLAGS@ $(LDFLAGS) \ @@ -31,5 +33,5 @@ index 3afef0a..299b297 100644 ARFLAGS = @ARFLAGS@ BACKLIBS = @BACKLIBS@ -- -2.7.4 +2.17.1 diff --git a/package/cups/S81cupsd b/package/cups/S81cupsd new file mode 100644 index 0000000000..f527d55f1e --- /dev/null +++ b/package/cups/S81cupsd @@ -0,0 +1,23 @@ +#!/bin/sh + +case "$1" in + start) + printf "Starting cupsd: " + start-stop-daemon -S -q -m -p /var/run/cupsd.pid \ + -b -x cupsd -- -C /etc/cups/cupsd.conf -s /etc/cups/cups-files + [ $? = 0 ] && echo "OK" || echo "FAIL" + ;; + stop) + printf "Stopping cupsd: " + start-stop-daemon -K -q -p /var/run/cupsd.pid + [ $? = 0 ] && echo "OK" || echo "FAIL" + ;; + restart) + "$0" stop + sleep 1 + "$0" start + ;; + *) + echo "Usage: $0 {start|stop|restart}" + ;; +esac diff --git a/package/cups/cups.hash b/package/cups/cups.hash index 8f037c6420..2eb289e209 100644 --- a/package/cups/cups.hash +++ b/package/cups/cups.hash @@ -1,4 +1,4 @@ # Locally calculated: -sha256 1bca9d89507e3f68cbc84482fe46ae8d5333af5bc2b9061347b2007182ac77ce cups-2.3.1-source.tar.gz +sha256 261fd948bce8647b6d5cb2a1784f0c24cc52b5c4e827b71d726020bcc502f3ee cups-2.3.3-source.tar.gz sha256 cfc7749b96f63bd31c3c42b5c471bf756814053e847c10f3eb003417bc523d30 LICENSE sha256 a5d616e6322a9cb1a971e18765025edfca4f3cd9c0eafc32d6d2eb4b8c8787b5 NOTICE diff --git a/package/cups/cups.mk b/package/cups/cups.mk index 18f01d8484..461e0d9143 100644 --- a/package/cups/cups.mk +++ b/package/cups/cups.mk @@ -4,7 +4,7 @@ # ################################################################################ -CUPS_VERSION = 2.3.1 +CUPS_VERSION = 2.3.3 CUPS_SOURCE = cups-$(CUPS_VERSION)-source.tar.gz CUPS_SITE = https://github.com/apple/cups/releases/download/v$(CUPS_VERSION) CUPS_LICENSE = Apache-2.0 with GPL-2.0/LGPL-2.0 exception @@ -21,7 +21,8 @@ CUPS_CONF_OPTS = \ --with-docdir=/usr/share/cups/doc-root \ --disable-gssapi \ --disable-pam \ - --libdir=/usr/lib + --libdir=/usr/lib \ + --with-rcdir=no CUPS_CONFIG_SCRIPTS = cups-config CUPS_DEPENDENCIES = \ host-autoconf \ @@ -71,4 +72,10 @@ else CUPS_CONF_OPTS += --disable-avahi endif +define CUPS_INSTALL_INIT_SYSV + @$(RM) $(TARGET_DIR)/etc/init.d/cups + $(INSTALL) -D -m 0755 package/cups/S81cupsd \ + $(TARGET_DIR)/etc/init.d/S81cupsd +endef + $(eval $(autotools-package))