| Message ID | 20241015163854.35204-1-simeddon@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Shuah Khan |
| Headers | show |
| Series | cpupower: add checks for xgettext and msgfmt | expand |
On Tue, Oct 15, 2024 at 10:08:54PM +0530, Siddharth Menon wrote: > Check whether xgettext and msgfmt are available on the system before > attempting to generate the .pot and .gmo files and generate. > In case of missing dependency, generate error message directing user > to install the necessary package. Thank you for your patch. Tried a quick few tests by running `make` without gettext installed. > $(OUTPUT)po/$(PACKAGE).pot: $(UTIL_SRC) > $(ECHO) " GETTEXT " $@ > - $(QUIET) xgettext --default-domain=$(PACKAGE) --add-comments \ > - --keyword=_ --keyword=N_ $(UTIL_SRC) -p $(@D) -o $(@F) > + $(QUIET) if ! command -v xgettext > /dev/null; then \ > + echo "Error: gettext not found. Please install gettext."; \ > + else \ > + xgettext --default-domain=$(PACKAGE) --add-comments \ > + --keyword=_ --keyword=N_ $(UTIL_SRC) -p $(@D) -o $(@F); \ > + fi I tested this on a Fedora 40 server. I was not able to get this message to print despite xgettext not being in the path. > > $(OUTPUT)po/%.gmo: po/%.po > $(ECHO) " MSGFMT " $@ > - $(QUIET) msgfmt -o $@ po/$*.po > + $(QUIET) if ! command -v msgfmt > /dev/null; then \ > + echo "Error: msgfmt not found. Make sure gettext is set up correctly."; \ > + else \ > + msgfmt -o $@ po/$*.po; \ > + fi I was able to get this to print without the gettext pkg being installed and it did not print with it installed. This ran fine.
On Tue, 15 Oct 2024 at 23:19, John B. Wyatt IV <jwyatt@redhat.com> wrote: > > I tested this on a Fedora 40 server. I was not able to get this message to > print despite xgettext not being in the path. Thank you for testing out my patch. The .pot file for cpupower is required in order to enter this code block. Should I send in another patch to generate cpupower.pot in the makefile itself? Resending my previous response as I accidentally left text-only mode disabled. -- Sincerely, Siddharth Menon
On 10/15/24 15:55, Sid wrote: > On Tue, 15 Oct 2024 at 23:19, John B. Wyatt IV <jwyatt@redhat.com> wrote: >> >> I tested this on a Fedora 40 server. I was not able to get this message to >> print despite xgettext not being in the path. > Thank you for testing out my patch. The .pot file for cpupower is required > in order to enter this code block. > > Should I send in another patch to generate cpupower.pot in the makefile > itself? > > Resending my previous response as I accidentally left text-only mode > disabled. > As mentioned earlier multiple error messages clutter the output. How about the following simpler fix: =========================================================================== diff --git a/tools/power/cpupower/Makefile b/tools/power/cpupower/Makefile index 6c02f401069e..d1171385b552 100644 --- a/tools/power/cpupower/Makefile +++ b/tools/power/cpupower/Makefile @@ -218,17 +218,28 @@ else endif $(QUIET) $(STRIPCMD) $@ +ifeq (, $(shell which xgettext)) +$(warning "Install xgettext to generate GNU gettext Language Translations") +else $(OUTPUT)po/$(PACKAGE).pot: $(UTIL_SRC) $(ECHO) " GETTEXT " $@ $(QUIET) xgettext --default-domain=$(PACKAGE) --add-comments \ --keyword=_ --keyword=N_ $(UTIL_SRC) -p $(@D) -o $(@F) +endif +ifeq (, $(shell which msgfmt)) +$(warning "Install msgfmt to generate GNU gettext Language Translations") +else $(OUTPUT)po/%.gmo: po/%.po $(ECHO) " MSGFMT " $@ $(QUIET) msgfmt -o $@ po/$*.po +endif create-gmo: ${GMO_FILES} +ifeq (, $(shell which msgmerge)) +$(warning "Install msgmerge to generate GNU gettext Language Translations") +else update-po: $(OUTPUT)po/$(PACKAGE).pot $(ECHO) " MSGMRG " $@ $(QUIET) @for HLANG in $(LANGUAGES); do \ @@ -241,6 +252,7 @@ update-po: $(OUTPUT)po/$(PACKAGE).pot rm -f $(OUTPUT)po/$$HLANG.new.po; \ fi; \ done; +endif compile-bench: $(OUTPUT)libcpupower.so.$(LIB_MAJ) @V=$(V) confdir=$(confdir) $(MAKE) -C bench O=$(OUTPUT) ===========================================================================
diff --git a/tools/power/cpupower/Makefile b/tools/power/cpupower/Makefile index 6c02f401069e..38e7daacecf4 100644 --- a/tools/power/cpupower/Makefile +++ b/tools/power/cpupower/Makefile @@ -220,12 +220,20 @@ endif $(OUTPUT)po/$(PACKAGE).pot: $(UTIL_SRC) $(ECHO) " GETTEXT " $@ - $(QUIET) xgettext --default-domain=$(PACKAGE) --add-comments \ - --keyword=_ --keyword=N_ $(UTIL_SRC) -p $(@D) -o $(@F) + $(QUIET) if ! command -v xgettext > /dev/null; then \ + echo "Error: gettext not found. Please install gettext."; \ + else \ + xgettext --default-domain=$(PACKAGE) --add-comments \ + --keyword=_ --keyword=N_ $(UTIL_SRC) -p $(@D) -o $(@F); \ + fi $(OUTPUT)po/%.gmo: po/%.po $(ECHO) " MSGFMT " $@ - $(QUIET) msgfmt -o $@ po/$*.po + $(QUIET) if ! command -v msgfmt > /dev/null; then \ + echo "Error: msgfmt not found. Make sure gettext is set up correctly."; \ + else \ + msgfmt -o $@ po/$*.po; \ + fi create-gmo: ${GMO_FILES}
Check whether xgettext and msgfmt are available on the system before attempting to generate the .pot and .gmo files and generate. In case of missing dependency, generate error message directing user to install the necessary package. Signed-off-by: Siddharth Menon <simeddon@gmail.com> --- tools/power/cpupower/Makefile | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)