From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Date: Fri, 18 Jun 2021 09:32:43 +0100 Message-ID: <943BB62E-71B9-4944-9366-649642C0A4ED@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0009546922338323383==" List-Id: --===============0009546922338323383== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, > On 17 Jun 2021, at 23:28, Robin Roevens wrote: >=20 > Hi >=20 > Michael Tremer schreef op do 10-06-2021 om 12:27 [+0100]: >> Hello, >>=20 >>> On 6 Jun 2021, at 19:41, Robin Roevens >>> wrote: >>>=20 >>> Hi Michael >>>=20 >>> I know/see a lot of activity here, so I assume you are always quite >>> busy and may have missed my last reaction? >>=20 >> Yes, sorry, I missed this, or didn=E2=80=99t know what to say. Feel free to >> remind me if you didn=E2=80=99t hear from me within a couple of days. >>=20 >>> Should I proceed with my proposition to add >>> Summary: SUMMARY >>> Services: SERVICES >>> to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro >>=20 >> I would say yes. Do we have any unanswered questions left? > Not for this patch-set at the moment, I think.. >=20 >>=20 >> Is Jonatan happy? > I don't know.. @Jonatan, are you happy? :-) Seems to be on holiday... >=20 >>=20 >>> Should I also add=20 >>> InitScripts: INITSCRIPTS >>> for non-service initscripts.. just for completion of metadata ? >>=20 >> Hmm, I would say no. If there is no point to it, then leave it. >>=20 >> 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: >=20 > $(call INSTALL_INITSCRIPTS,$(SERVICES)) >=20 >>=20 >>> 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) >>>=20 >>> Or should we find other methods to have this info ready for >>> services.cgi and/or monitoring agents ? >>=20 >> No, I believe this is a good solution. We can then have a function >> that queries the package database and shows all services correctly. >>=20 >> That sounds clean to me and is re-usable and extensible. > I think so too. >=20 > 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. Noth= ing 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 ? >=20 > This of course next to patches to add the INSTALL_INITSCRIPTS macro and > pakfire library change to read/use this additional metadata. -Michael >=20 > Regards >=20 > Robin >=20 >>=20 >> -Michael >>=20 >>>=20 >>> Regards >>> Robin >>>=20 >>> Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]: >>>> Hi Michael >>>>=20 >>>> Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]: >>>>>=20 >>>>>=20 >>>>>> On 14 May 2021, at 21:07, Robin Roevens < =20 >>>>>> robin.roevens(a)disroot.org> >>>>>> wrote: >>>>>>=20 >>>>>> Hi >>>>>>=20 >>>>>> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: >>>>>>> Hello Jonatan, >>>>>>>=20 >>>>>>> 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. >>>>>>>=20 >>>>>>> It is a weird corner case, but how do we want to handle >>>>>>> this? >>>>>>>=20 >>>>>>> -Michael >>>>>>=20 >>>>>> 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? >>>>>>=20 >>>>>> The INSTALL_INITSCRIPTS macro could then be renamed to >>>>>> INSTALL_SERVICE_INITSCRIPTS. >>>>>>=20 >>>>>> 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). >>>>>>=20 >>>>>> So this would then result in: >>>>>> --- >>>>>> define INSTALL_INITSCRIPT >>>>>> install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1)=20 >>>>>> /etc/rc.d/init.d/$(1) >>>>>> endef >>>>>>=20 >>>>>> define INSTALL_SERVICE_INITSCRIPTS >>>>>> for service in $(SERVICES); do \ >>>>>> $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ >>>>>> done >>>>>> endef >>>>>> --- >>>>>>=20 >>>>>> 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 >>>>>>=20 >>>>>> 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 >>>>>> --- >>>>>>=20 >>>>>> Maybe that info could also come in handy sooner or later ? >>>>>=20 >>>>> This sounds rather complicated to me. >>>>>=20 >>>>> What are our objectives here? Does Zabbix want/need to >>>>> start/stop >>>>> services? And if so, why? >>>>=20 >>>> 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).=20 >>>>=20 >>>> 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. >>>>=20 >>>> 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. >>>>=20 >>>> 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) >>>>=20 >>>> 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. >>>>=20 >>>> 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). >>>>=20 >>>> If it would make things more clear, I could work my last mail out >>>> in an >>>> actual patch ? >>>>=20 >>>> Regards >>>> Robin >>>>=20 >>>>>=20 >>>>>> 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. >>>>>>=20 >>>>>> 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.. >>>>>>=20 >>>>>> 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. >>>>>>=20 >>>>>> Robin >>>>>>=20 >>>>>>>=20 >>>>>>>> On 12 May 2021, at 19:49, Jonatan Schlag < >>>>>>>> jonatan.schlag(a)ipfire.org> >>>>>>>> wrote: >>>>>>>>=20 >>>>>>>> Hi, >>>>>>>>=20 >>>>>>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens < >>>>>>>>> robin.roevens(a)disroot.org>: >>>>>>>>>=20 >>>>>>>>> =EF=BB=BF* 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. >>>>>>>>>=20 >>>>>>>>> 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(-) >>>>>>>>>=20 >>>>>>>>> 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=3D%s >>>>>>>>> /install/packages/$(PROG)- >>>>>>>>> $(VER)-$(PAK_VER).ipfire)/g" \ >>>>>>>>> + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ >>>>>>>>> < /usr/src/src/pakfire/meta > >>>>>>>>> /install/packages/meta- >>>>>>>>> $(PROG) >>>>>>>>> endef >>>>>>>>>=20 >>>>>>>>> -define INSTALL_INITSCRIPT >>>>>>>>> - install -m 754 -v >>>>>>>>> $(DIR_SRC)/src/initscripts/packages/$(1)=20 >>>>>>>>> /etc/rc.d/init.d/$(1) >>>>>>>>> +define INSTALL_INITSCRIPTS >>>>>>>>> + for initscript in $(INITSCRIPTS); do \ >>>>>>>>> + install -m 754 -v >>>>>>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript=20 >>>>>>>>> /etc/rc.d/init.d/$$initscript; \ >>>>>>>>> + done >>>>>>>>> endef >>>>>>>>>=20 >>>>>>>>> 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 >>>>>>>>>=20 >>>>>>>>> VER =3D 6.5.0 >>>>>>>>> +SUMMARY =3D Server side daemon and supporting >>>>>>>>> files for >>>>>>>>> libvirt >>>>>>>>>=20 >>>>>>>>> THISAPP =3D libvirt-$(VER) >>>>>>>>> DL_FILE =3D $(THISAPP).tar.xz >>>>>>>>> @@ -37,6 +38,8 @@ PAK_VER =3D 25 >>>>>>>>>=20 >>>>>>>>> DEPS =3D ebtables libpciaccess libtirpc libyajl >>>>>>>>> ncat qemu >>>>>>>>>=20 >>>>>>>>> +INITSCRIPTS=3D libvirtd virtlogd >>>>>>>>> + >>>>>>>>> ####################################################### >>>>>>>>> ###### >>>>>>>>> ###### >>>>>>>>> ############ >>>>>>>>> # Top-level Rules >>>>>>>>> ####################################################### >>>>>>>>> ###### >>>>>>>>> ###### >>>>>>>>> ############ >>>>>>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst >>>>>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>>>>> cd $(DIR_APP)/build_libvirt && make install >>>>>>>>>=20 >>>>>>>>> #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.=20 >>>>>>>>=20 >>>>>>>> Jonatan >>>>>>>>>=20 >>>>>>>>> # 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 >>>>>>>>>=20 >>>>>>>>> VER =3D 4.2.6 >>>>>>>>> +SUMMARY =3D Zabbix Agent >>>>>>>>>=20 >>>>>>>>> THISAPP =3D zabbix-$(VER) >>>>>>>>> DL_FILE =3D $(THISAPP).tar.gz >>>>>>>>> @@ -35,6 +36,8 @@ PROG =3D zabbix_agentd >>>>>>>>> PAK_VER =3D 4 >>>>>>>>> DEPS =3D >>>>>>>>>=20 >>>>>>>>> +INITSCRIPTS=3D zabbix_agentd >>>>>>>>> + >>>>>>>>> ####################################################### >>>>>>>>> ###### >>>>>>>>> ###### >>>>>>>>> ############ >>>>>>>>> # Top-level Rules >>>>>>>>> ####################################################### >>>>>>>>> ###### >>>>>>>>> ###### >>>>>>>>> ############ >>>>>>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst >>>>>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>>>>> chown zabbix.zabbix /var/run/zabbix >>>>>>>>>=20 >>>>>>>>> # Install initscripts >>>>>>>>> - $(call INSTALL_INITSCRIPT,zabbix_agentd) >>>>>>>>> + @$(INSTALL_INITSCRIPTS) >>>>>>>>>=20 >>>>>>>>> # 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 >>>>>>>>> --=20 >>>>>>>>> 2.31.1 >>>>>>>>>=20 >>>>>>>>>=20 >>>>>>>>> --=20 >>>>>>>>> Dit bericht is gescanned op virussen en andere >>>>>>>>> gevaarlijke >>>>>>>>> inhoud door MailScanner en lijkt schoon te zijn. >>>>>>>=20 >>>>>>>=20 >>>>>>=20 >>>>>>=20 >>>>>> --=20 >>>>>> Dit bericht is gescanned op virussen en andere gevaarlijke >>>>>> inhoud door MailScanner en lijkt schoon te zijn. >>>>>=20 >>>>>=20 >>>>=20 >>>>=20 >>>=20 >>>=20 >>> --=20 >>> Dit bericht is gescanned op virussen en andere gevaarlijke >>> inhoud door MailScanner en lijkt schoon te zijn. >>>=20 >>=20 >>=20 >=20 >=20 > --=20 > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. --===============0009546922338323383==--