unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#47882] [PATCH] Makefile: Reimplement download-po target
@ 2021-04-18 22:09 Julien Lepiller
  2021-04-30 19:57 ` Ludovic Courtès
  0 siblings, 1 reply; 4+ messages in thread
From: Julien Lepiller @ 2021-04-18 22:09 UTC (permalink / raw)
  To: 47882

[-- Attachment #1: Type: text/plain, Size: 2207 bytes --]

Hi Guix!

The attached patch reimplements the download-po target in Makefile.am.
The previous implementation was based on the implementation for the TP
and would use the API of Weblate to get files.

However, the rate-limit is very close to the number of files we need to
download, which was a big limitation, and this target would only
download existing files, ignoring new translations.

With this, new translations and existing ones are added/updated. The
target will first do a shallow clone of the repository behind
Weblate. Then, it will check that files are not empty (contain at least
one translated string) and correct (according to scheme-format at
least). If they are correct, it will normalize them and copy them to
the po/ subdir in the repository.

Here is an example output:

copied po/doc/guix-manual.ru.po.
/tmp/tmp.dWvtATfWQA/translations/po/doc/guix-manual.si.po:6:
AVERTISSEMENT : Le champ d'en-tête « PO-Revision-Date » a encore sa
valeur initiale par défaut
WARN: po/doc/guix-manual.si.po (0 translated messages) was not
added/updated.
/tmp/tmp.dWvtATfWQA/translations/po/guix/nl.po:5135: Les spécifications
de format dans « msgid_plural » et « msgstr[1] » ne sont pas
équivalentes
msgfmt: 1 erreur fatale trouvée
WARN: po/guix/nl.po (1001 translated messages) was not added/updated.

In this output, we can see that the Russian manual was copied, the
Sinhala manual was empty, so not copied, and the Dutch translation had
an issue in one string that needs to be fixed, and was not copied.

I chose not to copy the file because an issue in msgfmt would break
guix pull. Unfortunately, this is not enough testing, as the manual and
cookbook are not checked. Checking them requires building the manual,
so before commiting changes, please run "make".

To document the new process:

make download-po
for any new file in po/guix and po/packages, add the language in a
single line in po/guix/LINGUAS or po/packages/LINGUAS.
for any new file in po/doc, add the file name to po/doc/local.mk, in
DOC_PO_FILES or DOC_COOKBOOK_PO_FILES depending on the translation
type. Add the texi file name to doc/local.mk too, in info_TEXINFOS.

[-- Attachment #2: 0001-Makefile-Reimplement-download-po-target.patch --]
[-- Type: text/x-patch, Size: 4276 bytes --]

From 5c14506d6af24b5307e03604cabf8ca10af56067 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Sun, 18 Apr 2021 23:56:48 +0200
Subject: [PATCH] Makefile: Reimplement `download-po` target.

The weblate API rate limit is very close to the number of files we need
to download.  The previous implementation did not add new translations.

* Makefile.am (download-po): Update target.
(make-download-po, make-check-po): Remove functions.
---
 Makefile.am | 87 ++++++++++++++---------------------------------------
 1 file changed, 22 insertions(+), 65 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 17ad236655..811e47dbfc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -969,71 +969,28 @@ cuirass-jobs: $(GOBJECTS)
 
 # Downloading up-to-date PO files.
 
-# make-download-po-rule DOMAIN DIRECTORY [FILE-NAME-PREFIX]
-define make-download-po-rule
-
-download-po.$(1):
-	if [ -f "$(top_srcdir)/$(2)/LINGUAS" ]; then				\
-	  LINGUAS="`grep -v '^[[:blank:]]*#' < $(top_srcdir)/$(2)/LINGUAS`" ;	\
-	else									\
-	  LINGUAS="`(cd $(top_srcdir)/$(2);					\
-	    for i in *.po; do echo $$$$i; done) | cut -d . -f 2`" ;		\
-	fi ;									\
-	for lang in $$$$LINGUAS; do						\
-	  if wget -nv -O "$(top_srcdir)/$(2)/$(3)$$$$lang.po.tmp"		\
-	     "https://translate.fedoraproject.org/api/translations/guix/$(1)/$$$$lang/file/" ; \
-	  then									\
-	    msgfilter --no-wrap -i "$(top_srcdir)/$(2)/$(3)$$$$lang.po.tmp"	\
-	      cat > "$(top_srcdir)/$(2)/$(3)$$$$lang.po.tmp2" ;			\
-	    rm "$(top_srcdir)/$(2)/$(3)$$$$lang.po.tmp" ;			\
-	    mv "$(top_srcdir)/$(2)/$(3)$$$$lang.po"{.tmp2,} ;			\
-	  else									\
-	    rm "$(top_srcdir)/$(2)/$(3)$$$$lang.po.tmp" ;			\
-	  fi ;									\
-	done
-
-.PHONY: download-po.$(1)
-
-endef
-
-# Checking po files for issues.  This is useful to run after downloading new
-# po files.
-
-# make-check-po-rule DOMAIN DIRECTORY [FILE-NAME-PREFIX]
-define make-check-po-rule
-
-check-po.$(1):
-	if [ -f "$(top_srcdir)/$(2)/LINGUAS" ]; then				\
-	  LINGUAS="`grep -v '^[[:blank:]]*#' < $(top_srcdir)/$(2)/LINGUAS`" ;	\
-	else									\
-	  LINGUAS="`(cd $(top_srcdir)/$(2);					\
-	    for i in *.po; do echo $$$$i; done) | cut -d . -f 2`" ;		\
-	fi ;									\
-	for lang in $$$$LINGUAS; do						\
-	  if [ -f "$(top_srcdir)/$(2)/$(3)$$$$lang.po" ];			\
-	  then									\
-	    if ! msgfmt -c "$(top_srcdir)/$(2)/$(3)$$$$lang.po" ;		\
-		then								\
-		  exit 1 ;							\
-	    fi ;								\
-	  fi ;									\
-	done
-
-.PHONY: check-po.$(1)
-
-endef
-
-$(eval $(call make-download-po-rule,documentation-cookbook,po/doc,guix-cookbook.))
-$(eval $(call make-download-po-rule,documentation-manual,po/doc,guix-manual.))
-$(eval $(call make-download-po-rule,guix,po/guix))
-$(eval $(call make-download-po-rule,packages,po/packages))
-
-$(eval $(call make-check-po-rule,documentation-cookbook,po/doc,guix-cookbook.))
-$(eval $(call make-check-po-rule,documentation-manual,po/doc,guix-manual.))
-$(eval $(call make-check-po-rule,guix,po/guix))
-$(eval $(call make-check-po-rule,packages,po/packages))
-
-download-po: $(foreach domain,guix packages documentation-manual documentation-cookbook,download-po.$(domain))
+WEBLATE_REPO="https://framagit.org/tyreunom/guix-translations"
+
+# shallow clone the git repository behind weblate and copy files from it if
+# they contain at least one translation, and they are well-formed (scheme format
+# only), warn otherwise.  Copied files are converted to a canonical form.
+download-po:
+	dir=$$(mktemp -d); \
+	git clone --depth 1 "$(WEBLATE_REPO)" "$$dir/translations"; \
+	for domain in po/doc po/guix po/packages; do \
+		for po in "$$dir/translations/$$domain"/*.po; do \
+			translated=$$(LANG=en_US.UTF-8 msgfmt --statistics "$$po" 2>&1 | cut -f1 -d' '); \
+			target=$$(basename "$$po"); \
+			target="$$domain/$$target"; \
+			if msgfmt -c "$$po" && [ "$$translated" != "0" ]; then \
+				msgfilter --no-wrap -i "$$po" cat > "$$target"; \
+				echo "copied $$target."; \
+			else \
+				echo "WARN: $$target ($$translated translated messages) was not added/updated."; \
+			fi; \
+		done; \
+	done; \
+	rm -rf "$$dir"
 .PHONY: download-po
 
 check-po: $(foreach domain,guix packages documentation-manual documentation-cookbook,check-po.$(domain))
-- 
2.31.1


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

* [bug#47882] [PATCH] Makefile: Reimplement download-po target
  2021-04-18 22:09 [bug#47882] [PATCH] Makefile: Reimplement download-po target Julien Lepiller
@ 2021-04-30 19:57 ` Ludovic Courtès
  2021-05-01 14:42   ` bug#47882: " Julien Lepiller
  0 siblings, 1 reply; 4+ messages in thread
From: Ludovic Courtès @ 2021-04-30 19:57 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: 47882

Hi!

Julien Lepiller <julien@lepiller.eu> skribis:

> The attached patch reimplements the download-po target in Makefile.am.
> The previous implementation was based on the implementation for the TP
> and would use the API of Weblate to get files.
>
> However, the rate-limit is very close to the number of files we need to
> download, which was a big limitation, and this target would only
> download existing files, ignoring new translations.

OK.

> To document the new process:
>
> make download-po
> for any new file in po/guix and po/packages, add the language in a
> single line in po/guix/LINGUAS or po/packages/LINGUAS.
> for any new file in po/doc, add the file name to po/doc/local.mk, in
> DOC_PO_FILES or DOC_COOKBOOK_PO_FILES depending on the translation
> type. Add the texi file name to doc/local.mk too, in info_TEXINFOS.

Alright.  This looks like a nice improvement!  Some nitpicking follows,
but it LGTM overall.

>>From 5c14506d6af24b5307e03604cabf8ca10af56067 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Sun, 18 Apr 2021 23:56:48 +0200
> Subject: [PATCH] Makefile: Reimplement `download-po` target.
>
> The weblate API rate limit is very close to the number of files we need
> to download.  The previous implementation did not add new translations.
>
> * Makefile.am (download-po): Update target.
> (make-download-po, make-check-po): Remove functions.

Should be: (make-download-po-rule, make-check-po-rule)

> -	    rm "$(top_srcdir)/$(2)/$(3)$$$$lang.po.tmp" ;			\
> -	    mv "$(top_srcdir)/$(2)/$(3)$$$$lang.po"{.tmp2,} ;			\

That was kinda crazy, I’m glad you divided the number of dollars by
two.  :-)

> +WEBLATE_REPO="https://framagit.org/tyreunom/guix-translations"

Since it’s not shell, rather:

  WEBLATE_REPO = https://framagit.org/tyreunom/guix-translations

BTW, I vaguely remember we asked for a Savannah repo but I’m not sure
what the outcome was; did it eventually stall for some obscure reason?

> +# shallow clone the git repository behind weblate and copy files from it if
> +# they contain at least one translation, and they are well-formed (scheme format
> +# only), warn otherwise.  Copied files are converted to a canonical form.

Please capitalize sentences and proper names.

> +download-po:
> +	dir=$$(mktemp -d); \
> +	git clone --depth 1 "$(WEBLATE_REPO)" "$$dir/translations"; \
> +	for domain in po/doc po/guix po/packages; do \
> +		for po in "$$dir/translations/$$domain"/*.po; do \
> +			translated=$$(LANG=en_US.UTF-8 msgfmt --statistics "$$po" 2>&1 | cut -f1 -d' '); \
> +			target=$$(basename "$$po"); \
> +			target="$$domain/$$target"; \
> +			if msgfmt -c "$$po" && [ "$$translated" != "0" ]; then \
> +				msgfilter --no-wrap -i "$$po" cat > "$$target"; \

Maybe write to $$target.tmp and then do “mv $$target.tmp $$target” so
it’s atomic and we don’t end up with corrupt files if ‘msgfilter’ fails.

I would use spaces rather than tabs to indent the ‘for’ loops.

Thanks!

Ludo’.




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

* bug#47882: [PATCH] Makefile: Reimplement download-po target
  2021-04-30 19:57 ` Ludovic Courtès
