Subject: Re: bug#53548: [PATCH] Harden beautify-description

From: Ludovic Courtès <[email-masked]>
Date: Tue, 01 Feb 2022 00:09:40 +0100

Hi Alice,

Alice BRENON <[email-masked]> skribis:

> Missing metadata in packages used to break their imports, at least for
> opam packages, until this was fixed by Julien in
> 24aa7b3c21309b63cc6e8e18d6417d2cddccf6c6. When Xinglu improved the
> output of descriptions in 155fc235b5e1b41b4665c782365dd2bf11beae9c, it
> made the imports break again in the case when the `description` field
> is missing and Julien's fix applies, returning #f in `metadata-ref`
> instead of crashing. Trouble is: beautify-description expects its
> description argument to be a string and will crash if this assumption
> isn't met (when calling the `string-prefix?` predicate).

Thanks for explaining!  Some comments:

> From 22f0523ef3599b45af9448bd4f31f7b8f8ce6af2 Mon Sep 17 00:00:00 2001
> From: Alice BRENON <[email-masked]>
> Date: Wed, 26 Jan 2022 09:27:12 +0100
> Subject: [PATCH] guix: import: Harden beautify-description.
>
> * guix/import/utils.scm (beautify-description): Handle non-string
> arguments.
> * guix/import/opam.scm: [use-modules] Make imports explicit for module
> (guix import utils).

[...]

>    #:use-module ((guix utils) #:select (cache-directory
>                                         version>?
>                                         call-with-temporary-output-file))
> -  #:use-module (guix import utils)
> +  #:use-module ((guix import utils) #:select (beautify-description
> +                                              guix-hash-url
> +                                              recursive-import
> +                                              spdx-string->license
> +                                              url-fetch))

It can’t hurt.

> +                  ((not (string? description))
> +                   (let ((home-url "https://guix.gnu.org/")
> +                         (doc-path "fr/manual/devel/en/html_node/")
> +                         (page
> +                           "Synopses-and-Descriptions.html#Synopses-and-Descriptions"))
> +                     (string-append
> +                       "Please fill in the description of your package before "
> +                       "submitting ! See "
> +                       home-url doc-path page)))

I’d avoid the URL and maybe make the string translatable, like so:

  (G_ "This package lacks a description …
Run \"info '(guix) Synopses and Descriptions'\" for more information.")

… where ‘G_’ comes from (guix i18n).

WDYT?

This looks like a welcome improvement to me.

Thanks!

Ludo’.