From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robin Roevens To: development@lists.ipfire.org Subject: Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Date: Fri, 18 Jun 2021 00:28:36 +0200 Message-ID: In-Reply-To: <5A6FBFC4-9888-4FD0-8972-8C2DE7495042@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0653344072689136406==" List-Id: --===============0653344072689136406== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Hi Michael Tremer schreef op do 10-06-2021 om 12:27 [+0100]: > Hello, > > > On 6 Jun 2021, at 19:41, Robin Roevens > > 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 > > > > > > > > > > > > > > > > --- > > > > > > > > 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. --===============0653344072689136406==--