* [PATCH 0/3] Pakfile metadata enhancements @ 2021-04-23 16:15 Robin Roevens 2021-04-23 16:15 ` [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Robin Roevens ` (4 more replies) 0 siblings, 5 replies; 26+ messages in thread From: Robin Roevens @ 2021-04-23 16:15 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 4623 bytes --] Hi folks During my discussion with Michael in the zabbix_agentd patchset thread about knowing what addon services should be running or not, it came up that it would be handy for several reasons if we had a bit more metadata for pak-files as we currently have. Mostly knowing which services (initscripts) are installed by a pak-file. This would allow for services.cgi to not have to manually try to find out if an installed addon actually has an initscript or not. Also paks like libvirt install 2 initscripts, those can now both be displayed on the services.cgi page. Idem for monitoring agents, which was my main objective. So here is an attempt to achieve this. This is not yet a patchset to be applied yet, but rather a proposal as this change would require all addon LFS-files to be changed. But I didn't want to do that yet as this patchset may very wel be rejected completely :-). The first patch introduces 2 new macro's: - SUMMARY for a short, one-line summary of the package - INITSCRIPTS for a space seperated list of initscripts provided by the package. And an alternative INSTALL_INITSCRIPTS method instead of the current INSTALL_INITSCRIPT method. As we now have all initscripts in the INITSCRIPTS macro, the INSTALL_INITSCRIPTS will install all initscripts listed in that macro, so a simple call to INSTALL_INITSCRIPTS will now do the job instead of multiple calls in case of multiple initscripts (for example libvirt. I noticed clamav actually uses 1 initscript for starting 2 services, this could maybe also be split up again) I included 2 examples in the first patch: libvirt and zabbix_agentd. But when implemented ofcourse all makefiles should be updated. During the pak packaging process in the build procedure, those new macro's will be inheritted in the generated pakfire meta-* files. The second patch adds an extra 'info <pak(s)>' commandline parameter to pakfire, which will in turn call a new Pakfire::pakinfo function. This function wil parse the meta-* file of the requested pak and functions in 2 modes: - "latest" which is the behaviour of the info parameter. This will display the latest available metadata of the pak and the status of the pak on the system as in: is it installed?, and if so, is it up-to-date. - "installed" wich will display only information about the currently installed pak and bail out of the requested pak is not currently installed. This function was added to provide a 'central' point/method to get pak information. I don't know if there are other scripts beside services.cgi that currently try parsing meta-* files. But they should then be changed to use this function instead. Example output of the new pakfire info command: `pakfire info zabbix_agentd`: when installed and up-to-date: --- Name: zabbix_agentd Version: 4.2.6-4 Summary: Zabbix Agent Size: 250.00 KB Dependencies: Pakfile: zabbix_agentd-4.2.6-4.ipfire InitScripts: zabbix_agentd Installed: Yes Status: up-to-date --- When an update is available: --- Name: zabbix_agentd Version: 5.0.10-5 Summary: Zabbix Agent Size: 276.00 KB Dependencies: fping Pakfile: zabbix_agentd-5.0.10-5.ipfire InitScripts: zabbix_agentd Installed: Yes Status: outdated (version 4.2.6-4 is installed) --- Or when a pak was discontinued and no longer supplied by ipfire, but still installed on the system: --- Name: zabbix_agentd Version: - Summary: Zabbix Agent Size: 250.00 KB Dependencies: Pakfile: zabbix_agentd-4.2.6-4.ipfire InitScripts: zabbix_agentd Installed: Yes Status: obsolete (version 4.2.6-4 is installed) --- and at last when a pak is available, but not installed: --- Name: zabbix_agentd Version: 5.0.10-5 Summary: Zabbix Agent Size: 276.00 KB Dependencies: fping Pakfile: zabbix_agentd-5.0.10-5.ipfire InitScripts: zabbix_agentd Installed: No Status: not installed --- And then the last patch is an update of service.cgi now using the new Pakfire::pakinfo function in "installed"-mode. If there are any suggestions on more metadata.. I think this is the moment to throw them at me. And ofcourse suggestions/comments are welcome as this is currently only a proposal for change. But I think we win in robustness of services.cgi and user experience in both using pakfire and ability to provide available services to monitoring agents. On top of that could the whole meta-* files system be overhauled in the future, if wanted, with only pakfire itself needing change as the rest will then depend on pakfire for correctly parsing it's "database". Regards Robin -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-04-23 16:15 [PATCH 0/3] Pakfile metadata enhancements Robin Roevens @ 2021-04-23 16:15 ` Robin Roevens 2021-05-12 18:49 ` Jonatan Schlag 2021-05-14 12:23 ` Michael Tremer 2021-04-23 16:15 ` [PATCH 2/3] pakfire: add 'info <pak(s)>' option Robin Roevens ` (3 subsequent siblings) 4 siblings, 2 replies; 26+ messages in thread From: Robin Roevens @ 2021-04-23 16:15 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 3868 bytes --] * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles. * Add a Summary and InitScripts field to the meta-* addon files containing the value's of those macro's. * Replace the INSTALL_INITSCRIPT makefile macro by a new INSTALL_INITSCRIPTS macro/method that will install all initscripts defined in the new INITSCRIPTS macro. * Adapt libvirt and zabbix_agentd pak as examples of how to use this. Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> --- lfs/Config | 8 ++++++-- lfs/libvirt | 6 ++++-- lfs/zabbix_agentd | 5 ++++- src/pakfire/meta | 2 ++ 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lfs/Config b/lfs/Config index eadbbc408..61d6f0c82 100644 --- a/lfs/Config +++ b/lfs/Config @@ -299,15 +299,19 @@ define PAK # Create meta file sed \ -e "s/NAME/$(PROG)/g" \ + -e "s/SUMMARY/$(SUMMARY)/g" \ -e "s/VER/$(VER)/g" \ -e "s/RELEASE/$(PAK_VER)/g" \ -e "s/DEPS/$(DEPS)/g" \ -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \ + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG) endef -define INSTALL_INITSCRIPT - install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) /etc/rc.d/init.d/$(1) +define INSTALL_INITSCRIPTS + for initscript in $(INITSCRIPTS); do \ + install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$$initscript /etc/rc.d/init.d/$$initscript; \ + done endef ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)" diff --git a/lfs/libvirt b/lfs/libvirt index 28a95d317..be5d3db3a 100644 --- a/lfs/libvirt +++ b/lfs/libvirt @@ -25,6 +25,7 @@ include Config VER = 6.5.0 +SUMMARY = Server side daemon and supporting files for libvirt THISAPP = libvirt-$(VER) DL_FILE = $(THISAPP).tar.xz @@ -37,6 +38,8 @@ PAK_VER = 25 DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu +INITSCRIPTS= libvirtd virtlogd + ############################################################################### # Top-level Rules ############################################################################### @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) cd $(DIR_APP)/build_libvirt && make install #install initscripts - $(call INSTALL_INITSCRIPT,libvirtd) - $(call INSTALL_INITSCRIPT,virtlogd) + @$(INSTALL_INITSCRIPTS) mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests # Backup diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd index c69643a54..a72fe024b 100644 --- a/lfs/zabbix_agentd +++ b/lfs/zabbix_agentd @@ -25,6 +25,7 @@ include Config VER = 4.2.6 +SUMMARY = Zabbix Agent THISAPP = zabbix-$(VER) DL_FILE = $(THISAPP).tar.gz @@ -35,6 +36,8 @@ PROG = zabbix_agentd PAK_VER = 4 DEPS = +INITSCRIPTS= zabbix_agentd + ############################################################################### # Top-level Rules ############################################################################### @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) chown zabbix.zabbix /var/run/zabbix # Install initscripts - $(call INSTALL_INITSCRIPT,zabbix_agentd) + @$(INSTALL_INITSCRIPTS) # Install sudoers include file install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ diff --git a/src/pakfire/meta b/src/pakfire/meta index d97b2a0fa..849b9cd6c 100644 --- a/src/pakfire/meta +++ b/src/pakfire/meta @@ -1,6 +1,8 @@ Name: NAME +Summary: SUMMARY ProgVersion: VER Release: RELEASE Size: SIZE Dependencies: DEPS File: NAME-VER-RELEASE.ipfire +InitScripts: INITSCRIPTS -- 2.31.1 -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-04-23 16:15 ` [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Robin Roevens @ 2021-05-12 18:49 ` Jonatan Schlag 2021-05-14 12:24 ` Michael Tremer 2021-05-14 12:23 ` Michael Tremer 1 sibling, 1 reply; 26+ messages in thread From: Jonatan Schlag @ 2021-05-12 18:49 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 4530 bytes --] Hi, > Am 23.04.2021 um 18:16 schrieb Robin Roevens <robin.roevens(a)disroot.org>: > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles. > * Add a Summary and InitScripts field to the meta-* addon files > containing the value's of those macro's. > * Replace the INSTALL_INITSCRIPT makefile macro by a new > INSTALL_INITSCRIPTS macro/method that will install all initscripts > defined in the new INITSCRIPTS macro. > * Adapt libvirt and zabbix_agentd pak as examples of how to use this. > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > --- > lfs/Config | 8 ++++++-- > lfs/libvirt | 6 ++++-- > lfs/zabbix_agentd | 5 ++++- > src/pakfire/meta | 2 ++ > 4 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/lfs/Config b/lfs/Config > index eadbbc408..61d6f0c82 100644 > --- a/lfs/Config > +++ b/lfs/Config > @@ -299,15 +299,19 @@ define PAK > # Create meta file > sed \ > -e "s/NAME/$(PROG)/g" \ > + -e "s/SUMMARY/$(SUMMARY)/g" \ > -e "s/VER/$(VER)/g" \ > -e "s/RELEASE/$(PAK_VER)/g" \ > -e "s/DEPS/$(DEPS)/g" \ > -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \ > + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ > < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG) > endef > > -define INSTALL_INITSCRIPT > - install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) /etc/rc.d/init.d/$(1) > +define INSTALL_INITSCRIPTS > + for initscript in $(INITSCRIPTS); do \ > + install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$$initscript /etc/rc.d/init.d/$$initscript; \ > + done > endef > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)" > diff --git a/lfs/libvirt b/lfs/libvirt > index 28a95d317..be5d3db3a 100644 > --- a/lfs/libvirt > +++ b/lfs/libvirt > @@ -25,6 +25,7 @@ > include Config > > VER = 6.5.0 > +SUMMARY = Server side daemon and supporting files for libvirt > > THISAPP = libvirt-$(VER) > DL_FILE = $(THISAPP).tar.xz > @@ -37,6 +38,8 @@ PAK_VER = 25 > > DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu > > +INITSCRIPTS= libvirtd virtlogd > + > ############################################################################### > # Top-level Rules > ############################################################################### > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) > cd $(DIR_APP)/build_libvirt && make install > > #install initscripts > - $(call INSTALL_INITSCRIPT,libvirtd) > - $(call INSTALL_INITSCRIPT,virtlogd) > + @$(INSTALL_INITSCRIPTS) > mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests And here my approach maybe breaks :-). How could we handle this? It is not a daemon, more something like a startup/shutdown scripts for machines... (And should not appear in the service.cgi). So parsing the root file may yield wrong results and your way would be the better one. Jonatan > > # Backup > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd > index c69643a54..a72fe024b 100644 > --- a/lfs/zabbix_agentd > +++ b/lfs/zabbix_agentd > @@ -25,6 +25,7 @@ > include Config > > VER = 4.2.6 > +SUMMARY = Zabbix Agent > > THISAPP = zabbix-$(VER) > DL_FILE = $(THISAPP).tar.gz > @@ -35,6 +36,8 @@ PROG = zabbix_agentd > PAK_VER = 4 > DEPS = > > +INITSCRIPTS= zabbix_agentd > + > ############################################################################### > # Top-level Rules > ############################################################################### > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) > chown zabbix.zabbix /var/run/zabbix > > # Install initscripts > - $(call INSTALL_INITSCRIPT,zabbix_agentd) > + @$(INSTALL_INITSCRIPTS) > > # Install sudoers include file > install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ > diff --git a/src/pakfire/meta b/src/pakfire/meta > index d97b2a0fa..849b9cd6c 100644 > --- a/src/pakfire/meta > +++ b/src/pakfire/meta > @@ -1,6 +1,8 @@ > Name: NAME > +Summary: SUMMARY > ProgVersion: VER > Release: RELEASE > Size: SIZE > Dependencies: DEPS > File: NAME-VER-RELEASE.ipfire > +InitScripts: INITSCRIPTS > -- > 2.31.1 > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-05-12 18:49 ` Jonatan Schlag @ 2021-05-14 12:24 ` Michael Tremer 2021-05-14 20:07 ` Robin Roevens 0 siblings, 1 reply; 26+ messages in thread From: Michael Tremer @ 2021-05-14 12:24 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 4959 bytes --] Hello Jonatan, Yes I get your point. Just because a package has more than one initscript does not mean they both should be listed in services.cgi. It is a weird corner case, but how do we want to handle this? -Michael > On 12 May 2021, at 19:49, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote: > > Hi, > >> Am 23.04.2021 um 18:16 schrieb Robin Roevens <robin.roevens(a)disroot.org>: >> >> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles. >> * Add a Summary and InitScripts field to the meta-* addon files >> containing the value's of those macro's. >> * Replace the INSTALL_INITSCRIPT makefile macro by a new >> INSTALL_INITSCRIPTS macro/method that will install all initscripts >> defined in the new INITSCRIPTS macro. >> * Adapt libvirt and zabbix_agentd pak as examples of how to use this. >> >> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> >> --- >> lfs/Config | 8 ++++++-- >> lfs/libvirt | 6 ++++-- >> lfs/zabbix_agentd | 5 ++++- >> src/pakfire/meta | 2 ++ >> 4 files changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/lfs/Config b/lfs/Config >> index eadbbc408..61d6f0c82 100644 >> --- a/lfs/Config >> +++ b/lfs/Config >> @@ -299,15 +299,19 @@ define PAK >> # Create meta file >> sed \ >> -e "s/NAME/$(PROG)/g" \ >> + -e "s/SUMMARY/$(SUMMARY)/g" \ >> -e "s/VER/$(VER)/g" \ >> -e "s/RELEASE/$(PAK_VER)/g" \ >> -e "s/DEPS/$(DEPS)/g" \ >> -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \ >> + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ >> < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG) >> endef >> >> -define INSTALL_INITSCRIPT >> - install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) /etc/rc.d/init.d/$(1) >> +define INSTALL_INITSCRIPTS >> + for initscript in $(INITSCRIPTS); do \ >> + install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$$initscript /etc/rc.d/init.d/$$initscript; \ >> + done >> endef >> >> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)" >> diff --git a/lfs/libvirt b/lfs/libvirt >> index 28a95d317..be5d3db3a 100644 >> --- a/lfs/libvirt >> +++ b/lfs/libvirt >> @@ -25,6 +25,7 @@ >> include Config >> >> VER = 6.5.0 >> +SUMMARY = Server side daemon and supporting files for libvirt >> >> THISAPP = libvirt-$(VER) >> DL_FILE = $(THISAPP).tar.xz >> @@ -37,6 +38,8 @@ PAK_VER = 25 >> >> DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu >> >> +INITSCRIPTS= libvirtd virtlogd >> + >> ############################################################################### >> # Top-level Rules >> ############################################################################### >> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) >> cd $(DIR_APP)/build_libvirt && make install >> >> #install initscripts >> - $(call INSTALL_INITSCRIPT,libvirtd) >> - $(call INSTALL_INITSCRIPT,virtlogd) >> + @$(INSTALL_INITSCRIPTS) >> mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests > And here my approach maybe breaks :-). How could we handle this? It is not a daemon, more something like a startup/shutdown scripts for machines... (And should not appear in the service.cgi). So parsing the root file may yield wrong results and your way would be the better one. > > Jonatan >> >> # Backup >> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd >> index c69643a54..a72fe024b 100644 >> --- a/lfs/zabbix_agentd >> +++ b/lfs/zabbix_agentd >> @@ -25,6 +25,7 @@ >> include Config >> >> VER = 4.2.6 >> +SUMMARY = Zabbix Agent >> >> THISAPP = zabbix-$(VER) >> DL_FILE = $(THISAPP).tar.gz >> @@ -35,6 +36,8 @@ PROG = zabbix_agentd >> PAK_VER = 4 >> DEPS = >> >> +INITSCRIPTS= zabbix_agentd >> + >> ############################################################################### >> # Top-level Rules >> ############################################################################### >> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) >> chown zabbix.zabbix /var/run/zabbix >> >> # Install initscripts >> - $(call INSTALL_INITSCRIPT,zabbix_agentd) >> + @$(INSTALL_INITSCRIPTS) >> >> # Install sudoers include file >> install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ >> diff --git a/src/pakfire/meta b/src/pakfire/meta >> index d97b2a0fa..849b9cd6c 100644 >> --- a/src/pakfire/meta >> +++ b/src/pakfire/meta >> @@ -1,6 +1,8 @@ >> Name: NAME >> +Summary: SUMMARY >> ProgVersion: VER >> Release: RELEASE >> Size: SIZE >> Dependencies: DEPS >> File: NAME-VER-RELEASE.ipfire >> +InitScripts: INITSCRIPTS >> -- >> 2.31.1 >> >> >> -- >> Dit bericht is gescanned op virussen en andere gevaarlijke >> inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-05-14 12:24 ` Michael Tremer @ 2021-05-14 20:07 ` Robin Roevens 2021-05-18 15:09 ` Michael Tremer 0 siblings, 1 reply; 26+ messages in thread From: Robin Roevens @ 2021-05-14 20:07 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 8139 bytes --] Hi Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: > Hello Jonatan, > > Yes I get your point. Just because a package has more than one > initscript does not mean they both should be listed in services.cgi. > > It is a weird corner case, but how do we want to handle this? > > -Michael Maybe a slight renaming of my proposed new variable INITSCRIPTS to SERVICES in the LFS files would make it a bit more clear that this concerns only service-initscripts? The INSTALL_INITSCRIPTS macro could then be renamed to INSTALL_SERVICE_INITSCRIPTS. If we then also keep the original INSTALL_INITSCRIPT macro, that macro can then be used to install non-service initscripts if required. Or even to install all required initscripts in the occasion that you want to skip some service-initscripts defined in SERVICES because they are included in the source, like the situation Jonatan pointed out (in this case the SERVICES variable is filled with the sole purpose to serve as metadata). So this would then result in: --- define INSTALL_INITSCRIPT install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) /etc/rc.d/init.d/$(1) endef define INSTALL_SERVICE_INITSCRIPTS for service in $(SERVICES); do \ $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ done endef --- Optionally we could also add again a variable INITSCRIPTS with the purpose to list non-service initscripts. so SERVICES - for service-initscripts INITSCRIPTS - for non-service-initscripts and in the meta-* files: --- Name: NAME Summary: SUMMARY ProgVersion: VER Release: RELEASE Size: SIZE Dependencies: DEPS File: NAME-VER-RELEASE.ipfire Services: SERVICES InitScripts: INITSCRIPTS --- Maybe that info could also come in handy sooner or later ? In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed back to INSTALL_INITSCRIPTS, now accepting a parameter which is a list of initscripts to install, so one could call: --- $(call INSTALL_INITSCRIPTS,$(SERVICES)) $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS)) --- to install all initscripts. Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS which would then install both SERVICES and INITSCRIPTS (or even replace the above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a macro to install all initscripts (SERVICES and INITSCRIPTS) and a macro to install a single initscript in case some of the ones listed in SERVICES and/or INITSCRIPTS need to be skipped.. But I think the INSTALL_INITSCRIPTS which takes a list of services to install as parameter is probably the most generic one, giving you the possibility to install all SERVICES in one go, and then individually install only those INITSCRIPTS not included in the source if INITSCRIPTS contains such initscript(s) or vice versa. Robin > > > On 12 May 2021, at 19:49, Jonatan Schlag <jonatan.schlag(a)ipfire.org> > > wrote: > > > > Hi, > > > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens < > > > robin.roevens(a)disroot.org>: > > > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles. > > > * Add a Summary and InitScripts field to the meta-* addon files > > > containing the value's of those macro's. > > > * Replace the INSTALL_INITSCRIPT makefile macro by a new > > > INSTALL_INITSCRIPTS macro/method that will install all initscripts > > > defined in the new INITSCRIPTS macro. > > > * Adapt libvirt and zabbix_agentd pak as examples of how to use > > > this. > > > > > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > > > --- > > > lfs/Config | 8 ++++++-- > > > lfs/libvirt | 6 ++++-- > > > lfs/zabbix_agentd | 5 ++++- > > > src/pakfire/meta | 2 ++ > > > 4 files changed, 16 insertions(+), 5 deletions(-) > > > > > > diff --git a/lfs/Config b/lfs/Config > > > index eadbbc408..61d6f0c82 100644 > > > --- a/lfs/Config > > > +++ b/lfs/Config > > > @@ -299,15 +299,19 @@ define PAK > > > # Create meta file > > > sed \ > > > -e "s/NAME/$(PROG)/g" \ > > > + -e "s/SUMMARY/$(SUMMARY)/g" \ > > > -e "s/VER/$(VER)/g" \ > > > -e "s/RELEASE/$(PAK_VER)/g" \ > > > -e "s/DEPS/$(DEPS)/g" \ > > > -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)- > > > $(VER)-$(PAK_VER).ipfire)/g" \ > > > + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ > > > < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG) > > > endef > > > > > > -define INSTALL_INITSCRIPT > > > - install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) > > > /etc/rc.d/init.d/$(1) > > > +define INSTALL_INITSCRIPTS > > > + for initscript in $(INITSCRIPTS); do \ > > > + install -m 754 -v > > > $(DIR_SRC)/src/initscripts/packages/$$initscript > > > /etc/rc.d/init.d/$$initscript; \ > > > + done > > > endef > > > > > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)" > > > diff --git a/lfs/libvirt b/lfs/libvirt > > > index 28a95d317..be5d3db3a 100644 > > > --- a/lfs/libvirt > > > +++ b/lfs/libvirt > > > @@ -25,6 +25,7 @@ > > > include Config > > > > > > VER = 6.5.0 > > > +SUMMARY = Server side daemon and supporting files for > > > libvirt > > > > > > THISAPP = libvirt-$(VER) > > > DL_FILE = $(THISAPP).tar.xz > > > @@ -37,6 +38,8 @@ PAK_VER = 25 > > > > > > DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu > > > > > > +INITSCRIPTS= libvirtd virtlogd > > > + > > > ################################################################### > > > ############ > > > # Top-level Rules > > > ################################################################### > > > ############ > > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst > > > %,$(DIR_DL)/%,$(objects)) > > > cd $(DIR_APP)/build_libvirt && make install > > > > > > #install initscripts > > > - $(call INSTALL_INITSCRIPT,libvirtd) > > > - $(call INSTALL_INITSCRIPT,virtlogd) > > > + @$(INSTALL_INITSCRIPTS) > > > mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests > > And here my approach maybe breaks :-). How could we handle this? It > > is not a daemon, more something like a startup/shutdown scripts for > > machines... (And should not appear in the service.cgi). So parsing > > the root file may yield wrong results and your way would be the > > better one. > > > > Jonatan > > > > > > # Backup > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd > > > index c69643a54..a72fe024b 100644 > > > --- a/lfs/zabbix_agentd > > > +++ b/lfs/zabbix_agentd > > > @@ -25,6 +25,7 @@ > > > include Config > > > > > > VER = 4.2.6 > > > +SUMMARY = Zabbix Agent > > > > > > THISAPP = zabbix-$(VER) > > > DL_FILE = $(THISAPP).tar.gz > > > @@ -35,6 +36,8 @@ PROG = zabbix_agentd > > > PAK_VER = 4 > > > DEPS = > > > > > > +INITSCRIPTS= zabbix_agentd > > > + > > > ################################################################### > > > ############ > > > # Top-level Rules > > > ################################################################### > > > ############ > > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst > > > %,$(DIR_DL)/%,$(objects)) > > > chown zabbix.zabbix /var/run/zabbix > > > > > > # Install initscripts > > > - $(call INSTALL_INITSCRIPT,zabbix_agentd) > > > + @$(INSTALL_INITSCRIPTS) > > > > > > # Install sudoers include file > > > install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ > > > diff --git a/src/pakfire/meta b/src/pakfire/meta > > > index d97b2a0fa..849b9cd6c 100644 > > > --- a/src/pakfire/meta > > > +++ b/src/pakfire/meta > > > @@ -1,6 +1,8 @@ > > > Name: NAME > > > +Summary: SUMMARY > > > ProgVersion: VER > > > Release: RELEASE > > > Size: SIZE > > > Dependencies: DEPS > > > File: NAME-VER-RELEASE.ipfire > > > +InitScripts: INITSCRIPTS > > > -- > > > 2.31.1 > > > > > > > > > -- > > > Dit bericht is gescanned op virussen en andere gevaarlijke > > > inhoud door MailScanner en lijkt schoon te zijn. > > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-05-14 20:07 ` Robin Roevens @ 2021-05-18 15:09 ` Michael Tremer 2021-05-24 20:23 ` Robin Roevens 0 siblings, 1 reply; 26+ messages in thread From: Michael Tremer @ 2021-05-18 15:09 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 8480 bytes --] > On 14 May 2021, at 21:07, Robin Roevens <robin.roevens(a)disroot.org> wrote: > > Hi > > Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: >> Hello Jonatan, >> >> Yes I get your point. Just because a package has more than one >> initscript does not mean they both should be listed in services.cgi. >> >> It is a weird corner case, but how do we want to handle this? >> >> -Michael > > Maybe a slight renaming of my proposed new variable INITSCRIPTS to > SERVICES in the LFS files would make it a bit more clear that this > concerns only service-initscripts? > > The INSTALL_INITSCRIPTS macro could then be renamed to > INSTALL_SERVICE_INITSCRIPTS. > > If we then also keep the original INSTALL_INITSCRIPT macro, that macro > can then be used to install non-service initscripts if required. Or > even to install all required initscripts in the occasion that you want > to skip some service-initscripts defined in SERVICES because they are > included in the source, like the situation Jonatan pointed out (in this > case the SERVICES variable is filled with the sole purpose to serve as > metadata). > > So this would then result in: > --- > define INSTALL_INITSCRIPT > install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) > /etc/rc.d/init.d/$(1) > endef > > define INSTALL_SERVICE_INITSCRIPTS > for service in $(SERVICES); do \ > $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ > done > endef > --- > > Optionally we could also add again a variable INITSCRIPTS with the > purpose to list non-service initscripts. so > SERVICES - for service-initscripts > INITSCRIPTS - for non-service-initscripts > > and in the meta-* files: > --- > Name: NAME > Summary: SUMMARY > ProgVersion: VER > Release: RELEASE > Size: SIZE > Dependencies: DEPS > File: NAME-VER-RELEASE.ipfire > Services: SERVICES > InitScripts: INITSCRIPTS > --- > > Maybe that info could also come in handy sooner or later ? This sounds rather complicated to me. What are our objectives here? Does Zabbix want/need to start/stop services? And if so, why? > In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed > back to INSTALL_INITSCRIPTS, now accepting a parameter which is a list > of initscripts to install, so one could call: > --- > $(call INSTALL_INITSCRIPTS,$(SERVICES)) > $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS)) > --- > to install all initscripts. > > Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS which > would then install both SERVICES and INITSCRIPTS (or even replace the > above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a macro to > install all initscripts (SERVICES and INITSCRIPTS) and a macro to > install a single initscript in case some of the ones listed in SERVICES > and/or INITSCRIPTS need to be skipped.. > > But I think the INSTALL_INITSCRIPTS which takes a list of services to > install as parameter is probably the most generic one, giving you the > possibility to install all SERVICES in one go, and then individually > install only those INITSCRIPTS not included in the source if > INITSCRIPTS contains such initscript(s) or vice versa. > > Robin > >> >>> On 12 May 2021, at 19:49, Jonatan Schlag <jonatan.schlag(a)ipfire.org> >>> wrote: >>> >>> Hi, >>> >>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens < >>>> robin.roevens(a)disroot.org>: >>>> >>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles. >>>> * Add a Summary and InitScripts field to the meta-* addon files >>>> containing the value's of those macro's. >>>> * Replace the INSTALL_INITSCRIPT makefile macro by a new >>>> INSTALL_INITSCRIPTS macro/method that will install all initscripts >>>> defined in the new INITSCRIPTS macro. >>>> * Adapt libvirt and zabbix_agentd pak as examples of how to use >>>> this. >>>> >>>> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> >>>> --- >>>> lfs/Config | 8 ++++++-- >>>> lfs/libvirt | 6 ++++-- >>>> lfs/zabbix_agentd | 5 ++++- >>>> src/pakfire/meta | 2 ++ >>>> 4 files changed, 16 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/lfs/Config b/lfs/Config >>>> index eadbbc408..61d6f0c82 100644 >>>> --- a/lfs/Config >>>> +++ b/lfs/Config >>>> @@ -299,15 +299,19 @@ define PAK >>>> # Create meta file >>>> sed \ >>>> -e "s/NAME/$(PROG)/g" \ >>>> + -e "s/SUMMARY/$(SUMMARY)/g" \ >>>> -e "s/VER/$(VER)/g" \ >>>> -e "s/RELEASE/$(PAK_VER)/g" \ >>>> -e "s/DEPS/$(DEPS)/g" \ >>>> -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)- >>>> $(VER)-$(PAK_VER).ipfire)/g" \ >>>> + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ >>>> < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG) >>>> endef >>>> >>>> -define INSTALL_INITSCRIPT >>>> - install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) >>>> /etc/rc.d/init.d/$(1) >>>> +define INSTALL_INITSCRIPTS >>>> + for initscript in $(INITSCRIPTS); do \ >>>> + install -m 754 -v >>>> $(DIR_SRC)/src/initscripts/packages/$$initscript >>>> /etc/rc.d/init.d/$$initscript; \ >>>> + done >>>> endef >>>> >>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)" >>>> diff --git a/lfs/libvirt b/lfs/libvirt >>>> index 28a95d317..be5d3db3a 100644 >>>> --- a/lfs/libvirt >>>> +++ b/lfs/libvirt >>>> @@ -25,6 +25,7 @@ >>>> include Config >>>> >>>> VER = 6.5.0 >>>> +SUMMARY = Server side daemon and supporting files for >>>> libvirt >>>> >>>> THISAPP = libvirt-$(VER) >>>> DL_FILE = $(THISAPP).tar.xz >>>> @@ -37,6 +38,8 @@ PAK_VER = 25 >>>> >>>> DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu >>>> >>>> +INITSCRIPTS= libvirtd virtlogd >>>> + >>>> ################################################################### >>>> ############ >>>> # Top-level Rules >>>> ################################################################### >>>> ############ >>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst >>>> %,$(DIR_DL)/%,$(objects)) >>>> cd $(DIR_APP)/build_libvirt && make install >>>> >>>> #install initscripts >>>> - $(call INSTALL_INITSCRIPT,libvirtd) >>>> - $(call INSTALL_INITSCRIPT,virtlogd) >>>> + @$(INSTALL_INITSCRIPTS) >>>> mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests >>> And here my approach maybe breaks :-). How could we handle this? It >>> is not a daemon, more something like a startup/shutdown scripts for >>> machines... (And should not appear in the service.cgi). So parsing >>> the root file may yield wrong results and your way would be the >>> better one. >>> >>> Jonatan >>>> >>>> # Backup >>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd >>>> index c69643a54..a72fe024b 100644 >>>> --- a/lfs/zabbix_agentd >>>> +++ b/lfs/zabbix_agentd >>>> @@ -25,6 +25,7 @@ >>>> include Config >>>> >>>> VER = 4.2.6 >>>> +SUMMARY = Zabbix Agent >>>> >>>> THISAPP = zabbix-$(VER) >>>> DL_FILE = $(THISAPP).tar.gz >>>> @@ -35,6 +36,8 @@ PROG = zabbix_agentd >>>> PAK_VER = 4 >>>> DEPS = >>>> >>>> +INITSCRIPTS= zabbix_agentd >>>> + >>>> ################################################################### >>>> ############ >>>> # Top-level Rules >>>> ################################################################### >>>> ############ >>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst >>>> %,$(DIR_DL)/%,$(objects)) >>>> chown zabbix.zabbix /var/run/zabbix >>>> >>>> # Install initscripts >>>> - $(call INSTALL_INITSCRIPT,zabbix_agentd) >>>> + @$(INSTALL_INITSCRIPTS) >>>> >>>> # Install sudoers include file >>>> install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ >>>> diff --git a/src/pakfire/meta b/src/pakfire/meta >>>> index d97b2a0fa..849b9cd6c 100644 >>>> --- a/src/pakfire/meta >>>> +++ b/src/pakfire/meta >>>> @@ -1,6 +1,8 @@ >>>> Name: NAME >>>> +Summary: SUMMARY >>>> ProgVersion: VER >>>> Release: RELEASE >>>> Size: SIZE >>>> Dependencies: DEPS >>>> File: NAME-VER-RELEASE.ipfire >>>> +InitScripts: INITSCRIPTS >>>> -- >>>> 2.31.1 >>>> >>>> >>>> -- >>>> Dit bericht is gescanned op virussen en andere gevaarlijke >>>> inhoud door MailScanner en lijkt schoon te zijn. >> >> > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-05-18 15:09 ` Michael Tremer @ 2021-05-24 20:23 ` Robin Roevens 2021-06-06 18:41 ` Robin Roevens 2021-06-10 11:24 ` Michael Tremer 0 siblings, 2 replies; 26+ messages in thread From: Robin Roevens @ 2021-05-24 20:23 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 12389 bytes --] Hi Michael Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]: > > > > On 14 May 2021, at 21:07, Robin Roevens <robin.roevens(a)disroot.org> > > wrote: > > > > Hi > > > > Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: > > > Hello Jonatan, > > > > > > Yes I get your point. Just because a package has more than one > > > initscript does not mean they both should be listed in > > > services.cgi. > > > > > > It is a weird corner case, but how do we want to handle this? > > > > > > -Michael > > > > Maybe a slight renaming of my proposed new variable INITSCRIPTS to > > SERVICES in the LFS files would make it a bit more clear that this > > concerns only service-initscripts? > > > > The INSTALL_INITSCRIPTS macro could then be renamed to > > INSTALL_SERVICE_INITSCRIPTS. > > > > If we then also keep the original INSTALL_INITSCRIPT macro, that > > macro > > can then be used to install non-service initscripts if required. > > Or > > even to install all required initscripts in the occasion that you > > want > > to skip some service-initscripts defined in SERVICES because they > > are > > included in the source, like the situation Jonatan pointed out (in > > this > > case the SERVICES variable is filled with the sole purpose to serve > > as > > metadata). > > > > So this would then result in: > > --- > > define INSTALL_INITSCRIPT > > install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) > > /etc/rc.d/init.d/$(1) > > endef > > > > define INSTALL_SERVICE_INITSCRIPTS > > for service in $(SERVICES); do \ > > $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ > > done > > endef > > --- > > > > Optionally we could also add again a variable INITSCRIPTS with the > > purpose to list non-service initscripts. so > > SERVICES - for service-initscripts > > INITSCRIPTS - for non-service-initscripts > > > > and in the meta-* files: > > --- > > Name: NAME > > Summary: SUMMARY > > ProgVersion: VER > > Release: RELEASE > > Size: SIZE > > Dependencies: DEPS > > File: NAME-VER-RELEASE.ipfire > > Services: SERVICES > > InitScripts: INITSCRIPTS > > --- > > > > Maybe that info could also come in handy sooner or later ? > > This sounds rather complicated to me. > > What are our objectives here? Does Zabbix want/need to start/stop > services? And if so, why? I was not directly thinking of Zabbix specifically here. The objective is to have a central 'database' of metadata for the pak's that can come in handy for current and possibly future functionality of IPFire. And a single way of retrieving this information (both by using "pakfire info" or directly accessing the pakfire library, in my proposal). Knowing which pak's provide which services can be used in service.cgi (which currently does give users the possibility to start/stop services indeed) and can be used for monitoring agents like Zabbix, but also Nagios or possibly others to know which services to monitor. This instead of my earlier proposal of re-using parts of the services.cgi code in a separate script for zabbix_agentd, which you pointed out that should be done cleaner using some central way of storing/retrieving that information. Zabbix does have functionality to implement starting/stopping those services using a context menu in the Zabbix web GUI; and a user can easily implement that on it's own (as this should be done mainly on server-side anyway) if he wants to and has the information of which services should run. This could be done by zabbix server logging into the IPFire machine using ssh and using addonctrl or by the agent, which would then need permission to addonctrl start/stop. But I think that is something for the user to decide on his own as you pointed out previously, giving zabbix agent root permission on addonctrl start/stop is a possible extra security risk. So I don't think we should provide that out of the box. (Maybe it could be a configurable 'setting' in the IPFire webgui some time in the future, but not something to focus on currently) But with that said, I don't really understand your remark here as I only propose a way for the LFS files to populate the "Services" meta- data and I only threw in "Initscripts" for non-service initscripts, just for completeness of the meta-data. And to prevent duplication of the list of initscripts, I added a macro to install those in one go instead of again 'defining' this list by needing to call INSTALL_INITSCRIPT individually for each initscript while we now have a list of them in a variable. And due to the remark of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak maintainer to use in corner cases. But this has nothing to do with actually stopping/starting services other that providing information about which services are installed (and maybe could be stopped/started if wanted). If it would make things more clear, I could work my last mail out in an actual patch ? Regards Robin > > > In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed > > back to INSTALL_INITSCRIPTS, now accepting a parameter which is a > > list > > of initscripts to install, so one could call: > > --- > > $(call INSTALL_INITSCRIPTS,$(SERVICES)) > > $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS)) > > --- > > to install all initscripts. > > > > Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS > > which > > would then install both SERVICES and INITSCRIPTS (or even replace > > the > > above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a > > macro to > > install all initscripts (SERVICES and INITSCRIPTS) and a macro to > > install a single initscript in case some of the ones listed in > > SERVICES > > and/or INITSCRIPTS need to be skipped.. > > > > But I think the INSTALL_INITSCRIPTS which takes a list of services > > to > > install as parameter is probably the most generic one, giving you > > the > > possibility to install all SERVICES in one go, and then > > individually > > install only those INITSCRIPTS not included in the source if > > INITSCRIPTS contains such initscript(s) or vice versa. > > > > Robin > > > > > > > > > On 12 May 2021, at 19:49, Jonatan Schlag < > > > > jonatan.schlag(a)ipfire.org> > > > > wrote: > > > > > > > > Hi, > > > > > > > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens < > > > > > robin.roevens(a)disroot.org>: > > > > > > > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS- > > > > > makefiles. > > > > > * Add a Summary and InitScripts field to the meta-* addon > > > > > files > > > > > containing the value's of those macro's. > > > > > * Replace the INSTALL_INITSCRIPT makefile macro by a new > > > > > INSTALL_INITSCRIPTS macro/method that will install all > > > > > initscripts > > > > > defined in the new INITSCRIPTS macro. > > > > > * Adapt libvirt and zabbix_agentd pak as examples of how to > > > > > use > > > > > this. > > > > > > > > > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > > > > > --- > > > > > lfs/Config | 8 ++++++-- > > > > > lfs/libvirt | 6 ++++-- > > > > > lfs/zabbix_agentd | 5 ++++- > > > > > src/pakfire/meta | 2 ++ > > > > > 4 files changed, 16 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/lfs/Config b/lfs/Config > > > > > index eadbbc408..61d6f0c82 100644 > > > > > --- a/lfs/Config > > > > > +++ b/lfs/Config > > > > > @@ -299,15 +299,19 @@ define PAK > > > > > # Create meta file > > > > > sed \ > > > > > -e "s/NAME/$(PROG)/g" \ > > > > > + -e "s/SUMMARY/$(SUMMARY)/g" \ > > > > > -e "s/VER/$(VER)/g" \ > > > > > -e "s/RELEASE/$(PAK_VER)/g" \ > > > > > -e "s/DEPS/$(DEPS)/g" \ > > > > > -e "s/SIZE/$$(stat --format=%s > > > > > /install/packages/$(PROG)- > > > > > $(VER)-$(PAK_VER).ipfire)/g" \ > > > > > + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ > > > > > < /usr/src/src/pakfire/meta > /install/packages/meta- > > > > > $(PROG) > > > > > endef > > > > > > > > > > -define INSTALL_INITSCRIPT > > > > > - install -m 754 -v > > > > > $(DIR_SRC)/src/initscripts/packages/$(1) > > > > > /etc/rc.d/init.d/$(1) > > > > > +define INSTALL_INITSCRIPTS > > > > > + for initscript in $(INITSCRIPTS); do \ > > > > > + install -m 754 -v > > > > > $(DIR_SRC)/src/initscripts/packages/$$initscript > > > > > /etc/rc.d/init.d/$$initscript; \ > > > > > + done > > > > > endef > > > > > > > > > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 > > > > > riscv64)" > > > > > diff --git a/lfs/libvirt b/lfs/libvirt > > > > > index 28a95d317..be5d3db3a 100644 > > > > > --- a/lfs/libvirt > > > > > +++ b/lfs/libvirt > > > > > @@ -25,6 +25,7 @@ > > > > > include Config > > > > > > > > > > VER = 6.5.0 > > > > > +SUMMARY = Server side daemon and supporting files for > > > > > libvirt > > > > > > > > > > THISAPP = libvirt-$(VER) > > > > > DL_FILE = $(THISAPP).tar.xz > > > > > @@ -37,6 +38,8 @@ PAK_VER = 25 > > > > > > > > > > DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu > > > > > > > > > > +INITSCRIPTS= libvirtd virtlogd > > > > > + > > > > > ############################################################# > > > > > ###### > > > > > ############ > > > > > # Top-level Rules > > > > > ############################################################# > > > > > ###### > > > > > ############ > > > > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst > > > > > %,$(DIR_DL)/%,$(objects)) > > > > > cd $(DIR_APP)/build_libvirt && make install > > > > > > > > > > #install initscripts > > > > > - $(call INSTALL_INITSCRIPT,libvirtd) > > > > > - $(call INSTALL_INITSCRIPT,virtlogd) > > > > > + @$(INSTALL_INITSCRIPTS) > > > > > mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt- > > > > > guests > > > > And here my approach maybe breaks :-). How could we handle > > > > this? It > > > > is not a daemon, more something like a startup/shutdown scripts > > > > for > > > > machines... (And should not appear in the service.cgi). So > > > > parsing > > > > the root file may yield wrong results and your way would be the > > > > better one. > > > > > > > > Jonatan > > > > > > > > > > # Backup > > > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd > > > > > index c69643a54..a72fe024b 100644 > > > > > --- a/lfs/zabbix_agentd > > > > > +++ b/lfs/zabbix_agentd > > > > > @@ -25,6 +25,7 @@ > > > > > include Config > > > > > > > > > > VER = 4.2.6 > > > > > +SUMMARY = Zabbix Agent > > > > > > > > > > THISAPP = zabbix-$(VER) > > > > > DL_FILE = $(THISAPP).tar.gz > > > > > @@ -35,6 +36,8 @@ PROG = zabbix_agentd > > > > > PAK_VER = 4 > > > > > DEPS = > > > > > > > > > > +INITSCRIPTS= zabbix_agentd > > > > > + > > > > > ############################################################# > > > > > ###### > > > > > ############ > > > > > # Top-level Rules > > > > > ############################################################# > > > > > ###### > > > > > ############ > > > > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst > > > > > %,$(DIR_DL)/%,$(objects)) > > > > > chown zabbix.zabbix /var/run/zabbix > > > > > > > > > > # Install initscripts > > > > > - $(call INSTALL_INITSCRIPT,zabbix_agentd) > > > > > + @$(INSTALL_INITSCRIPTS) > > > > > > > > > > # Install sudoers include file > > > > > install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ > > > > > diff --git a/src/pakfire/meta b/src/pakfire/meta > > > > > index d97b2a0fa..849b9cd6c 100644 > > > > > --- a/src/pakfire/meta > > > > > +++ b/src/pakfire/meta > > > > > @@ -1,6 +1,8 @@ > > > > > Name: NAME > > > > > +Summary: SUMMARY > > > > > ProgVersion: VER > > > > > Release: RELEASE > > > > > Size: SIZE > > > > > Dependencies: DEPS > > > > > File: NAME-VER-RELEASE.ipfire > > > > > +InitScripts: INITSCRIPTS > > > > > -- > > > > > 2.31.1 > > > > > > > > > > > > > > > -- > > > > > Dit bericht is gescanned op virussen en andere gevaarlijke > > > > > inhoud door MailScanner en lijkt schoon te zijn. > > > > > > > > > > > > -- > > Dit bericht is gescanned op virussen en andere gevaarlijke > > inhoud door MailScanner en lijkt schoon te zijn. > > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-05-24 20:23 ` Robin Roevens @ 2021-06-06 18:41 ` Robin Roevens 2021-06-10 11:27 ` Michael Tremer 2021-06-10 11:24 ` Michael Tremer 1 sibling, 1 reply; 26+ messages in thread From: Robin Roevens @ 2021-06-06 18:41 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 14167 bytes --] Hi Michael I know/see a lot of activity here, so I assume you are always quite busy and may have missed my last reaction? Should I proceed with my proposition to add Summary: SUMMARY Services: SERVICES to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro Should I also add InitScripts: INITSCRIPTS for non-service initscripts.. just for completion of metadata ? And in that case a more generic INSTALL_INITSCRIPTS macro accepting a list of initscripts to install? (see mail history below for explanation of my intentions) Or should we find other methods to have this info ready for services.cgi and/or monitoring agents ? Regards Robin Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]: > Hi Michael > > Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]: > > > > > > > On 14 May 2021, at 21:07, Robin Roevens <robin.roevens(a)disroot.org> > > > wrote: > > > > > > Hi > > > > > > Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: > > > > Hello Jonatan, > > > > > > > > Yes I get your point. Just because a package has more than one > > > > initscript does not mean they both should be listed in > > > > services.cgi. > > > > > > > > It is a weird corner case, but how do we want to handle this? > > > > > > > > -Michael > > > > > > Maybe a slight renaming of my proposed new variable INITSCRIPTS to > > > SERVICES in the LFS files would make it a bit more clear that this > > > concerns only service-initscripts? > > > > > > The INSTALL_INITSCRIPTS macro could then be renamed to > > > INSTALL_SERVICE_INITSCRIPTS. > > > > > > If we then also keep the original INSTALL_INITSCRIPT macro, that > > > macro > > > can then be used to install non-service initscripts if required. > > > Or > > > even to install all required initscripts in the occasion that you > > > want > > > to skip some service-initscripts defined in SERVICES because they > > > are > > > included in the source, like the situation Jonatan pointed out (in > > > this > > > case the SERVICES variable is filled with the sole purpose to serve > > > as > > > metadata). > > > > > > So this would then result in: > > > --- > > > define INSTALL_INITSCRIPT > > > install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) > > > /etc/rc.d/init.d/$(1) > > > endef > > > > > > define INSTALL_SERVICE_INITSCRIPTS > > > for service in $(SERVICES); do \ > > > $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ > > > done > > > endef > > > --- > > > > > > Optionally we could also add again a variable INITSCRIPTS with the > > > purpose to list non-service initscripts. so > > > SERVICES - for service-initscripts > > > INITSCRIPTS - for non-service-initscripts > > > > > > and in the meta-* files: > > > --- > > > Name: NAME > > > Summary: SUMMARY > > > ProgVersion: VER > > > Release: RELEASE > > > Size: SIZE > > > Dependencies: DEPS > > > File: NAME-VER-RELEASE.ipfire > > > Services: SERVICES > > > InitScripts: INITSCRIPTS > > > --- > > > > > > Maybe that info could also come in handy sooner or later ? > > > > This sounds rather complicated to me. > > > > What are our objectives here? Does Zabbix want/need to start/stop > > services? And if so, why? > > I was not directly thinking of Zabbix specifically here. The objective > is to have a central 'database' of metadata for the pak's that can come > in handy for current and possibly future functionality of IPFire. And a > single way of retrieving this information (both by using "pakfire info" > or directly accessing the pakfire library, in my proposal). > > Knowing which pak's provide which services can be used in service.cgi > (which currently does give users the possibility to start/stop services > indeed) and can be used for monitoring agents like Zabbix, but also > Nagios or possibly others to know which services to monitor. > > This instead of my earlier proposal of re-using parts of the > services.cgi code in a separate script for zabbix_agentd, which you > pointed out that should be done cleaner using some central way of > storing/retrieving that information. > > Zabbix does have functionality to implement starting/stopping those > services using a context menu in the Zabbix web GUI; and a user can > easily implement that on it's own (as this should be done mainly on > server-side anyway) if he wants to and has the information of which > services should run. This could be done by zabbix server logging into > the IPFire machine using ssh and using addonctrl or by the agent, which > would then need permission to addonctrl start/stop. But I think that is > something for the user to decide on his own as you pointed out > previously, giving zabbix agent root permission on addonctrl start/stop > is a possible extra security risk. So I don't think we should provide > that out of the box. (Maybe it could be a configurable 'setting' in the > IPFire webgui some time in the future, but not something to focus on > currently) > > But with that said, I don't really understand your remark here as I > only propose a way for the LFS files to populate the "Services" meta- > data and I only threw in "Initscripts" for non-service initscripts, > just for completeness of the meta-data. > And to prevent duplication of the list of initscripts, I added a macro > to install those in one go instead of again 'defining' this list by > needing to call INSTALL_INITSCRIPT individually for each initscript > while we now have a list of them in a variable. And due to the remark > of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak > maintainer to use in corner cases. > > But this has nothing to do with actually stopping/starting services > other that providing information about which services are installed > (and maybe could be stopped/started if wanted). > > If it would make things more clear, I could work my last mail out in an > actual patch ? > > Regards > Robin > > > > > > In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed > > > back to INSTALL_INITSCRIPTS, now accepting a parameter which is a > > > list > > > of initscripts to install, so one could call: > > > --- > > > $(call INSTALL_INITSCRIPTS,$(SERVICES)) > > > $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS)) > > > --- > > > to install all initscripts. > > > > > > Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS > > > which > > > would then install both SERVICES and INITSCRIPTS (or even replace > > > the > > > above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a > > > macro to > > > install all initscripts (SERVICES and INITSCRIPTS) and a macro to > > > install a single initscript in case some of the ones listed in > > > SERVICES > > > and/or INITSCRIPTS need to be skipped.. > > > > > > But I think the INSTALL_INITSCRIPTS which takes a list of services > > > to > > > install as parameter is probably the most generic one, giving you > > > the > > > possibility to install all SERVICES in one go, and then > > > individually > > > install only those INITSCRIPTS not included in the source if > > > INITSCRIPTS contains such initscript(s) or vice versa. > > > > > > Robin > > > > > > > > > > > > On 12 May 2021, at 19:49, Jonatan Schlag < > > > > > jonatan.schlag(a)ipfire.org> > > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens < > > > > > > robin.roevens(a)disroot.org>: > > > > > > > > > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS- > > > > > > makefiles. > > > > > > * Add a Summary and InitScripts field to the meta-* addon > > > > > > files > > > > > > containing the value's of those macro's. > > > > > > * Replace the INSTALL_INITSCRIPT makefile macro by a new > > > > > > INSTALL_INITSCRIPTS macro/method that will install all > > > > > > initscripts > > > > > > defined in the new INITSCRIPTS macro. > > > > > > * Adapt libvirt and zabbix_agentd pak as examples of how to > > > > > > use > > > > > > this. > > > > > > > > > > > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > > > > > > --- > > > > > > lfs/Config | 8 ++++++-- > > > > > > lfs/libvirt | 6 ++++-- > > > > > > lfs/zabbix_agentd | 5 ++++- > > > > > > src/pakfire/meta | 2 ++ > > > > > > 4 files changed, 16 insertions(+), 5 deletions(-) > > > > > > > > > > > > diff --git a/lfs/Config b/lfs/Config > > > > > > index eadbbc408..61d6f0c82 100644 > > > > > > --- a/lfs/Config > > > > > > +++ b/lfs/Config > > > > > > @@ -299,15 +299,19 @@ define PAK > > > > > > # Create meta file > > > > > > sed \ > > > > > > -e "s/NAME/$(PROG)/g" \ > > > > > > + -e "s/SUMMARY/$(SUMMARY)/g" \ > > > > > > -e "s/VER/$(VER)/g" \ > > > > > > -e "s/RELEASE/$(PAK_VER)/g" \ > > > > > > -e "s/DEPS/$(DEPS)/g" \ > > > > > > -e "s/SIZE/$$(stat --format=%s > > > > > > /install/packages/$(PROG)- > > > > > > $(VER)-$(PAK_VER).ipfire)/g" \ > > > > > > + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ > > > > > > < /usr/src/src/pakfire/meta > /install/packages/meta- > > > > > > $(PROG) > > > > > > endef > > > > > > > > > > > > -define INSTALL_INITSCRIPT > > > > > > - install -m 754 -v > > > > > > $(DIR_SRC)/src/initscripts/packages/$(1) > > > > > > /etc/rc.d/init.d/$(1) > > > > > > +define INSTALL_INITSCRIPTS > > > > > > + for initscript in $(INITSCRIPTS); do \ > > > > > > + install -m 754 -v > > > > > > $(DIR_SRC)/src/initscripts/packages/$$initscript > > > > > > /etc/rc.d/init.d/$$initscript; \ > > > > > > + done > > > > > > endef > > > > > > > > > > > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 > > > > > > riscv64)" > > > > > > diff --git a/lfs/libvirt b/lfs/libvirt > > > > > > index 28a95d317..be5d3db3a 100644 > > > > > > --- a/lfs/libvirt > > > > > > +++ b/lfs/libvirt > > > > > > @@ -25,6 +25,7 @@ > > > > > > include Config > > > > > > > > > > > > VER = 6.5.0 > > > > > > +SUMMARY = Server side daemon and supporting files for > > > > > > libvirt > > > > > > > > > > > > THISAPP = libvirt-$(VER) > > > > > > DL_FILE = $(THISAPP).tar.xz > > > > > > @@ -37,6 +38,8 @@ PAK_VER = 25 > > > > > > > > > > > > DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu > > > > > > > > > > > > +INITSCRIPTS= libvirtd virtlogd > > > > > > + > > > > > > ############################################################# > > > > > > ###### > > > > > > ############ > > > > > > # Top-level Rules > > > > > > ############################################################# > > > > > > ###### > > > > > > ############ > > > > > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst > > > > > > %,$(DIR_DL)/%,$(objects)) > > > > > > cd $(DIR_APP)/build_libvirt && make install > > > > > > > > > > > > #install initscripts > > > > > > - $(call INSTALL_INITSCRIPT,libvirtd) > > > > > > - $(call INSTALL_INITSCRIPT,virtlogd) > > > > > > + @$(INSTALL_INITSCRIPTS) > > > > > > mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt- > > > > > > guests > > > > > And here my approach maybe breaks :-). How could we handle > > > > > this? It > > > > > is not a daemon, more something like a startup/shutdown scripts > > > > > for > > > > > machines... (And should not appear in the service.cgi). So > > > > > parsing > > > > > the root file may yield wrong results and your way would be the > > > > > better one. > > > > > > > > > > Jonatan > > > > > > > > > > > > # Backup > > > > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd > > > > > > index c69643a54..a72fe024b 100644 > > > > > > --- a/lfs/zabbix_agentd > > > > > > +++ b/lfs/zabbix_agentd > > > > > > @@ -25,6 +25,7 @@ > > > > > > include Config > > > > > > > > > > > > VER = 4.2.6 > > > > > > +SUMMARY = Zabbix Agent > > > > > > > > > > > > THISAPP = zabbix-$(VER) > > > > > > DL_FILE = $(THISAPP).tar.gz > > > > > > @@ -35,6 +36,8 @@ PROG = zabbix_agentd > > > > > > PAK_VER = 4 > > > > > > DEPS = > > > > > > > > > > > > +INITSCRIPTS= zabbix_agentd > > > > > > + > > > > > > ############################################################# > > > > > > ###### > > > > > > ############ > > > > > > # Top-level Rules > > > > > > ############################################################# > > > > > > ###### > > > > > > ############ > > > > > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst > > > > > > %,$(DIR_DL)/%,$(objects)) > > > > > > chown zabbix.zabbix /var/run/zabbix > > > > > > > > > > > > # Install initscripts > > > > > > - $(call INSTALL_INITSCRIPT,zabbix_agentd) > > > > > > + @$(INSTALL_INITSCRIPTS) > > > > > > > > > > > > # Install sudoers include file > > > > > > install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ > > > > > > diff --git a/src/pakfire/meta b/src/pakfire/meta > > > > > > index d97b2a0fa..849b9cd6c 100644 > > > > > > --- a/src/pakfire/meta > > > > > > +++ b/src/pakfire/meta > > > > > > @@ -1,6 +1,8 @@ > > > > > > Name: NAME > > > > > > +Summary: SUMMARY > > > > > > ProgVersion: VER > > > > > > Release: RELEASE > > > > > > Size: SIZE > > > > > > Dependencies: DEPS > > > > > > File: NAME-VER-RELEASE.ipfire > > > > > > +InitScripts: INITSCRIPTS > > > > > > -- > > > > > > 2.31.1 > > > > > > > > > > > > > > > > > > -- > > > > > > Dit bericht is gescanned op virussen en andere gevaarlijke > > > > > > inhoud door MailScanner en lijkt schoon te zijn. > > > > > > > > > > > > > > > > > -- > > > Dit bericht is gescanned op virussen en andere gevaarlijke > > > inhoud door MailScanner en lijkt schoon te zijn. > > > > > > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-06-06 18:41 ` Robin Roevens @ 2021-06-10 11:27 ` Michael Tremer 2021-06-17 22:28 ` Robin Roevens 0 siblings, 1 reply; 26+ messages in thread From: Michael Tremer @ 2021-06-10 11:27 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 14003 bytes --] Hello, > On 6 Jun 2021, at 19:41, Robin Roevens <robin.roevens(a)disroot.org> wrote: > > Hi Michael > > I know/see a lot of activity here, so I assume you are always quite > busy and may have missed my last reaction? Yes, sorry, I missed this, or didn’t know what to say. Feel free to remind me if you didn’t hear from me within a couple of days. > Should I proceed with my proposition to add > Summary: SUMMARY > Services: SERVICES > to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro I would say yes. Do we have any unanswered questions left? Is Jonatan happy? > Should I also add > InitScripts: INITSCRIPTS > for non-service initscripts.. just for completion of metadata ? Hmm, I would say no. If there is no point to it, then leave it. We can always add it later if we decide that we want it later. > And in that case a more generic INSTALL_INITSCRIPTS macro accepting a > list of initscripts to install? (see mail history below for explanation > of my intentions) > > Or should we find other methods to have this info ready for > services.cgi and/or monitoring agents ? No, I believe this is a good solution. We can then have a function that queries the package database and shows all services correctly. That sounds clean to me and is re-usable and extensible. -Michael > > Regards > Robin > > Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]: >> Hi Michael >> >> Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]: >>> >>> >>>> On 14 May 2021, at 21:07, Robin Roevens <robin.roevens(a)disroot.org> >>>> wrote: >>>> >>>> Hi >>>> >>>> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: >>>>> Hello Jonatan, >>>>> >>>>> Yes I get your point. Just because a package has more than one >>>>> initscript does not mean they both should be listed in >>>>> services.cgi. >>>>> >>>>> It is a weird corner case, but how do we want to handle this? >>>>> >>>>> -Michael >>>> >>>> Maybe a slight renaming of my proposed new variable INITSCRIPTS to >>>> SERVICES in the LFS files would make it a bit more clear that this >>>> concerns only service-initscripts? >>>> >>>> The INSTALL_INITSCRIPTS macro could then be renamed to >>>> INSTALL_SERVICE_INITSCRIPTS. >>>> >>>> If we then also keep the original INSTALL_INITSCRIPT macro, that >>>> macro >>>> can then be used to install non-service initscripts if required. >>>> Or >>>> even to install all required initscripts in the occasion that you >>>> want >>>> to skip some service-initscripts defined in SERVICES because they >>>> are >>>> included in the source, like the situation Jonatan pointed out (in >>>> this >>>> case the SERVICES variable is filled with the sole purpose to serve >>>> as >>>> metadata). >>>> >>>> So this would then result in: >>>> --- >>>> define INSTALL_INITSCRIPT >>>> install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) >>>> /etc/rc.d/init.d/$(1) >>>> endef >>>> >>>> define INSTALL_SERVICE_INITSCRIPTS >>>> for service in $(SERVICES); do \ >>>> $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ >>>> done >>>> endef >>>> --- >>>> >>>> Optionally we could also add again a variable INITSCRIPTS with the >>>> purpose to list non-service initscripts. so >>>> SERVICES - for service-initscripts >>>> INITSCRIPTS - for non-service-initscripts >>>> >>>> and in the meta-* files: >>>> --- >>>> Name: NAME >>>> Summary: SUMMARY >>>> ProgVersion: VER >>>> Release: RELEASE >>>> Size: SIZE >>>> Dependencies: DEPS >>>> File: NAME-VER-RELEASE.ipfire >>>> Services: SERVICES >>>> InitScripts: INITSCRIPTS >>>> --- >>>> >>>> Maybe that info could also come in handy sooner or later ? >>> >>> This sounds rather complicated to me. >>> >>> What are our objectives here? Does Zabbix want/need to start/stop >>> services? And if so, why? >> >> I was not directly thinking of Zabbix specifically here. The objective >> is to have a central 'database' of metadata for the pak's that can come >> in handy for current and possibly future functionality of IPFire. And a >> single way of retrieving this information (both by using "pakfire info" >> or directly accessing the pakfire library, in my proposal). >> >> Knowing which pak's provide which services can be used in service.cgi >> (which currently does give users the possibility to start/stop services >> indeed) and can be used for monitoring agents like Zabbix, but also >> Nagios or possibly others to know which services to monitor. >> >> This instead of my earlier proposal of re-using parts of the >> services.cgi code in a separate script for zabbix_agentd, which you >> pointed out that should be done cleaner using some central way of >> storing/retrieving that information. >> >> Zabbix does have functionality to implement starting/stopping those >> services using a context menu in the Zabbix web GUI; and a user can >> easily implement that on it's own (as this should be done mainly on >> server-side anyway) if he wants to and has the information of which >> services should run. This could be done by zabbix server logging into >> the IPFire machine using ssh and using addonctrl or by the agent, which >> would then need permission to addonctrl start/stop. But I think that is >> something for the user to decide on his own as you pointed out >> previously, giving zabbix agent root permission on addonctrl start/stop >> is a possible extra security risk. So I don't think we should provide >> that out of the box. (Maybe it could be a configurable 'setting' in the >> IPFire webgui some time in the future, but not something to focus on >> currently) >> >> But with that said, I don't really understand your remark here as I >> only propose a way for the LFS files to populate the "Services" meta- >> data and I only threw in "Initscripts" for non-service initscripts, >> just for completeness of the meta-data. >> And to prevent duplication of the list of initscripts, I added a macro >> to install those in one go instead of again 'defining' this list by >> needing to call INSTALL_INITSCRIPT individually for each initscript >> while we now have a list of them in a variable. And due to the remark >> of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak >> maintainer to use in corner cases. >> >> But this has nothing to do with actually stopping/starting services >> other that providing information about which services are installed >> (and maybe could be stopped/started if wanted). >> >> If it would make things more clear, I could work my last mail out in an >> actual patch ? >> >> Regards >> Robin >> >>> >>>> In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed >>>> back to INSTALL_INITSCRIPTS, now accepting a parameter which is a >>>> list >>>> of initscripts to install, so one could call: >>>> --- >>>> $(call INSTALL_INITSCRIPTS,$(SERVICES)) >>>> $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS)) >>>> --- >>>> to install all initscripts. >>>> >>>> Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS >>>> which >>>> would then install both SERVICES and INITSCRIPTS (or even replace >>>> the >>>> above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a >>>> macro to >>>> install all initscripts (SERVICES and INITSCRIPTS) and a macro to >>>> install a single initscript in case some of the ones listed in >>>> SERVICES >>>> and/or INITSCRIPTS need to be skipped.. >>>> >>>> But I think the INSTALL_INITSCRIPTS which takes a list of services >>>> to >>>> install as parameter is probably the most generic one, giving you >>>> the >>>> possibility to install all SERVICES in one go, and then >>>> individually >>>> install only those INITSCRIPTS not included in the source if >>>> INITSCRIPTS contains such initscript(s) or vice versa. >>>> >>>> Robin >>>> >>>>> >>>>>> On 12 May 2021, at 19:49, Jonatan Schlag < >>>>>> jonatan.schlag(a)ipfire.org> >>>>>> wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens < >>>>>>> robin.roevens(a)disroot.org>: >>>>>>> >>>>>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS- >>>>>>> makefiles. >>>>>>> * Add a Summary and InitScripts field to the meta-* addon >>>>>>> files >>>>>>> containing the value's of those macro's. >>>>>>> * Replace the INSTALL_INITSCRIPT makefile macro by a new >>>>>>> INSTALL_INITSCRIPTS macro/method that will install all >>>>>>> initscripts >>>>>>> defined in the new INITSCRIPTS macro. >>>>>>> * Adapt libvirt and zabbix_agentd pak as examples of how to >>>>>>> use >>>>>>> this. >>>>>>> >>>>>>> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> >>>>>>> --- >>>>>>> lfs/Config | 8 ++++++-- >>>>>>> lfs/libvirt | 6 ++++-- >>>>>>> lfs/zabbix_agentd | 5 ++++- >>>>>>> src/pakfire/meta | 2 ++ >>>>>>> 4 files changed, 16 insertions(+), 5 deletions(-) >>>>>>> >>>>>>> diff --git a/lfs/Config b/lfs/Config >>>>>>> index eadbbc408..61d6f0c82 100644 >>>>>>> --- a/lfs/Config >>>>>>> +++ b/lfs/Config >>>>>>> @@ -299,15 +299,19 @@ define PAK >>>>>>> # Create meta file >>>>>>> sed \ >>>>>>> -e "s/NAME/$(PROG)/g" \ >>>>>>> + -e "s/SUMMARY/$(SUMMARY)/g" \ >>>>>>> -e "s/VER/$(VER)/g" \ >>>>>>> -e "s/RELEASE/$(PAK_VER)/g" \ >>>>>>> -e "s/DEPS/$(DEPS)/g" \ >>>>>>> -e "s/SIZE/$$(stat --format=%s >>>>>>> /install/packages/$(PROG)- >>>>>>> $(VER)-$(PAK_VER).ipfire)/g" \ >>>>>>> + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ >>>>>>> < /usr/src/src/pakfire/meta > /install/packages/meta- >>>>>>> $(PROG) >>>>>>> endef >>>>>>> >>>>>>> -define INSTALL_INITSCRIPT >>>>>>> - install -m 754 -v >>>>>>> $(DIR_SRC)/src/initscripts/packages/$(1) >>>>>>> /etc/rc.d/init.d/$(1) >>>>>>> +define INSTALL_INITSCRIPTS >>>>>>> + for initscript in $(INITSCRIPTS); do \ >>>>>>> + install -m 754 -v >>>>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript >>>>>>> /etc/rc.d/init.d/$$initscript; \ >>>>>>> + done >>>>>>> endef >>>>>>> >>>>>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 >>>>>>> riscv64)" >>>>>>> diff --git a/lfs/libvirt b/lfs/libvirt >>>>>>> index 28a95d317..be5d3db3a 100644 >>>>>>> --- a/lfs/libvirt >>>>>>> +++ b/lfs/libvirt >>>>>>> @@ -25,6 +25,7 @@ >>>>>>> include Config >>>>>>> >>>>>>> VER = 6.5.0 >>>>>>> +SUMMARY = Server side daemon and supporting files for >>>>>>> libvirt >>>>>>> >>>>>>> THISAPP = libvirt-$(VER) >>>>>>> DL_FILE = $(THISAPP).tar.xz >>>>>>> @@ -37,6 +38,8 @@ PAK_VER = 25 >>>>>>> >>>>>>> DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu >>>>>>> >>>>>>> +INITSCRIPTS= libvirtd virtlogd >>>>>>> + >>>>>>> ############################################################# >>>>>>> ###### >>>>>>> ############ >>>>>>> # Top-level Rules >>>>>>> ############################################################# >>>>>>> ###### >>>>>>> ############ >>>>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst >>>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>>> cd $(DIR_APP)/build_libvirt && make install >>>>>>> >>>>>>> #install initscripts >>>>>>> - $(call INSTALL_INITSCRIPT,libvirtd) >>>>>>> - $(call INSTALL_INITSCRIPT,virtlogd) >>>>>>> + @$(INSTALL_INITSCRIPTS) >>>>>>> mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt- >>>>>>> guests >>>>>> And here my approach maybe breaks :-). How could we handle >>>>>> this? It >>>>>> is not a daemon, more something like a startup/shutdown scripts >>>>>> for >>>>>> machines... (And should not appear in the service.cgi). So >>>>>> parsing >>>>>> the root file may yield wrong results and your way would be the >>>>>> better one. >>>>>> >>>>>> Jonatan >>>>>>> >>>>>>> # Backup >>>>>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd >>>>>>> index c69643a54..a72fe024b 100644 >>>>>>> --- a/lfs/zabbix_agentd >>>>>>> +++ b/lfs/zabbix_agentd >>>>>>> @@ -25,6 +25,7 @@ >>>>>>> include Config >>>>>>> >>>>>>> VER = 4.2.6 >>>>>>> +SUMMARY = Zabbix Agent >>>>>>> >>>>>>> THISAPP = zabbix-$(VER) >>>>>>> DL_FILE = $(THISAPP).tar.gz >>>>>>> @@ -35,6 +36,8 @@ PROG = zabbix_agentd >>>>>>> PAK_VER = 4 >>>>>>> DEPS = >>>>>>> >>>>>>> +INITSCRIPTS= zabbix_agentd >>>>>>> + >>>>>>> ############################################################# >>>>>>> ###### >>>>>>> ############ >>>>>>> # Top-level Rules >>>>>>> ############################################################# >>>>>>> ###### >>>>>>> ############ >>>>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst >>>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>>> chown zabbix.zabbix /var/run/zabbix >>>>>>> >>>>>>> # Install initscripts >>>>>>> - $(call INSTALL_INITSCRIPT,zabbix_agentd) >>>>>>> + @$(INSTALL_INITSCRIPTS) >>>>>>> >>>>>>> # Install sudoers include file >>>>>>> install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ >>>>>>> diff --git a/src/pakfire/meta b/src/pakfire/meta >>>>>>> index d97b2a0fa..849b9cd6c 100644 >>>>>>> --- a/src/pakfire/meta >>>>>>> +++ b/src/pakfire/meta >>>>>>> @@ -1,6 +1,8 @@ >>>>>>> Name: NAME >>>>>>> +Summary: SUMMARY >>>>>>> ProgVersion: VER >>>>>>> Release: RELEASE >>>>>>> Size: SIZE >>>>>>> Dependencies: DEPS >>>>>>> File: NAME-VER-RELEASE.ipfire >>>>>>> +InitScripts: INITSCRIPTS >>>>>>> -- >>>>>>> 2.31.1 >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Dit bericht is gescanned op virussen en andere gevaarlijke >>>>>>> inhoud door MailScanner en lijkt schoon te zijn. >>>>> >>>>> >>>> >>>> >>>> -- >>>> Dit bericht is gescanned op virussen en andere gevaarlijke >>>> inhoud door MailScanner en lijkt schoon te zijn. >>> >>> >> >> > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-06-10 11:27 ` Michael Tremer @ 2021-06-17 22:28 ` Robin Roevens 2021-06-18 8:32 ` Michael Tremer 0 siblings, 1 reply; 26+ messages in thread From: Robin Roevens @ 2021-06-17 22:28 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 17976 bytes --] Hi Michael Tremer schreef op do 10-06-2021 om 12:27 [+0100]: > Hello, > > > On 6 Jun 2021, at 19:41, Robin Roevens <robin.roevens(a)disroot.org> > > wrote: > > > > Hi Michael > > > > I know/see a lot of activity here, so I assume you are always quite > > busy and may have missed my last reaction? > > Yes, sorry, I missed this, or didn’t know what to say. Feel free to > remind me if you didn’t hear from me within a couple of days. > > > Should I proceed with my proposition to add > > Summary: SUMMARY > > Services: SERVICES > > to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro > > I would say yes. Do we have any unanswered questions left? Not for this patch-set at the moment, I think.. > > Is Jonatan happy? I don't know.. @Jonatan, are you happy? :-) > > > Should I also add > > InitScripts: INITSCRIPTS > > for non-service initscripts.. just for completion of metadata ? > > Hmm, I would say no. If there is no point to it, then leave it. > > We can always add it later if we decide that we want it later. In the light of possibly adding this later, I opt for the more generic INSTALL_INITSCRIPTS macro accepting a list of initscripts to install. Most common way of calling it from the lfs files would then be: $(call INSTALL_INITSCRIPTS,$(SERVICES)) > > > And in that case a more generic INSTALL_INITSCRIPTS macro accepting > > a > > list of initscripts to install? (see mail history below for > > explanation > > of my intentions) > > > > Or should we find other methods to have this info ready for > > services.cgi and/or monitoring agents ? > > No, I believe this is a good solution. We can then have a function > that queries the package database and shows all services correctly. > > That sounds clean to me and is re-usable and extensible. I think so too. I started working on changing all pak-lfs files with the additional fields. When finished and tested, how should I post this change ? As there are about 200+ lfs files that wil be modified, mostly just adding a SUMMARY and SERVICES field. Should I post those changes as one big patch? I assume no-one will be happy if I start posting 200+ patches into the mailing list? I could try splitting up into patches containing only non-service paks, and another containing all paks installing initscripts, and then maybe some separate patches for lfs files that contain some corner cases should I encounter such paks ? This of course next to patches to add the INSTALL_INITSCRIPTS macro and pakfire library change to read/use this additional metadata. Regards Robin > > -Michael > > > > > Regards > > Robin > > > > Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]: > > > Hi Michael > > > > > > Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]: > > > > > > > > > > > > > On 14 May 2021, at 21:07, Robin Roevens < > > > > > robin.roevens(a)disroot.org> > > > > > wrote: > > > > > > > > > > Hi > > > > > > > > > > Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: > > > > > > Hello Jonatan, > > > > > > > > > > > > Yes I get your point. Just because a package has more than > > > > > > one > > > > > > initscript does not mean they both should be listed in > > > > > > services.cgi. > > > > > > > > > > > > It is a weird corner case, but how do we want to handle > > > > > > this? > > > > > > > > > > > > -Michael > > > > > > > > > > Maybe a slight renaming of my proposed new variable > > > > > INITSCRIPTS to > > > > > SERVICES in the LFS files would make it a bit more clear that > > > > > this > > > > > concerns only service-initscripts? > > > > > > > > > > The INSTALL_INITSCRIPTS macro could then be renamed to > > > > > INSTALL_SERVICE_INITSCRIPTS. > > > > > > > > > > If we then also keep the original INSTALL_INITSCRIPT macro, > > > > > that > > > > > macro > > > > > can then be used to install non-service initscripts if > > > > > required. > > > > > Or > > > > > even to install all required initscripts in the occasion that > > > > > you > > > > > want > > > > > to skip some service-initscripts defined in SERVICES because > > > > > they > > > > > are > > > > > included in the source, like the situation Jonatan pointed > > > > > out (in > > > > > this > > > > > case the SERVICES variable is filled with the sole purpose to > > > > > serve > > > > > as > > > > > metadata). > > > > > > > > > > So this would then result in: > > > > > --- > > > > > define INSTALL_INITSCRIPT > > > > > install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) > > > > > /etc/rc.d/init.d/$(1) > > > > > endef > > > > > > > > > > define INSTALL_SERVICE_INITSCRIPTS > > > > > for service in $(SERVICES); do \ > > > > > $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ > > > > > done > > > > > endef > > > > > --- > > > > > > > > > > Optionally we could also add again a variable INITSCRIPTS > > > > > with the > > > > > purpose to list non-service initscripts. so > > > > > SERVICES - for service-initscripts > > > > > INITSCRIPTS - for non-service-initscripts > > > > > > > > > > and in the meta-* files: > > > > > --- > > > > > Name: NAME > > > > > Summary: SUMMARY > > > > > ProgVersion: VER > > > > > Release: RELEASE > > > > > Size: SIZE > > > > > Dependencies: DEPS > > > > > File: NAME-VER-RELEASE.ipfire > > > > > Services: SERVICES > > > > > InitScripts: INITSCRIPTS > > > > > --- > > > > > > > > > > Maybe that info could also come in handy sooner or later ? > > > > > > > > This sounds rather complicated to me. > > > > > > > > What are our objectives here? Does Zabbix want/need to > > > > start/stop > > > > services? And if so, why? > > > > > > I was not directly thinking of Zabbix specifically here. The > > > objective > > > is to have a central 'database' of metadata for the pak's that > > > can come > > > in handy for current and possibly future functionality of IPFire. > > > And a > > > single way of retrieving this information (both by using "pakfire > > > info" > > > or directly accessing the pakfire library, in my proposal). > > > > > > Knowing which pak's provide which services can be used in > > > service.cgi > > > (which currently does give users the possibility to start/stop > > > services > > > indeed) and can be used for monitoring agents like Zabbix, but > > > also > > > Nagios or possibly others to know which services to monitor. > > > > > > This instead of my earlier proposal of re-using parts of the > > > services.cgi code in a separate script for zabbix_agentd, which > > > you > > > pointed out that should be done cleaner using some central way of > > > storing/retrieving that information. > > > > > > Zabbix does have functionality to implement starting/stopping > > > those > > > services using a context menu in the Zabbix web GUI; and a user > > > can > > > easily implement that on it's own (as this should be done mainly > > > on > > > server-side anyway) if he wants to and has the information of > > > which > > > services should run. This could be done by zabbix server logging > > > into > > > the IPFire machine using ssh and using addonctrl or by the agent, > > > which > > > would then need permission to addonctrl start/stop. But I think > > > that is > > > something for the user to decide on his own as you pointed out > > > previously, giving zabbix agent root permission on addonctrl > > > start/stop > > > is a possible extra security risk. So I don't think we should > > > provide > > > that out of the box. (Maybe it could be a configurable 'setting' > > > in the > > > IPFire webgui some time in the future, but not something to focus > > > on > > > currently) > > > > > > But with that said, I don't really understand your remark here as > > > I > > > only propose a way for the LFS files to populate the "Services" > > > meta- > > > data and I only threw in "Initscripts" for non-service > > > initscripts, > > > just for completeness of the meta-data. > > > And to prevent duplication of the list of initscripts, I added a > > > macro > > > to install those in one go instead of again 'defining' this list > > > by > > > needing to call INSTALL_INITSCRIPT individually for each > > > initscript > > > while we now have a list of them in a variable. And due to the > > > remark > > > of Jonatan I now also kept the original INSTALL_INITSCRIPT for > > > the pak > > > maintainer to use in corner cases. > > > > > > But this has nothing to do with actually stopping/starting > > > services > > > other that providing information about which services are > > > installed > > > (and maybe could be stopped/started if wanted). > > > > > > If it would make things more clear, I could work my last mail out > > > in an > > > actual patch ? > > > > > > Regards > > > Robin > > > > > > > > > > > > In this case the INSTALL_SERVICE_INITSCRIPTS could again be > > > > > renamed > > > > > back to INSTALL_INITSCRIPTS, now accepting a parameter which > > > > > is a > > > > > list > > > > > of initscripts to install, so one could call: > > > > > --- > > > > > $(call INSTALL_INITSCRIPTS,$(SERVICES)) > > > > > $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS)) > > > > > --- > > > > > to install all initscripts. > > > > > > > > > > Or even provide again an additional macro > > > > > INSTALL_ALL_INITSCRIPTS > > > > > which > > > > > would then install both SERVICES and INITSCRIPTS (or even > > > > > replace > > > > > the > > > > > above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in > > > > > a > > > > > macro to > > > > > install all initscripts (SERVICES and INITSCRIPTS) and a > > > > > macro to > > > > > install a single initscript in case some of the ones listed > > > > > in > > > > > SERVICES > > > > > and/or INITSCRIPTS need to be skipped.. > > > > > > > > > > But I think the INSTALL_INITSCRIPTS which takes a list of > > > > > services > > > > > to > > > > > install as parameter is probably the most generic one, giving > > > > > you > > > > > the > > > > > possibility to install all SERVICES in one go, and then > > > > > individually > > > > > install only those INITSCRIPTS not included in the source if > > > > > INITSCRIPTS contains such initscript(s) or vice versa. > > > > > > > > > > Robin > > > > > > > > > > > > > > > > > > On 12 May 2021, at 19:49, Jonatan Schlag < > > > > > > > jonatan.schlag(a)ipfire.org> > > > > > > > wrote: > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens < > > > > > > > > robin.roevens(a)disroot.org>: > > > > > > > > > > > > > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS- > > > > > > > > makefiles. > > > > > > > > * Add a Summary and InitScripts field to the meta-* > > > > > > > > addon > > > > > > > > files > > > > > > > > containing the value's of those macro's. > > > > > > > > * Replace the INSTALL_INITSCRIPT makefile macro by a > > > > > > > > new > > > > > > > > INSTALL_INITSCRIPTS macro/method that will install all > > > > > > > > initscripts > > > > > > > > defined in the new INITSCRIPTS macro. > > > > > > > > * Adapt libvirt and zabbix_agentd pak as examples of > > > > > > > > how to > > > > > > > > use > > > > > > > > this. > > > > > > > > > > > > > > > > Signed-off-by: Robin Roevens > > > > > > > > <robin.roevens(a)disroot.org> > > > > > > > > --- > > > > > > > > lfs/Config | 8 ++++++-- > > > > > > > > lfs/libvirt | 6 ++++-- > > > > > > > > lfs/zabbix_agentd | 5 ++++- > > > > > > > > src/pakfire/meta | 2 ++ > > > > > > > > 4 files changed, 16 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > > diff --git a/lfs/Config b/lfs/Config > > > > > > > > index eadbbc408..61d6f0c82 100644 > > > > > > > > --- a/lfs/Config > > > > > > > > +++ b/lfs/Config > > > > > > > > @@ -299,15 +299,19 @@ define PAK > > > > > > > > # Create meta file > > > > > > > > sed \ > > > > > > > > -e "s/NAME/$(PROG)/g" \ > > > > > > > > + -e "s/SUMMARY/$(SUMMARY)/g" \ > > > > > > > > -e "s/VER/$(VER)/g" \ > > > > > > > > -e "s/RELEASE/$(PAK_VER)/g" \ > > > > > > > > -e "s/DEPS/$(DEPS)/g" \ > > > > > > > > -e "s/SIZE/$$(stat --format=%s > > > > > > > > /install/packages/$(PROG)- > > > > > > > > $(VER)-$(PAK_VER).ipfire)/g" \ > > > > > > > > + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ > > > > > > > > < /usr/src/src/pakfire/meta > > > > > > > > > /install/packages/meta- > > > > > > > > $(PROG) > > > > > > > > endef > > > > > > > > > > > > > > > > -define INSTALL_INITSCRIPT > > > > > > > > - install -m 754 -v > > > > > > > > $(DIR_SRC)/src/initscripts/packages/$(1) > > > > > > > > /etc/rc.d/init.d/$(1) > > > > > > > > +define INSTALL_INITSCRIPTS > > > > > > > > + for initscript in $(INITSCRIPTS); do \ > > > > > > > > + install -m 754 -v > > > > > > > > $(DIR_SRC)/src/initscripts/packages/$$initscript > > > > > > > > /etc/rc.d/init.d/$$initscript; \ > > > > > > > > + done > > > > > > > > endef > > > > > > > > > > > > > > > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 > > > > > > > > riscv64)" > > > > > > > > diff --git a/lfs/libvirt b/lfs/libvirt > > > > > > > > index 28a95d317..be5d3db3a 100644 > > > > > > > > --- a/lfs/libvirt > > > > > > > > +++ b/lfs/libvirt > > > > > > > > @@ -25,6 +25,7 @@ > > > > > > > > include Config > > > > > > > > > > > > > > > > VER = 6.5.0 > > > > > > > > +SUMMARY = Server side daemon and supporting > > > > > > > > files for > > > > > > > > libvirt > > > > > > > > > > > > > > > > THISAPP = libvirt-$(VER) > > > > > > > > DL_FILE = $(THISAPP).tar.xz > > > > > > > > @@ -37,6 +38,8 @@ PAK_VER = 25 > > > > > > > > > > > > > > > > DEPS = ebtables libpciaccess libtirpc libyajl > > > > > > > > ncat qemu > > > > > > > > > > > > > > > > +INITSCRIPTS= libvirtd virtlogd > > > > > > > > + > > > > > > > > ####################################################### > > > > > > > > ###### > > > > > > > > ###### > > > > > > > > ############ > > > > > > > > # Top-level Rules > > > > > > > > ####################################################### > > > > > > > > ###### > > > > > > > > ###### > > > > > > > > ############ > > > > > > > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst > > > > > > > > %,$(DIR_DL)/%,$(objects)) > > > > > > > > cd $(DIR_APP)/build_libvirt && make install > > > > > > > > > > > > > > > > #install initscripts > > > > > > > > - $(call INSTALL_INITSCRIPT,libvirtd) > > > > > > > > - $(call INSTALL_INITSCRIPT,virtlogd) > > > > > > > > + @$(INSTALL_INITSCRIPTS) > > > > > > > > mv /usr/libexec/libvirt-guests.sh > > > > > > > > /etc/rc.d/init.d/libvirt- > > > > > > > > guests > > > > > > > And here my approach maybe breaks :-). How could we > > > > > > > handle > > > > > > > this? It > > > > > > > is not a daemon, more something like a startup/shutdown > > > > > > > scripts > > > > > > > for > > > > > > > machines... (And should not appear in the service.cgi). > > > > > > > So > > > > > > > parsing > > > > > > > the root file may yield wrong results and your way would > > > > > > > be the > > > > > > > better one. > > > > > > > > > > > > > > Jonatan > > > > > > > > > > > > > > > > # Backup > > > > > > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd > > > > > > > > index c69643a54..a72fe024b 100644 > > > > > > > > --- a/lfs/zabbix_agentd > > > > > > > > +++ b/lfs/zabbix_agentd > > > > > > > > @@ -25,6 +25,7 @@ > > > > > > > > include Config > > > > > > > > > > > > > > > > VER = 4.2.6 > > > > > > > > +SUMMARY = Zabbix Agent > > > > > > > > > > > > > > > > THISAPP = zabbix-$(VER) > > > > > > > > DL_FILE = $(THISAPP).tar.gz > > > > > > > > @@ -35,6 +36,8 @@ PROG = zabbix_agentd > > > > > > > > PAK_VER = 4 > > > > > > > > DEPS = > > > > > > > > > > > > > > > > +INITSCRIPTS= zabbix_agentd > > > > > > > > + > > > > > > > > ####################################################### > > > > > > > > ###### > > > > > > > > ###### > > > > > > > > ############ > > > > > > > > # Top-level Rules > > > > > > > > ####################################################### > > > > > > > > ###### > > > > > > > > ###### > > > > > > > > ############ > > > > > > > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst > > > > > > > > %,$(DIR_DL)/%,$(objects)) > > > > > > > > chown zabbix.zabbix /var/run/zabbix > > > > > > > > > > > > > > > > # Install initscripts > > > > > > > > - $(call INSTALL_INITSCRIPT,zabbix_agentd) > > > > > > > > + @$(INSTALL_INITSCRIPTS) > > > > > > > > > > > > > > > > # Install sudoers include file > > > > > > > > install -v -m 644 > > > > > > > > $(DIR_SRC)/config/zabbix_agentd/sudoers \ > > > > > > > > diff --git a/src/pakfire/meta b/src/pakfire/meta > > > > > > > > index d97b2a0fa..849b9cd6c 100644 > > > > > > > > --- a/src/pakfire/meta > > > > > > > > +++ b/src/pakfire/meta > > > > > > > > @@ -1,6 +1,8 @@ > > > > > > > > Name: NAME > > > > > > > > +Summary: SUMMARY > > > > > > > > ProgVersion: VER > > > > > > > > Release: RELEASE > > > > > > > > Size: SIZE > > > > > > > > Dependencies: DEPS > > > > > > > > File: NAME-VER-RELEASE.ipfire > > > > > > > > +InitScripts: INITSCRIPTS > > > > > > > > -- > > > > > > > > 2.31.1 > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > Dit bericht is gescanned op virussen en andere > > > > > > > > gevaarlijke > > > > > > > > inhoud door MailScanner en lijkt schoon te zijn. > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Dit bericht is gescanned op virussen en andere gevaarlijke > > > > > inhoud door MailScanner en lijkt schoon te zijn. > > > > > > > > > > > > > > > > > > > > -- > > Dit bericht is gescanned op virussen en andere gevaarlijke > > inhoud door MailScanner en lijkt schoon te zijn. > > > > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-06-17 22:28 ` Robin Roevens @ 2021-06-18 8:32 ` Michael Tremer 0 siblings, 0 replies; 26+ messages in thread From: Michael Tremer @ 2021-06-18 8:32 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 17188 bytes --] Hello, > On 17 Jun 2021, at 23:28, Robin Roevens <robin.roevens(a)disroot.org> wrote: > > Hi > > Michael Tremer schreef op do 10-06-2021 om 12:27 [+0100]: >> Hello, >> >>> On 6 Jun 2021, at 19:41, Robin Roevens <robin.roevens(a)disroot.org> >>> wrote: >>> >>> Hi Michael >>> >>> I know/see a lot of activity here, so I assume you are always quite >>> busy and may have missed my last reaction? >> >> Yes, sorry, I missed this, or didn’t know what to say. Feel free to >> remind me if you didn’t hear from me within a couple of days. >> >>> Should I proceed with my proposition to add >>> Summary: SUMMARY >>> Services: SERVICES >>> to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro >> >> I would say yes. Do we have any unanswered questions left? > Not for this patch-set at the moment, I think.. > >> >> Is Jonatan happy? > I don't know.. @Jonatan, are you happy? :-) Seems to be on holiday... > >> >>> Should I also add >>> InitScripts: INITSCRIPTS >>> for non-service initscripts.. just for completion of metadata ? >> >> Hmm, I would say no. If there is no point to it, then leave it. >> >> We can always add it later if we decide that we want it later. > In the light of possibly adding this later, I opt for the more generic > INSTALL_INITSCRIPTS macro accepting a list of initscripts to install. > Most common way of calling it from the lfs files would then be: > > $(call INSTALL_INITSCRIPTS,$(SERVICES)) > >> >>> And in that case a more generic INSTALL_INITSCRIPTS macro accepting >>> a >>> list of initscripts to install? (see mail history below for >>> explanation >>> of my intentions) >>> >>> Or should we find other methods to have this info ready for >>> services.cgi and/or monitoring agents ? >> >> No, I believe this is a good solution. We can then have a function >> that queries the package database and shows all services correctly. >> >> That sounds clean to me and is re-usable and extensible. > I think so too. > > I started working on changing all pak-lfs files with the additional > fields. When finished and tested, how should I post this change ? > As there are about 200+ lfs files that wil be modified, mostly just > adding a SUMMARY and SERVICES field. Should I post those changes as one > big patch? I assume no-one will be happy if I start posting 200+ > patches into the mailing list? Would actually that many files be affected? Changing the macro can be one patch for all lfs files. It does not have to be one patch per lfs file. Grouping logical changes is what we want to do. Nothing more, nothing less. > I could try splitting up into patches containing only non-service paks, > and another containing all paks installing initscripts, and then maybe > some separate patches for lfs files that contain some corner cases > should I encounter such paks ? > > This of course next to patches to add the INSTALL_INITSCRIPTS macro and > pakfire library change to read/use this additional metadata. -Michael > > Regards > > Robin > >> >> -Michael >> >>> >>> Regards >>> Robin >>> >>> Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]: >>>> Hi Michael >>>> >>>> Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]: >>>>> >>>>> >>>>>> On 14 May 2021, at 21:07, Robin Roevens < >>>>>> robin.roevens(a)disroot.org> >>>>>> wrote: >>>>>> >>>>>> Hi >>>>>> >>>>>> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: >>>>>>> Hello Jonatan, >>>>>>> >>>>>>> Yes I get your point. Just because a package has more than >>>>>>> one >>>>>>> initscript does not mean they both should be listed in >>>>>>> services.cgi. >>>>>>> >>>>>>> It is a weird corner case, but how do we want to handle >>>>>>> this? >>>>>>> >>>>>>> -Michael >>>>>> >>>>>> Maybe a slight renaming of my proposed new variable >>>>>> INITSCRIPTS to >>>>>> SERVICES in the LFS files would make it a bit more clear that >>>>>> this >>>>>> concerns only service-initscripts? >>>>>> >>>>>> The INSTALL_INITSCRIPTS macro could then be renamed to >>>>>> INSTALL_SERVICE_INITSCRIPTS. >>>>>> >>>>>> If we then also keep the original INSTALL_INITSCRIPT macro, >>>>>> that >>>>>> macro >>>>>> can then be used to install non-service initscripts if >>>>>> required. >>>>>> Or >>>>>> even to install all required initscripts in the occasion that >>>>>> you >>>>>> want >>>>>> to skip some service-initscripts defined in SERVICES because >>>>>> they >>>>>> are >>>>>> included in the source, like the situation Jonatan pointed >>>>>> out (in >>>>>> this >>>>>> case the SERVICES variable is filled with the sole purpose to >>>>>> serve >>>>>> as >>>>>> metadata). >>>>>> >>>>>> So this would then result in: >>>>>> --- >>>>>> define INSTALL_INITSCRIPT >>>>>> install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) >>>>>> /etc/rc.d/init.d/$(1) >>>>>> endef >>>>>> >>>>>> define INSTALL_SERVICE_INITSCRIPTS >>>>>> for service in $(SERVICES); do \ >>>>>> $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ >>>>>> done >>>>>> endef >>>>>> --- >>>>>> >>>>>> Optionally we could also add again a variable INITSCRIPTS >>>>>> with the >>>>>> purpose to list non-service initscripts. so >>>>>> SERVICES - for service-initscripts >>>>>> INITSCRIPTS - for non-service-initscripts >>>>>> >>>>>> and in the meta-* files: >>>>>> --- >>>>>> Name: NAME >>>>>> Summary: SUMMARY >>>>>> ProgVersion: VER >>>>>> Release: RELEASE >>>>>> Size: SIZE >>>>>> Dependencies: DEPS >>>>>> File: NAME-VER-RELEASE.ipfire >>>>>> Services: SERVICES >>>>>> InitScripts: INITSCRIPTS >>>>>> --- >>>>>> >>>>>> Maybe that info could also come in handy sooner or later ? >>>>> >>>>> This sounds rather complicated to me. >>>>> >>>>> What are our objectives here? Does Zabbix want/need to >>>>> start/stop >>>>> services? And if so, why? >>>> >>>> I was not directly thinking of Zabbix specifically here. The >>>> objective >>>> is to have a central 'database' of metadata for the pak's that >>>> can come >>>> in handy for current and possibly future functionality of IPFire. >>>> And a >>>> single way of retrieving this information (both by using "pakfire >>>> info" >>>> or directly accessing the pakfire library, in my proposal). >>>> >>>> Knowing which pak's provide which services can be used in >>>> service.cgi >>>> (which currently does give users the possibility to start/stop >>>> services >>>> indeed) and can be used for monitoring agents like Zabbix, but >>>> also >>>> Nagios or possibly others to know which services to monitor. >>>> >>>> This instead of my earlier proposal of re-using parts of the >>>> services.cgi code in a separate script for zabbix_agentd, which >>>> you >>>> pointed out that should be done cleaner using some central way of >>>> storing/retrieving that information. >>>> >>>> Zabbix does have functionality to implement starting/stopping >>>> those >>>> services using a context menu in the Zabbix web GUI; and a user >>>> can >>>> easily implement that on it's own (as this should be done mainly >>>> on >>>> server-side anyway) if he wants to and has the information of >>>> which >>>> services should run. This could be done by zabbix server logging >>>> into >>>> the IPFire machine using ssh and using addonctrl or by the agent, >>>> which >>>> would then need permission to addonctrl start/stop. But I think >>>> that is >>>> something for the user to decide on his own as you pointed out >>>> previously, giving zabbix agent root permission on addonctrl >>>> start/stop >>>> is a possible extra security risk. So I don't think we should >>>> provide >>>> that out of the box. (Maybe it could be a configurable 'setting' >>>> in the >>>> IPFire webgui some time in the future, but not something to focus >>>> on >>>> currently) >>>> >>>> But with that said, I don't really understand your remark here as >>>> I >>>> only propose a way for the LFS files to populate the "Services" >>>> meta- >>>> data and I only threw in "Initscripts" for non-service >>>> initscripts, >>>> just for completeness of the meta-data. >>>> And to prevent duplication of the list of initscripts, I added a >>>> macro >>>> to install those in one go instead of again 'defining' this list >>>> by >>>> needing to call INSTALL_INITSCRIPT individually for each >>>> initscript >>>> while we now have a list of them in a variable. And due to the >>>> remark >>>> of Jonatan I now also kept the original INSTALL_INITSCRIPT for >>>> the pak >>>> maintainer to use in corner cases. >>>> >>>> But this has nothing to do with actually stopping/starting >>>> services >>>> other that providing information about which services are >>>> installed >>>> (and maybe could be stopped/started if wanted). >>>> >>>> If it would make things more clear, I could work my last mail out >>>> in an >>>> actual patch ? >>>> >>>> Regards >>>> Robin >>>> >>>>> >>>>>> In this case the INSTALL_SERVICE_INITSCRIPTS could again be >>>>>> renamed >>>>>> back to INSTALL_INITSCRIPTS, now accepting a parameter which >>>>>> is a >>>>>> list >>>>>> of initscripts to install, so one could call: >>>>>> --- >>>>>> $(call INSTALL_INITSCRIPTS,$(SERVICES)) >>>>>> $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS)) >>>>>> --- >>>>>> to install all initscripts. >>>>>> >>>>>> Or even provide again an additional macro >>>>>> INSTALL_ALL_INITSCRIPTS >>>>>> which >>>>>> would then install both SERVICES and INITSCRIPTS (or even >>>>>> replace >>>>>> the >>>>>> above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in >>>>>> a >>>>>> macro to >>>>>> install all initscripts (SERVICES and INITSCRIPTS) and a >>>>>> macro to >>>>>> install a single initscript in case some of the ones listed >>>>>> in >>>>>> SERVICES >>>>>> and/or INITSCRIPTS need to be skipped.. >>>>>> >>>>>> But I think the INSTALL_INITSCRIPTS which takes a list of >>>>>> services >>>>>> to >>>>>> install as parameter is probably the most generic one, giving >>>>>> you >>>>>> the >>>>>> possibility to install all SERVICES in one go, and then >>>>>> individually >>>>>> install only those INITSCRIPTS not included in the source if >>>>>> INITSCRIPTS contains such initscript(s) or vice versa. >>>>>> >>>>>> Robin >>>>>> >>>>>>> >>>>>>>> On 12 May 2021, at 19:49, Jonatan Schlag < >>>>>>>> jonatan.schlag(a)ipfire.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens < >>>>>>>>> robin.roevens(a)disroot.org>: >>>>>>>>> >>>>>>>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS- >>>>>>>>> makefiles. >>>>>>>>> * Add a Summary and InitScripts field to the meta-* >>>>>>>>> addon >>>>>>>>> files >>>>>>>>> containing the value's of those macro's. >>>>>>>>> * Replace the INSTALL_INITSCRIPT makefile macro by a >>>>>>>>> new >>>>>>>>> INSTALL_INITSCRIPTS macro/method that will install all >>>>>>>>> initscripts >>>>>>>>> defined in the new INITSCRIPTS macro. >>>>>>>>> * Adapt libvirt and zabbix_agentd pak as examples of >>>>>>>>> how to >>>>>>>>> use >>>>>>>>> this. >>>>>>>>> >>>>>>>>> Signed-off-by: Robin Roevens >>>>>>>>> <robin.roevens(a)disroot.org> >>>>>>>>> --- >>>>>>>>> lfs/Config | 8 ++++++-- >>>>>>>>> lfs/libvirt | 6 ++++-- >>>>>>>>> lfs/zabbix_agentd | 5 ++++- >>>>>>>>> src/pakfire/meta | 2 ++ >>>>>>>>> 4 files changed, 16 insertions(+), 5 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/lfs/Config b/lfs/Config >>>>>>>>> index eadbbc408..61d6f0c82 100644 >>>>>>>>> --- a/lfs/Config >>>>>>>>> +++ b/lfs/Config >>>>>>>>> @@ -299,15 +299,19 @@ define PAK >>>>>>>>> # Create meta file >>>>>>>>> sed \ >>>>>>>>> -e "s/NAME/$(PROG)/g" \ >>>>>>>>> + -e "s/SUMMARY/$(SUMMARY)/g" \ >>>>>>>>> -e "s/VER/$(VER)/g" \ >>>>>>>>> -e "s/RELEASE/$(PAK_VER)/g" \ >>>>>>>>> -e "s/DEPS/$(DEPS)/g" \ >>>>>>>>> -e "s/SIZE/$$(stat --format=%s >>>>>>>>> /install/packages/$(PROG)- >>>>>>>>> $(VER)-$(PAK_VER).ipfire)/g" \ >>>>>>>>> + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ >>>>>>>>> < /usr/src/src/pakfire/meta > >>>>>>>>> /install/packages/meta- >>>>>>>>> $(PROG) >>>>>>>>> endef >>>>>>>>> >>>>>>>>> -define INSTALL_INITSCRIPT >>>>>>>>> - install -m 754 -v >>>>>>>>> $(DIR_SRC)/src/initscripts/packages/$(1) >>>>>>>>> /etc/rc.d/init.d/$(1) >>>>>>>>> +define INSTALL_INITSCRIPTS >>>>>>>>> + for initscript in $(INITSCRIPTS); do \ >>>>>>>>> + install -m 754 -v >>>>>>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript >>>>>>>>> /etc/rc.d/init.d/$$initscript; \ >>>>>>>>> + done >>>>>>>>> endef >>>>>>>>> >>>>>>>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 >>>>>>>>> riscv64)" >>>>>>>>> diff --git a/lfs/libvirt b/lfs/libvirt >>>>>>>>> index 28a95d317..be5d3db3a 100644 >>>>>>>>> --- a/lfs/libvirt >>>>>>>>> +++ b/lfs/libvirt >>>>>>>>> @@ -25,6 +25,7 @@ >>>>>>>>> include Config >>>>>>>>> >>>>>>>>> VER = 6.5.0 >>>>>>>>> +SUMMARY = Server side daemon and supporting >>>>>>>>> files for >>>>>>>>> libvirt >>>>>>>>> >>>>>>>>> THISAPP = libvirt-$(VER) >>>>>>>>> DL_FILE = $(THISAPP).tar.xz >>>>>>>>> @@ -37,6 +38,8 @@ PAK_VER = 25 >>>>>>>>> >>>>>>>>> DEPS = ebtables libpciaccess libtirpc libyajl >>>>>>>>> ncat qemu >>>>>>>>> >>>>>>>>> +INITSCRIPTS= libvirtd virtlogd >>>>>>>>> + >>>>>>>>> ####################################################### >>>>>>>>> ###### >>>>>>>>> ###### >>>>>>>>> ############ >>>>>>>>> # Top-level Rules >>>>>>>>> ####################################################### >>>>>>>>> ###### >>>>>>>>> ###### >>>>>>>>> ############ >>>>>>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst >>>>>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>>>>> cd $(DIR_APP)/build_libvirt && make install >>>>>>>>> >>>>>>>>> #install initscripts >>>>>>>>> - $(call INSTALL_INITSCRIPT,libvirtd) >>>>>>>>> - $(call INSTALL_INITSCRIPT,virtlogd) >>>>>>>>> + @$(INSTALL_INITSCRIPTS) >>>>>>>>> mv /usr/libexec/libvirt-guests.sh >>>>>>>>> /etc/rc.d/init.d/libvirt- >>>>>>>>> guests >>>>>>>> And here my approach maybe breaks :-). How could we >>>>>>>> handle >>>>>>>> this? It >>>>>>>> is not a daemon, more something like a startup/shutdown >>>>>>>> scripts >>>>>>>> for >>>>>>>> machines... (And should not appear in the service.cgi). >>>>>>>> So >>>>>>>> parsing >>>>>>>> the root file may yield wrong results and your way would >>>>>>>> be the >>>>>>>> better one. >>>>>>>> >>>>>>>> Jonatan >>>>>>>>> >>>>>>>>> # Backup >>>>>>>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd >>>>>>>>> index c69643a54..a72fe024b 100644 >>>>>>>>> --- a/lfs/zabbix_agentd >>>>>>>>> +++ b/lfs/zabbix_agentd >>>>>>>>> @@ -25,6 +25,7 @@ >>>>>>>>> include Config >>>>>>>>> >>>>>>>>> VER = 4.2.6 >>>>>>>>> +SUMMARY = Zabbix Agent >>>>>>>>> >>>>>>>>> THISAPP = zabbix-$(VER) >>>>>>>>> DL_FILE = $(THISAPP).tar.gz >>>>>>>>> @@ -35,6 +36,8 @@ PROG = zabbix_agentd >>>>>>>>> PAK_VER = 4 >>>>>>>>> DEPS = >>>>>>>>> >>>>>>>>> +INITSCRIPTS= zabbix_agentd >>>>>>>>> + >>>>>>>>> ####################################################### >>>>>>>>> ###### >>>>>>>>> ###### >>>>>>>>> ############ >>>>>>>>> # Top-level Rules >>>>>>>>> ####################################################### >>>>>>>>> ###### >>>>>>>>> ###### >>>>>>>>> ############ >>>>>>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst >>>>>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>>>>> chown zabbix.zabbix /var/run/zabbix >>>>>>>>> >>>>>>>>> # Install initscripts >>>>>>>>> - $(call INSTALL_INITSCRIPT,zabbix_agentd) >>>>>>>>> + @$(INSTALL_INITSCRIPTS) >>>>>>>>> >>>>>>>>> # Install sudoers include file >>>>>>>>> install -v -m 644 >>>>>>>>> $(DIR_SRC)/config/zabbix_agentd/sudoers \ >>>>>>>>> diff --git a/src/pakfire/meta b/src/pakfire/meta >>>>>>>>> index d97b2a0fa..849b9cd6c 100644 >>>>>>>>> --- a/src/pakfire/meta >>>>>>>>> +++ b/src/pakfire/meta >>>>>>>>> @@ -1,6 +1,8 @@ >>>>>>>>> Name: NAME >>>>>>>>> +Summary: SUMMARY >>>>>>>>> ProgVersion: VER >>>>>>>>> Release: RELEASE >>>>>>>>> Size: SIZE >>>>>>>>> Dependencies: DEPS >>>>>>>>> File: NAME-VER-RELEASE.ipfire >>>>>>>>> +InitScripts: INITSCRIPTS >>>>>>>>> -- >>>>>>>>> 2.31.1 >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Dit bericht is gescanned op virussen en andere >>>>>>>>> gevaarlijke >>>>>>>>> inhoud door MailScanner en lijkt schoon te zijn. >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Dit bericht is gescanned op virussen en andere gevaarlijke >>>>>> inhoud door MailScanner en lijkt schoon te zijn. >>>>> >>>>> >>>> >>>> >>> >>> >>> -- >>> Dit bericht is gescanned op virussen en andere gevaarlijke >>> inhoud door MailScanner en lijkt schoon te zijn. >>> >> >> > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-05-24 20:23 ` Robin Roevens 2021-06-06 18:41 ` Robin Roevens @ 2021-06-10 11:24 ` Michael Tremer 1 sibling, 0 replies; 26+ messages in thread From: Michael Tremer @ 2021-06-10 11:24 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 12568 bytes --] Hello, > On 24 May 2021, at 21:23, Robin Roevens <robin.roevens(a)disroot.org> wrote: > > Hi Michael > > Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]: >> >> >>> On 14 May 2021, at 21:07, Robin Roevens <robin.roevens(a)disroot.org> >>> wrote: >>> >>> Hi >>> >>> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: >>>> Hello Jonatan, >>>> >>>> Yes I get your point. Just because a package has more than one >>>> initscript does not mean they both should be listed in >>>> services.cgi. >>>> >>>> It is a weird corner case, but how do we want to handle this? >>>> >>>> -Michael >>> >>> Maybe a slight renaming of my proposed new variable INITSCRIPTS to >>> SERVICES in the LFS files would make it a bit more clear that this >>> concerns only service-initscripts? >>> >>> The INSTALL_INITSCRIPTS macro could then be renamed to >>> INSTALL_SERVICE_INITSCRIPTS. >>> >>> If we then also keep the original INSTALL_INITSCRIPT macro, that >>> macro >>> can then be used to install non-service initscripts if required. >>> Or >>> even to install all required initscripts in the occasion that you >>> want >>> to skip some service-initscripts defined in SERVICES because they >>> are >>> included in the source, like the situation Jonatan pointed out (in >>> this >>> case the SERVICES variable is filled with the sole purpose to serve >>> as >>> metadata). >>> >>> So this would then result in: >>> --- >>> define INSTALL_INITSCRIPT >>> install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) >>> /etc/rc.d/init.d/$(1) >>> endef >>> >>> define INSTALL_SERVICE_INITSCRIPTS >>> for service in $(SERVICES); do \ >>> $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ >>> done >>> endef >>> --- >>> >>> Optionally we could also add again a variable INITSCRIPTS with the >>> purpose to list non-service initscripts. so >>> SERVICES - for service-initscripts >>> INITSCRIPTS - for non-service-initscripts >>> >>> and in the meta-* files: >>> --- >>> Name: NAME >>> Summary: SUMMARY >>> ProgVersion: VER >>> Release: RELEASE >>> Size: SIZE >>> Dependencies: DEPS >>> File: NAME-VER-RELEASE.ipfire >>> Services: SERVICES >>> InitScripts: INITSCRIPTS >>> --- >>> >>> Maybe that info could also come in handy sooner or later ? >> >> This sounds rather complicated to me. >> >> What are our objectives here? Does Zabbix want/need to start/stop >> services? And if so, why? > > I was not directly thinking of Zabbix specifically here. The objective > is to have a central 'database' of metadata for the pak's that can come > in handy for current and possibly future functionality of IPFire. And a > single way of retrieving this information (both by using "pakfire info" > or directly accessing the pakfire library, in my proposal). I understood that, but my question remains. Why does Zabbix need to start/stop individual services on the firewall? I just want to understand the use-case so I can help to build a better solution. > Knowing which pak's provide which services can be used in service.cgi > (which currently does give users the possibility to start/stop services > indeed) and can be used for monitoring agents like Zabbix, but also > Nagios or possibly others to know which services to monitor. > > This instead of my earlier proposal of re-using parts of the > services.cgi code in a separate script for zabbix_agentd, which you > pointed out that should be done cleaner using some central way of > storing/retrieving that information. > > Zabbix does have functionality to implement starting/stopping those > services using a context menu in the Zabbix web GUI; and a user can > easily implement that on it's own (as this should be done mainly on > server-side anyway) if he wants to and has the information of which > services should run. This could be done by zabbix server logging into > the IPFire machine using ssh and using addonctrl or by the agent, which > would then need permission to addonctrl start/stop. But I think that is > something for the user to decide on his own as you pointed out > previously, giving zabbix agent root permission on addonctrl start/stop > is a possible extra security risk. So I don't think we should provide > that out of the box. (Maybe it could be a configurable 'setting' in the > IPFire webgui some time in the future, but not something to focus on > currently) > > But with that said, I don't really understand your remark here as I > only propose a way for the LFS files to populate the "Services" meta- > data and I only threw in "Initscripts" for non-service initscripts, > just for completeness of the meta-data. > And to prevent duplication of the list of initscripts, I added a macro > to install those in one go instead of again 'defining' this list by > needing to call INSTALL_INITSCRIPT individually for each initscript > while we now have a list of them in a variable. And due to the remark > of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak > maintainer to use in corner cases. > > But this has nothing to do with actually stopping/starting services > other that providing information about which services are installed > (and maybe could be stopped/started if wanted). Noted. > If it would make things more clear, I could work my last mail out in an > actual patch ? To remind me again, what does that entail? -Michael > Regards > Robin > >> >>> In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed >>> back to INSTALL_INITSCRIPTS, now accepting a parameter which is a >>> list >>> of initscripts to install, so one could call: >>> --- >>> $(call INSTALL_INITSCRIPTS,$(SERVICES)) >>> $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS)) >>> --- >>> to install all initscripts. >>> >>> Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS >>> which >>> would then install both SERVICES and INITSCRIPTS (or even replace >>> the >>> above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a >>> macro to >>> install all initscripts (SERVICES and INITSCRIPTS) and a macro to >>> install a single initscript in case some of the ones listed in >>> SERVICES >>> and/or INITSCRIPTS need to be skipped.. >>> >>> But I think the INSTALL_INITSCRIPTS which takes a list of services >>> to >>> install as parameter is probably the most generic one, giving you >>> the >>> possibility to install all SERVICES in one go, and then >>> individually >>> install only those INITSCRIPTS not included in the source if >>> INITSCRIPTS contains such initscript(s) or vice versa. >>> >>> Robin >>> >>>> >>>>> On 12 May 2021, at 19:49, Jonatan Schlag < >>>>> jonatan.schlag(a)ipfire.org> >>>>> wrote: >>>>> >>>>> Hi, >>>>> >>>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens < >>>>>> robin.roevens(a)disroot.org>: >>>>>> >>>>>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS- >>>>>> makefiles. >>>>>> * Add a Summary and InitScripts field to the meta-* addon >>>>>> files >>>>>> containing the value's of those macro's. >>>>>> * Replace the INSTALL_INITSCRIPT makefile macro by a new >>>>>> INSTALL_INITSCRIPTS macro/method that will install all >>>>>> initscripts >>>>>> defined in the new INITSCRIPTS macro. >>>>>> * Adapt libvirt and zabbix_agentd pak as examples of how to >>>>>> use >>>>>> this. >>>>>> >>>>>> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> >>>>>> --- >>>>>> lfs/Config | 8 ++++++-- >>>>>> lfs/libvirt | 6 ++++-- >>>>>> lfs/zabbix_agentd | 5 ++++- >>>>>> src/pakfire/meta | 2 ++ >>>>>> 4 files changed, 16 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/lfs/Config b/lfs/Config >>>>>> index eadbbc408..61d6f0c82 100644 >>>>>> --- a/lfs/Config >>>>>> +++ b/lfs/Config >>>>>> @@ -299,15 +299,19 @@ define PAK >>>>>> # Create meta file >>>>>> sed \ >>>>>> -e "s/NAME/$(PROG)/g" \ >>>>>> + -e "s/SUMMARY/$(SUMMARY)/g" \ >>>>>> -e "s/VER/$(VER)/g" \ >>>>>> -e "s/RELEASE/$(PAK_VER)/g" \ >>>>>> -e "s/DEPS/$(DEPS)/g" \ >>>>>> -e "s/SIZE/$$(stat --format=%s >>>>>> /install/packages/$(PROG)- >>>>>> $(VER)-$(PAK_VER).ipfire)/g" \ >>>>>> + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ >>>>>> < /usr/src/src/pakfire/meta > /install/packages/meta- >>>>>> $(PROG) >>>>>> endef >>>>>> >>>>>> -define INSTALL_INITSCRIPT >>>>>> - install -m 754 -v >>>>>> $(DIR_SRC)/src/initscripts/packages/$(1) >>>>>> /etc/rc.d/init.d/$(1) >>>>>> +define INSTALL_INITSCRIPTS >>>>>> + for initscript in $(INITSCRIPTS); do \ >>>>>> + install -m 754 -v >>>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript >>>>>> /etc/rc.d/init.d/$$initscript; \ >>>>>> + done >>>>>> endef >>>>>> >>>>>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 >>>>>> riscv64)" >>>>>> diff --git a/lfs/libvirt b/lfs/libvirt >>>>>> index 28a95d317..be5d3db3a 100644 >>>>>> --- a/lfs/libvirt >>>>>> +++ b/lfs/libvirt >>>>>> @@ -25,6 +25,7 @@ >>>>>> include Config >>>>>> >>>>>> VER = 6.5.0 >>>>>> +SUMMARY = Server side daemon and supporting files for >>>>>> libvirt >>>>>> >>>>>> THISAPP = libvirt-$(VER) >>>>>> DL_FILE = $(THISAPP).tar.xz >>>>>> @@ -37,6 +38,8 @@ PAK_VER = 25 >>>>>> >>>>>> DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu >>>>>> >>>>>> +INITSCRIPTS= libvirtd virtlogd >>>>>> + >>>>>> ############################################################# >>>>>> ###### >>>>>> ############ >>>>>> # Top-level Rules >>>>>> ############################################################# >>>>>> ###### >>>>>> ############ >>>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst >>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>> cd $(DIR_APP)/build_libvirt && make install >>>>>> >>>>>> #install initscripts >>>>>> - $(call INSTALL_INITSCRIPT,libvirtd) >>>>>> - $(call INSTALL_INITSCRIPT,virtlogd) >>>>>> + @$(INSTALL_INITSCRIPTS) >>>>>> mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt- >>>>>> guests >>>>> And here my approach maybe breaks :-). How could we handle >>>>> this? It >>>>> is not a daemon, more something like a startup/shutdown scripts >>>>> for >>>>> machines... (And should not appear in the service.cgi). So >>>>> parsing >>>>> the root file may yield wrong results and your way would be the >>>>> better one. >>>>> >>>>> Jonatan >>>>>> >>>>>> # Backup >>>>>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd >>>>>> index c69643a54..a72fe024b 100644 >>>>>> --- a/lfs/zabbix_agentd >>>>>> +++ b/lfs/zabbix_agentd >>>>>> @@ -25,6 +25,7 @@ >>>>>> include Config >>>>>> >>>>>> VER = 4.2.6 >>>>>> +SUMMARY = Zabbix Agent >>>>>> >>>>>> THISAPP = zabbix-$(VER) >>>>>> DL_FILE = $(THISAPP).tar.gz >>>>>> @@ -35,6 +36,8 @@ PROG = zabbix_agentd >>>>>> PAK_VER = 4 >>>>>> DEPS = >>>>>> >>>>>> +INITSCRIPTS= zabbix_agentd >>>>>> + >>>>>> ############################################################# >>>>>> ###### >>>>>> ############ >>>>>> # Top-level Rules >>>>>> ############################################################# >>>>>> ###### >>>>>> ############ >>>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst >>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>> chown zabbix.zabbix /var/run/zabbix >>>>>> >>>>>> # Install initscripts >>>>>> - $(call INSTALL_INITSCRIPT,zabbix_agentd) >>>>>> + @$(INSTALL_INITSCRIPTS) >>>>>> >>>>>> # Install sudoers include file >>>>>> install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ >>>>>> diff --git a/src/pakfire/meta b/src/pakfire/meta >>>>>> index d97b2a0fa..849b9cd6c 100644 >>>>>> --- a/src/pakfire/meta >>>>>> +++ b/src/pakfire/meta >>>>>> @@ -1,6 +1,8 @@ >>>>>> Name: NAME >>>>>> +Summary: SUMMARY >>>>>> ProgVersion: VER >>>>>> Release: RELEASE >>>>>> Size: SIZE >>>>>> Dependencies: DEPS >>>>>> File: NAME-VER-RELEASE.ipfire >>>>>> +InitScripts: INITSCRIPTS >>>>>> -- >>>>>> 2.31.1 >>>>>> >>>>>> >>>>>> -- >>>>>> Dit bericht is gescanned op virussen en andere gevaarlijke >>>>>> inhoud door MailScanner en lijkt schoon te zijn. >>>> >>>> >>> >>> >>> -- >>> Dit bericht is gescanned op virussen en andere gevaarlijke >>> inhoud door MailScanner en lijkt schoon te zijn. >> >> > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-04-23 16:15 ` [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Robin Roevens 2021-05-12 18:49 ` Jonatan Schlag @ 2021-05-14 12:23 ` Michael Tremer 2021-05-14 20:16 ` Robin Roevens 1 sibling, 1 reply; 26+ messages in thread From: Michael Tremer @ 2021-05-14 12:23 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 4518 bytes --] Hello, > On 23 Apr 2021, at 17:15, Robin Roevens <robin.roevens(a)disroot.org> wrote: > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles. > * Add a Summary and InitScripts field to the meta-* addon files > containing the value's of those macro's. > * Replace the INSTALL_INITSCRIPT makefile macro by a new > INSTALL_INITSCRIPTS macro/method that will install all initscripts > defined in the new INITSCRIPTS macro. > * Adapt libvirt and zabbix_agentd pak as examples of how to use this. > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > --- > lfs/Config | 8 ++++++-- > lfs/libvirt | 6 ++++-- > lfs/zabbix_agentd | 5 ++++- > src/pakfire/meta | 2 ++ > 4 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/lfs/Config b/lfs/Config > index eadbbc408..61d6f0c82 100644 > --- a/lfs/Config > +++ b/lfs/Config > @@ -299,15 +299,19 @@ define PAK > # Create meta file > sed \ > -e "s/NAME/$(PROG)/g" \ > + -e "s/SUMMARY/$(SUMMARY)/g" \ > -e "s/VER/$(VER)/g" \ > -e "s/RELEASE/$(PAK_VER)/g" \ > -e "s/DEPS/$(DEPS)/g" \ > -e "s/SIZE/$$(stat --format=%s /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \ > + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ > < /usr/src/src/pakfire/meta > /install/packages/meta-$(PROG) > endef > > -define INSTALL_INITSCRIPT > - install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) /etc/rc.d/init.d/$(1) > +define INSTALL_INITSCRIPTS > + for initscript in $(INITSCRIPTS); do \ > + install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$$initscript /etc/rc.d/init.d/$$initscript; \ > + done > endef This won’t abort any more if there install command failed. The exit code is ignored and the loop will continue with its next iteration. You could add “|| exit 1” after the install command. There should be plenty of examples in the code where this has been used. > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)" > diff --git a/lfs/libvirt b/lfs/libvirt > index 28a95d317..be5d3db3a 100644 > --- a/lfs/libvirt > +++ b/lfs/libvirt > @@ -25,6 +25,7 @@ > include Config > > VER = 6.5.0 > +SUMMARY = Server side daemon and supporting files for libvirt > > THISAPP = libvirt-$(VER) > DL_FILE = $(THISAPP).tar.xz > @@ -37,6 +38,8 @@ PAK_VER = 25 > > DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu > > +INITSCRIPTS= libvirtd virtlogd > + > ############################################################################### > # Top-level Rules > ############################################################################### > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) > cd $(DIR_APP)/build_libvirt && make install > > #install initscripts > - $(call INSTALL_INITSCRIPT,libvirtd) > - $(call INSTALL_INITSCRIPT,virtlogd) > + @$(INSTALL_INITSCRIPTS) > mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-guests > > # Backup > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd > index c69643a54..a72fe024b 100644 > --- a/lfs/zabbix_agentd > +++ b/lfs/zabbix_agentd > @@ -25,6 +25,7 @@ > include Config > > VER = 4.2.6 > +SUMMARY = Zabbix Agent > > THISAPP = zabbix-$(VER) > DL_FILE = $(THISAPP).tar.gz > @@ -35,6 +36,8 @@ PROG = zabbix_agentd > PAK_VER = 4 > DEPS = > > +INITSCRIPTS= zabbix_agentd > + > ############################################################################### > # Top-level Rules > ############################################################################### > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) > chown zabbix.zabbix /var/run/zabbix > > # Install initscripts > - $(call INSTALL_INITSCRIPT,zabbix_agentd) > + @$(INSTALL_INITSCRIPTS) > > # Install sudoers include file > install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ > diff --git a/src/pakfire/meta b/src/pakfire/meta > index d97b2a0fa..849b9cd6c 100644 > --- a/src/pakfire/meta > +++ b/src/pakfire/meta > @@ -1,6 +1,8 @@ > Name: NAME > +Summary: SUMMARY > ProgVersion: VER > Release: RELEASE > Size: SIZE > Dependencies: DEPS > File: NAME-VER-RELEASE.ipfire > +InitScripts: INITSCRIPTS > -- > 2.31.1 Also, what happens with all the other files that used to call “INSTALL_INITSCRIPT”? -Michael > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-05-14 12:23 ` Michael Tremer @ 2021-05-14 20:16 ` Robin Roevens 2021-05-18 11:12 ` Michael Tremer 0 siblings, 1 reply; 26+ messages in thread From: Robin Roevens @ 2021-05-14 20:16 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 6339 bytes --] Hi Michael Tremer schreef op vr 14-05-2021 om 13:23 [+0100]: > Hello, > > > On 23 Apr 2021, at 17:15, Robin Roevens <robin.roevens(a)disroot.org> > > wrote: > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles. > > * Add a Summary and InitScripts field to the meta-* addon files > > containing the value's of those macro's. > > * Replace the INSTALL_INITSCRIPT makefile macro by a new > > INSTALL_INITSCRIPTS macro/method that will install all initscripts > > defined in the new INITSCRIPTS macro. > > * Adapt libvirt and zabbix_agentd pak as examples of how to use > > this. > > > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > > --- > > lfs/Config | 8 ++++++-- > > lfs/libvirt | 6 ++++-- > > lfs/zabbix_agentd | 5 ++++- > > src/pakfire/meta | 2 ++ > > 4 files changed, 16 insertions(+), 5 deletions(-) > > > > diff --git a/lfs/Config b/lfs/Config > > index eadbbc408..61d6f0c82 100644 > > --- a/lfs/Config > > +++ b/lfs/Config > > @@ -299,15 +299,19 @@ define PAK > > # Create meta file > > sed \ > > -e "s/NAME/$(PROG)/g" \ > > + -e "s/SUMMARY/$(SUMMARY)/g" \ > > -e "s/VER/$(VER)/g" \ > > -e "s/RELEASE/$(PAK_VER)/g" \ > > -e "s/DEPS/$(DEPS)/g" \ > > -e "s/SIZE/$$(stat --format=%s > > /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \ > > + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ > > < /usr/src/src/pakfire/meta > /install/packages/meta- > > $(PROG) > > endef > > > > -define INSTALL_INITSCRIPT > > - install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) > > /etc/rc.d/init.d/$(1) > > +define INSTALL_INITSCRIPTS > > + for initscript in $(INITSCRIPTS); do \ > > + install -m 754 -v > > $(DIR_SRC)/src/initscripts/packages/$$initscript > > /etc/rc.d/init.d/$$initscript; \ > > + done > > endef > > This won’t abort any more if there install command failed. The exit > code is ignored and the loop will continue with its next iteration. > > You could add “|| exit 1” after the install command. There should be > plenty of examples in the code where this has been used. > Good point. will add that. > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)" > > diff --git a/lfs/libvirt b/lfs/libvirt > > index 28a95d317..be5d3db3a 100644 > > --- a/lfs/libvirt > > +++ b/lfs/libvirt > > @@ -25,6 +25,7 @@ > > include Config > > > > VER = 6.5.0 > > +SUMMARY = Server side daemon and supporting files for > > libvirt > > > > THISAPP = libvirt-$(VER) > > DL_FILE = $(THISAPP).tar.xz > > @@ -37,6 +38,8 @@ PAK_VER = 25 > > > > DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu > > > > +INITSCRIPTS= libvirtd virtlogd > > + > > ################################################################### > > ############ > > # Top-level Rules > > ################################################################### > > ############ > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst > > %,$(DIR_DL)/%,$(objects)) > > cd $(DIR_APP)/build_libvirt && make install > > > > #install initscripts > > - $(call INSTALL_INITSCRIPT,libvirtd) > > - $(call INSTALL_INITSCRIPT,virtlogd) > > + @$(INSTALL_INITSCRIPTS) > > mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt- > > guests > > > > # Backup > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd > > index c69643a54..a72fe024b 100644 > > --- a/lfs/zabbix_agentd > > +++ b/lfs/zabbix_agentd > > @@ -25,6 +25,7 @@ > > include Config > > > > VER = 4.2.6 > > +SUMMARY = Zabbix Agent > > > > THISAPP = zabbix-$(VER) > > DL_FILE = $(THISAPP).tar.gz > > @@ -35,6 +36,8 @@ PROG = zabbix_agentd > > PAK_VER = 4 > > DEPS = > > > > +INITSCRIPTS= zabbix_agentd > > + > > ################################################################### > > ############ > > # Top-level Rules > > ################################################################### > > ############ > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst > > %,$(DIR_DL)/%,$(objects)) > > chown zabbix.zabbix /var/run/zabbix > > > > # Install initscripts > > - $(call INSTALL_INITSCRIPT,zabbix_agentd) > > + @$(INSTALL_INITSCRIPTS) > > > > # Install sudoers include file > > install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ > > diff --git a/src/pakfire/meta b/src/pakfire/meta > > index d97b2a0fa..849b9cd6c 100644 > > --- a/src/pakfire/meta > > +++ b/src/pakfire/meta > > @@ -1,6 +1,8 @@ > > Name: NAME > > +Summary: SUMMARY > > ProgVersion: VER > > Release: RELEASE > > Size: SIZE > > Dependencies: DEPS > > File: NAME-VER-RELEASE.ipfire > > +InitScripts: INITSCRIPTS > > -- > > 2.31.1 > > Also, what happens with all the other files that used to call > “INSTALL_INITSCRIPT”? > What other files call INSTALL_INITSCRIPT ? Is it not only called from the lfs files? The intention is of course, in the final patch-set to adapt all lfs files to use the INITSCRIPTS variable and call INSTALL_INITSCIPTS instead of individual calls to INSTALL_INITSCRIPT. But I did not yet want to do this, since my idea could also be shot down or drastically adapted, making that work obsolete. (already proven by my new SERVICES/INITSCRIPTS proposal in my previous mail). SO I currently only included 2 examples (zabbix_agentd and libvirt) of the changes to the lfs files involved. If that is what you meant ? Regards Robin > -Michael > > > -- > > Dit bericht is gescanned op virussen en andere gevaarlijke > > inhoud door MailScanner en lijkt schoon te zijn. > > > > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files 2021-05-14 20:16 ` Robin Roevens @ 2021-05-18 11:12 ` Michael Tremer 0 siblings, 0 replies; 26+ messages in thread From: Michael Tremer @ 2021-05-18 11:12 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 6453 bytes --] Hello, > On 14 May 2021, at 21:16, Robin Roevens <robin.roevens(a)disroot.org> wrote: > > Hi > > Michael Tremer schreef op vr 14-05-2021 om 13:23 [+0100]: >> Hello, >> >>> On 23 Apr 2021, at 17:15, Robin Roevens <robin.roevens(a)disroot.org> >>> wrote: >>> >>> * Introduce SUMMARY and INITSCRIPTS macro's in LFS-makefiles. >>> * Add a Summary and InitScripts field to the meta-* addon files >>> containing the value's of those macro's. >>> * Replace the INSTALL_INITSCRIPT makefile macro by a new >>> INSTALL_INITSCRIPTS macro/method that will install all initscripts >>> defined in the new INITSCRIPTS macro. >>> * Adapt libvirt and zabbix_agentd pak as examples of how to use >>> this. >>> >>> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> >>> --- >>> lfs/Config | 8 ++++++-- >>> lfs/libvirt | 6 ++++-- >>> lfs/zabbix_agentd | 5 ++++- >>> src/pakfire/meta | 2 ++ >>> 4 files changed, 16 insertions(+), 5 deletions(-) >>> >>> diff --git a/lfs/Config b/lfs/Config >>> index eadbbc408..61d6f0c82 100644 >>> --- a/lfs/Config >>> +++ b/lfs/Config >>> @@ -299,15 +299,19 @@ define PAK >>> # Create meta file >>> sed \ >>> -e "s/NAME/$(PROG)/g" \ >>> + -e "s/SUMMARY/$(SUMMARY)/g" \ >>> -e "s/VER/$(VER)/g" \ >>> -e "s/RELEASE/$(PAK_VER)/g" \ >>> -e "s/DEPS/$(DEPS)/g" \ >>> -e "s/SIZE/$$(stat --format=%s >>> /install/packages/$(PROG)-$(VER)-$(PAK_VER).ipfire)/g" \ >>> + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ >>> < /usr/src/src/pakfire/meta > /install/packages/meta- >>> $(PROG) >>> endef >>> >>> -define INSTALL_INITSCRIPT >>> - install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) >>> /etc/rc.d/init.d/$(1) >>> +define INSTALL_INITSCRIPTS >>> + for initscript in $(INITSCRIPTS); do \ >>> + install -m 754 -v >>> $(DIR_SRC)/src/initscripts/packages/$$initscript >>> /etc/rc.d/init.d/$$initscript; \ >>> + done >>> endef >> >> This won’t abort any more if there install command failed. The exit >> code is ignored and the loop will continue with its next iteration. >> >> You could add “|| exit 1” after the install command. There should be >> plenty of examples in the code where this has been used. >> > > Good point. will add that. > >>> ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64 riscv64)" >>> diff --git a/lfs/libvirt b/lfs/libvirt >>> index 28a95d317..be5d3db3a 100644 >>> --- a/lfs/libvirt >>> +++ b/lfs/libvirt >>> @@ -25,6 +25,7 @@ >>> include Config >>> >>> VER = 6.5.0 >>> +SUMMARY = Server side daemon and supporting files for >>> libvirt >>> >>> THISAPP = libvirt-$(VER) >>> DL_FILE = $(THISAPP).tar.xz >>> @@ -37,6 +38,8 @@ PAK_VER = 25 >>> >>> DEPS = ebtables libpciaccess libtirpc libyajl ncat qemu >>> >>> +INITSCRIPTS= libvirtd virtlogd >>> + >>> ################################################################### >>> ############ >>> # Top-level Rules >>> ################################################################### >>> ############ >>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst >>> %,$(DIR_DL)/%,$(objects)) >>> cd $(DIR_APP)/build_libvirt && make install >>> >>> #install initscripts >>> - $(call INSTALL_INITSCRIPT,libvirtd) >>> - $(call INSTALL_INITSCRIPT,virtlogd) >>> + @$(INSTALL_INITSCRIPTS) >>> mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt- >>> guests >>> >>> # Backup >>> diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd >>> index c69643a54..a72fe024b 100644 >>> --- a/lfs/zabbix_agentd >>> +++ b/lfs/zabbix_agentd >>> @@ -25,6 +25,7 @@ >>> include Config >>> >>> VER = 4.2.6 >>> +SUMMARY = Zabbix Agent >>> >>> THISAPP = zabbix-$(VER) >>> DL_FILE = $(THISAPP).tar.gz >>> @@ -35,6 +36,8 @@ PROG = zabbix_agentd >>> PAK_VER = 4 >>> DEPS = >>> >>> +INITSCRIPTS= zabbix_agentd >>> + >>> ################################################################### >>> ############ >>> # Top-level Rules >>> ################################################################### >>> ############ >>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst >>> %,$(DIR_DL)/%,$(objects)) >>> chown zabbix.zabbix /var/run/zabbix >>> >>> # Install initscripts >>> - $(call INSTALL_INITSCRIPT,zabbix_agentd) >>> + @$(INSTALL_INITSCRIPTS) >>> >>> # Install sudoers include file >>> install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \ >>> diff --git a/src/pakfire/meta b/src/pakfire/meta >>> index d97b2a0fa..849b9cd6c 100644 >>> --- a/src/pakfire/meta >>> +++ b/src/pakfire/meta >>> @@ -1,6 +1,8 @@ >>> Name: NAME >>> +Summary: SUMMARY >>> ProgVersion: VER >>> Release: RELEASE >>> Size: SIZE >>> Dependencies: DEPS >>> File: NAME-VER-RELEASE.ipfire >>> +InitScripts: INITSCRIPTS >>> -- >>> 2.31.1 >> >> Also, what happens with all the other files that used to call >> “INSTALL_INITSCRIPT”? >> > > What other files call INSTALL_INITSCRIPT ? Is it not only called from > the lfs files? > The intention is of course, in the final patch-set to adapt all lfs > files to use the INITSCRIPTS variable and call INSTALL_INITSCIPTS > instead of individual calls to INSTALL_INITSCRIPT. But I did not yet > want to do this, since my idea could also be shot down or drastically > adapted, making that work obsolete. (already proven by my new > SERVICES/INITSCRIPTS proposal in my previous mail). SO I currently only > included 2 examples (zabbix_agentd and libvirt) of the changes to the > lfs files involved. > If that is what you meant ? Okay, that wasn’t clear to me, but this is what I was referring to. INSTALL_INITSCRIPTS is called 56 times and you want to change it, all those files would need to be changed, too. However, let’s sort out Jonatan’s point first and see what a good solution would be. Best, -Michael > > Regards > Robin > >> -Michael >> >>> -- >>> Dit bericht is gescanned op virussen en andere gevaarlijke >>> inhoud door MailScanner en lijkt schoon te zijn. >>> >> >> > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/3] pakfire: add 'info <pak(s)>' option 2021-04-23 16:15 [PATCH 0/3] Pakfile metadata enhancements Robin Roevens 2021-04-23 16:15 ` [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Robin Roevens @ 2021-04-23 16:15 ` Robin Roevens 2021-05-13 8:02 ` Jonatan Schlag 2021-04-23 16:15 ` [PATCH 3/3] services.cgi: use new Pakfire::pakinfo function Robin Roevens ` (2 subsequent siblings) 4 siblings, 1 reply; 26+ messages in thread From: Robin Roevens @ 2021-04-23 16:15 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 5844 bytes --] Add an 'info <pak(s)>' option to pakfire to display pakfile metadata and current state of the pak on the system. Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> --- src/pakfire/lib/functions.pl | 124 ++++++++++++++++++++++++++++++++++- src/pakfire/pakfire | 21 ++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/pakfire/lib/functions.pl b/src/pakfire/lib/functions.pl index 4d5c6219a..46da06a88 100644 --- a/src/pakfire/lib/functions.pl +++ b/src/pakfire/lib/functions.pl @@ -110,7 +110,8 @@ sub usage { &Pakfire::message("Usage: pakfire <install|remove> [options] <pak(s)>"); &Pakfire::message(" <update> - Contacts the servers for new lists of paks."); &Pakfire::message(" <upgrade> - Installs the latest version of all paks."); - &Pakfire::message(" <list> - Outputs a short list with all available paks."); + &Pakfire::message(" <list> [installed/notinstalled] - Outputs a list with all, installed or available paks."); + &Pakfire::message(" <info> <pak(s)> - Output pak metadata."); &Pakfire::message(" <status> - Outputs a summary about available core upgrades, updates and a required reboot"); &Pakfire::message(""); &Pakfire::message(" Global options:"); @@ -538,6 +539,127 @@ sub dblist { } } +sub pakinfo { + ### This subroutine shows available info for a package + # Pass package name and type of info as argument: &Pakfire::pakinfo(package, "latest") + # Type can be "latest" or "installed" + # Usage is always with two argument. + my $pak = shift; + my $info_type = shift; + + my $found = 0; + my @templine; + my ($available_version, $available_release); + + if ("$info_type" eq "latest") { + ### Make sure that the info is not outdated. + &Pakfire::dbgetlist("noforce"); + + ### Check if package is in packages_list and get latest available version + open(FILE, "<$Conf::dbdir/lists/packages_list.db"); + my @db = <FILE>; + close(FILE); + + foreach (@db) { + @templine = split(/;/,$_); + if ("$templine[0]" eq "$pak" ) { + $found = 1; + $available_version = $templine[1]; + $available_release = $templine[2]; + break; + } + } + } + + ### Parse latest package metadata + my $installed = !&isinstalled("$pak"); + my @file; + + if ($found && "$info_type" eq "latest") { + getmetafile("$pak"); + + open(FILE, "<$Conf::dbdir/meta/meta-$pak"); + @file = <FILE>; + close(FILE); + } elsif ($installed) { + open(FILE, "<$Conf::dbdir/installed/meta-$pak"); + @file = <FILE>; + close(FILE); + } else { + message("PAKFIRE WARN: Pak '$pak' not found."); + return 1; + } + + my ($name, $summary, $size, $dependencies, $pakfile, $initscripts); + foreach $line (@file) { + @templine = split(/\: /,$line); + if ("$templine[0]" eq "Name") { + $name = $templine[1]; + chomp($name); + } elsif ("$templine[0]" eq "Summary") { + $summary = $templine[1]; + chomp($summary); + } elsif ("$templine[0]" eq "Size") { + $size = &beautifysize("$templine[1]"); + chomp($size); + } elsif ("$templine[0]" eq "Dependencies") { + $dependencies = $templine[1]; + chomp($dependencies); + } elsif ("$templine[0]" eq "File") { + $pakfile = $templine[1]; + chomp($pakfile); + } elsif ("$templine[0]" eq "InitScripts") { + $initscripts = $templine[1]; + chomp($initscripts); + } + } + + ### Get installed version information + my ($installed_version, $installed_release); + + if ($installed) { + open(FILE, "<$Conf::dbdir/installed/meta-$pak"); + @file = <FILE>; + close(FILE); + + foreach $line (@file) { + @templine = split(/\: /,$line); + if ("$templine[0]" eq "ProgVersion") { + $installed_version = $templine[1]; + chomp($installed_version); + } elsif ("$templine[0]" eq "Release") { + $installed_release = $templine[1]; + chomp($installed_release); + } + } + } + + print "Name: $name\n"; + if ("$info_type" eq "latest") { + print "Version: $available_version-$available_release\n"; + } else { + print "Version: $installed_version-$installed_release\n"; + } + print "Summary: $summary\nSize: $size\nDependencies: $dependencies\nPakfile: $pakfile\nInitScripts: $initscripts\n"; + if ($installed) { + print "Installed: Yes\n"; + } else { + print "Installed: No\n"; + } + if ("$info_type" eq "latest") { + if (!$found) { + print "Status: obsolete (version $installed_version-$installed_release is installed)\n"; + } elsif ($installed && "$installed_release" < "$available_release") { + print "Status: outdated (version $installed_version-$installed_release is installed)\n"; + } elsif ($installed) { + print "Status: up-to-date\n"; + } else { + print "Status: not installed\n"; + } + } + print "\n"; +} + sub resolvedeps_one { my $pak = shift; diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire index c69a8d3ad..5507c8e92 100644 --- a/src/pakfire/pakfire +++ b/src/pakfire/pakfire @@ -303,6 +303,27 @@ &Pakfire::message("PAKFIRE WARN: Not a known option $ARGV[1]") if ($ARGV[1]); &Pakfire::dblist("all", "noweb"); } + + } elsif ("$ARGV[0]" eq "info") { + shift; + + my @paks; + my $pak; + foreach $pak (@ARGV) { + unless ("$pak" =~ "^-") { + push(@paks,$pak); + } + } + + unless ("@paks") { + &Pakfire::message("PAKFIRE ERROR: missing package name"); + &Pakfire::usage; + exit 1; + } + + foreach $pak (@paks) { + &Pakfire::pakinfo("$pak", "latest"); + } } elsif ("$ARGV[0]" eq "resolvedeps") { foreach (@ARGV) { -- 2.31.1 -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] pakfire: add 'info <pak(s)>' option 2021-04-23 16:15 ` [PATCH 2/3] pakfire: add 'info <pak(s)>' option Robin Roevens @ 2021-05-13 8:02 ` Jonatan Schlag 2021-05-13 19:11 ` Robin Roevens 0 siblings, 1 reply; 26+ messages in thread From: Jonatan Schlag @ 2021-05-13 8:02 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 7728 bytes --] Hi, had a look at your code and found some more generic possibilities to improve it :-) Am 2021-04-23 18:15, schrieb Robin Roevens: > Add an 'info <pak(s)>' option to pakfire to display pakfile metadata > and current state of the pak on the system. > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > --- > src/pakfire/lib/functions.pl | 124 ++++++++++++++++++++++++++++++++++- > src/pakfire/pakfire | 21 ++++++ > 2 files changed, 144 insertions(+), 1 deletion(-) > > diff --git a/src/pakfire/lib/functions.pl > b/src/pakfire/lib/functions.pl > index 4d5c6219a..46da06a88 100644 > --- a/src/pakfire/lib/functions.pl > +++ b/src/pakfire/lib/functions.pl > @@ -110,7 +110,8 @@ sub usage { > &Pakfire::message("Usage: pakfire <install|remove> [options] > <pak(s)>"); > &Pakfire::message(" <update> - Contacts the servers > for new lists of paks."); > &Pakfire::message(" <upgrade> - Installs the latest > version of all paks."); > - &Pakfire::message(" <list> - Outputs a short list > with all available paks."); > + &Pakfire::message(" <list> [installed/notinstalled] - > Outputs a list with all, installed or available paks."); > + &Pakfire::message(" <info> <pak(s)> - Output pak > metadata."); > &Pakfire::message(" <status> - Outputs a summary > about available core upgrades, updates and a required reboot"); > &Pakfire::message(""); > &Pakfire::message(" Global options:"); > @@ -538,6 +539,127 @@ sub dblist { > } > } > > +sub pakinfo { > + ### This subroutine shows available info for a package > + # Pass package name and type of info as argument: > &Pakfire::pakinfo(package, "latest") > + # Type can be "latest" or "installed" > + # Usage is always with two argument. > + my $pak = shift; > + my $info_type = shift; > + > + my $found = 0; > + my @templine; > + my ($available_version, $available_release); > + > + if ("$info_type" eq "latest") { > + ### Make sure that the info is not outdated. > + &Pakfire::dbgetlist("noforce"); > + > + ### Check if package is in packages_list and get latest available > version > + open(FILE, "<$Conf::dbdir/lists/packages_list.db"); > + my @db = <FILE>; > + close(FILE); > + > + foreach (@db) { > + @templine = split(/;/,$_); > + if ("$templine[0]" eq "$pak" ) { > + $found = 1; > + $available_version = $templine[1]; > + $available_release = $templine[2]; > + break; > + } > + } > + } > + We have somehow similar code in dblist(). A very good example of why UI code and logic should be split up. Just want to mention it. It is definitely not your fault, and I do not want to blame you for the errors others made. But maybe it would be worth it to think about a function which either returns something like that: { (package-name, status in {installed, needs-upgrade, notinstalled })} or that: [ installed: {set of packages which are installed} needs upgrade: {self explanatory } not installed: {self explanatory } ] [] is a dictionary so in perl somehthing like %data = (); () is a tupel so in perl an array {} is a set so in perl an array and use this function here and in dblist. > + ### Parse latest package metadata > + my $installed = !&isinstalled("$pak"); > + my @file; > + > + if ($found && "$info_type" eq "latest") { > + getmetafile("$pak"); > + > + open(FILE, "<$Conf::dbdir/meta/meta-$pak"); > + @file = <FILE>; > + close(FILE); > + } elsif ($installed) { > + open(FILE, "<$Conf::dbdir/installed/meta-$pak"); > + @file = <FILE>; > + close(FILE); You could open the file at 1 > + } else { > + message("PAKFIRE WARN: Pak '$pak' not found."); > + return 1; > + } Here is 1 and deduplicate the code. > + > + my ($name, $summary, $size, $dependencies, $pakfile, $initscripts); > + foreach $line (@file) { > + @templine = split(/\: /,$line); > + if ("$templine[0]" eq "Name") { > + $name = $templine[1]; > + chomp($name); > + } elsif ("$templine[0]" eq "Summary") { > + $summary = $templine[1]; > + chomp($summary); > + } elsif ("$templine[0]" eq "Size") { > + $size = &beautifysize("$templine[1]"); > + chomp($size); > + } elsif ("$templine[0]" eq "Dependencies") { > + $dependencies = $templine[1]; > + chomp($dependencies); > + } elsif ("$templine[0]" eq "File") { > + $pakfile = $templine[1]; > + chomp($pakfile); > + } elsif ("$templine[0]" eq "InitScripts") { > + $initscripts = $templine[1]; > + chomp($initscripts); > + } > + } And here I somehow do not know what to say :-). The problem is that both blocks are code duplication. But we already have this kind of code duplication at a lot of places (I counted 5). So it is ok to do it that way, but it would be better to have a function which just returns a hash, and you could do something like that: metadata = get_metadate("$pak") metdata["installed_version"] Both functions would be also highly testable as we could lay down an example file and test if this file is read correctly. If now someone wonders why I wrote this down, it may help to learn new things, as I do when I read the list. I am unsure what advice a should give you @Robin: it is not your fault that the code looks like it looks, but on the other hand, making things "wrong" only because others did it before is also a dead end. So I hope others have an idea, what to do. Greetings Jonatan > + > + ### Get installed version information > + my ($installed_version, $installed_release); > + > + if ($installed) { > + open(FILE, "<$Conf::dbdir/installed/meta-$pak"); > + @file = <FILE>; > + close(FILE); > + > + foreach $line (@file) { > + @templine = split(/\: /,$line); > + if ("$templine[0]" eq "ProgVersion") { > + $installed_version = $templine[1]; > + chomp($installed_version); > + } elsif ("$templine[0]" eq "Release") { > + $installed_release = $templine[1]; > + chomp($installed_release); > + } > + } > + } > + > + print "Name: $name\n"; > + if ("$info_type" eq "latest") { > + print "Version: $available_version-$available_release\n"; > + } else { > + print "Version: $installed_version-$installed_release\n"; > + } > + print "Summary: $summary\nSize: $size\nDependencies: > $dependencies\nPakfile: $pakfile\nInitScripts: $initscripts\n"; > + if ($installed) { > + print "Installed: Yes\n"; > + } else { > + print "Installed: No\n"; > + } > + if ("$info_type" eq "latest") { > + if (!$found) { > + print "Status: obsolete (version > $installed_version-$installed_release is installed)\n"; > + } elsif ($installed && "$installed_release" < "$available_release") > { > + print "Status: outdated (version > $installed_version-$installed_release is installed)\n"; > + } elsif ($installed) { > + print "Status: up-to-date\n"; > + } else { > + print "Status: not installed\n"; > + } > + } > + print "\n"; > +} > + > sub resolvedeps_one { > my $pak = shift; > > diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire > index c69a8d3ad..5507c8e92 100644 > --- a/src/pakfire/pakfire > +++ b/src/pakfire/pakfire > @@ -303,6 +303,27 @@ > &Pakfire::message("PAKFIRE WARN: Not a known option $ARGV[1]") if > ($ARGV[1]); > &Pakfire::dblist("all", "noweb"); > } > + > + } elsif ("$ARGV[0]" eq "info") { > + shift; > + > + my @paks; > + my $pak; > + foreach $pak (@ARGV) { > + unless ("$pak" =~ "^-") { > + push(@paks,$pak); > + } > + } > + > + unless ("@paks") { > + &Pakfire::message("PAKFIRE ERROR: missing package name"); > + &Pakfire::usage; > + exit 1; > + } > + > + foreach $pak (@paks) { > + &Pakfire::pakinfo("$pak", "latest"); > + } > > } elsif ("$ARGV[0]" eq "resolvedeps") { > foreach (@ARGV) { > -- > 2.31.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] pakfire: add 'info <pak(s)>' option 2021-05-13 8:02 ` Jonatan Schlag @ 2021-05-13 19:11 ` Robin Roevens 2021-05-14 12:19 ` Michael Tremer 0 siblings, 1 reply; 26+ messages in thread From: Robin Roevens @ 2021-05-13 19:11 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 14005 bytes --] Hi Jonatan I completely agree with your remarks about the code duplication and the separation of GUI vs logic.. Actually even a bit relieved.. I only continued this way since I didn't want to 'break' with the way code was currently written and as I'm both very new here in this group and I never actually coded in Perl before.. I more or less assumed this is the way it is done here and it was not (yet) my 'right' to question it :-).. But I would be happy to introduce more reusable functions, less to no duplicate code and a more (m)vc-like approach to the code. In the meantime I have also been working on a .cgi page for the zabbix agent configuration.. completely in the style currently all cgi pages are where duplicate code is everywhere and logic and view are mangled together.. even with 'goto's all around the place (in most languages where 'goto' exists, it is some kind of taboo to actually use it.. except for plain old BASIC or so.. And it seems to be accepted in the Perl community too.. but still I frowned upon it :-) Anyway, I was about to start thinking about giving up on it since I would not want to maintain such code in the long run. But if it is ok for you guys to revamp the coding style to a more "modern" style, I'm willing to give that a try.. Regards Robin Jonatan Schlag schreef op do 13-05-2021 om 10:02 [+0200]: > Hi, > > had a look at your code and found some more generic possibilities to > improve it :-) > > Am 2021-04-23 18:15, schrieb Robin Roevens: > > Add an 'info <pak(s)>' option to pakfire to display pakfile > > metadata > > and current state of the pak on the system. > > > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > > --- > > src/pakfire/lib/functions.pl | 124 > > ++++++++++++++++++++++++++++++++++- > > src/pakfire/pakfire | 21 ++++++ > > 2 files changed, 144 insertions(+), 1 deletion(-) > > > > diff --git a/src/pakfire/lib/functions.pl > > b/src/pakfire/lib/functions.pl > > index 4d5c6219a..46da06a88 100644 > > --- a/src/pakfire/lib/functions.pl > > +++ b/src/pakfire/lib/functions.pl > > @@ -110,7 +110,8 @@ sub usage { > > &Pakfire::message("Usage: pakfire <install|remove> [options] > > <pak(s)>"); > > &Pakfire::message(" <update> - Contacts the > > servers > > for new lists of paks."); > > &Pakfire::message(" <upgrade> - Installs the > > latest > > version of all paks."); > > - &Pakfire::message(" <list> - Outputs a short list > > with all available paks."); > > + &Pakfire::message(" <list> > > [installed/notinstalled] - > > Outputs a list with all, installed or available paks."); > > + &Pakfire::message(" <info> <pak(s)> - Output pak > > metadata."); > > &Pakfire::message(" <status> - Outputs a summary > > about available core upgrades, updates and a required reboot"); > > &Pakfire::message(""); > > &Pakfire::message(" Global options:"); > > @@ -538,6 +539,127 @@ sub dblist { > > } > > } > > > > +sub pakinfo { > > + ### This subroutine shows available info for a package > > + # Pass package name and type of info as argument: > > &Pakfire::pakinfo(package, "latest") > > + # Type can be "latest" or "installed" > > + # Usage is always with two argument. > > + my $pak = shift; > > + my $info_type = shift; > > + > > + my $found = 0; > > + my @templine; > > + my ($available_version, $available_release); > > + > > + if ("$info_type" eq "latest") { > > + ### Make sure that the info is not outdated. > > + &Pakfire::dbgetlist("noforce"); > > + > > + ### Check if package is in packages_list and get > > latest available > > version > > + open(FILE, "<$Conf::dbdir/lists/packages_list.db"); > > + my @db = <FILE>; > > + close(FILE); > > + > > + foreach (@db) { > > + @templine = split(/;/,$_); > > + if ("$templine[0]" eq "$pak" ) { > > + $found = 1; > > + $available_version = $templine[1]; > > + $available_release = $templine[2]; > > + break; > > + } > > + } > > + } > > + > We have somehow similar code in dblist(). A very good example of why > UI > code and logic should be split up. > Just want to mention it. It is definitely not your fault, and I do > not > want to blame you for the errors others made. But maybe it would > be worth it to think about a function which either returns something > like that: > > { (package-name, status in {installed, needs-upgrade, notinstalled > })} > > or that: > > [ > installed: {set of packages which are installed} > needs upgrade: {self explanatory } > not installed: {self explanatory } > ] > > [] is a dictionary so in perl somehthing like %data = (); > () is a tupel so in perl an array > {} is a set so in perl an array > > and use this function here and in dblist. > > > + ### Parse latest package metadata > > + my $installed = !&isinstalled("$pak"); > > + my @file; > > + > > + if ($found && "$info_type" eq "latest") { > > + getmetafile("$pak"); > > + > > + open(FILE, "<$Conf::dbdir/meta/meta-$pak"); > > + @file = <FILE>; > > + close(FILE); > > + } elsif ($installed) { > > + open(FILE, "<$Conf::dbdir/installed/meta-$pak"); > > + @file = <FILE>; > > + close(FILE); > You could open the file at 1 > > + } else { > > + message("PAKFIRE WARN: Pak '$pak' not found."); > > + return 1; > > + } > > Here is 1 and deduplicate the code. > > + > > + my ($name, $summary, $size, $dependencies, $pakfile, > > $initscripts); > > + foreach $line (@file) { > > + @templine = split(/\: /,$line); > > + if ("$templine[0]" eq "Name") { > > + $name = $templine[1]; > > + chomp($name); > > + } elsif ("$templine[0]" eq "Summary") { > > + $summary = $templine[1]; > > + chomp($summary); > > + } elsif ("$templine[0]" eq "Size") { > > + $size = &beautifysize("$templine[1]"); > > + chomp($size); > > + } elsif ("$templine[0]" eq "Dependencies") { > > + $dependencies = $templine[1]; > > + chomp($dependencies); > > + } elsif ("$templine[0]" eq "File") { > > + $pakfile = $templine[1]; > > + chomp($pakfile); > > + } elsif ("$templine[0]" eq "InitScripts") { > > + $initscripts = $templine[1]; > > + chomp($initscripts); > > + } > > + } > > And here I somehow do not know what to say :-). The problem is that > both > blocks are code duplication. But we already have this kind of code > duplication at a lot of places (I counted 5). > So it is ok to do it that way, but it would be better to have a > function > which just returns a hash, and you could do something like that: > > metadata = get_metadate("$pak") > > metdata["installed_version"] > > Both functions would be also highly testable as we could lay down an > example file and test if this file is read correctly. > > If now someone wonders why I wrote this down, it may help to learn > new > things, as I do when I read the list. I am unsure what advice a > should > give you @Robin: it is not your fault that the code looks like it > looks, > but on the other hand, making things "wrong" only because others did > it > before is also a dead end. So I hope others have an idea, what to do. > > Greetings Jonatan > > + > > + ### Get installed version information > > + my ($installed_version, $installed_release); > > + > > + if ($installed) { > > + open(FILE, "<$Conf::dbdir/installed/meta-$pak"); > > + @file = <FILE>; > > + close(FILE); > > + > > + foreach $line (@file) { > > + @templine = split(/\: /,$line); > > + if ("$templine[0]" eq "ProgVersion") { > > + $installed_version = $templine[1]; > > + chomp($installed_version); > > + } elsif ("$templine[0]" eq "Release") { > > + $installed_release = $templine[1]; > > + chomp($installed_release); > > + } > > + } > > + } > > + > > + print "Name: $name\n"; > > + if ("$info_type" eq "latest") { > > + print "Version: $available_version- > > $available_release\n"; > > + } else { > > + print "Version: $installed_version- > > $installed_release\n"; > > + } > > + print "Summary: $summary\nSize: $size\nDependencies: > > $dependencies\nPakfile: $pakfile\nInitScripts: $initscripts\n"; > > + if ($installed) { > > + print "Installed: Yes\n"; > > + } else { > > + print "Installed: No\n"; > > + } > > + if ("$info_type" eq "latest") { > > + if (!$found) { > > + print "Status: obsolete (version > > $installed_version-$installed_release is installed)\n"; > > + } elsif ($installed && "$installed_release" < > > "$available_release") > > { > > + print "Status: outdated (version > > $installed_version-$installed_release is installed)\n"; > > + } elsif ($installed) { > > + print "Status: up-to-date\n"; > > + } else { > > + print "Status: not installed\n"; > > + } > > + } > > + print "\n"; > > +} > > + > > sub resolvedeps_one { > > my $pak = shift; > > > > diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire > > index c69a8d3ad..5507c8e92 100644 > > --- a/src/pakfire/pakfire > > +++ b/src/pakfire/pakfire > > @@ -303,6 +303,27 @@ > > &Pakfire::message("PAKFIRE WARN: Not a > > known option $ARGV[1]") if > > ($ARGV[1]); > > &Pakfire::dblist("all", "noweb"); > > } > > + > > + } elsif ("$ARGV[0]" eq "info") { > > + shift; > > + > > + my @paks; > > + my $pak; > > + foreach $pak (@ARGV) { > > + unless ("$pak" =~ "^-") { > > + push(@paks,$pak); > > + } > > + } > > + > > + unless ("@paks") { > > + &Pakfire::message("PAKFIRE ERROR: missing > > package name"); > > + &Pakfire::usage; > > + exit 1; > > + } > > + > > + foreach $pak (@paks) { > > + &Pakfire::pakinfo("$pak", "latest"); > > + } > > > > } elsif ("$ARGV[0]" eq "resolvedeps") { > > foreach (@ARGV) { > > -- > > 2.31.1 > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] pakfire: add 'info <pak(s)>' option 2021-05-13 19:11 ` Robin Roevens @ 2021-05-14 12:19 ` Michael Tremer 2021-05-14 20:24 ` Robin Roevens 0 siblings, 1 reply; 26+ messages in thread From: Michael Tremer @ 2021-05-14 12:19 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 12680 bytes --] Hello, All the mess is probably my fault. I was young and had no clue what I was doing. Now I am older… still not having a clue what I am doing. I would say your solution looks clean, but as Jonatan suggested, a unique function to read the package metadata and return it as a hash would help. If you want to add that, I am happy to merge the patch. Generally I am interested in seeing pakfire being tidied up a little bit, but that has to happen in small steps that are reviewable and testable. Anything that breaks might stop people from installing updates and us from shipping a fix. -Michael > On 13 May 2021, at 20:11, Robin Roevens <robin.roevens(a)disroot.org> wrote: > > Hi Jonatan > > I completely agree with your remarks about the code duplication and the > separation of GUI vs logic.. Actually even a bit relieved.. > > I only continued this way since I didn't want to 'break' with the way > code was currently written and as I'm both very new here in this group > and I never actually coded in Perl before.. I more or less assumed this > is the way it is done here and it was not (yet) my 'right' to question > it :-).. > > But I would be happy to introduce more reusable functions, less to no > duplicate code and a more (m)vc-like approach to the code. > > In the meantime I have also been working on a .cgi page for the zabbix > agent configuration.. completely in the style currently all cgi pages > are where duplicate code is everywhere and logic and view are mangled > together.. even with 'goto's all around the place (in most languages > where 'goto' exists, it is some kind of taboo to actually use it.. > except for plain old BASIC or so.. And it seems to be accepted in the > Perl community too.. but still I frowned upon it :-) > Anyway, I was about to start thinking about giving up on it since I > would not want to maintain such code in the long run. > > But if it is ok for you guys to revamp the coding style to a more > "modern" style, I'm willing to give that a try.. > > Regards > Robin > > Jonatan Schlag schreef op do 13-05-2021 om 10:02 [+0200]: >> Hi, >> >> had a look at your code and found some more generic possibilities to >> improve it :-) >> >> Am 2021-04-23 18:15, schrieb Robin Roevens: >>> Add an 'info <pak(s)>' option to pakfire to display pakfile >>> metadata >>> and current state of the pak on the system. >>> >>> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> >>> --- >>> src/pakfire/lib/functions.pl | 124 >>> ++++++++++++++++++++++++++++++++++- >>> src/pakfire/pakfire | 21 ++++++ >>> 2 files changed, 144 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/pakfire/lib/functions.pl >>> b/src/pakfire/lib/functions.pl >>> index 4d5c6219a..46da06a88 100644 >>> --- a/src/pakfire/lib/functions.pl >>> +++ b/src/pakfire/lib/functions.pl >>> @@ -110,7 +110,8 @@ sub usage { >>> &Pakfire::message("Usage: pakfire <install|remove> [options] >>> <pak(s)>"); >>> &Pakfire::message(" <update> - Contacts the >>> servers >>> for new lists of paks."); >>> &Pakfire::message(" <upgrade> - Installs the >>> latest >>> version of all paks."); >>> - &Pakfire::message(" <list> - Outputs a short list >>> with all available paks."); >>> + &Pakfire::message(" <list> >>> [installed/notinstalled] - >>> Outputs a list with all, installed or available paks."); >>> + &Pakfire::message(" <info> <pak(s)> - Output pak >>> metadata."); >>> &Pakfire::message(" <status> - Outputs a summary >>> about available core upgrades, updates and a required reboot"); >>> &Pakfire::message(""); >>> &Pakfire::message(" Global options:"); >>> @@ -538,6 +539,127 @@ sub dblist { >>> } >>> } >>> >>> +sub pakinfo { >>> + ### This subroutine shows available info for a package >>> + # Pass package name and type of info as argument: >>> &Pakfire::pakinfo(package, "latest") >>> + # Type can be "latest" or "installed" >>> + # Usage is always with two argument. >>> + my $pak = shift; >>> + my $info_type = shift; >>> + >>> + my $found = 0; >>> + my @templine; >>> + my ($available_version, $available_release); >>> + >>> + if ("$info_type" eq "latest") { >>> + ### Make sure that the info is not outdated. >>> + &Pakfire::dbgetlist("noforce"); >>> + >>> + ### Check if package is in packages_list and get >>> latest available >>> version >>> + open(FILE, "<$Conf::dbdir/lists/packages_list.db"); >>> + my @db = <FILE>; >>> + close(FILE); >>> + >>> + foreach (@db) { >>> + @templine = split(/;/,$_); >>> + if ("$templine[0]" eq "$pak" ) { >>> + $found = 1; >>> + $available_version = $templine[1]; >>> + $available_release = $templine[2]; >>> + break; >>> + } >>> + } >>> + } >>> + >> We have somehow similar code in dblist(). A very good example of why >> UI >> code and logic should be split up. >> Just want to mention it. It is definitely not your fault, and I do >> not >> want to blame you for the errors others made. But maybe it would >> be worth it to think about a function which either returns something >> like that: >> >> { (package-name, status in {installed, needs-upgrade, notinstalled >> })} >> >> or that: >> >> [ >> installed: {set of packages which are installed} >> needs upgrade: {self explanatory } >> not installed: {self explanatory } >> ] >> >> [] is a dictionary so in perl somehthing like %data = (); >> () is a tupel so in perl an array >> {} is a set so in perl an array >> >> and use this function here and in dblist. >> >>> + ### Parse latest package metadata >>> + my $installed = !&isinstalled("$pak"); >>> + my @file; >>> + >>> + if ($found && "$info_type" eq "latest") { >>> + getmetafile("$pak"); >>> + >>> + open(FILE, "<$Conf::dbdir/meta/meta-$pak"); >>> + @file = <FILE>; >>> + close(FILE); >>> + } elsif ($installed) { >>> + open(FILE, "<$Conf::dbdir/installed/meta-$pak"); >>> + @file = <FILE>; >>> + close(FILE); >> You could open the file at 1 >>> + } else { >>> + message("PAKFIRE WARN: Pak '$pak' not found."); >>> + return 1; >>> + } >> >> Here is 1 and deduplicate the code. >>> + >>> + my ($name, $summary, $size, $dependencies, $pakfile, >>> $initscripts); >>> + foreach $line (@file) { >>> + @templine = split(/\: /,$line); >>> + if ("$templine[0]" eq "Name") { >>> + $name = $templine[1]; >>> + chomp($name); >>> + } elsif ("$templine[0]" eq "Summary") { >>> + $summary = $templine[1]; >>> + chomp($summary); >>> + } elsif ("$templine[0]" eq "Size") { >>> + $size = &beautifysize("$templine[1]"); >>> + chomp($size); >>> + } elsif ("$templine[0]" eq "Dependencies") { >>> + $dependencies = $templine[1]; >>> + chomp($dependencies); >>> + } elsif ("$templine[0]" eq "File") { >>> + $pakfile = $templine[1]; >>> + chomp($pakfile); >>> + } elsif ("$templine[0]" eq "InitScripts") { >>> + $initscripts = $templine[1]; >>> + chomp($initscripts); >>> + } >>> + } >> >> And here I somehow do not know what to say :-). The problem is that >> both >> blocks are code duplication. But we already have this kind of code >> duplication at a lot of places (I counted 5). >> So it is ok to do it that way, but it would be better to have a >> function >> which just returns a hash, and you could do something like that: >> >> metadata = get_metadate("$pak") >> >> metdata["installed_version"] >> >> Both functions would be also highly testable as we could lay down an >> example file and test if this file is read correctly. >> >> If now someone wonders why I wrote this down, it may help to learn >> new >> things, as I do when I read the list. I am unsure what advice a >> should >> give you @Robin: it is not your fault that the code looks like it >> looks, >> but on the other hand, making things "wrong" only because others did >> it >> before is also a dead end. So I hope others have an idea, what to do. >> >> Greetings Jonatan >>> + >>> + ### Get installed version information >>> + my ($installed_version, $installed_release); >>> + >>> + if ($installed) { >>> + open(FILE, "<$Conf::dbdir/installed/meta-$pak"); >>> + @file = <FILE>; >>> + close(FILE); >>> + >>> + foreach $line (@file) { >>> + @templine = split(/\: /,$line); >>> + if ("$templine[0]" eq "ProgVersion") { >>> + $installed_version = $templine[1]; >>> + chomp($installed_version); >>> + } elsif ("$templine[0]" eq "Release") { >>> + $installed_release = $templine[1]; >>> + chomp($installed_release); >>> + } >>> + } >>> + } >>> + >>> + print "Name: $name\n"; >>> + if ("$info_type" eq "latest") { >>> + print "Version: $available_version- >>> $available_release\n"; >>> + } else { >>> + print "Version: $installed_version- >>> $installed_release\n"; >>> + } >>> + print "Summary: $summary\nSize: $size\nDependencies: >>> $dependencies\nPakfile: $pakfile\nInitScripts: $initscripts\n"; >>> + if ($installed) { >>> + print "Installed: Yes\n"; >>> + } else { >>> + print "Installed: No\n"; >>> + } >>> + if ("$info_type" eq "latest") { >>> + if (!$found) { >>> + print "Status: obsolete (version >>> $installed_version-$installed_release is installed)\n"; >>> + } elsif ($installed && "$installed_release" < >>> "$available_release") >>> { >>> + print "Status: outdated (version >>> $installed_version-$installed_release is installed)\n"; >>> + } elsif ($installed) { >>> + print "Status: up-to-date\n"; >>> + } else { >>> + print "Status: not installed\n"; >>> + } >>> + } >>> + print "\n"; >>> +} >>> + >>> sub resolvedeps_one { >>> my $pak = shift; >>> >>> diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire >>> index c69a8d3ad..5507c8e92 100644 >>> --- a/src/pakfire/pakfire >>> +++ b/src/pakfire/pakfire >>> @@ -303,6 +303,27 @@ >>> &Pakfire::message("PAKFIRE WARN: Not a >>> known option $ARGV[1]") if >>> ($ARGV[1]); >>> &Pakfire::dblist("all", "noweb"); >>> } >>> + >>> + } elsif ("$ARGV[0]" eq "info") { >>> + shift; >>> + >>> + my @paks; >>> + my $pak; >>> + foreach $pak (@ARGV) { >>> + unless ("$pak" =~ "^-") { >>> + push(@paks,$pak); >>> + } >>> + } >>> + >>> + unless ("@paks") { >>> + &Pakfire::message("PAKFIRE ERROR: missing >>> package name"); >>> + &Pakfire::usage; >>> + exit 1; >>> + } >>> + >>> + foreach $pak (@paks) { >>> + &Pakfire::pakinfo("$pak", "latest"); >>> + } >>> >>> } elsif ("$ARGV[0]" eq "resolvedeps") { >>> foreach (@ARGV) { >>> -- >>> 2.31.1 >> > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] pakfire: add 'info <pak(s)>' option 2021-05-14 12:19 ` Michael Tremer @ 2021-05-14 20:24 ` Robin Roevens 2021-05-25 11:22 ` Michael Tremer 0 siblings, 1 reply; 26+ messages in thread From: Robin Roevens @ 2021-05-14 20:24 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 16742 bytes --] Hi Michael Tremer schreef op vr 14-05-2021 om 13:19 [+0100]: > Hello, > > All the mess is probably my fault. I was young and had no clue what I > was doing. Now I am older… still not having a clue what I am doing. > We all were young once.. and who really does have a clue of what he is actually doing :-).. > I would say your solution looks clean, but as Jonatan suggested, a > unique function to read the package metadata and return it as a hash > would help. If you want to add that, I am happy to merge the patch. > I will certainly look into that. > Generally I am interested in seeing pakfire being tidied up a little > bit, but that has to happen in small steps that are reviewable and > testable. Anything that breaks might stop people from installing > updates and us from shipping a fix. > Indeed pakfire is quite essential to be as bug-free as possible, so we will have to be very carefully with it. I'll see what I can do over time as I'm currently still a rookie in Perl; but I have plenty of experience in different kinds of languages and scripts, that can come in handy. Robin > -Michael > > > On 13 May 2021, at 20:11, Robin Roevens <robin.roevens(a)disroot.org> > > wrote: > > > > Hi Jonatan > > > > I completely agree with your remarks about the code duplication and > > the > > separation of GUI vs logic.. Actually even a bit relieved.. > > > > I only continued this way since I didn't want to 'break' with the > > way > > code was currently written and as I'm both very new here in this > > group > > and I never actually coded in Perl before.. I more or less assumed > > this > > is the way it is done here and it was not (yet) my 'right' to > > question > > it :-).. > > > > But I would be happy to introduce more reusable functions, less to > > no > > duplicate code and a more (m)vc-like approach to the code. > > > > In the meantime I have also been working on a .cgi page for the > > zabbix > > agent configuration.. completely in the style currently all cgi > > pages > > are where duplicate code is everywhere and logic and view are > > mangled > > together.. even with 'goto's all around the place (in most > > languages > > where 'goto' exists, it is some kind of taboo to actually use it.. > > except for plain old BASIC or so.. And it seems to be accepted in > > the > > Perl community too.. but still I frowned upon it :-) > > Anyway, I was about to start thinking about giving up on it since I > > would not want to maintain such code in the long run. > > > > But if it is ok for you guys to revamp the coding style to a more > > "modern" style, I'm willing to give that a try.. > > > > Regards > > Robin > > > > Jonatan Schlag schreef op do 13-05-2021 om 10:02 [+0200]: > > > Hi, > > > > > > had a look at your code and found some more generic possibilities > > > to > > > improve it :-) > > > > > > Am 2021-04-23 18:15, schrieb Robin Roevens: > > > > Add an 'info <pak(s)>' option to pakfire to display pakfile > > > > metadata > > > > and current state of the pak on the system. > > > > > > > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> > > > > --- > > > > src/pakfire/lib/functions.pl | 124 > > > > ++++++++++++++++++++++++++++++++++- > > > > src/pakfire/pakfire | 21 ++++++ > > > > 2 files changed, 144 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/pakfire/lib/functions.pl > > > > b/src/pakfire/lib/functions.pl > > > > index 4d5c6219a..46da06a88 100644 > > > > --- a/src/pakfire/lib/functions.pl > > > > +++ b/src/pakfire/lib/functions.pl > > > > @@ -110,7 +110,8 @@ sub usage { > > > > &Pakfire::message("Usage: pakfire <install|remove> [options] > > > > <pak(s)>"); > > > > &Pakfire::message(" <update> - Contacts the > > > > servers > > > > for new lists of paks."); > > > > &Pakfire::message(" <upgrade> - Installs the > > > > latest > > > > version of all paks."); > > > > - &Pakfire::message(" <list> - Outputs a short > > > > list > > > > with all available paks."); > > > > + &Pakfire::message(" <list> > > > > [installed/notinstalled] - > > > > Outputs a list with all, installed or available paks."); > > > > + &Pakfire::message(" <info> <pak(s)> - Output > > > > pak > > > > metadata."); > > > > &Pakfire::message(" <status> - Outputs a > > > > summary > > > > about available core upgrades, updates and a required reboot"); > > > > &Pakfire::message(""); > > > > &Pakfire::message(" Global options:"); > > > > @@ -538,6 +539,127 @@ sub dblist { > > > > } > > > > } > > > > > > > > +sub pakinfo { > > > > + ### This subroutine shows available info for a package > > > > + # Pass package name and type of info as argument: > > > > &Pakfire::pakinfo(package, "latest") > > > > + # Type can be "latest" or "installed" > > > > + # Usage is always with two argument. > > > > + my $pak = shift; > > > > + my $info_type = shift; > > > > + > > > > + my $found = 0; > > > > + my @templine; > > > > + my ($available_version, $available_release); > > > > + > > > > + if ("$info_type" eq "latest") { > > > > + ### Make sure that the info is not outdated. > > > > + &Pakfire::dbgetlist("noforce"); > > > > + > > > > + ### Check if package is in packages_list and > > > > get > > > > latest available > > > > version > > > > + open(FILE, > > > > "<$Conf::dbdir/lists/packages_list.db"); > > > > + my @db = <FILE>; > > > > + close(FILE); > > > > + > > > > + foreach (@db) { > > > > + @templine = split(/;/,$_); > > > > + if ("$templine[0]" eq "$pak" ) { > > > > + $found = 1; > > > > + $available_version = > > > > $templine[1]; > > > > + $available_release = > > > > $templine[2]; > > > > + break; > > > > + } > > > > + } > > > > + } > > > > + > > > We have somehow similar code in dblist(). A very good example of > > > why > > > UI > > > code and logic should be split up. > > > Just want to mention it. It is definitely not your fault, and I > > > do > > > not > > > want to blame you for the errors others made. But maybe it would > > > be worth it to think about a function which either returns > > > something > > > like that: > > > > > > { (package-name, status in {installed, needs-upgrade, > > > notinstalled > > > })} > > > > > > or that: > > > > > > [ > > > installed: {set of packages which are installed} > > > needs upgrade: {self explanatory } > > > not installed: {self explanatory } > > > ] > > > > > > [] is a dictionary so in perl somehthing like %data = (); > > > () is a tupel so in perl an array > > > {} is a set so in perl an array > > > > > > and use this function here and in dblist. > > > > > > > + ### Parse latest package metadata > > > > + my $installed = !&isinstalled("$pak"); > > > > + my @file; > > > > + > > > > + if ($found && "$info_type" eq "latest") { > > > > + getmetafile("$pak"); > > > > + > > > > + open(FILE, "<$Conf::dbdir/meta/meta-$pak"); > > > > + @file = <FILE>; > > > > + close(FILE); > > > > + } elsif ($installed) { > > > > + open(FILE, "<$Conf::dbdir/installed/meta- > > > > $pak"); > > > > + @file = <FILE>; > > > > + close(FILE); > > > You could open the file at 1 > > > > + } else { > > > > + message("PAKFIRE WARN: Pak '$pak' not found."); > > > > + return 1; > > > > + } > > > > > > Here is 1 and deduplicate the code. > > > > + > > > > + my ($name, $summary, $size, $dependencies, $pakfile, > > > > $initscripts); > > > > + foreach $line (@file) { > > > > + @templine = split(/\: /,$line); > > > > + if ("$templine[0]" eq "Name") { > > > > + $name = $templine[1]; > > > > + chomp($name); > > > > + } elsif ("$templine[0]" eq "Summary") { > > > > + $summary = $templine[1]; > > > > + chomp($summary); > > > > + } elsif ("$templine[0]" eq "Size") { > > > > + $size = &beautifysize("$templine[1]"); > > > > + chomp($size); > > > > + } elsif ("$templine[0]" eq "Dependencies") { > > > > + $dependencies = $templine[1]; > > > > + chomp($dependencies); > > > > + } elsif ("$templine[0]" eq "File") { > > > > + $pakfile = $templine[1]; > > > > + chomp($pakfile); > > > > + } elsif ("$templine[0]" eq "InitScripts") { > > > > + $initscripts = $templine[1]; > > > > + chomp($initscripts); > > > > + } > > > > + } > > > > > > And here I somehow do not know what to say :-). The problem is > > > that > > > both > > > blocks are code duplication. But we already have this kind of > > > code > > > duplication at a lot of places (I counted 5). > > > So it is ok to do it that way, but it would be better to have a > > > function > > > which just returns a hash, and you could do something like that: > > > > > > metadata = get_metadate("$pak") > > > > > > metdata["installed_version"] > > > > > > Both functions would be also highly testable as we could lay down > > > an > > > example file and test if this file is read correctly. > > > > > > If now someone wonders why I wrote this down, it may help to > > > learn > > > new > > > things, as I do when I read the list. I am unsure what advice a > > > should > > > give you @Robin: it is not your fault that the code looks like it > > > looks, > > > but on the other hand, making things "wrong" only because others > > > did > > > it > > > before is also a dead end. So I hope others have an idea, what to > > > do. > > > > > > Greetings Jonatan > > > > + > > > > + ### Get installed version information > > > > + my ($installed_version, $installed_release); > > > > + > > > > + if ($installed) { > > > > + open(FILE, "<$Conf::dbdir/installed/meta- > > > > $pak"); > > > > + @file = <FILE>; > > > > + close(FILE); > > > > + > > > > + foreach $line (@file) { > > > > + @templine = split(/\: /,$line); > > > > + if ("$templine[0]" eq "ProgVersion") { > > > > + $installed_version = > > > > $templine[1]; > > > > + chomp($installed_version); > > > > + } elsif ("$templine[0]" eq "Release") { > > > > + $installed_release = > > > > $templine[1]; > > > > + chomp($installed_release); > > > > + } > > > > + } > > > > + } > > > > + > > > > + print "Name: $name\n"; > > > > + if ("$info_type" eq "latest") { > > > > + print "Version: $available_version- > > > > $available_release\n"; > > > > + } else { > > > > + print "Version: $installed_version- > > > > $installed_release\n"; > > > > + } > > > > + print "Summary: $summary\nSize: $size\nDependencies: > > > > $dependencies\nPakfile: $pakfile\nInitScripts: $initscripts\n"; > > > > + if ($installed) { > > > > + print "Installed: Yes\n"; > > > > + } else { > > > > + print "Installed: No\n"; > > > > + } > > > > + if ("$info_type" eq "latest") { > > > > + if (!$found) { > > > > + print "Status: obsolete (version > > > > $installed_version-$installed_release is installed)\n"; > > > > + } elsif ($installed && "$installed_release" < > > > > "$available_release") > > > > { > > > > + print "Status: outdated (version > > > > $installed_version-$installed_release is installed)\n"; > > > > + } elsif ($installed) { > > > > + print "Status: up-to-date\n"; > > > > + } else { > > > > + print "Status: not installed\n"; > > > > + } > > > > + } > > > > + print "\n"; > > > > +} > > > > + > > > > sub resolvedeps_one { > > > > my $pak = shift; > > > > > > > > diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire > > > > index c69a8d3ad..5507c8e92 100644 > > > > --- a/src/pakfire/pakfire > > > > +++ b/src/pakfire/pakfire > > > > @@ -303,6 +303,27 @@ > > > > &Pakfire::message("PAKFIRE WARN: Not a > > > > known option $ARGV[1]") if > > > > ($ARGV[1]); > > > > &Pakfire::dblist("all", "noweb"); > > > > } > > > > + > > > > + } elsif ("$ARGV[0]" eq "info") { > > > > + shift; > > > > + > > > > + my @paks; > > > > + my $pak; > > > > + foreach $pak (@ARGV) { > > > > + unless ("$pak" =~ "^-") { > > > > + push(@paks,$pak); > > > > + } > > > > + } > > > > + > > > > + unless ("@paks") { > > > > + &Pakfire::message("PAKFIRE ERROR: > > > > missing > > > > package name"); > > > > + &Pakfire::usage; > > > > + exit 1; > > > > + } > > > > + > > > > + foreach $pak (@paks) { > > > > + &Pakfire::pakinfo("$pak", "latest"); > > > > + } > > > > > > > > } elsif ("$ARGV[0]" eq "resolvedeps") { > > > > foreach (@ARGV) { > > > > -- > > > > 2.31.1 > > > > > > > > > -- > > Dit bericht is gescanned op virussen en andere gevaarlijke > > inhoud door MailScanner en lijkt schoon te zijn. > > > > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/3] pakfire: add 'info <pak(s)>' option 2021-05-14 20:24 ` Robin Roevens @ 2021-05-25 11:22 ` Michael Tremer 0 siblings, 0 replies; 26+ messages in thread From: Michael Tremer @ 2021-05-25 11:22 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 15026 bytes --] Hello, > On 14 May 2021, at 21:24, Robin Roevens <robin.roevens(a)disroot.org> wrote: > > Hi > > Michael Tremer schreef op vr 14-05-2021 om 13:19 [+0100]: >> Hello, >> >> All the mess is probably my fault. I was young and had no clue what I >> was doing. Now I am older… still not having a clue what I am doing. >> > > We all were young once.. and who really does have a clue of what he is > actually doing :-).. I hope I learned a couple of things on the way and produce a little bit less bullshit. But that is why we have this list, so that we can call each other out if we produce garbage :) >> I would say your solution looks clean, but as Jonatan suggested, a >> unique function to read the package metadata and return it as a hash >> would help. If you want to add that, I am happy to merge the patch. >> > > I will certainly look into that. > >> Generally I am interested in seeing pakfire being tidied up a little >> bit, but that has to happen in small steps that are reviewable and >> testable. Anything that breaks might stop people from installing >> updates and us from shipping a fix. >> > > Indeed pakfire is quite essential to be as bug-free as possible, so we > will have to be very carefully with it. I'll see what I can do over > time as I'm currently still a rookie in Perl; but I have plenty of > experience in different kinds of languages and scripts, that can come > in handy. Perl is always full of surprises. So many things happen implicitly and I constantly run into unexpected behaviour. It probably is documented somewhere, but it takes about 10 years of experience to get Perl right in the first place. It is also a write-only language. I recently wrote those functions. It is very hard to understand what is actually happening: https://git.ipfire.org/?p=people/ms/ipfire-2.x.git;a=commitdiff;h=dc5a53feed5059c43bb0d40b9ca6d10e6c128990 -Michael > > Robin > >> -Michael >> >>> On 13 May 2021, at 20:11, Robin Roevens <robin.roevens(a)disroot.org> >>> wrote: >>> >>> Hi Jonatan >>> >>> I completely agree with your remarks about the code duplication and >>> the >>> separation of GUI vs logic.. Actually even a bit relieved.. >>> >>> I only continued this way since I didn't want to 'break' with the >>> way >>> code was currently written and as I'm both very new here in this >>> group >>> and I never actually coded in Perl before.. I more or less assumed >>> this >>> is the way it is done here and it was not (yet) my 'right' to >>> question >>> it :-).. >>> >>> But I would be happy to introduce more reusable functions, less to >>> no >>> duplicate code and a more (m)vc-like approach to the code. >>> >>> In the meantime I have also been working on a .cgi page for the >>> zabbix >>> agent configuration.. completely in the style currently all cgi >>> pages >>> are where duplicate code is everywhere and logic and view are >>> mangled >>> together.. even with 'goto's all around the place (in most >>> languages >>> where 'goto' exists, it is some kind of taboo to actually use it.. >>> except for plain old BASIC or so.. And it seems to be accepted in >>> the >>> Perl community too.. but still I frowned upon it :-) >>> Anyway, I was about to start thinking about giving up on it since I >>> would not want to maintain such code in the long run. >>> >>> But if it is ok for you guys to revamp the coding style to a more >>> "modern" style, I'm willing to give that a try.. >>> >>> Regards >>> Robin >>> >>> Jonatan Schlag schreef op do 13-05-2021 om 10:02 [+0200]: >>>> Hi, >>>> >>>> had a look at your code and found some more generic possibilities >>>> to >>>> improve it :-) >>>> >>>> Am 2021-04-23 18:15, schrieb Robin Roevens: >>>>> Add an 'info <pak(s)>' option to pakfire to display pakfile >>>>> metadata >>>>> and current state of the pak on the system. >>>>> >>>>> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> >>>>> --- >>>>> src/pakfire/lib/functions.pl | 124 >>>>> ++++++++++++++++++++++++++++++++++- >>>>> src/pakfire/pakfire | 21 ++++++ >>>>> 2 files changed, 144 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/src/pakfire/lib/functions.pl >>>>> b/src/pakfire/lib/functions.pl >>>>> index 4d5c6219a..46da06a88 100644 >>>>> --- a/src/pakfire/lib/functions.pl >>>>> +++ b/src/pakfire/lib/functions.pl >>>>> @@ -110,7 +110,8 @@ sub usage { >>>>> &Pakfire::message("Usage: pakfire <install|remove> [options] >>>>> <pak(s)>"); >>>>> &Pakfire::message(" <update> - Contacts the >>>>> servers >>>>> for new lists of paks."); >>>>> &Pakfire::message(" <upgrade> - Installs the >>>>> latest >>>>> version of all paks."); >>>>> - &Pakfire::message(" <list> - Outputs a short >>>>> list >>>>> with all available paks."); >>>>> + &Pakfire::message(" <list> >>>>> [installed/notinstalled] - >>>>> Outputs a list with all, installed or available paks."); >>>>> + &Pakfire::message(" <info> <pak(s)> - Output >>>>> pak >>>>> metadata."); >>>>> &Pakfire::message(" <status> - Outputs a >>>>> summary >>>>> about available core upgrades, updates and a required reboot"); >>>>> &Pakfire::message(""); >>>>> &Pakfire::message(" Global options:"); >>>>> @@ -538,6 +539,127 @@ sub dblist { >>>>> } >>>>> } >>>>> >>>>> +sub pakinfo { >>>>> + ### This subroutine shows available info for a package >>>>> + # Pass package name and type of info as argument: >>>>> &Pakfire::pakinfo(package, "latest") >>>>> + # Type can be "latest" or "installed" >>>>> + # Usage is always with two argument. >>>>> + my $pak = shift; >>>>> + my $info_type = shift; >>>>> + >>>>> + my $found = 0; >>>>> + my @templine; >>>>> + my ($available_version, $available_release); >>>>> + >>>>> + if ("$info_type" eq "latest") { >>>>> + ### Make sure that the info is not outdated. >>>>> + &Pakfire::dbgetlist("noforce"); >>>>> + >>>>> + ### Check if package is in packages_list and >>>>> get >>>>> latest available >>>>> version >>>>> + open(FILE, >>>>> "<$Conf::dbdir/lists/packages_list.db"); >>>>> + my @db = <FILE>; >>>>> + close(FILE); >>>>> + >>>>> + foreach (@db) { >>>>> + @templine = split(/;/,$_); >>>>> + if ("$templine[0]" eq "$pak" ) { >>>>> + $found = 1; >>>>> + $available_version = >>>>> $templine[1]; >>>>> + $available_release = >>>>> $templine[2]; >>>>> + break; >>>>> + } >>>>> + } >>>>> + } >>>>> + >>>> We have somehow similar code in dblist(). A very good example of >>>> why >>>> UI >>>> code and logic should be split up. >>>> Just want to mention it. It is definitely not your fault, and I >>>> do >>>> not >>>> want to blame you for the errors others made. But maybe it would >>>> be worth it to think about a function which either returns >>>> something >>>> like that: >>>> >>>> { (package-name, status in {installed, needs-upgrade, >>>> notinstalled >>>> })} >>>> >>>> or that: >>>> >>>> [ >>>> installed: {set of packages which are installed} >>>> needs upgrade: {self explanatory } >>>> not installed: {self explanatory } >>>> ] >>>> >>>> [] is a dictionary so in perl somehthing like %data = (); >>>> () is a tupel so in perl an array >>>> {} is a set so in perl an array >>>> >>>> and use this function here and in dblist. >>>> >>>>> + ### Parse latest package metadata >>>>> + my $installed = !&isinstalled("$pak"); >>>>> + my @file; >>>>> + >>>>> + if ($found && "$info_type" eq "latest") { >>>>> + getmetafile("$pak"); >>>>> + >>>>> + open(FILE, "<$Conf::dbdir/meta/meta-$pak"); >>>>> + @file = <FILE>; >>>>> + close(FILE); >>>>> + } elsif ($installed) { >>>>> + open(FILE, "<$Conf::dbdir/installed/meta- >>>>> $pak"); >>>>> + @file = <FILE>; >>>>> + close(FILE); >>>> You could open the file at 1 >>>>> + } else { >>>>> + message("PAKFIRE WARN: Pak '$pak' not found."); >>>>> + return 1; >>>>> + } >>>> >>>> Here is 1 and deduplicate the code. >>>>> + >>>>> + my ($name, $summary, $size, $dependencies, $pakfile, >>>>> $initscripts); >>>>> + foreach $line (@file) { >>>>> + @templine = split(/\: /,$line); >>>>> + if ("$templine[0]" eq "Name") { >>>>> + $name = $templine[1]; >>>>> + chomp($name); >>>>> + } elsif ("$templine[0]" eq "Summary") { >>>>> + $summary = $templine[1]; >>>>> + chomp($summary); >>>>> + } elsif ("$templine[0]" eq "Size") { >>>>> + $size = &beautifysize("$templine[1]"); >>>>> + chomp($size); >>>>> + } elsif ("$templine[0]" eq "Dependencies") { >>>>> + $dependencies = $templine[1]; >>>>> + chomp($dependencies); >>>>> + } elsif ("$templine[0]" eq "File") { >>>>> + $pakfile = $templine[1]; >>>>> + chomp($pakfile); >>>>> + } elsif ("$templine[0]" eq "InitScripts") { >>>>> + $initscripts = $templine[1]; >>>>> + chomp($initscripts); >>>>> + } >>>>> + } >>>> >>>> And here I somehow do not know what to say :-). The problem is >>>> that >>>> both >>>> blocks are code duplication. But we already have this kind of >>>> code >>>> duplication at a lot of places (I counted 5). >>>> So it is ok to do it that way, but it would be better to have a >>>> function >>>> which just returns a hash, and you could do something like that: >>>> >>>> metadata = get_metadate("$pak") >>>> >>>> metdata["installed_version"] >>>> >>>> Both functions would be also highly testable as we could lay down >>>> an >>>> example file and test if this file is read correctly. >>>> >>>> If now someone wonders why I wrote this down, it may help to >>>> learn >>>> new >>>> things, as I do when I read the list. I am unsure what advice a >>>> should >>>> give you @Robin: it is not your fault that the code looks like it >>>> looks, >>>> but on the other hand, making things "wrong" only because others >>>> did >>>> it >>>> before is also a dead end. So I hope others have an idea, what to >>>> do. >>>> >>>> Greetings Jonatan >>>>> + >>>>> + ### Get installed version information >>>>> + my ($installed_version, $installed_release); >>>>> + >>>>> + if ($installed) { >>>>> + open(FILE, "<$Conf::dbdir/installed/meta- >>>>> $pak"); >>>>> + @file = <FILE>; >>>>> + close(FILE); >>>>> + >>>>> + foreach $line (@file) { >>>>> + @templine = split(/\: /,$line); >>>>> + if ("$templine[0]" eq "ProgVersion") { >>>>> + $installed_version = >>>>> $templine[1]; >>>>> + chomp($installed_version); >>>>> + } elsif ("$templine[0]" eq "Release") { >>>>> + $installed_release = >>>>> $templine[1]; >>>>> + chomp($installed_release); >>>>> + } >>>>> + } >>>>> + } >>>>> + >>>>> + print "Name: $name\n"; >>>>> + if ("$info_type" eq "latest") { >>>>> + print "Version: $available_version- >>>>> $available_release\n"; >>>>> + } else { >>>>> + print "Version: $installed_version- >>>>> $installed_release\n"; >>>>> + } >>>>> + print "Summary: $summary\nSize: $size\nDependencies: >>>>> $dependencies\nPakfile: $pakfile\nInitScripts: $initscripts\n"; >>>>> + if ($installed) { >>>>> + print "Installed: Yes\n"; >>>>> + } else { >>>>> + print "Installed: No\n"; >>>>> + } >>>>> + if ("$info_type" eq "latest") { >>>>> + if (!$found) { >>>>> + print "Status: obsolete (version >>>>> $installed_version-$installed_release is installed)\n"; >>>>> + } elsif ($installed && "$installed_release" < >>>>> "$available_release") >>>>> { >>>>> + print "Status: outdated (version >>>>> $installed_version-$installed_release is installed)\n"; >>>>> + } elsif ($installed) { >>>>> + print "Status: up-to-date\n"; >>>>> + } else { >>>>> + print "Status: not installed\n"; >>>>> + } >>>>> + } >>>>> + print "\n"; >>>>> +} >>>>> + >>>>> sub resolvedeps_one { >>>>> my $pak = shift; >>>>> >>>>> diff --git a/src/pakfire/pakfire b/src/pakfire/pakfire >>>>> index c69a8d3ad..5507c8e92 100644 >>>>> --- a/src/pakfire/pakfire >>>>> +++ b/src/pakfire/pakfire >>>>> @@ -303,6 +303,27 @@ >>>>> &Pakfire::message("PAKFIRE WARN: Not a >>>>> known option $ARGV[1]") if >>>>> ($ARGV[1]); >>>>> &Pakfire::dblist("all", "noweb"); >>>>> } >>>>> + >>>>> + } elsif ("$ARGV[0]" eq "info") { >>>>> + shift; >>>>> + >>>>> + my @paks; >>>>> + my $pak; >>>>> + foreach $pak (@ARGV) { >>>>> + unless ("$pak" =~ "^-") { >>>>> + push(@paks,$pak); >>>>> + } >>>>> + } >>>>> + >>>>> + unless ("@paks") { >>>>> + &Pakfire::message("PAKFIRE ERROR: >>>>> missing >>>>> package name"); >>>>> + &Pakfire::usage; >>>>> + exit 1; >>>>> + } >>>>> + >>>>> + foreach $pak (@paks) { >>>>> + &Pakfire::pakinfo("$pak", "latest"); >>>>> + } >>>>> >>>>> } elsif ("$ARGV[0]" eq "resolvedeps") { >>>>> foreach (@ARGV) { >>>>> -- >>>>> 2.31.1 >>>> >>> >>> >>> -- >>> Dit bericht is gescanned op virussen en andere gevaarlijke >>> inhoud door MailScanner en lijkt schoon te zijn. >>> >> >> > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 3/3] services.cgi: use new Pakfire::pakinfo function 2021-04-23 16:15 [PATCH 0/3] Pakfile metadata enhancements Robin Roevens 2021-04-23 16:15 ` [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Robin Roevens 2021-04-23 16:15 ` [PATCH 2/3] pakfire: add 'info <pak(s)>' option Robin Roevens @ 2021-04-23 16:15 ` Robin Roevens 2021-05-11 19:30 ` group call for review/opinions/idea's (was: [PATCH 0/3] Pakfile metadata enhancements) Robin Roevens 2021-05-12 18:45 ` [PATCH 0/3] Pakfile metadata enhancements Jonatan Schlag 4 siblings, 0 replies; 26+ messages in thread From: Robin Roevens @ 2021-04-23 16:15 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 4532 bytes --] Use new Pakfire::pakinfo function to determine installed addons and related services/initscripts more reliable. Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org> --- html/cgi-bin/services.cgi | 96 ++++++++++++++++++++++++--------------- 1 file changed, 59 insertions(+), 37 deletions(-) diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi index 38b89ef1e..66b0d711f 100644 --- a/html/cgi-bin/services.cgi +++ b/html/cgi-bin/services.cgi @@ -29,6 +29,7 @@ require '/var/ipfire/general-functions.pl'; require "${General::swroot}/lang.pl"; require "${General::swroot}/header.pl"; require "${General::swroot}/graphs.pl"; +require "/opt/pakfire/lib/functions.pl"; my %color = (); my %mainsettings = (); @@ -160,48 +161,69 @@ END my $lines=0; # Used to count the outputlines to make different bgcolor - # Generate list of installed addon pak's - opendir (DIR, "/opt/pakfire/db/installed") || die "Cannot opendir /opt/pakfire/db/installed/: $!"; - my @pak = sort readdir DIR; - foreach (@pak){ - chomp($_); - next unless (m/^meta-/); - s/^meta-//; - - # Check which of the paks are services - if (-e "/etc/init.d/$_") { - # blacklist some packages - # - # alsa has trouble with the volume saving and was not really stopped - # mdadm should not stopped with webif because this could crash the system - # - if ( $_ eq 'squid' ) { - next; - } - if ( ($_ ne "alsa") && ($_ ne "mdadm") ) { - $lines++; - if ($lines % 2){ - print "<tr>"; - $col="bgcolor='$color{'color22'}'"; - }else{ - print "<tr>"; - $col="bgcolor='$color{'color20'}'"; + my ($paklist, $pakinfo); + my (@templine, @templine2); + my ($listline, $infoline); + my @paks; + my @addon_services; + + # Generate list of installed addon pak services + $Pakfire::enable_colors = 0; + open OUT, '>', \$paklist; + select OUT; + eval { \&Pakfire::dblist("installed", "noweb"); }; + select STDOUT; + + foreach $listline (split(/^/, "$paklist")) { + chomp($listline); + @templine = split(/\: /, $listline); + + if ("@templine[0]" eq "Name") { + open OUT, '>', \$pakinfo; + select OUT; + eval { \&Pakfire::pakinfo("@templine[1]", "installed"); }; + select STDOUT; + + foreach $infoline (split(/^/, $pakinfo)) { + chomp($infoline); + @templine2 = split(/\: /, $infoline); + + if ("@templine2[0]" eq "InitScripts" && "@templine2[1]") { + push(@addon_services, split(" ", "@templine2[1]")); } - print "<td align='left' $col width='31%'>$_</td> "; - my $status = isautorun($_,$col); - print "$status "; - print "<td align='center' $col width='8%'><a href='services.cgi?$_!start'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>"; - print "<td align='center' $col width='8%'><a href='services.cgi?$_!stop'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> "; - my $status = &isrunningaddon($_,$col); - $status =~ s/\^[\[[0-1]\;[0-9]+m//g; - - chomp($status); - print "$status"; - print "</tr>"; } } } + foreach (@addon_services) { + $lines++; + if ($lines % 2){ + print "<tr>"; + $col="bgcolor='$color{'color22'}'"; + }else{ + print "<tr>"; + $col="bgcolor='$color{'color20'}'"; + } + print "<td align='left' $col width='31%'>$_</td> "; + my $status = isautorun($_,$col); + print "$status "; + # Don't allow user to start/stop folowing services from webui: + # - alsa has trouble with the volume saving and was not really stopped + if ($_ eq "alsa") { + print "<td align='center' $col width='8%'></td>"; + print "<td align='center' $col width='8%'></td> "; + } else { + print "<td align='center' $col width='8%'><a href='services.cgi?$_!start'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>"; + print "<td align='center' $col width='8%'><a href='services.cgi?$_!stop'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> "; + } + my $status = &isrunningaddon($_,$col); + $status =~ s/\^[\[[0-1]\;[0-9]+m//g; + + chomp($status); + print "$status"; + print "</tr>"; + } + print "</table></div>\n"; &Header::closebox(); -- 2.31.1 -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* group call for review/opinions/idea's (was: [PATCH 0/3] Pakfile metadata enhancements) 2021-04-23 16:15 [PATCH 0/3] Pakfile metadata enhancements Robin Roevens ` (2 preceding siblings ...) 2021-04-23 16:15 ` [PATCH 3/3] services.cgi: use new Pakfire::pakinfo function Robin Roevens @ 2021-05-11 19:30 ` Robin Roevens 2021-05-12 18:45 ` [PATCH 0/3] Pakfile metadata enhancements Jonatan Schlag 4 siblings, 0 replies; 26+ messages in thread From: Robin Roevens @ 2021-05-11 19:30 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 5306 bytes --] A gentle reminder to the group, please check out this proposal for adding and consulting extra metadata to paks, and comment on it, maybe suggest other metadata besides initscripts and summary that I implemented in this proposal patch-set, that could be handy for other addons or cgi pages or just plain info to the user... Thanks.. Robin Robin Roevens schreef op vr 23-04-2021 om 18:15 [+0200]: > Hi folks > > During my discussion with Michael in the zabbix_agentd patchset > thread > about knowing what addon services should be running or not, it came > up > that it would be handy for several reasons if we had a bit more > metadata > for pak-files as we currently have. Mostly knowing which services > (initscripts) are installed by a pak-file. > This would allow for services.cgi to not have to manually try to find > out if an installed addon actually has an initscript or not. > Also paks like libvirt install 2 initscripts, those can now both be > displayed on the services.cgi page. > Idem for monitoring agents, which was my main objective. > > So here is an attempt to achieve this. This is not yet a patchset to > be applied yet, but rather a proposal as this change would require > all > addon LFS-files to be changed. But I didn't want to do that yet as > this patchset may very wel be rejected completely :-). > > The first patch introduces 2 new macro's: > - SUMMARY for a short, one-line summary of the package > - INITSCRIPTS for a space seperated list of initscripts provided by > the > package. > And an alternative INSTALL_INITSCRIPTS method instead of the current > INSTALL_INITSCRIPT method. As we now have all initscripts in the > INITSCRIPTS macro, the INSTALL_INITSCRIPTS will install all > initscripts > listed in that macro, so a simple call to INSTALL_INITSCRIPTS will > now > do the job instead of multiple calls in case of multiple initscripts > (for example libvirt. I noticed clamav actually uses 1 initscript for > starting 2 services, this could maybe also be split up again) > I included 2 examples in the first patch: libvirt and zabbix_agentd. > But > when implemented ofcourse all makefiles should be updated. > > During the pak packaging process in the build procedure, those new > macro's will be inheritted in the generated pakfire meta-* files. > > The second patch adds an extra 'info <pak(s)>' commandline parameter > to > pakfire, which will in turn call a new Pakfire::pakinfo function. > This function wil parse the meta-* file of the requested pak and > functions in 2 modes: > - "latest" which is the behaviour of the info parameter. This will > display the latest available metadata of the pak and the status of > the pak on the system as in: is it installed?, and if so, is it > up-to-date. > - "installed" wich will display only information about the currently > installed pak and bail out of the requested pak is not currently > installed. > This function was added to provide a 'central' point/method to get > pak > information. I don't know if there are other scripts beside > services.cgi > that currently try parsing meta-* files. But they should then be > changed > to use this function instead. > > Example output of the new pakfire info command: `pakfire info > zabbix_agentd`: > when installed and up-to-date: > --- > Name: zabbix_agentd > Version: 4.2.6-4 > Summary: Zabbix Agent > Size: 250.00 KB > Dependencies: > Pakfile: zabbix_agentd-4.2.6-4.ipfire > InitScripts: zabbix_agentd > Installed: Yes > Status: up-to-date > --- > When an update is available: > --- > Name: zabbix_agentd > Version: 5.0.10-5 > Summary: Zabbix Agent > Size: 276.00 KB > Dependencies: fping > Pakfile: zabbix_agentd-5.0.10-5.ipfire > InitScripts: zabbix_agentd > Installed: Yes > Status: outdated (version 4.2.6-4 is installed) > --- > Or when a pak was discontinued and no longer supplied by ipfire, but > still installed on the system: > --- > Name: zabbix_agentd > Version: - > Summary: Zabbix Agent > Size: 250.00 KB > Dependencies: > Pakfile: zabbix_agentd-4.2.6-4.ipfire > InitScripts: zabbix_agentd > Installed: Yes > Status: obsolete (version 4.2.6-4 is installed) > --- > and at last when a pak is available, but not installed: > --- > Name: zabbix_agentd > Version: 5.0.10-5 > Summary: Zabbix Agent > Size: 276.00 KB > Dependencies: fping > Pakfile: zabbix_agentd-5.0.10-5.ipfire > InitScripts: zabbix_agentd > Installed: No > Status: not installed > --- > > And then the last patch is an update of service.cgi now using the new > Pakfire::pakinfo function in "installed"-mode. > > If there are any suggestions on more metadata.. I think this is the > moment to throw them at me. > And ofcourse suggestions/comments are welcome as this is currently > only > a proposal for change. But I think we win in robustness of > services.cgi > and user experience in both using pakfire and ability to provide > available services to monitoring agents. > On top of that could the whole meta-* files system be overhauled in > the > future, if wanted, with only pakfire itself needing change as the > rest > will then depend on pakfire for correctly parsing it's "database". > > > Regards > Robin > > > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/3] Pakfile metadata enhancements 2021-04-23 16:15 [PATCH 0/3] Pakfile metadata enhancements Robin Roevens ` (3 preceding siblings ...) 2021-05-11 19:30 ` group call for review/opinions/idea's (was: [PATCH 0/3] Pakfile metadata enhancements) Robin Roevens @ 2021-05-12 18:45 ` Jonatan Schlag 2021-05-12 22:31 ` Robin Roevens 4 siblings, 1 reply; 26+ messages in thread From: Jonatan Schlag @ 2021-05-12 18:45 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 6067 bytes --] Hi, > Am 23.04.2021 um 18:16 schrieb Robin Roevens <robin.roevens(a)disroot.org>: > > Hi folks > > During my discussion with Michael in the zabbix_agentd patchset thread > about knowing what addon services should be running or not, it came up > that it would be handy for several reasons if we had a bit more metadata > for pak-files as we currently have. Mostly knowing which services > (initscripts) are installed by a pak-file. > This would allow for services.cgi to not have to manually try to find > out if an installed addon actually has an initscript or not. > Also paks like libvirt install 2 initscripts, those can now both be > displayed on the services.cgi page. > Idem for monitoring agents, which was my main objective. > > So here is an attempt to achieve this. This is not yet a patchset to > be applied yet, but rather a proposal as this change would require all > addon LFS-files to be changed. But I didn't want to do that yet as > this patchset may very wel be rejected completely :-). > The first patch introduces 2 new macro's: So it could be done in two separate patches or even better patch sets. Makes reviewing easier and patches shorter :-) > - SUMMARY for a short, one-line summary of the package > - INITSCRIPTS for a space seperated list of initscripts provided by the > package. How it is supposed to be handled when a package (like libvirt) install its own init scripts? So not a initscript we have in our source, but which is delivered in the source of the package itself. If we put this in the list, the macro will break. If we leave it out we lose information. Do you did some research how other distribution handle this problem? ( If they handle it at all.) Another approach which comes to my mind: Why not parsing the root file for /etc/rc.d/init.d/ (I currently do not know if it is the right path)? So trying to detect which initscripts are part of the root file? > And an alternative INSTALL_INITSCRIPTS method instead of the current > INSTALL_INITSCRIPT method. As we now have all initscripts in the > INITSCRIPTS macro, the INSTALL_INITSCRIPTS will install all initscripts > listed in that macro, so a simple call to INSTALL_INITSCRIPTS will now > do the job instead of multiple calls in case of multiple initscripts > (for example libvirt. I noticed clamav actually uses 1 initscript for > starting 2 services, this could maybe also be split up again) > I included 2 examples in the first patch: libvirt and zabbix_agentd. But > when implemented ofcourse all makefiles should be updated. > > During the pak packaging process in the build procedure, those new > macro's will be inheritted in the generated pakfire meta-* files. > > The second patch adds an extra 'info <pak(s)>' commandline parameter to > pakfire, which will in turn call a new Pakfire::pakinfo function. > This function wil parse the meta-* file of the requested pak and > functions in 2 modes: > - "latest" which is the behaviour of the info parameter. This will > display the latest available metadata of the pak and the status of > the pak on the system as in: is it installed?, and if so, is it > up-to-date. > - "installed" wich will display only information about the currently > installed pak and bail out of the requested pak is not currently > installed. > This function was added to provide a 'central' point/method to get pak > information. I don't know if there are other scripts beside services.cgi > that currently try parsing meta-* files. But they should then be changed > to use this function instead. > > Example output of the new pakfire info command: `pakfire info zabbix_agentd`: > when installed and up-to-date: > --- > Name: zabbix_agentd > Version: 4.2.6-4 > Summary: Zabbix Agent > Size: 250.00 KB > Dependencies: > Pakfile: zabbix_agentd-4.2.6-4.ipfire > InitScripts: zabbix_agentd > Installed: Yes > Status: up-to-date > --- > When an update is available: > --- > Name: zabbix_agentd > Version: 5.0.10-5 > Summary: Zabbix Agent > Size: 276.00 KB > Dependencies: fping > Pakfile: zabbix_agentd-5.0.10-5.ipfire > InitScripts: zabbix_agentd > Installed: Yes > Status: outdated (version 4.2.6-4 is installed) > --- > Or when a pak was discontinued and no longer supplied by ipfire, but > still installed on the system: > --- > Name: zabbix_agentd > Version: - > Summary: Zabbix Agent > Size: 250.00 KB > Dependencies: > Pakfile: zabbix_agentd-4.2.6-4.ipfire > InitScripts: zabbix_agentd > Installed: Yes > Status: obsolete (version 4.2.6-4 is installed) > --- > and at last when a pak is available, but not installed: > --- > Name: zabbix_agentd > Version: 5.0.10-5 > Summary: Zabbix Agent > Size: 276.00 KB > Dependencies: fping > Pakfile: zabbix_agentd-5.0.10-5.ipfire > InitScripts: zabbix_agentd > Installed: No > Status: not installed > --- > > And then the last patch is an update of service.cgi now using the new > Pakfire::pakinfo function in "installed"-mode. > > If there are any suggestions on more metadata.. I think this is the > moment to throw them at me. > And ofcourse suggestions/comments are welcome as this is currently only > a proposal for change. But I think we win in robustness of services.cgi > and user experience in both using pakfire and ability to provide > available services to monitoring agents. Just want to say thank you for taking this way, which might be “harder” but yields better result from my experience. Please do not take my points as “this is not right”, more like “there might me other ways, are they better?”. Greetings Jonatan > On top of that could the whole meta-* files system be overhauled in the > future, if wanted, with only pakfire itself needing change as the rest > will then depend on pakfire for correctly parsing it's "database". > > > Regards > Robin > > > > -- > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/3] Pakfile metadata enhancements 2021-05-12 18:45 ` [PATCH 0/3] Pakfile metadata enhancements Jonatan Schlag @ 2021-05-12 22:31 ` Robin Roevens 2021-05-15 12:10 ` Adolf Belka 0 siblings, 1 reply; 26+ messages in thread From: Robin Roevens @ 2021-05-12 22:31 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 8989 bytes --] Hi Jonatan Jonatan Schlag schreef op wo 12-05-2021 om 20:45 [+0200]: > Hi, > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens < > > robin.roevens(a)disroot.org>: > > > > Hi folks > > > > During my discussion with Michael in the zabbix_agentd patchset > > thread > > about knowing what addon services should be running or not, it came > > up > > that it would be handy for several reasons if we had a bit more > > metadata > > for pak-files as we currently have. Mostly knowing which services > > (initscripts) are installed by a pak-file. > > This would allow for services.cgi to not have to manually try to find > > out if an installed addon actually has an initscript or not. > > Also paks like libvirt install 2 initscripts, those can now both be > > displayed on the services.cgi page. > > Idem for monitoring agents, which was my main objective. > > > > So here is an attempt to achieve this. This is not yet a patchset to > > be applied yet, but rather a proposal as this change would require > > all > > addon LFS-files to be changed. But I didn't want to do that yet as > > this patchset may very wel be rejected completely :-). > > The first patch introduces 2 new macro's: > So it could be done in two separate patches or even better patch sets. > Makes reviewing easier and patches shorter :-) > > - SUMMARY for a short, one-line summary of the package > > - INITSCRIPTS for a space seperated list of initscripts provided by > > the > > package. > > How it is supposed to be handled when a package (like libvirt) install > its own init scripts? So not a initscript we have in our source, but > which is delivered in the source of the package itself. If we put this > in the list, the macro will break. If we leave it out we lose > information. > You have a point there. In the case of libvirt it, as you point out in your other mail, is indeed an initscript that does not start a service so does not pose a problem using this method. But it is not unthinkable that another pak (or a possible future pak) includes an initscript in the source itself. So this is may indeed be something to take into account.. > Do you did some research how other distribution handle this problem? > ( If they handle it at all.) "normal" distributions generally don't really care which packages run which services and if/or those are actually services. However most modern distro's use systemd making it easier to know which units are services, and which are so called one-shots as you can just ask systemd. It has a lot more info about all "initscripts" which would make this task easier. But we don't have systemd here :-) Also Zabbix monitoring agent for example has no built-in methods to automatically discover available and/or running services when the monitored distro is not running systemd. While it already does this for decades for Windows hosts. And more recently also for systemd. In Suse, each package containing a service also created a link /usr/sbin/rc<servicename> which links to the 'service' command which currently is a wrapper script to systemd. Not sure how they did it before systemd, possibly those rc-binaries where just symlinks to the actual initscripts. That is maybe something we may also need to consider, but then rather for better user cli experience as I don't immediately see that solving the problem you posed. > > Another approach which comes to my mind: > Why not parsing the root file for /etc/rc.d/init.d/ (I currently do not > know if it is the right path)? So trying to detect which initscripts > are part of the root file? > As you concluded in your other mail, not all initscripts are services/daemons so that would probably require some hardcoded exceptions and alike. Which is exactly what we are trying to avoid and is currently done in services.cgi. :-) To tackle the problem with service-initscripts included in and installed by the source in my approach; I currently see 2 possible solutions: - also provide the old INSTALL_INITSCRIPT macro to manually install one or more initscripts, so you are able to skip initscripts listed in INITSCRIPTS but which are installed by the source. - make it a common practice to prevent the source from installing an initscript, extracting it and installing it the IPFire way. (I think initscript investigation is probably required anyway to make sure it is compatible on IPFire..) > > And an alternative INSTALL_INITSCRIPTS method instead of the > > current > > INSTALL_INITSCRIPT method. As we now have all initscripts in the > > INITSCRIPTS macro, the INSTALL_INITSCRIPTS will install all > > initscripts > > listed in that macro, so a simple call to INSTALL_INITSCRIPTS will > > now > > do the job instead of multiple calls in case of multiple > > initscripts > > (for example libvirt. I noticed clamav actually uses 1 initscript > > for > > starting 2 services, this could maybe also be split up again) > > I included 2 examples in the first patch: libvirt and > > zabbix_agentd. But > > when implemented ofcourse all makefiles should be updated. > > > > During the pak packaging process in the build procedure, those new > > macro's will be inheritted in the generated pakfire meta-* files. > > > > The second patch adds an extra 'info <pak(s)>' commandline > > parameter to > > pakfire, which will in turn call a new Pakfire::pakinfo function. > > This function wil parse the meta-* file of the requested pak and > > functions in 2 modes: > > - "latest" which is the behaviour of the info parameter. This will > > display the latest available metadata of the pak and the status of > > the pak on the system as in: is it installed?, and if so, is it > > up-to-date. > > - "installed" wich will display only information about the > > currently > > installed pak and bail out of the requested pak is not currently > > installed. > > This function was added to provide a 'central' point/method to get > > pak > > information. I don't know if there are other scripts beside > > services.cgi > > that currently try parsing meta-* files. But they should then be > > changed > > to use this function instead. > > > > Example output of the new pakfire info command: `pakfire info > > zabbix_agentd`: > > when installed and up-to-date: > > --- > > Name: zabbix_agentd > > Version: 4.2.6-4 > > Summary: Zabbix Agent > > Size: 250.00 KB > > Dependencies: > > Pakfile: zabbix_agentd-4.2.6-4.ipfire > > InitScripts: zabbix_agentd > > Installed: Yes > > Status: up-to-date > > --- > > When an update is available: > > --- > > Name: zabbix_agentd > > Version: 5.0.10-5 > > Summary: Zabbix Agent > > Size: 276.00 KB > > Dependencies: fping > > Pakfile: zabbix_agentd-5.0.10-5.ipfire > > InitScripts: zabbix_agentd > > Installed: Yes > > Status: outdated (version 4.2.6-4 is installed) > > --- > > Or when a pak was discontinued and no longer supplied by ipfire, > > but > > still installed on the system: > > --- > > Name: zabbix_agentd > > Version: - > > Summary: Zabbix Agent > > Size: 250.00 KB > > Dependencies: > > Pakfile: zabbix_agentd-4.2.6-4.ipfire > > InitScripts: zabbix_agentd > > Installed: Yes > > Status: obsolete (version 4.2.6-4 is installed) > > --- > > and at last when a pak is available, but not installed: > > --- > > Name: zabbix_agentd > > Version: 5.0.10-5 > > Summary: Zabbix Agent > > Size: 276.00 KB > > Dependencies: fping > > Pakfile: zabbix_agentd-5.0.10-5.ipfire > > InitScripts: zabbix_agentd > > Installed: No > > Status: not installed > > --- > > > > And then the last patch is an update of service.cgi now using the > > new > > Pakfire::pakinfo function in "installed"-mode. > > > > If there are any suggestions on more metadata.. I think this is the > > moment to throw them at me. > > And ofcourse suggestions/comments are welcome as this is currently > > only > > a proposal for change. But I think we win in robustness of > > services.cgi > > and user experience in both using pakfire and ability to provide > > available services to monitoring agents. > Just want to say thank you for taking this way, which might be > “harder” but yields better result from my experience. Please do not > take my points as “this is not right”, more like “there might me > other ways, are they better?”. > That is exactly why I threw this in the group; to check if there are better idea's/approaches/things I overlooked/.. So thank you for your constructive comments! Regards Robin > Greetings Jonatan > > On top of that could the whole meta-* files system be overhauled in > > the > > future, if wanted, with only pakfire itself needing change as the > > rest > > will then depend on pakfire for correctly parsing it's "database". > > > > > > Regards > > Robin > > > > > > > > -- > > Dit bericht is gescanned op virussen en andere gevaarlijke > > inhoud door MailScanner en lijkt schoon te zijn. > > > -- Dit bericht is gescanned op virussen en andere gevaarlijke inhoud door MailScanner en lijkt schoon te zijn. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/3] Pakfile metadata enhancements 2021-05-12 22:31 ` Robin Roevens @ 2021-05-15 12:10 ` Adolf Belka 0 siblings, 0 replies; 26+ messages in thread From: Adolf Belka @ 2021-05-15 12:10 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 9973 bytes --] Hi Robin, On 13/05/2021 00:31, Robin Roevens wrote: > Hi Jonatan > > Jonatan Schlag schreef op wo 12-05-2021 om 20:45 [+0200]: >> Hi, >> >>> Am 23.04.2021 um 18:16 schrieb Robin Roevens < >>> robin.roevens(a)disroot.org>: >>> >>> Hi folks >>> >>> During my discussion with Michael in the zabbix_agentd patchset >>> thread >>> about knowing what addon services should be running or not, it came >>> up >>> that it would be handy for several reasons if we had a bit more >>> metadata >>> for pak-files as we currently have. Mostly knowing which services >>> (initscripts) are installed by a pak-file. >>> This would allow for services.cgi to not have to manually try to find >>> out if an installed addon actually has an initscript or not. >>> Also paks like libvirt install 2 initscripts, those can now both be >>> displayed on the services.cgi page. >>> Idem for monitoring agents, which was my main objective. >>> >>> So here is an attempt to achieve this. This is not yet a patchset to >>> be applied yet, but rather a proposal as this change would require >>> all >>> addon LFS-files to be changed. But I didn't want to do that yet as >>> this patchset may very wel be rejected completely :-). >>> The first patch introduces 2 new macro's: >> So it could be done in two separate patches or even better patch sets. >> Makes reviewing easier and patches shorter :-) >>> - SUMMARY for a short, one-line summary of the package >>> - INITSCRIPTS for a space seperated list of initscripts provided by >>> the >>> package. >> How it is supposed to be handled when a package (like libvirt) install >> its own init scripts? So not a initscript we have in our source, but >> which is delivered in the source of the package itself. If we put this >> in the list, the macro will break. If we leave it out we lose >> information. >> > You have a point there. In the case of libvirt it, as you point out in > your other mail, is indeed an initscript that does not start a service > so does not pose a problem using this method. > But it is not unthinkable that another pak (or a possible future pak) > includes an initscript in the source itself. > So this is may indeed be something to take into account.. > >> Do you did some research how other distribution handle this problem? >> ( If they handle it at all.) > "normal" distributions generally don't really care which packages run > which services and if/or those are actually services. > However most modern distro's use systemd making it easier to know which > units are services, and which are so called one-shots as you can just > ask systemd. It has a lot more info about all "initscripts" which would > make this task easier. But we don't have systemd here :-) > > Also Zabbix monitoring agent for example has no built-in methods to > automatically discover available and/or running services when the > monitored distro is not running systemd. While it already does this for > decades for Windows hosts. And more recently also for systemd. > > In Suse, each package containing a service also created a link > /usr/sbin/rc<servicename> which links to the 'service' command which > currently is a wrapper script to systemd. Not sure how they did it > before systemd, possibly those rc-binaries where just symlinks to the > actual initscripts. > That is maybe something we may also need to consider, but then rather > for better user cli experience as I don't immediately see that solving > the problem you posed. > >> Another approach which comes to my mind: >> Why not parsing the root file for /etc/rc.d/init.d/ (I currently do not >> know if it is the right path)? So trying to detect which initscripts >> are part of the root file? >> > As you concluded in your other mail, not all initscripts are > services/daemons so that would probably require some hardcoded > exceptions and alike. Which is exactly what we are trying to avoid and > is currently done in services.cgi. :-) > > To tackle the problem with service-initscripts included in and > installed by the source in my approach; I currently see 2 possible > solutions: > > - also provide the old INSTALL_INITSCRIPT macro to manually install one > or more initscripts, so you are able to skip initscripts listed in > INITSCRIPTS but which are installed by the source. > > - make it a common practice to prevent the source from installing an > initscript, extracting it and installing it the IPFire way. (I think > initscript investigation is probably required anyway to make sure it is > compatible on IPFire..) I don't believe that a source automatically installing an initscript is something I have ever come across. As the package is from source then the package has limited idea on what initscript system (System V, systemd, upstart etc) is being used and what the locations for the scripts should be. The closest I have come is the bacula source file which also has a range of different startup options available which it can choose based on the distro it detects but the user has to specifically run make install-autostart in the build after make install, so it doesn't happen automatically, you have to choose to do it. So I think you shouldn't have to worry about auto installation of initscripts. Regards, Adolf. > >>> And an alternative INSTALL_INITSCRIPTS method instead of the >>> current >>> INSTALL_INITSCRIPT method. As we now have all initscripts in the >>> INITSCRIPTS macro, the INSTALL_INITSCRIPTS will install all >>> initscripts >>> listed in that macro, so a simple call to INSTALL_INITSCRIPTS will >>> now >>> do the job instead of multiple calls in case of multiple >>> initscripts >>> (for example libvirt. I noticed clamav actually uses 1 initscript >>> for >>> starting 2 services, this could maybe also be split up again) >>> I included 2 examples in the first patch: libvirt and >>> zabbix_agentd. But >>> when implemented ofcourse all makefiles should be updated. >>> >>> During the pak packaging process in the build procedure, those new >>> macro's will be inheritted in the generated pakfire meta-* files. >>> >>> The second patch adds an extra 'info <pak(s)>' commandline >>> parameter to >>> pakfire, which will in turn call a new Pakfire::pakinfo function. >>> This function wil parse the meta-* file of the requested pak and >>> functions in 2 modes: >>> - "latest" which is the behaviour of the info parameter. This will >>> display the latest available metadata of the pak and the status of >>> the pak on the system as in: is it installed?, and if so, is it >>> up-to-date. >>> - "installed" wich will display only information about the >>> currently >>> installed pak and bail out of the requested pak is not currently >>> installed. >>> This function was added to provide a 'central' point/method to get >>> pak >>> information. I don't know if there are other scripts beside >>> services.cgi >>> that currently try parsing meta-* files. But they should then be >>> changed >>> to use this function instead. >>> >>> Example output of the new pakfire info command: `pakfire info >>> zabbix_agentd`: >>> when installed and up-to-date: >>> --- >>> Name: zabbix_agentd >>> Version: 4.2.6-4 >>> Summary: Zabbix Agent >>> Size: 250.00 KB >>> Dependencies: >>> Pakfile: zabbix_agentd-4.2.6-4.ipfire >>> InitScripts: zabbix_agentd >>> Installed: Yes >>> Status: up-to-date >>> --- >>> When an update is available: >>> --- >>> Name: zabbix_agentd >>> Version: 5.0.10-5 >>> Summary: Zabbix Agent >>> Size: 276.00 KB >>> Dependencies: fping >>> Pakfile: zabbix_agentd-5.0.10-5.ipfire >>> InitScripts: zabbix_agentd >>> Installed: Yes >>> Status: outdated (version 4.2.6-4 is installed) >>> --- >>> Or when a pak was discontinued and no longer supplied by ipfire, >>> but >>> still installed on the system: >>> --- >>> Name: zabbix_agentd >>> Version: - >>> Summary: Zabbix Agent >>> Size: 250.00 KB >>> Dependencies: >>> Pakfile: zabbix_agentd-4.2.6-4.ipfire >>> InitScripts: zabbix_agentd >>> Installed: Yes >>> Status: obsolete (version 4.2.6-4 is installed) >>> --- >>> and at last when a pak is available, but not installed: >>> --- >>> Name: zabbix_agentd >>> Version: 5.0.10-5 >>> Summary: Zabbix Agent >>> Size: 276.00 KB >>> Dependencies: fping >>> Pakfile: zabbix_agentd-5.0.10-5.ipfire >>> InitScripts: zabbix_agentd >>> Installed: No >>> Status: not installed >>> --- >>> >>> And then the last patch is an update of service.cgi now using the >>> new >>> Pakfire::pakinfo function in "installed"-mode. >>> >>> If there are any suggestions on more metadata.. I think this is the >>> moment to throw them at me. >>> And ofcourse suggestions/comments are welcome as this is currently >>> only >>> a proposal for change. But I think we win in robustness of >>> services.cgi >>> and user experience in both using pakfire and ability to provide >>> available services to monitoring agents. >> Just want to say thank you for taking this way, which might be >> “harder” but yields better result from my experience. Please do not >> take my points as “this is not right”, more like “there might me >> other ways, are they better?”. >> > That is exactly why I threw this in the group; to check if there are > better idea's/approaches/things I overlooked/.. > So thank you for your constructive comments! > > Regards > Robin > >> Greetings Jonatan >>> On top of that could the whole meta-* files system be overhauled in >>> the >>> future, if wanted, with only pakfire itself needing change as the >>> rest >>> will then depend on pakfire for correctly parsing it's "database". >>> >>> >>> Regards >>> Robin >>> >>> >>> >>> -- >>> Dit bericht is gescanned op virussen en andere gevaarlijke >>> inhoud door MailScanner en lijkt schoon te zijn. >>> > ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-06-18 8:32 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-23 16:15 [PATCH 0/3] Pakfile metadata enhancements Robin Roevens 2021-04-23 16:15 ` [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Robin Roevens 2021-05-12 18:49 ` Jonatan Schlag 2021-05-14 12:24 ` Michael Tremer 2021-05-14 20:07 ` Robin Roevens 2021-05-18 15:09 ` Michael Tremer 2021-05-24 20:23 ` Robin Roevens 2021-06-06 18:41 ` Robin Roevens 2021-06-10 11:27 ` Michael Tremer 2021-06-17 22:28 ` Robin Roevens 2021-06-18 8:32 ` Michael Tremer 2021-06-10 11:24 ` Michael Tremer 2021-05-14 12:23 ` Michael Tremer 2021-05-14 20:16 ` Robin Roevens 2021-05-18 11:12 ` Michael Tremer 2021-04-23 16:15 ` [PATCH 2/3] pakfire: add 'info <pak(s)>' option Robin Roevens 2021-05-13 8:02 ` Jonatan Schlag 2021-05-13 19:11 ` Robin Roevens 2021-05-14 12:19 ` Michael Tremer 2021-05-14 20:24 ` Robin Roevens 2021-05-25 11:22 ` Michael Tremer 2021-04-23 16:15 ` [PATCH 3/3] services.cgi: use new Pakfire::pakinfo function Robin Roevens 2021-05-11 19:30 ` group call for review/opinions/idea's (was: [PATCH 0/3] Pakfile metadata enhancements) Robin Roevens 2021-05-12 18:45 ` [PATCH 0/3] Pakfile metadata enhancements Jonatan Schlag 2021-05-12 22:31 ` Robin Roevens 2021-05-15 12:10 ` Adolf Belka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox