GNU bug report logs - #47882
[PATCH] Makefile: Reimplement download-po target

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47882 in the body.
You can then email your comments to 47882 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: guix-patches <at> gnu.org
Subject: [PATCH] Makefile: Reimplement download-po target
Date: Mon, 19 Apr 2021 00:09:58 +0200

[Message part 1 (text/plain, inline)]

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.

[0001-Makefile-Reimplement-download-po-target.patch (text/x-patch, attachment)]

Message #8 received at 47882 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Julien Lepiller <julien <at> lepiller.eu>
Cc: 47882 <at> debbugs.gnu.org
Subject: Re: bug#47882: [PATCH] Makefile: Reimplement download-po target
Date: Fri, 30 Apr 2021 21:57:05 +0200

Hi!

Julien Lepiller <julien <at> 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 <at> 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’.

Message #13 received at 47882-done <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 47882-done <at> debbugs.gnu.org
Subject: Re: bug#47882: [PATCH] Makefile: Reimplement download-po target
Date: Sat, 1 May 2021 16:42:01 +0200

Le Fri, 30 Apr 2021 21:57:05 +0200,
Ludovic Courtès <ludo <at> 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...

Message #16 received at 47882-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Julien Lepiller <julien <at> lepiller.eu>
Cc: 47882-done <at> debbugs.gnu.org
Subject: Re: bug#47882: [PATCH] Makefile: Reimplement download-po target
Date: Sat, 01 May 2021 17:08:24 +0200

Julien Lepiller <julien <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.