@ 2021-05-01 14:42   ` Julien Lepiller
  2021-05-01 15:08     ` [bug#47882] " Ludovic Courtès
  0 siblings, 1 reply; 4+ messages in thread
From: Julien Lepiller @ 2021-05-01 14:42 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 47882-done

Le Fri, 30 Apr 2021 21:57:05 +0200,
Ludovic Courtès <ludo@gnu.org> a écrit :

> Hi!
> 
> Alright.  This looks like a nice improvement!  Some nitpicking
> follows, but it LGTM overall.

Thank you! I fixed your comments and pushed to master as
45549e9f31e462d0d1519fd678dd782da6ed5cb9. Thank you!

> BTW, I vaguely remember we asked for a Savannah repo but I’m not sure
> what the outcome was; did it eventually stall for some obscure reason?

Yeah, there were still some issues related to copyright. To be honest I
lost my energy to push on this front...




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

* [bug#47882] [PATCH] Makefile: Reimplement download-po target
  2021-05-01 14:42   ` bug#47882: " Julien Lepiller
@ 2021-05-01 15:08     ` Ludovic Courtès
  0 siblings, 0 replies; 4+ messages in thread
From: Ludovic Courtès @ 2021-05-01 15:08 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: 47882-done

Julien Lepiller <julien@lepiller.eu> skribis:

> Le Fri, 30 Apr 2021 21:57:05 +0200,

[...]

>> Alright.  This looks like a nice improvement!  Some nitpicking
>> follows, but it LGTM overall.
>
> Thank you! I fixed your comments and pushed to master as
> 45549e9f31e462d0d1519fd678dd782da6ed5cb9. Thank you!

Cool!

>> BTW, I vaguely remember we asked for a Savannah repo but I’m not sure
>> what the outcome was; did it eventually stall for some obscure reason?
>
> Yeah, there were still some issues related to copyright. To be honest I
> lost my energy to push on this front...

Yeah, I sympathize.  It’s fine and more productive to use the repo you
set up.

Thank you!

Ludo’.




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

end of thread, other threads:[~2021-05-01 15:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-18 22:09 [bug#47882] [PATCH] Makefile: Reimplement download-po target Julien Lepiller
2021-04-30 19:57 ` Ludovic Courtès
2021-05-01 14:42   ` bug#47882: " Julien Lepiller
2021-05-01 15:08     ` [bug#47882] " Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).