guix-patches
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Nicolas Graves
Subject: [bug#73439] [PATCH v5 1/5] import: Add %libreoffice-updater.
Date: Fri, 18 Oct 2024 16:29:48 +0200 
On 2024-10-18 14:31, Ludovic Courtès wrote:

> Hi,
>
> Nicolas Graves <ngraves@ngraves.fr> 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?

No issue for that, but I don't get how this works by looking at the
source code only, hence my dirty hacks ;)

> 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?

Actually now I don't really now anymore what was possible and what was
not. I remember trying and successfully updating libreoffice without
this patch once, but not able to reproduce.

With more experimentation :
- the current release-monitoring-url is able to update to the
latest-version, but isn't able to udpate to a --target-version
- same thing for my libreoffice-latest-url

I guess we don't want to maintain this code if we are able to keep being
up-to-date. Let's skip this commit and merge others once QA is done with
them. I can always cherry-pick this patch in the future if I need it.

>
> Thanks,
> Ludo’.
>
>
>

-- 
Best regards,
Nicolas Graves
reply via email to
[Prev in Thread] Current Thread [Next in Thread]