Subject: Re: [bug#73439] [PATCH v5 1/5] import: Add %libreoffice-updater.

From: Ludovic Courtès <[email-masked]>
Date: Fri, 18 Oct 2024 14:31:47 +0200

Hi,

Nicolas Graves <[email-masked]> skribis:

> Change-Id: I481b1175db531c4fea4a57838fe190f679cd1a85
> ---
>  Makefile.am                 |  1 +
>  guix/import/libreoffice.scm | 98 +++++++++++++++++++++++++++++++++++++

Neat.  Could you add a test, an @item in doc/guix.texi next to the other
updaters, and a commit log?

> +(define (libreoffice-latest-version)

Please add a docstring to all top-level procedures.

> +  (let* ((response port (http-get libreoffice-latest-url
> +                                  #:streaming? #t))
> +         (content (get-string-all port))

PORT is not closed.  If you’re going to load it all in memory, just
leave #:streaming? to #f and you’ll get a string instead of a port.

> +         ;; xml->sxml is not flexible enough for html.
> +         ;; For instance, <img> tags don't have closing </img>.
> +         ;; This trick preprocesses html to extract all <a> tags in
> +         ;; a <body> wrapper, which sxml-match can handle well.
> +         (xml (xml->sxml
> +               (string-append
> +                "<body><"
> +                (string-join
> +                 (filter (cute string-prefix? "a " <>)
> +                         (string-split content #\<))
> +                 "</a><")
> +                "></a></body>")
> +               #:trim-whitespace? #t)))

This is terrible.  :-)

(guix import go) and (guix gnu-maintenance) use (htmlprag) for that.
Would that work for you?

Thinking about it, the strategy looks very similar to that of the
‘generic-html’ updater.

Would it be enough to add a ‘release-monitoring-url’ property to the
relevant LibreOffice packages?

Thanks,
Ludo’.