Re: [PATCH 1/2] bootstrap: Break automake dependency on generated files. (was Re: Let’s translate!)

[Ludovic Courtès]

Hi,

Julien Lepiller <address@hidden> scribes:

> From 77c33ee55115475f582eb49da8dc045432fbdb3b Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <address@hidden>
> Date: Fri, 26 Apr 2019 14:54:52 +0200
> Subject: [PATCH] self: Rebuild translated manuals.
>
> * guix/self.scm (info-manual): Run po4a and related commands to generate
> translated texi files before building translated manuals.
> * guix/build/po.scm: New file.
> * Makefile.am (MODULES): Add it.

I would put po.scm in MODULES_NOT_COMPILED.

> +(define (read-po-file port)
> +  "Read a .po file from PORT and returns an alist of msgid and msgstr."
                                          ^
“return”

> +(define (translate-texi-manuals source)
> +  "Retrun the translated texinfo manuals built from SOURCE"
         ^                                                   ^
Typos.  :-)

> +          (define (translate-tmp-texi po source tmp-name)
> +            (invoke #+(file-append po4a "/bin/po4a-translate")
> +              "-M" "UTF-8" "-L" "UTF-8" "-k" "0" "-f" "texinfo"
> +              "-m" source "-p" po "-l" tmp-name))

I’d remove all the ‘tmp-’ in here since they don’t add anything, and I’d
suggest adding a comment below the ‘define’, like:

  ;; Translate Texinfo file SOURCE using messages from PO, and write the
  ;; result to OUTPUT.

> +          (define (make-ref-regex msgid end)
> +            (make-regexp (string-append
> +                           "ref\\{" (string-join (string-split msgid #\ ) "[ 
> \n]+")
> +                           end)))

The thing in the middle should be wrapped like this:

  (regexp-quote (string-join (string-split …)))

> +          (define (translate-cross-reference content translations)
> +            "Take CONTENT, a string representing a .texi file and translate 
> any
> +croos-reference in it (@ref, @xref and @pxref) that have a translation in
     ^^
Typo.

Should be ‘translate-cross-references’ (plural), no?

> +TRANSLATIONS, an alist of msgid and msgstr."
> +                   (if (or (equal? msgstr "")
> +                           (string-any (lambda (chr)
> +                                         (member chr '(#\{ #\} #\( #\) 
> #\newline #\,)))
> +                                       msgid))
> +                     content
> +                     ;; Otherwise, they might be the name of a section, so we
> +                     ;; need to translate any occurence in @(p?x?)ref{...}.
> +                     (let ((regexp1 (make-ref-regex msgid ","))
> +                           (regexp2 (make-ref-regex msgid "\\}")))
> +                       (regexp-substitute/global
> +                         #f regexp2
> +                         (regexp-substitute/global
> +                           #f regexp1 content 'pre "ref{" msgstr "," 'post)
> +                         'pre "ref{" msgstr "}" 'post))))))
> +              content translations))

Please align ‘if’ like so:

  (if condition
      then
      else)

You probably need something like this to add (guix build po) to the
result of ‘guix pull’:

diff --git a/guix/self.scm b/guix/self.scm
index 2a10d1d25f..12bc816fa8 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -578,6 +578,7 @@ Info manual."
                  ;; us to avoid an extra dependency on guile-gdbm-ffi.
                  #:extra-files
                  `(("guix/man-db.scm" ,(local-file "../guix/man-db.scm"))
+                   ("guix/build/po.scm" ,(local-file "../guix/build/po.scm"))
                    ("guix/store/schema.sql"
                     ,(local-file "../guix/store/schema.sql")))
 

Otherwise LGTM!  Please make sure that ‘make as-derivation’ works as
intended (builds the translations, installs (guix build po)) if you
haven’t done it already.

Thanks!

Ludo’.