Error message for missing synopsis in opam importer

Hello,

I triggered a confusing behaviour from the opam importer trying to

import package iter 1.2.1 today on a Guix System install.

The package iter is missing a "synopsis" field as can be seen on

https://opam.ocaml.org/packages/iter/, which when I tried

guix import opam iter

yielded the following backtrace:

Backtrace:

8 (primitive-load "/home/alice/.config/guix/current/bin/g…")

In guix/ui.scm:

2185:7 7 (run-guix . _)

2148:10 6 (run-guix-command _ . _)

In guix/scripts/import.scm:

120:11 5 (guix-import . _)

In guix/scripts/import/opam.scm:

104:23 4 (guix-import-opam . _)

In guix/utils.scm:

752:8 3 (call-with-temporary-output-file _)

In guix/import/opam.scm:

337:34 2 (_ _ _)

In srfi/srfi-1.scm:

460:18 1 (fold #<procedure 7f3baca56fe0 at guix/import/opam.scm…> …)

In guix/import/opam.scm:

193:15 0 (_ _ _)

guix/import/opam.scm:193:15: Throw to key `match-error' with args

`("match" "no matching pattern" string-pat)'.

the final error is raised l.193 of guix/import/opam.scm because

metadata-ref supports various types for a metadata field, but not the

lack of it. As discussed with Maxime Devos on the IRC channel, it would

be helpful to either allow the import of a package with a missing field

(possibly filling it in the output scheme code for the imported package

with some bad value requiring the user to fill it and causing any build

to crash until replaced properly) or at least to handle that missing

field with a more explicit error message than the above backtrace

(something like "Can't import that package because it's missing such or

such field").

Thanks,

Alice BRENON

Hi,

Thanks for the report. I'm CC'ing Simon since they have been working on

improved error handling/reporting for the importers.

Alice BRENON <alice.brenon@ens-lyon.fr> writes:

IMO, a warning should be emitted, but the package should be buildable if

at all possible; it's the submitter's responsibility to vet imported

packages.

Simon, how's that error handling rework coming? ;)

--

Sarah

Hello,

Thanks for your answer Sarah. Simon, I don't know if you have been able

to make any progress but I wanted to make sure you had seen the patch

proposal I sent to let the opam importer work from more repositories

than the few initially defined (opam's official and three for coq):

Though I had a local "bypass" on the metadata reader to be able to

perform the import I wanted and orginally designed my patch for, I paid

attention not to commit it to keep matters separated. Any insight on the

general form the improved error handling will take ? Please let me

know if I can update my #49958 patch to play along more nicely with

your rework.

Alice

Le Mon, 02 Aug 2021 12:28:20 -0700,

Sarah Morgensen <iskarian@mgsn.dev> a écrit :

Hi,

I am back from holidays. :-)

On Wed, 11 Aug 2021 at 15:15, Alice BRENON <alice.brenon@ens-lyon.fr> wrote:

I have seen but not read in details. I will do.

From my understanding, there is 2 issues:

- gentle handler for error

- warn for incomplete metadata

With Jérémy (jeko), we have started to work time to time using

experimental pair-programming to fix the former. Currently, each

importer uses its own error mechanism and obviously incoherence between

them happens; especially when ’--recursive’. We are trying to unify

that.

Thanks for the report of this use case. :-)

Cheers,

simon

Hello,

Thanks for your answer !

Le Tue, 17 Aug 2021 09:43:10 +0200,

zimoun <zimon.toutoune@gmail.com> a écrit :

Yes, absolutely, because currently understanding the cause of the error

requires to delve into the source to understand what is going on. The

warning part is more optional, but if this pattern matching is modified

to handle that special case of a missing metadata instead of entirely

crashing, I thought it could be useful not to be too permissive either,

and to at least mention that a missing metadata was caught and should

be filled by hand.

This could take the form of a message above the output of the actual

scheme code for the package declaration while the importer is running,

or of an invalid value for that missing field in the generated scheme

output, something like "<FILL-ME>" or such that would be invalid in

scheme and would make guix build fail when trying to use the output

directly without manually editing it to fill the missing metadata.

Glad to learn my report could help : )

Hi,

First, today when running guix import opam iter, I get a synopsis, and

#f as the description because the field is missing.

I also pushed a small patch to master, as

24aa7b3c21309b63cc6e8e18d6417d2cddccf6c6, that ensures that, when the

field exists but contains unknown data, we also return #f instead of a

match error that produces the ugly backtrace you saw.

Thanks for the report, closing :)