Subject: Re: [bug#55242] [PATCH 07/10] guix: import: go: More resilience wrt network errors; add logging.

From: Maxime Devos <[email-masked]>
Date: Mon, 09 May 2022 19:45:07 +0200

Attila Lendvai schreef op ma 09-05-2022 om 12:34 [+0000]:
> [...] does it make enough sense to justify my proposed behavior?

(For a positive note, skip to the last paragraph!)

If we do something like this (*) (this = catching errors for
dependencies and skipping those packages), I don't want this to be
something Go-specific.  Could this be done in 'recursive-import' or
something like that instead, such that all importers benefit and Go
isn't some weird special case?

Also, given that we appear to have a number of people effectively
maintaining the Go importer ("git log guix/import.go.scm" mentions a
few ‘Maxim Cournoyer’, ‘Ludovic Courtès’ ‘Xinglu Chen’ and ‘Sarah
Morgensen’, and more recently maybe you?) and I don't see any open old
Go importer bugs/patches on issues.guix.gnu.org, I don't think we have
to worry about

> realistically? because there are years old issues, and long bitrotten
> patches in every project's issue tracker, Guix included.

in this case.

> short of hard data, this can be argued forever. my intuition is
> clearly pointing towards 2), because i doubt that the choking point
> of improving the importer is due to not having enough examples on
> which it breaks.

If you know of some examples on which the Go importer broke, could you
add them to tests/go, to make sure it won't break again later
(regression), in case of future changes?

Anyway, let's take a look at for which kind of things I think of in
case of ‘buggy importer’:

1. (Maybe [PATCH 04/10]?)  The parser for a file format cannot parse 
   <something>, so it throws an exception.

   (I consider this more a ‘limitation’ than a ‘bug’.)

   For this kind of thing, maybe we can just throw a ‘oops I don't
   understand the file format’ exception, report an appropriate error
   message (package name + maybe line number, but no need for a
   backtrace), skip that package and continue.  That way, the Go
   importer will be to some degree robust to future changes.

   That's already done in this patch series, except for the ‘no need
   for a backtrace’ part and this patch series catching all exceptions
   instead of just networking errors, ‘I don't understand the file
   format’ errors, ...

2. The Go importer misinterprets some kind of field or such.

   These kind of things don't (typically?) cause errors, so skipping.
   (Still a bug though!)

3. The Go importer cannot produce a package definition because it
   doesn't support <some Go packaging method>.

   Similar (1).

4. The Go importer has some kind of type error or such.

   This case looks like an actual bug to me, where I think that
   catching the resulting exceptions is just sweeping things under
   the rug, implicitly teaching other people that it's ok to just
   ignore problems without making any attempt of fixing them or
   reporting them.  Vaguely similar to ‘broken windows’
   (<https://en.wikipedia.org/wiki/Broken_windows_theory>).

   Sure, for a single particular user and short term this makes the
   importer a bit less useful, but knowing bugs is a prerequisite for
   fixing them, and long term, by actually knowing about the bugs and
   fixing them, more users will benefit.

That said, like "guix build" has a "--keep-going" option to temporarily
ignore to some degree packages that fail to build, maybe "guix import"
(when using --recursive) can have a "--keep-going" option.

Greetings,
Maxime.