Message ID | 20201126144745.502228-1-nolange79@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | [v3,1/1] package/systemd: add support for creating journal catalog DB | expand |
Hello Norbert, Le 26/11/2020 à 15:47, Norbert Lange a écrit : > journald supports catalog files, or rather a binary database of > those. > Functionality added includes: > > - A config option allows enabling the binary database. > > - if the option is enabled, the database is built and moved to > /usr/share/factory. A symlink is created in /var pointing to > that file. > If BR2_ENABLE_LOCALE_PURGE is enabled, the catalogs not in the > language whitelist are deleted first. > > - the service normally used for creating the DB during boot, > and the catalog source files used as input are deleted. > > The move to /usr/share/factory is helpful for having /usr as whole > system distribution. > > Signed-off-by: Norbert Lange <nolange79@gmail.com> > --- > v1->v2: > - Moved all logic into systemd.mk > - solved the LOCALE_PURGE order that way > - use the factory to store the file > - option to enable the DB (similar to udev HWDB) > - cant be anabled with !ROOTFS_RW, tons of > issues with that one > v2->v3: > - use backticks instead of $$() > - be more explicit about what happens in > SYSTEMD_LOCALE_PURGE_CATALOGS > > Signed-off-by: Norbert Lange <nolange79@gmail.com> > --- > package/systemd/Config.in | 14 +++++++++++++ > package/systemd/systemd.mk | 41 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 55 insertions(+) > > diff --git a/package/systemd/Config.in b/package/systemd/Config.in > index ec34478e3d..d576e6035b 100644 > --- a/package/systemd/Config.in > +++ b/package/systemd/Config.in > @@ -269,6 +269,20 @@ config BR2_PACKAGE_SYSTEMD_IMPORTD > > http://www.freedesktop.org/software/systemd/man/machinectl.html#Image%20Transfer%20Commands > > +config BR2_PACKAGE_SYSTEMD_CATALOGDB > + bool "enable journal catalog database installation" > + depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW # conflicting tmpfiles magic > + help > + Build and install the journal catalog database. > + > + catalog files are used to provide extended and potentially localized > + messages for the journal. > + > + The original catalog files will be built into a DB at > + /usr/share/factory/var/lib/systemd/catalog/database. > + > + https://www.freedesktop.org/wiki/Software/systemd/catalog/ > + > config BR2_PACKAGE_SYSTEMD_LOCALED > bool "enable locale daemon" > help > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > index cb12f667d6..ae99c02abf 100644 > --- a/package/systemd/systemd.mk > +++ b/package/systemd/systemd.mk > @@ -613,6 +613,47 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD > $(SYSTEMD_INSTALL_NETWORK_CONFS) > endef > > +ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) > +define SYSTEMD_LOCALE_PURGE_CATALOGS > + # go through all files with scheme <basename>.<langext>.catalog > + # and remove those where <langext> is not in LOCALE_NOPURGE > + for cfile in `find $(TARGET_DIR)/usr/lib/systemd/catalog -name '*.*.catalog'`; \ Why not "systemd.*.catalog" ? On my system, all catalog files with <langext> are nammed starting with "systemd." > + do \ > + basename=$${cfile##*/}; \ > + basename=$${basename%.catalog}; \ > + langext=$${basename#*.}; \ > + [ "$$langext" = "$${basename}" ] && continue; \ > + expr '$(LOCALE_NOPURGE)' : ".*\b$${langext}\b" >/dev/null && continue; \ > + rm -f "$$cfile"; \ > + done > +endef > + > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_LOCALE_PURGE_CATALOGS > +endif > + > +ifeq ($(BR2_PACKAGE_SYSTEMD_CATALOGDB),y) > +define SYSTEMD_UPDATE_CATALOGS > + $(HOST_DIR)/bin/journalctl --root=$(TARGET_DIR) --update-catalog > + install -D $(TARGET_DIR)/var/lib/systemd/catalog/database \ > + $(TARGET_DIR)/usr/share/factory/var/lib/systemd/catalog/database > + rm $(TARGET_DIR)/var/lib/systemd/catalog/database > + ln -sf /usr/share/factory/var/lib/systemd/catalog/database \ > + $(TARGET_DIR)/var/lib/systemd/catalog/database > + grep -q '^L /var/lib/systemd/catalog/database' $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf || \ > + printf "\nL /var/lib/systemd/catalog/database\n" >> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf > +endef > + > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_UPDATE_CATALOGS > +endif > + > +define SYSTEMD_RM_CATALOG_UPDATE_SERVICE > + rm -rf $(TARGET_DIR)/usr/lib/systemd/catalog \ So we remove this directory. Why adding SYSTEMD_LOCALE_PURGE_CATALOGS then ? Best regards, Romain > + $(TARGET_DIR)/usr/lib/systemd/system/systemd-journal-catalog-update.service \ > + $(TARGET_DIR)/usr/lib/systemd/system/*/systemd-journal-catalog-update.service > +endef > + > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE > + > define SYSTEMD_PRESET_ALL > $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all > endef >
Am Sa., 8. Jan. 2022 um 12:55 Uhr schrieb Romain Naour <romain.naour@smile.fr>: > > Hello Norbert, > > Le 26/11/2020 à 15:47, Norbert Lange a écrit : > > journald supports catalog files, or rather a binary database of > > those. > > Functionality added includes: > > > > - A config option allows enabling the binary database. > > > > - if the option is enabled, the database is built and moved to > > /usr/share/factory. A symlink is created in /var pointing to > > that file. > > If BR2_ENABLE_LOCALE_PURGE is enabled, the catalogs not in the > > language whitelist are deleted first. > > > > - the service normally used for creating the DB during boot, > > and the catalog source files used as input are deleted. > > > > The move to /usr/share/factory is helpful for having /usr as whole > > system distribution. > > > > Signed-off-by: Norbert Lange <nolange79@gmail.com> > > --- > > v1->v2: > > - Moved all logic into systemd.mk > > - solved the LOCALE_PURGE order that way > > - use the factory to store the file > > - option to enable the DB (similar to udev HWDB) > > - cant be anabled with !ROOTFS_RW, tons of > > issues with that one > > v2->v3: > > - use backticks instead of $$() > > - be more explicit about what happens in > > SYSTEMD_LOCALE_PURGE_CATALOGS > > > > Signed-off-by: Norbert Lange <nolange79@gmail.com> > > --- > > package/systemd/Config.in | 14 +++++++++++++ > > package/systemd/systemd.mk | 41 ++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 55 insertions(+) > > > > diff --git a/package/systemd/Config.in b/package/systemd/Config.in > > index ec34478e3d..d576e6035b 100644 > > --- a/package/systemd/Config.in > > +++ b/package/systemd/Config.in > > @@ -269,6 +269,20 @@ config BR2_PACKAGE_SYSTEMD_IMPORTD > > > > http://www.freedesktop.org/software/systemd/man/machinectl.html#Image%20Transfer%20Commands > > > > +config BR2_PACKAGE_SYSTEMD_CATALOGDB > > + bool "enable journal catalog database installation" > > + depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW # conflicting tmpfiles magic > > + help > > + Build and install the journal catalog database. > > + > > + catalog files are used to provide extended and potentially localized > > + messages for the journal. > > + > > + The original catalog files will be built into a DB at > > + /usr/share/factory/var/lib/systemd/catalog/database. > > + > > + https://www.freedesktop.org/wiki/Software/systemd/catalog/ > > + > > config BR2_PACKAGE_SYSTEMD_LOCALED > > bool "enable locale daemon" > > help > > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > > index cb12f667d6..ae99c02abf 100644 > > --- a/package/systemd/systemd.mk > > +++ b/package/systemd/systemd.mk > > @@ -613,6 +613,47 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD > > $(SYSTEMD_INSTALL_NETWORK_CONFS) > > endef > > > > +ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) > > +define SYSTEMD_LOCALE_PURGE_CATALOGS > > + # go through all files with scheme <basename>.<langext>.catalog > > + # and remove those where <langext> is not in LOCALE_NOPURGE > > + for cfile in `find $(TARGET_DIR)/usr/lib/systemd/catalog -name '*.*.catalog'`; \ > > Why not "systemd.*.catalog" ? > > On my system, all catalog files with <langext> are nammed starting with "systemd." Cause the idea is that other packages could extend log messages by dropping catalogs there, there might be a dbus-broker.catalog for example, see [1]. > > > + do \ > > + basename=$${cfile##*/}; \ > > + basename=$${basename%.catalog}; \ > > + langext=$${basename#*.}; \ > > + [ "$$langext" = "$${basename}" ] && continue; \ > > + expr '$(LOCALE_NOPURGE)' : ".*\b$${langext}\b" >/dev/null && continue; \ > > + rm -f "$$cfile"; \ > > + done > > +endef > > + > > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_LOCALE_PURGE_CATALOGS > > +endif > > + > > +ifeq ($(BR2_PACKAGE_SYSTEMD_CATALOGDB),y) > > +define SYSTEMD_UPDATE_CATALOGS > > + $(HOST_DIR)/bin/journalctl --root=$(TARGET_DIR) --update-catalog > > + install -D $(TARGET_DIR)/var/lib/systemd/catalog/database \ > > + $(TARGET_DIR)/usr/share/factory/var/lib/systemd/catalog/database > > + rm $(TARGET_DIR)/var/lib/systemd/catalog/database > > + ln -sf /usr/share/factory/var/lib/systemd/catalog/database \ > > + $(TARGET_DIR)/var/lib/systemd/catalog/database > > + grep -q '^L /var/lib/systemd/catalog/database' $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf || \ > > + printf "\nL /var/lib/systemd/catalog/database\n" >> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf > > +endef > > + > > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_UPDATE_CATALOGS > > +endif > > + > > +define SYSTEMD_RM_CATALOG_UPDATE_SERVICE > > + rm -rf $(TARGET_DIR)/usr/lib/systemd/catalog \ > > So we remove this directory. > > Why adding SYSTEMD_LOCALE_PURGE_CATALOGS then ? The order is relevant, assuming LOCALE_NOPURGE = de, then - first remove the source-files for other languages (SYSTEMD_LOCALE_PURGE_CATALOGS) - then build and install the database from the source-files with the chosen languages - then remove all language source-files > > Best regards, > Romain > > > > + $(TARGET_DIR)/usr/lib/systemd/system/systemd-journal-catalog-update.service \ > > + $(TARGET_DIR)/usr/lib/systemd/system/*/systemd-journal-catalog-update.service > > +endef > > + > > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE > > + > > define SYSTEMD_PRESET_ALL > > $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all > > endef > > > Norbert [1] - https://github.com/bus1/dbus-broker/tree/main/src/catalog
Le 08/01/2022 à 15:33, Norbert Lange a écrit : > Am Sa., 8. Jan. 2022 um 12:55 Uhr schrieb Romain Naour <romain.naour@smile.fr>: >> [...] >>> >>> +ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) >>> +define SYSTEMD_LOCALE_PURGE_CATALOGS >>> + # go through all files with scheme <basename>.<langext>.catalog >>> + # and remove those where <langext> is not in LOCALE_NOPURGE >>> + for cfile in `find $(TARGET_DIR)/usr/lib/systemd/catalog -name '*.*.catalog'`; \ >> >> Why not "systemd.*.catalog" ? >> >> On my system, all catalog files with <langext> are nammed starting with "systemd." > > Cause the idea is that other packages could extend log messages by > dropping catalogs there, > there might be a dbus-broker.catalog for example, see [1]. dbus-broker.catalog doesn't match the find regex '*.*.catalog', this file is not removed. > >> >>> + do \ >>> + basename=$${cfile##*/}; \ >>> + basename=$${basename%.catalog}; \ >>> + langext=$${basename#*.}; \ >>> + [ "$$langext" = "$${basename}" ] && continue; \ >>> + expr '$(LOCALE_NOPURGE)' : ".*\b$${langext}\b" >/dev/null && continue; \ >>> + rm -f "$$cfile"; \ >>> + done >>> +endef >>> + >>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_LOCALE_PURGE_CATALOGS >>> +endif >>> + >>> +ifeq ($(BR2_PACKAGE_SYSTEMD_CATALOGDB),y) >>> +define SYSTEMD_UPDATE_CATALOGS >>> + $(HOST_DIR)/bin/journalctl --root=$(TARGET_DIR) --update-catalog >>> + install -D $(TARGET_DIR)/var/lib/systemd/catalog/database \ >>> + $(TARGET_DIR)/usr/share/factory/var/lib/systemd/catalog/database >>> + rm $(TARGET_DIR)/var/lib/systemd/catalog/database >>> + ln -sf /usr/share/factory/var/lib/systemd/catalog/database \ >>> + $(TARGET_DIR)/var/lib/systemd/catalog/database >>> + grep -q '^L /var/lib/systemd/catalog/database' $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf || \ >>> + printf "\nL /var/lib/systemd/catalog/database\n" >> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf >>> +endef >>> + >>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_UPDATE_CATALOGS >>> +endif >>> + >>> +define SYSTEMD_RM_CATALOG_UPDATE_SERVICE >>> + rm -rf $(TARGET_DIR)/usr/lib/systemd/catalog \ >> >> So we remove this directory. >> >> Why adding SYSTEMD_LOCALE_PURGE_CATALOGS then ? > > The order is relevant, assuming LOCALE_NOPURGE = de, then > > - first remove the source-files for other languages > (SYSTEMD_LOCALE_PURGE_CATALOGS) > - then build and install the database from the source-files with the > chosen languages > - then remove all language source-files Ok, maybe a comment would be necessary here. Best regards, Romain > >> >> Best regards, >> Romain >> >> >>> + $(TARGET_DIR)/usr/lib/systemd/system/systemd-journal-catalog-update.service \ >>> + $(TARGET_DIR)/usr/lib/systemd/system/*/systemd-journal-catalog-update.service >>> +endef >>> + >>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE >>> + >>> define SYSTEMD_PRESET_ALL >>> $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all >>> endef >>> >> > > Norbert > > [1] - https://github.com/bus1/dbus-broker/tree/main/src/catalog >
Hi Norbert, On 26/11/2020 15:47, Norbert Lange wrote: > journald supports catalog files, or rather a binary database of > those. > Functionality added includes: > > - A config option allows enabling the binary database. > > - if the option is enabled, the database is built and moved to > /usr/share/factory. A symlink is created in /var pointing to > that file. > If BR2_ENABLE_LOCALE_PURGE is enabled, the catalogs not in the > language whitelist are deleted first. > > - the service normally used for creating the DB during boot, > and the catalog source files used as input are deleted. > > The move to /usr/share/factory is helpful for having /usr as whole > system distribution. > > Signed-off-by: Norbert Lange <nolange79@gmail.com> I was going to apply a second time, but then I noticed that I made some comments on v2 [1] that you haven't answered to and I still think they're valid. So I've marked this patch as Changes Requested. If you respin, please also include in the commit message the additional explanations you made in reply to Romain. Regards, Arnout [1] https://lore.kernel.org/buildroot/1d09c49a-6d08-1cd7-9785-92b56c17ae06@mind.be/ > --- > v1->v2: > - Moved all logic into systemd.mk > - solved the LOCALE_PURGE order that way > - use the factory to store the file > - option to enable the DB (similar to udev HWDB) > - cant be anabled with !ROOTFS_RW, tons of > issues with that one > v2->v3: > - use backticks instead of $$() > - be more explicit about what happens in > SYSTEMD_LOCALE_PURGE_CATALOGS > > Signed-off-by: Norbert Lange <nolange79@gmail.com> > --- > package/systemd/Config.in | 14 +++++++++++++ > package/systemd/systemd.mk | 41 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 55 insertions(+) > > diff --git a/package/systemd/Config.in b/package/systemd/Config.in > index ec34478e3d..d576e6035b 100644 > --- a/package/systemd/Config.in > +++ b/package/systemd/Config.in > @@ -269,6 +269,20 @@ config BR2_PACKAGE_SYSTEMD_IMPORTD > > http://www.freedesktop.org/software/systemd/man/machinectl.html#Image%20Transfer%20Commands > > +config BR2_PACKAGE_SYSTEMD_CATALOGDB > + bool "enable journal catalog database installation" > + depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW # conflicting tmpfiles magic > + help > + Build and install the journal catalog database. > + > + catalog files are used to provide extended and potentially localized > + messages for the journal. > + > + The original catalog files will be built into a DB at > + /usr/share/factory/var/lib/systemd/catalog/database. > + > + https://www.freedesktop.org/wiki/Software/systemd/catalog/ > + > config BR2_PACKAGE_SYSTEMD_LOCALED > bool "enable locale daemon" > help > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > index cb12f667d6..ae99c02abf 100644 > --- a/package/systemd/systemd.mk > +++ b/package/systemd/systemd.mk > @@ -613,6 +613,47 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD > $(SYSTEMD_INSTALL_NETWORK_CONFS) > endef > > +ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) > +define SYSTEMD_LOCALE_PURGE_CATALOGS > + # go through all files with scheme <basename>.<langext>.catalog > + # and remove those where <langext> is not in LOCALE_NOPURGE > + for cfile in `find $(TARGET_DIR)/usr/lib/systemd/catalog -name '*.*.catalog'`; \ > + do \ > + basename=$${cfile##*/}; \ > + basename=$${basename%.catalog}; \ > + langext=$${basename#*.}; \ > + [ "$$langext" = "$${basename}" ] && continue; \ > + expr '$(LOCALE_NOPURGE)' : ".*\b$${langext}\b" >/dev/null && continue; \ > + rm -f "$$cfile"; \ > + done > +endef > + > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_LOCALE_PURGE_CATALOGS > +endif > + > +ifeq ($(BR2_PACKAGE_SYSTEMD_CATALOGDB),y) > +define SYSTEMD_UPDATE_CATALOGS > + $(HOST_DIR)/bin/journalctl --root=$(TARGET_DIR) --update-catalog > + install -D $(TARGET_DIR)/var/lib/systemd/catalog/database \ > + $(TARGET_DIR)/usr/share/factory/var/lib/systemd/catalog/database > + rm $(TARGET_DIR)/var/lib/systemd/catalog/database > + ln -sf /usr/share/factory/var/lib/systemd/catalog/database \ > + $(TARGET_DIR)/var/lib/systemd/catalog/database > + grep -q '^L /var/lib/systemd/catalog/database' $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf || \ > + printf "\nL /var/lib/systemd/catalog/database\n" >> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf > +endef > + > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_UPDATE_CATALOGS > +endif > + > +define SYSTEMD_RM_CATALOG_UPDATE_SERVICE > + rm -rf $(TARGET_DIR)/usr/lib/systemd/catalog \ > + $(TARGET_DIR)/usr/lib/systemd/system/systemd-journal-catalog-update.service \ > + $(TARGET_DIR)/usr/lib/systemd/system/*/systemd-journal-catalog-update.service > +endef > + > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE > + > define SYSTEMD_PRESET_ALL > $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all > endef >
Am Sa., 8. Jan. 2022 um 20:41 Uhr schrieb Arnout Vandecappelle <arnout@mind.be>: > > Hi Norbert, > > On 26/11/2020 15:47, Norbert Lange wrote: > > journald supports catalog files, or rather a binary database of > > those. > > Functionality added includes: > > > > - A config option allows enabling the binary database. > > > > - if the option is enabled, the database is built and moved to > > /usr/share/factory. A symlink is created in /var pointing to > > that file. > > If BR2_ENABLE_LOCALE_PURGE is enabled, the catalogs not in the > > language whitelist are deleted first. > > > > - the service normally used for creating the DB during boot, > > and the catalog source files used as input are deleted. > > > > The move to /usr/share/factory is helpful for having /usr as whole > > system distribution. > > > > Signed-off-by: Norbert Lange <nolange79@gmail.com> > > I was going to apply a second time, but then I noticed that I made some > comments on v2 [1] that you haven't answered to and I still think they're valid. > So I've marked this patch as Changes Requested. I did reply and cover the comments in [2], not sure whats missing? Separating into 2 independent patches is not really helpfull IMHO. > > If you respin, please also include in the commit message the additional > explanations you made in reply to Romain. > > Regards, > Arnout > > [1] https://lore.kernel.org/buildroot/1d09c49a-6d08-1cd7-9785-92b56c17ae06@mind.be/ > > > > --- > > v1->v2: > > - Moved all logic into systemd.mk > > - solved the LOCALE_PURGE order that way > > - use the factory to store the file > > - option to enable the DB (similar to udev HWDB) > > - cant be anabled with !ROOTFS_RW, tons of > > issues with that one > > v2->v3: > > - use backticks instead of $$() > > - be more explicit about what happens in > > SYSTEMD_LOCALE_PURGE_CATALOGS > > > > Signed-off-by: Norbert Lange <nolange79@gmail.com> > > --- > > package/systemd/Config.in | 14 +++++++++++++ > > package/systemd/systemd.mk | 41 ++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 55 insertions(+) > > > > diff --git a/package/systemd/Config.in b/package/systemd/Config.in > > index ec34478e3d..d576e6035b 100644 > > --- a/package/systemd/Config.in > > +++ b/package/systemd/Config.in > > @@ -269,6 +269,20 @@ config BR2_PACKAGE_SYSTEMD_IMPORTD > > > > http://www.freedesktop.org/software/systemd/man/machinectl.html#Image%20Transfer%20Commands > > > > +config BR2_PACKAGE_SYSTEMD_CATALOGDB > > + bool "enable journal catalog database installation" > > + depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW # conflicting tmpfiles magic > > + help > > + Build and install the journal catalog database. > > + > > + catalog files are used to provide extended and potentially localized > > + messages for the journal. > > + > > + The original catalog files will be built into a DB at > > + /usr/share/factory/var/lib/systemd/catalog/database. > > + > > + https://www.freedesktop.org/wiki/Software/systemd/catalog/ > > + > > config BR2_PACKAGE_SYSTEMD_LOCALED > > bool "enable locale daemon" > > help > > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > > index cb12f667d6..ae99c02abf 100644 > > --- a/package/systemd/systemd.mk > > +++ b/package/systemd/systemd.mk > > @@ -613,6 +613,47 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD > > $(SYSTEMD_INSTALL_NETWORK_CONFS) > > endef > > > > +ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) > > +define SYSTEMD_LOCALE_PURGE_CATALOGS > > + # go through all files with scheme <basename>.<langext>.catalog > > + # and remove those where <langext> is not in LOCALE_NOPURGE > > + for cfile in `find $(TARGET_DIR)/usr/lib/systemd/catalog -name '*.*.catalog'`; \ > > + do \ > > + basename=$${cfile##*/}; \ > > + basename=$${basename%.catalog}; \ > > + langext=$${basename#*.}; \ > > + [ "$$langext" = "$${basename}" ] && continue; \ > > + expr '$(LOCALE_NOPURGE)' : ".*\b$${langext}\b" >/dev/null && continue; \ > > + rm -f "$$cfile"; \ > > + done > > +endef > > + > > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_LOCALE_PURGE_CATALOGS > > +endif > > + > > +ifeq ($(BR2_PACKAGE_SYSTEMD_CATALOGDB),y) > > +define SYSTEMD_UPDATE_CATALOGS > > + $(HOST_DIR)/bin/journalctl --root=$(TARGET_DIR) --update-catalog > > + install -D $(TARGET_DIR)/var/lib/systemd/catalog/database \ > > + $(TARGET_DIR)/usr/share/factory/var/lib/systemd/catalog/database > > + rm $(TARGET_DIR)/var/lib/systemd/catalog/database > > + ln -sf /usr/share/factory/var/lib/systemd/catalog/database \ > > + $(TARGET_DIR)/var/lib/systemd/catalog/database > > + grep -q '^L /var/lib/systemd/catalog/database' $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf || \ > > + printf "\nL /var/lib/systemd/catalog/database\n" >> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf > > +endef > > + > > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_UPDATE_CATALOGS > > +endif > > + > > +define SYSTEMD_RM_CATALOG_UPDATE_SERVICE > > + rm -rf $(TARGET_DIR)/usr/lib/systemd/catalog \ > > + $(TARGET_DIR)/usr/lib/systemd/system/systemd-journal-catalog-update.service \ > > + $(TARGET_DIR)/usr/lib/systemd/system/*/systemd-journal-catalog-update.service > > +endef > > + > > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE > > + > > define SYSTEMD_PRESET_ALL > > $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all > > endef > > Norbert [2] - https://lore.kernel.org/buildroot/1d09c49a-6d08-1cd7-9785-92b56c17ae06@mind.be/
On 08/01/2022 22:46, Norbert Lange wrote: > Am Sa., 8. Jan. 2022 um 20:41 Uhr schrieb Arnout Vandecappelle <arnout@mind.be>: >> >> Hi Norbert, >> >> On 26/11/2020 15:47, Norbert Lange wrote: >>> journald supports catalog files, or rather a binary database of >>> those. >>> Functionality added includes: >>> >>> - A config option allows enabling the binary database. >>> >>> - if the option is enabled, the database is built and moved to >>> /usr/share/factory. A symlink is created in /var pointing to >>> that file. >>> If BR2_ENABLE_LOCALE_PURGE is enabled, the catalogs not in the >>> language whitelist are deleted first. >>> >>> - the service normally used for creating the DB during boot, >>> and the catalog source files used as input are deleted. >>> >>> The move to /usr/share/factory is helpful for having /usr as whole >>> system distribution. >>> >>> Signed-off-by: Norbert Lange <nolange79@gmail.com> >> >> I was going to apply a second time, but then I noticed that I made some >> comments on v2 [1] that you haven't answered to and I still think they're valid. >> So I've marked this patch as Changes Requested. > > I did reply and cover the comments in [2], not sure whats missing? > Separating into 2 independent patches is not really helpfull IMHO. Oops, you're right. I'll look at the patch again tomorrow morning. Regards, Arnout > >> >> If you respin, please also include in the commit message the additional >> explanations you made in reply to Romain. >> >> Regards, >> Arnout >> >> [1] https://lore.kernel.org/buildroot/1d09c49a-6d08-1cd7-9785-92b56c17ae06@mind.be/ >> >> >>> --- >>> v1->v2: >>> - Moved all logic into systemd.mk >>> - solved the LOCALE_PURGE order that way >>> - use the factory to store the file >>> - option to enable the DB (similar to udev HWDB) >>> - cant be anabled with !ROOTFS_RW, tons of >>> issues with that one >>> v2->v3: >>> - use backticks instead of $$() >>> - be more explicit about what happens in >>> SYSTEMD_LOCALE_PURGE_CATALOGS >>> >>> Signed-off-by: Norbert Lange <nolange79@gmail.com> >>> --- >>> package/systemd/Config.in | 14 +++++++++++++ >>> package/systemd/systemd.mk | 41 ++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 55 insertions(+) >>> >>> diff --git a/package/systemd/Config.in b/package/systemd/Config.in >>> index ec34478e3d..d576e6035b 100644 >>> --- a/package/systemd/Config.in >>> +++ b/package/systemd/Config.in >>> @@ -269,6 +269,20 @@ config BR2_PACKAGE_SYSTEMD_IMPORTD >>> >>> http://www.freedesktop.org/software/systemd/man/machinectl.html#Image%20Transfer%20Commands >>> >>> +config BR2_PACKAGE_SYSTEMD_CATALOGDB >>> + bool "enable journal catalog database installation" >>> + depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW # conflicting tmpfiles magic >>> + help >>> + Build and install the journal catalog database. >>> + >>> + catalog files are used to provide extended and potentially localized >>> + messages for the journal. >>> + >>> + The original catalog files will be built into a DB at >>> + /usr/share/factory/var/lib/systemd/catalog/database. >>> + >>> + https://www.freedesktop.org/wiki/Software/systemd/catalog/ >>> + >>> config BR2_PACKAGE_SYSTEMD_LOCALED >>> bool "enable locale daemon" >>> help >>> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk >>> index cb12f667d6..ae99c02abf 100644 >>> --- a/package/systemd/systemd.mk >>> +++ b/package/systemd/systemd.mk >>> @@ -613,6 +613,47 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD >>> $(SYSTEMD_INSTALL_NETWORK_CONFS) >>> endef >>> >>> +ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) >>> +define SYSTEMD_LOCALE_PURGE_CATALOGS >>> + # go through all files with scheme <basename>.<langext>.catalog >>> + # and remove those where <langext> is not in LOCALE_NOPURGE >>> + for cfile in `find $(TARGET_DIR)/usr/lib/systemd/catalog -name '*.*.catalog'`; \ >>> + do \ >>> + basename=$${cfile##*/}; \ >>> + basename=$${basename%.catalog}; \ >>> + langext=$${basename#*.}; \ >>> + [ "$$langext" = "$${basename}" ] && continue; \ >>> + expr '$(LOCALE_NOPURGE)' : ".*\b$${langext}\b" >/dev/null && continue; \ >>> + rm -f "$$cfile"; \ >>> + done >>> +endef >>> + >>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_LOCALE_PURGE_CATALOGS >>> +endif >>> + >>> +ifeq ($(BR2_PACKAGE_SYSTEMD_CATALOGDB),y) >>> +define SYSTEMD_UPDATE_CATALOGS >>> + $(HOST_DIR)/bin/journalctl --root=$(TARGET_DIR) --update-catalog >>> + install -D $(TARGET_DIR)/var/lib/systemd/catalog/database \ >>> + $(TARGET_DIR)/usr/share/factory/var/lib/systemd/catalog/database >>> + rm $(TARGET_DIR)/var/lib/systemd/catalog/database >>> + ln -sf /usr/share/factory/var/lib/systemd/catalog/database \ >>> + $(TARGET_DIR)/var/lib/systemd/catalog/database >>> + grep -q '^L /var/lib/systemd/catalog/database' $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf || \ >>> + printf "\nL /var/lib/systemd/catalog/database\n" >> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf >>> +endef >>> + >>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_UPDATE_CATALOGS >>> +endif >>> + >>> +define SYSTEMD_RM_CATALOG_UPDATE_SERVICE >>> + rm -rf $(TARGET_DIR)/usr/lib/systemd/catalog \ >>> + $(TARGET_DIR)/usr/lib/systemd/system/systemd-journal-catalog-update.service \ >>> + $(TARGET_DIR)/usr/lib/systemd/system/*/systemd-journal-catalog-update.service >>> +endef >>> + >>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE >>> + >>> define SYSTEMD_PRESET_ALL >>> $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all >>> endef >>> > > Norbert > > [2] - https://lore.kernel.org/buildroot/1d09c49a-6d08-1cd7-9785-92b56c17ae06@mind.be/ >
On 08/01/2022 23:38, Arnout Vandecappelle wrote: > > > On 08/01/2022 22:46, Norbert Lange wrote: >> Am Sa., 8. Jan. 2022 um 20:41 Uhr schrieb Arnout Vandecappelle <arnout@mind.be>: >>> >>> Hi Norbert, >>> >>> On 26/11/2020 15:47, Norbert Lange wrote: >>>> journald supports catalog files, or rather a binary database of >>>> those. >>>> Functionality added includes: >>>> >>>> - A config option allows enabling the binary database. >>>> >>>> - if the option is enabled, the database is built and moved to >>>> /usr/share/factory. A symlink is created in /var pointing to >>>> that file. >>>> If BR2_ENABLE_LOCALE_PURGE is enabled, the catalogs not in the >>>> language whitelist are deleted first. >>>> >>>> - the service normally used for creating the DB during boot, >>>> and the catalog source files used as input are deleted. >>>> >>>> The move to /usr/share/factory is helpful for having /usr as whole >>>> system distribution. >>>> >>>> Signed-off-by: Norbert Lange <nolange79@gmail.com> >>> >>> I was going to apply a second time, but then I noticed that I made some >>> comments on v2 [1] that you haven't answered to and I still think they're >>> valid. >>> So I've marked this patch as Changes Requested. >> >> I did reply and cover the comments in [2], not sure whats missing? >> Separating into 2 independent patches is not really helpfull IMHO. > > Oops, you're right. I'll look at the patch again tomorrow morning. As promised, I applied to master now. I changed the commit message quite a bit to (hopefully) better explain what is going on. package/systemd: add support for creating journal catalog DB journald supports catalog files, or rather a binary database of those. Functionality added includes: - A config option allows enabling the binary database. - If BR2_ENABLE_LOCALE_PURGE is enabled, the catalogs not in the language whitelist are deleted first. This is done independently of the new option, since the catalogs are removed later anyway. - If the option is enabled, the database is built and moved to /usr/share/factory. This makes sure that /usr contains the entire system. A symlink is created in /var pointing to that file. - The catalog source files are deleted. They serve no purpose on the target once the database exists. - All of the above is done in a ROOTFS_PRE_CMD_HOOK rather than in the build/install step, because other packages than systemd itself may also install catalogs. This also makes sure that it is possible to do a re-build, because the catalog files are not removed in $(TARGET_DIR) itself, only in the temporary copy for rootfs creation. - The service normally used for creating the DB during boot is deleted. If the DB is not enabled, we also don't want to waste time and space on re-generating every boot. Conversely, if the DB is enabled, it is already there so doesn't need to be re-done on every boot either. The new option depends on !BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW because if the rootfs is not RW, /var is overmounted with a tmpfs. The factory should handle this, but this only half-works [1]. [1] http://lists.busybox.net/pipermail/buildroot/2020-July/287016.html Signed-off-by: Norbert Lange <nolange79@gmail.com> Reviewed-by: Jérémy Rosen <jeremy.rosen@smile.fr> Reviewed-by: Adam Duskett <aduskett@gmail.com> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Two remarks about this: - By digging deeply into the mailing list archives, I discovered that in fact Jérémy and Adam had already approved this patch. In this case, it helps a lot if you carry the Reviewed-by tag in follow-up versions. With a Reviewed-by we can have a lot more confidence about applying something (that we don't fully understand anyway). And you can't expect the maintainers to either remember everything that was ever posted on the list, or to go digging through the mailing list to find such details (we often do do that, but it's exactly because we have to do that that it takes a long time before patches get applied). - In the patch changelog, it helps if you mention that a change is due to a review request, e.g.: v2->v3: - use backticks instead of $$() [Arnout] And one final thing. This BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW thing is really shitty. I hope we can one day have a properly working systemd with readonly rootfs... Regards, Arnout > > Regards, > Arnout > >> >>> >>> If you respin, please also include in the commit message the additional >>> explanations you made in reply to Romain. >>> >>> Regards, >>> Arnout >>> >>> [1] >>> https://lore.kernel.org/buildroot/1d09c49a-6d08-1cd7-9785-92b56c17ae06@mind.be/ >>> >>> >>>> --- >>>> v1->v2: >>>> - Moved all logic into systemd.mk >>>> - solved the LOCALE_PURGE order that way >>>> - use the factory to store the file >>>> - option to enable the DB (similar to udev HWDB) >>>> - cant be anabled with !ROOTFS_RW, tons of >>>> issues with that one >>>> v2->v3: >>>> - use backticks instead of $$() >>>> - be more explicit about what happens in >>>> SYSTEMD_LOCALE_PURGE_CATALOGS >>>> >>>> Signed-off-by: Norbert Lange <nolange79@gmail.com> >>>> --- >>>> package/systemd/Config.in | 14 +++++++++++++ >>>> package/systemd/systemd.mk | 41 ++++++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 55 insertions(+) >>>> >>>> diff --git a/package/systemd/Config.in b/package/systemd/Config.in >>>> index ec34478e3d..d576e6035b 100644 >>>> --- a/package/systemd/Config.in >>>> +++ b/package/systemd/Config.in >>>> @@ -269,6 +269,20 @@ config BR2_PACKAGE_SYSTEMD_IMPORTD >>>> >>>> http://www.freedesktop.org/software/systemd/man/machinectl.html#Image%20Transfer%20Commands >>>> >>>> +config BR2_PACKAGE_SYSTEMD_CATALOGDB >>>> + bool "enable journal catalog database installation" >>>> + depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW # conflicting >>>> tmpfiles magic >>>> + help >>>> + Build and install the journal catalog database. >>>> + >>>> + catalog files are used to provide extended and potentially localized >>>> + messages for the journal. >>>> + >>>> + The original catalog files will be built into a DB at >>>> + /usr/share/factory/var/lib/systemd/catalog/database. >>>> + >>>> + https://www.freedesktop.org/wiki/Software/systemd/catalog/ >>>> + >>>> config BR2_PACKAGE_SYSTEMD_LOCALED >>>> bool "enable locale daemon" >>>> help >>>> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk >>>> index cb12f667d6..ae99c02abf 100644 >>>> --- a/package/systemd/systemd.mk >>>> +++ b/package/systemd/systemd.mk >>>> @@ -613,6 +613,47 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD >>>> $(SYSTEMD_INSTALL_NETWORK_CONFS) >>>> endef >>>> >>>> +ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) >>>> +define SYSTEMD_LOCALE_PURGE_CATALOGS >>>> + # go through all files with scheme <basename>.<langext>.catalog >>>> + # and remove those where <langext> is not in LOCALE_NOPURGE >>>> + for cfile in `find $(TARGET_DIR)/usr/lib/systemd/catalog -name >>>> '*.*.catalog'`; \ >>>> + do \ >>>> + basename=$${cfile##*/}; \ >>>> + basename=$${basename%.catalog}; \ >>>> + langext=$${basename#*.}; \ >>>> + [ "$$langext" = "$${basename}" ] && continue; \ >>>> + expr '$(LOCALE_NOPURGE)' : ".*\b$${langext}\b" >/dev/null && >>>> continue; \ >>>> + rm -f "$$cfile"; \ >>>> + done >>>> +endef >>>> + >>>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_LOCALE_PURGE_CATALOGS >>>> +endif >>>> + >>>> +ifeq ($(BR2_PACKAGE_SYSTEMD_CATALOGDB),y) >>>> +define SYSTEMD_UPDATE_CATALOGS >>>> + $(HOST_DIR)/bin/journalctl --root=$(TARGET_DIR) --update-catalog >>>> + install -D $(TARGET_DIR)/var/lib/systemd/catalog/database \ >>>> + $(TARGET_DIR)/usr/share/factory/var/lib/systemd/catalog/database >>>> + rm $(TARGET_DIR)/var/lib/systemd/catalog/database >>>> + ln -sf /usr/share/factory/var/lib/systemd/catalog/database \ >>>> + $(TARGET_DIR)/var/lib/systemd/catalog/database >>>> + grep -q '^L /var/lib/systemd/catalog/database' >>>> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf || \ >>>> + printf "\nL /var/lib/systemd/catalog/database\n" >> >>>> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf >>>> +endef >>>> + >>>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_UPDATE_CATALOGS >>>> +endif >>>> + >>>> +define SYSTEMD_RM_CATALOG_UPDATE_SERVICE >>>> + rm -rf $(TARGET_DIR)/usr/lib/systemd/catalog \ >>>> + >>>> $(TARGET_DIR)/usr/lib/systemd/system/systemd-journal-catalog-update.service \ >>>> + $(TARGET_DIR)/usr/lib/systemd/system/*/systemd-journal-catalog-update.service >>>> +endef >>>> + >>>> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE >>>> + >>>> define SYSTEMD_PRESET_ALL >>>> $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all >>>> endef >>>> >> >> Norbert >> >> [2] - >> https://lore.kernel.org/buildroot/1d09c49a-6d08-1cd7-9785-92b56c17ae06@mind.be/ >>
diff --git a/package/systemd/Config.in b/package/systemd/Config.in index ec34478e3d..d576e6035b 100644 --- a/package/systemd/Config.in +++ b/package/systemd/Config.in @@ -269,6 +269,20 @@ config BR2_PACKAGE_SYSTEMD_IMPORTD http://www.freedesktop.org/software/systemd/man/machinectl.html#Image%20Transfer%20Commands +config BR2_PACKAGE_SYSTEMD_CATALOGDB + bool "enable journal catalog database installation" + depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW # conflicting tmpfiles magic + help + Build and install the journal catalog database. + + catalog files are used to provide extended and potentially localized + messages for the journal. + + The original catalog files will be built into a DB at + /usr/share/factory/var/lib/systemd/catalog/database. + + https://www.freedesktop.org/wiki/Software/systemd/catalog/ + config BR2_PACKAGE_SYSTEMD_LOCALED bool "enable locale daemon" help diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk index cb12f667d6..ae99c02abf 100644 --- a/package/systemd/systemd.mk +++ b/package/systemd/systemd.mk @@ -613,6 +613,47 @@ define SYSTEMD_INSTALL_INIT_SYSTEMD $(SYSTEMD_INSTALL_NETWORK_CONFS) endef +ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) +define SYSTEMD_LOCALE_PURGE_CATALOGS + # go through all files with scheme <basename>.<langext>.catalog + # and remove those where <langext> is not in LOCALE_NOPURGE + for cfile in `find $(TARGET_DIR)/usr/lib/systemd/catalog -name '*.*.catalog'`; \ + do \ + basename=$${cfile##*/}; \ + basename=$${basename%.catalog}; \ + langext=$${basename#*.}; \ + [ "$$langext" = "$${basename}" ] && continue; \ + expr '$(LOCALE_NOPURGE)' : ".*\b$${langext}\b" >/dev/null && continue; \ + rm -f "$$cfile"; \ + done +endef + +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_LOCALE_PURGE_CATALOGS +endif + +ifeq ($(BR2_PACKAGE_SYSTEMD_CATALOGDB),y) +define SYSTEMD_UPDATE_CATALOGS + $(HOST_DIR)/bin/journalctl --root=$(TARGET_DIR) --update-catalog + install -D $(TARGET_DIR)/var/lib/systemd/catalog/database \ + $(TARGET_DIR)/usr/share/factory/var/lib/systemd/catalog/database + rm $(TARGET_DIR)/var/lib/systemd/catalog/database + ln -sf /usr/share/factory/var/lib/systemd/catalog/database \ + $(TARGET_DIR)/var/lib/systemd/catalog/database + grep -q '^L /var/lib/systemd/catalog/database' $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf || \ + printf "\nL /var/lib/systemd/catalog/database\n" >> $(TARGET_DIR)/usr/lib/tmpfiles.d/var.conf +endef + +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_UPDATE_CATALOGS +endif + +define SYSTEMD_RM_CATALOG_UPDATE_SERVICE + rm -rf $(TARGET_DIR)/usr/lib/systemd/catalog \ + $(TARGET_DIR)/usr/lib/systemd/system/systemd-journal-catalog-update.service \ + $(TARGET_DIR)/usr/lib/systemd/system/*/systemd-journal-catalog-update.service +endef + +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE + define SYSTEMD_PRESET_ALL $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all endef