zimoun writes: Hello, Thanks for the v4. > Hi, > > On Sun, 27 Jun 2021 at 06:46, Sarah Morgensen wrote: > >> 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)) > > Well, why not, even if I am not convinced the reason is meaningful > because it is mostly an incorrect parsing which returns: > > reason: ("match" "no matching pattern" #f). > Yes, it is a less than ideal compromise... I could not quickly figure out how to properly format it without a lot of complexity (like guix/ui.scm does in 'call-with-error-handling'). I found it hard to read the full exception object, but I would not object strongly to printing the full exception object either. As you say, your patch will fix it anyway ;) > and I think it is better to display the complete 'args' instead of > extract the URL (package-name) from 'args'. You're not wrong; I was just trying to keep it somewhat consistent with the other error message. >> 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... > > Thanks for the idea. As explained patch#45984 [1], all the UI > messages must be in guix/scripts/import and not in guix/import and Yes, this was my secret trick: having separated out the error reporting, it could be easily be moved to scripts/import later. > therefore, indeed, error reporting needs to be unified between all the > importers and raised accordingly; that's what we are working on with > jeko (Jérémy Korwin-Zmijowski) as pair-programming exercise. :-) I look forward to the results! > Back to the initial patch, I think it is better to simply fix with the > minor improvements of v3 your proposed and let this last proposal for > #45984; feel free to comment there. ;-) I agree. Your v4 looks good to me, except... > #:repo->guix-package > (lambda* (name #:key version repo) I apologize for not being clear earlier; by "put [memoize] back in later on" I meant "later on in the call chain," e.g. #:repo->guix-package + (memoize (lambda* (name #:key version repo) That's my only nit this time! ;) Thanks for bearing with me. Regards, Sarah