GNU bug report logs - #49196
[PATCH] "guix import go" Improve error handling

Previous Next

Package: guix-patches;

Reported by: Sarah Morgensen <iskarian <at> mgsn.dev>

Date: Wed, 23 Jun 2021 20:50:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Sarah Morgensen <iskarian <at> mgsn.dev>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 49196 <at> debbugs.gnu.org
Subject: [bug#49196] [PATCH] import: utils: 'recursive-import' skips unfound packages
Date: Sat, 26 Jun 2021 21:46:10 -0700
Hi,

Apologies for the delay. Thanks for the work.

zimoun <zimon.toutoune <at> gmail.com> writes:

> -  #:export (go-module->guix-package
> +  #:export (go-module->guix-package*

Would it be better to export both, in case a callee wants access to the
actual errors?

> +(define go-module->guix-package*
> +  (memoize
> +   (lambda args

I would remove the memoize from this method (to put it back in later
on), because multiple invocations with errors would only yield one error
reported. I do not think this makes sense if another tool is using this.

> +               (else
> +                (warning (G_ "Something went wrong with ~s.~%") args)

A catch-all is fine, but we should at least report the actual error,
even if it's not pretty:

    (warning (G_ "Failed to import package ~s.~%  reason: ~s")
                  package-name (exception-args c))

However, if we want to move in the direction of proper error handling
like guix/packages.scm and guix/ui.scm, we can do something like...

--8<---------------cut here---------------start------------->8---
(define (report-import-error package-name error)
  (cond
   ((http-get-error? error)
    [...]
   (else
    [...]))))

(define* (go-module->guix-package* module-path
                                   #:key (on-error report-import-error)
                                   #:allow-other-keys #:rest args)
  (with-exception-handler
      (lambda (error)
        (on-error module-path error)
        (values #f '()))
    (lambda () (apply go-module->guix-package module-path args))
    #:unwind? #t))

(define* (go-module-recursive-import package-name
                                     #:key (goproxy "https://proxy.golang.org")
                                     version
                                     pin-versions?)
  (recursive-import
   package-name
   #:repo->guix-package
   (memoize
    (lambda* (name #:key version repo)
      (go-module->guix-package* name #:goproxy goproxy
                                #:version version
                                #:pin-versions? pin-versions?)))
   #:guix-name go-module->guix-package-name
   #:version version))
--8<---------------cut here---------------end--------------->8---

Looks like I got a little carried away :) But breaking out the error
reporting gives us the future option of "plugging in" other error
reporting strategies, such as collating them and returning them
alongside a programmatic recursive import, or being able to provide a
list of packages which failed to import at the end. This will be much
more useful once we have a proper set of import error conditions.

Too much, perhaps?

Sarah




This bug report was last modified 3 years and 262 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.