public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] cpupower: add checks for xgettext and msgfmt
@ 2024-10-17 13:32 Siddharth Menon
  2024-10-17 16:04 ` Shuah Khan
  0 siblings, 1 reply; 3+ messages in thread
From: Siddharth Menon @ 2024-10-17 13:32 UTC (permalink / raw)
  To: shuah, trenn, jwyatt, jkacur; +Cc: Siddharth Menon, linux-pm

Check whether xgettext and msgfmt are available on the system before
attempting to  generate GNU gettext Language Translations.
In case of missing dependency, generate warning message directing user
to install the necessary package.

Signed-off-by: Siddharth Menon <simeddon@gmail.com>
---
 v1->v2:
 - Checks for gettext tools outside the target definitions instead
 of inline
 - Replace command with which
 v2->v3:
 - Update commit message
 
 tools/power/cpupower/Makefile | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/power/cpupower/Makefile b/tools/power/cpupower/Makefile
index 6c02f401069e..84987f91d11f 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 extract translatable strings.")
+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 binary message catalogs.")
+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 merge 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)
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] cpupower: add checks for xgettext and msgfmt
  2024-10-17 13:32 [PATCH v3] cpupower: add checks for xgettext and msgfmt Siddharth Menon
@ 2024-10-17 16:04 ` Shuah Khan
  2024-10-17 19:52   ` John B. Wyatt IV
  0 siblings, 1 reply; 3+ messages in thread
From: Shuah Khan @ 2024-10-17 16:04 UTC (permalink / raw)
  To: Siddharth Menon, shuah, trenn, jwyatt, jkacur; +Cc: linux-pm, Shuah Khan

On 10/17/24 07:32, Siddharth Menon wrote:
> Check whether xgettext and msgfmt are available on the system before
> attempting to  generate GNU gettext Language Translations.
> In case of missing dependency, generate warning message directing user
> to install the necessary package.
> 
> Signed-off-by: Siddharth Menon <simeddon@gmail.com>
> ---
>   v1->v2:
>   - Checks for gettext tools outside the target definitions instead
>   of inline
>   - Replace command with which
>   v2->v3:
>   - Update commit message

Looks like there is more than just updating commit log
in this version. You fixed a problem in your v2. :)

Also when someone suggests an approach and give you a
diff, the practice is to add Suggested-by

John, Do you have time to test this? Asking since you
tested the previous version. Just want to make sure
this one works as well.

>   
>   tools/power/cpupower/Makefile | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/tools/power/cpupower/Makefile b/tools/power/cpupower/Makefile
> index 6c02f401069e..84987f91d11f 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 extract translatable strings.")
> +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 binary message catalogs.")
> +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 merge 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)

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] cpupower: add checks for xgettext and msgfmt
  2024-10-17 16:04 ` Shuah Khan
@ 2024-10-17 19:52   ` John B. Wyatt IV
  0 siblings, 0 replies; 3+ messages in thread
From: John B. Wyatt IV @ 2024-10-17 19:52 UTC (permalink / raw)
  To: Shuah Khan; +Cc: Siddharth Menon, shuah, trenn, jkacur, linux-pm

On Thu, Oct 17, 2024 at 10:04:29AM -0600, Shuah Khan wrote:
> On 10/17/24 07:32, Siddharth Menon wrote:
> > Check whether xgettext and msgfmt are available on the system before
> > attempting to  generate GNU gettext Language Translations.
> > In case of missing dependency, generate warning message directing user
> > to install the necessary package.
> > 
> > Signed-off-by: Siddharth Menon <simeddon@gmail.com>
> > ---
> >   v1->v2:
> >   - Checks for gettext tools outside the target definitions instead
> >   of inline
> >   - Replace command with which
> >   v2->v3:
> >   - Update commit message
> 
> Looks like there is more than just updating commit log
> in this version. You fixed a problem in your v2. :)
> 
> Also when someone suggests an approach and give you a
> diff, the practice is to add Suggested-by
> 
> John, Do you have time to test this? Asking since you
> tested the previous version. Just want to make sure
> this one works as well.
> 

Yes I do, all 3 messages only appear once when the gettext pkg is not
installed on a Fedora 40 server using the make command. When it is
installed none of the messages show. Works as expected.

Siddharth, assuming no other changes. :) Please include my Tested-by
tags so others know your code was tested in your V4 along with Shuah's
Suggested-by and document the clean up of V3's msgmerge.

Tested-by: John B. Wyatt IV <jwyatt@redhat.com>
Tested-by: John B. Wyatt IV <sageofredondo@gmail.com>

> thanks,
> -- Shuah
> 

-- 
Sincerely,
John Wyatt
Software Engineer, Core Kernel
Red Hat


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-10-17 19:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-17 13:32 [PATCH v3] cpupower: add checks for xgettext and msgfmt Siddharth Menon
2024-10-17 16:04 ` Shuah Khan
2024-10-17 19:52   ` John B. Wyatt IV

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox