[PATCH 01/10] guix: import: Print the number of packages at the end.

OpenSubmitted by Attila Lendvai.
Details
3 participants
  • Attila Lendvai
  • Maxim Cournoyer
  • Maxime Devos
Owner
unassigned
Severity
normal
A
A
Attila Lendvai wrote on 3 May 13:16 +0200
(address . guix-patches@gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503111620.3139-1-attila@lendvai.name
---

this will be a series of patches that were needed to be able to
(mostly) successfully run these two imports (go-ethereum and
ethersphere/bee):

RUN=12
clear && ./pre-inst-env guix import go -r --pin-versions github.com/ethersphere/bee@v1.5.1 > >(tee -a ~/workspace/guix/importing/bee-run-${RUN}.scm) 2> >(tee -a ~/workspace/guix/importing/bee-run-${RUN}.log >&2)

RUN=36
clear && ./pre-inst-env guix import go -r --pin-versions github.com/ethereum/go-ethereum@v1.10.17 > >(tee -a ~/workspace/guix/importing/go-ethereum-run-${RUN}.scm) 2> >(tee -a ~/workspace/guix/importing/go-ethereum-run-${RUN}.log >&2)

note that i only have a mediocre knowledge of the golang
infrastructure, so this should be reviewed by someone who
more deeply understands the golang build process, and its
implementation within guix.

i think most of it is not very controversial, maybe except
the last commit.

i'm willing to reshape this under the guidance of someone who
has a better bird's eye view perspective on this all.

guix/scripts/import.scm | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

Toggle diff (25 lines)
diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index 40fa6759ae..891f558b97 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -127,10 +127,14 @@ (define-command (guix-import . args)
                             ('define-public _ ...)))
               (print expr))
              ((? list? expressions)
-              (for-each (lambda (expr)
-                          (print expr)
-                          (newline))
-                        expressions))
+              (let ((count 0))
+                (for-each (lambda (expr)
+                            (print expr)
+                            (set! count (1+ count))
+                            (newline))
+                          expressions)
+                (format (current-warning-port)
+                        (G_ "Imported ~a packages~%") count)))
              (x
               (leave (G_ "'~a' import failed~%") importer))))
          (let ((hint (string-closest importer importers #:threshold 3)))
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:42 +0200
[PATCH 02/10] guix: import: go: Rename go.pkg.dev-info to pkg.go.dev-info.
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-2-attila@lendvai.name
To properly mimic the name of the website. Also employ it at one more place.
---
guix/import/go.scm | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

Toggle diff (48 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index d00c13475a..bb4bb7bb7b 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -120,8 +120,11 @@ (define (escape occurrence)
 ;; Prevent inlining of this procedure, which is accessed by unit tests.
 (set! go-path-escape go-path-escape)
 
-(define (go.pkg.dev-info name)
-  (http-fetch* (string-append "https://pkg.go.dev/" name)))
+(define (pkg.go.dev-info module-path)
+  ;; FIXME if MODULE-PATH is a fork on github, then it will try to get the
+  ;; info of the fork (instead of the upstream), which most probably does not
+  ;; exist in pkg.go.dev.  Example: https://github.com/bonitoo-io/oapi-codegen
+  (http-fetch* (string-append "https://pkg.go.dev/" module-path)))
 
 (define* (go-module-version-string goproxy name #:key version)
   "Fetch the version string of the latest version for NAME from the given
@@ -147,7 +150,7 @@ (define* (go-module-available-versions goproxy name)
 (define (go-package-licenses name)
   "Retrieve the list of licenses that apply to NAME, a Go package or module
 name (e.g. \"github.com/golang/protobuf/proto\")."
-  (let* ((body (go.pkg.dev-info (string-append name "?tab=licenses")))
+  (let* ((body (pkg.go.dev-info (string-append name "?tab=licenses")))
          ;; Extract the text contained in a h2 child node of any
          ;; element marked with a "License" class attribute.
          (select (sxpath `(// (* (@ (equal? (class "License"))))
@@ -169,7 +172,7 @@ (define (sxml->texi sxml-node)
 (define (go-package-description name)
   "Retrieve a short description for NAME, a Go package name,
 e.g. \"google.golang.org/protobuf/proto\"."
-  (let* ((body (go.pkg.dev-info name))
+  (let* ((body (pkg.go.dev-info name))
          (sxml (html->sxml body #:strict? #t))
          (overview ((sxpath
                      `(//
@@ -206,8 +209,7 @@ (define (go-package-synopsis module-name)
 the https://pkg.go.dev/ web site."
   ;; Note: Only the *module* (rather than package) page has the README title
   ;; used as a synopsis on the https://pkg.go.dev web site.
-  (let* ((url (string-append "https://pkg.go.dev/" module-name))
-         (body (http-fetch* url))
+  (let* ((body (pkg.go.dev-info module-name))
          ;; Extract the text contained in a h2 child node of any
          ;; element marked with a "License" class attribute.
          (select-title (sxpath
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:42 +0200
[PATCH 03/10] guix: import: go: Add mockup logging facility.
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-3-attila@lendvai.name
Introduce a (local) mockup logger, so that we don't need to keep adding and
deleting format's when debugging the codebase.

* guix/import/go.scm (log.info) (log.debug): New macros.
---
guix/import/go.scm | 11 +++++++++++
1 file changed, 11 insertions(+)

Toggle diff (24 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index bb4bb7bb7b..0af5e4b5e2 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -100,6 +100,17 @@ (define-module (guix import go)
 
 ;;; Code:
 
+;; FIXME set up logging for the entire project, and replace this poor man's
+;; logger with the proper one.
+(define-syntax-rule (log.info format-string ...)
+  (let ((port (current-warning-port)))
+    (format port format-string ...)
+    (newline port)))
+
+(define-syntax-rule (log.debug format-string ...)
+  ;;(log.info format-string ...)
+  '())
+
 (define http-fetch*
   ;; Like http-fetch, but memoized and returning the body as a string.
   (memoize (lambda args
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:42 +0200
[PATCH 05/10] guix: import: go: Harden sxml->texi conversion.
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-5-attila@lendvai.name
* guix/import/go.scm (sxml->texi): Escape @ as @@, and only emit the text part
of URLs if they are plain strings.
---
guix/import/go.scm | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)

Toggle diff (40 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index a115c61adc..6b0fbaa8b6 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -172,13 +172,26 @@ (define (sxml->texi sxml-node)
   "A very basic SXML to Texinfo converter which attempts to preserve HTML
 formatting and links as text."
   (sxml-match sxml-node
-              ((strong ,text)
-               (format #f "@strong{~a}" text))
-              ((a (@ (href ,url)) ,text)
-               (format #f "@url{~a,~a}" url text))
-              ((code ,text)
-               (format #f "@code{~a}" text))
-              (,something-else something-else)))
+    ((strong ,text)
+     (format #f "@strong{~a}" text))
+    ((a (@ (href ,url)) ,body)
+     ;; Examples: image in the url: github.com/go-openapi/jsonpointer
+     ;; (code ...) in the URL body: github.com/mwitkow/go-conntrack
+     (if (string? body)
+         (format #f "@url{~a,~a}" url body)
+         (sxml-match body
+                     ((code ,text)
+                      (format #f "@url{~a,~a}" url (sxml->texi body)))
+                     (,_
+                      (format #f "@url{~a}" url)))))
+    ((code ,text)
+     (format #f "@code{~a}" text))
+    (,something-else
+     ;; Example: @ in the description: github.com/ethersphere/langos
+     (if (string? something-else)
+         (string-replace-substring something-else
+                                   "@" "@@")
+         something-else))))
 
 (define (go-package-description name)
   "Retrieve a short description for NAME, a Go package name,
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:42 +0200
[PATCH 06/10] guix: import: go: Add a local duplicate of http-fetch.
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-6-attila@lendvai.name
This is needed when dealing with golang packages, as per:

A page may return 404, but at the same time also contain the sought after
`go-import` meta tag. An example for such a project/page is:

It's not enough to just handle the thrown exception, because we need to be
able to get hold of the fetched content, too.

Discussion why it's duplicated here: https://issues.guix.gnu.org/54836

* guix/import/go.scm (http-fetch): Add a copy and extend it with the
accept-all-response-codes? param.
---
guix/import/go.scm | 107 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 105 insertions(+), 2 deletions(-)

Toggle diff (132 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 6b0fbaa8b6..a08005d090 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -33,13 +33,22 @@ (define-module (guix import go)
   #:use-module (guix import json)
   #:use-module (guix packages)
   #:use-module ((guix utils) #:select (string-replace-substring))
-  #:use-module (guix http-client)
+  ;; FIXME? We use a local copy of http-fetch.
+  ;; See https://issues.guix.gnu.org/54836
+  #:use-module ((guix http-client) #:hide (http-fetch))
+  #:use-module (guix base64)
+  #:use-module (rnrs bytevectors)
+  #:use-module ((guix build download)
+                #:select (open-socket-for-uri
+                          (open-connection-for-uri
+                           . guix:open-connection-for-uri)
+                          resolve-uri-reference))
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix memoization)
   #:autoload   (htmlprag) (html->sxml)            ;from Guile-Lib
   #:autoload   (guix serialization) (write-file)
   #:autoload   (guix base32) (bytevector->nix-base32-string)
-  #:autoload   (guix build utils) (mkdir-p)
+  #:autoload   (guix build utils) (mkdir-p dump-port)
   #:autoload   (gcrypt hash) (hash-algorithm sha256)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
@@ -69,6 +78,100 @@ (define-module (guix import go)
             go-module->guix-package*
             go-module-recursive-import))
 
+;; This is a duplicate from (guix http-client) with the addition of a
+;; #:accept-all-response-codes? param. See https://issues.guix.gnu.org/54836
+(define* (http-fetch uri #:key port (text? #f) (buffered? #t)
+                     (open-connection guix:open-connection-for-uri)
+                     (keep-alive? #f)
+                     (verify-certificate? #t)
+                     (headers '((user-agent . "GNU Guile")))
+                     (log-port (current-error-port))
+                     timeout
+                     (accept-all-response-codes? #f))
+  "Return an input port containing the data at URI, and the expected number of
+bytes available or #f.  If TEXT? is true, the data at URI is considered to be
+textual.  Follow any HTTP redirection.  When BUFFERED? is #f, return an
+unbuffered port, suitable for use in `filtered-port'.  HEADERS is an alist of
+extra HTTP headers.
+
+When KEEP-ALIVE? is true, the connection is marked as 'keep-alive' and PORT is
+not closed upon completion.
+
+When VERIFY-CERTIFICATE? is true, verify HTTPS server certificates.
+
+TIMEOUT specifies the timeout in seconds for connection establishment; when
+TIMEOUT is #f, connection establishment never times out.
+
+Write information about redirects to LOG-PORT.
+
+When ACCEPT-ALL-RESPONSE-CODES? is false then raise an '&http-get-error'
+condition if downloading fails, otherwise return the response regardless
+of the reponse code."
+  (define parsed-initial-uri
+    (if (string? uri) (string->uri uri) uri))
+
+  (define (open-connection* uri)
+    (open-connection uri
+                     #:verify-certificate? verify-certificate?
+                     #:timeout timeout))
+
+  (let loop ((current-uri parsed-initial-uri)
+             (current-port (or port (open-connection parsed-initial-uri))))
+    (let ((headers (match (uri-userinfo current-uri)
+                     ((? string? str)
+                      (cons (cons 'Authorization
+                                  (string-append "Basic "
+                                                 (base64-encode
+                                                  (string->utf8 str))))
+                            headers))
+                     (_ headers))))
+      (unless (or buffered? (not (file-port? current-port)))
+        (setvbuf current-port 'none))
+      (let*-values (((resp data)
+                     (http-get current-uri #:streaming? #t #:port current-port
+                               #:keep-alive? keep-alive?
+                               #:headers headers))
+                    ((code)
+                     (response-code resp)))
+        (case code
+          ((200)
+           (values data (response-content-length resp)))
+          ((301                         ; moved permanently
+            302                         ; found (redirection)
+            303                         ; see other
+            307                         ; temporary redirection
+            308)                        ; permanent redirection
+           (let ((host (uri-host current-uri))
+                 (new-uri (resolve-uri-reference (response-location resp)
+                                                 current-uri)))
+             (if keep-alive?
+                 (dump-port data (%make-void-port "w0")
+                            (response-content-length resp))
+                 (close-port current-port))
+             (format log-port (G_ "following redirection to `~a'...~%")
+                     (uri->string new-uri))
+             (loop new-uri
+                   (or (and keep-alive?
+                            (or (not (uri-host new-uri))
+                                (string=? host (uri-host new-uri)))
+                            current-port)
+                       (open-connection* new-uri)))))
+          (else
+           (if accept-all-response-codes?
+               (values data (response-content-length resp))
+               (raise (condition (&http-get-error
+                                  (uri current-uri)
+                                  (code code)
+                                  (reason (response-reason-phrase resp))
+                                  (headers (response-headers resp)))
+                                 (&message
+                                  (message
+                                   (format
+                                    #f
+                                    (G_ "~a: HTTP download failed: ~a (~s)")
+                                    (uri->string current-uri) code
+                                    (response-reason-phrase resp)))))))))))))
+
 ;;; Commentary:
 ;;;
 ;;; (guix import go) attempts to make it easier to create Guix package
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:42 +0200
[PATCH 04/10] guix: import: go: Fix the interpretation of the replace directive.
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-4-attila@lendvai.name
When a version is specified in a replace directive, then we must also match
the version against any potential replacements.

* guix/import/go.scm (go.mod-requirements): Fix by also matching the version.
---
guix/import/go.scm | 53 +++++++++++++++++++++++++++++++---------------
1 file changed, 36 insertions(+), 17 deletions(-)

Toggle diff (82 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 0af5e4b5e2..a115c61adc 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -331,7 +331,7 @@ (define-peg-pattern original all (or (and module-path version) module-path))
   (define-peg-pattern with all (or (and module-path version) file-path))
   (define-peg-pattern replace all (and original => with EOL))
   (define-peg-pattern replace-top body
-    (and (ignore "replace") 
+    (and (ignore "replace")
          (or (and block-start (* (or replace block-line)) block-end) replace)))
 
   ;; RetractSpec = ( Version | "[" Version "," Version "]" ) newline .
@@ -365,22 +365,39 @@ (define (go.mod-directives go.mod directive)
 (define (go.mod-requirements go.mod)
   "Compute and return the list of requirements specified by GO.MOD."
   (define (replace directive requirements)
-    (define (maybe-replace module-path new-requirement)
-      ;; Do not allow version updates for indirect dependencies (see:
-      ;; https://golang.org/ref/mod#go-mod-file-replace).
-      (if (and (equal? module-path (first new-requirement))
-               (not (assoc-ref requirements module-path)))
-          requirements
-          (cons new-requirement (alist-delete module-path requirements))))
-
+    ;; Specification: https://golang.org/ref/mod#go-mod-file-replace
+    ;; Interesting test cases:
+    ;;   github.com/influxdata/influxdb-client-go/v2@v2.4.0
+    (log.debug "inside replace, directive ~S, requirements ~S" directive requirements)
+    (define (maybe-replace old-module-path old-version new-module-path new-version)
+      (let ((matched-entry (assoc-ref requirements old-module-path)))
+        (log.debug "inside maybe-replace, ~S ~S => ~S ~S, matched-entry ~S"
+                   old-module-path old-version new-module-path new-version matched-entry)
+        (cond ((and (equal? old-module-path new-module-path)
+                    (not matched-entry))
+               ;; "A replace directive has no effect if the module version on the left
+               ;; side is not required."
+               ;; Do not allow version updates for indirect dependencies.
+               requirements)
+              ((and matched-entry
+                    (or (not old-version)
+                        (equal? old-version new-version)))
+               (cons (list new-module-path new-version)
+                     (alist-delete old-module-path requirements)))
+              (else
+               requirements))))
+    (log.debug "toplevel directive is ~S" directive)
     (match directive
-      ((('original ('module-path module-path) . _) with . _)
+      ((('original ('module-path old-module-path) ('version old-version) ...) with)
+       (unless (null? old-version)
+         (set! old-version (first old-version)))
        (match with
-         (('with ('file-path _) . _)
-          (alist-delete module-path requirements))
-         (('with ('module-path new-module-path) ('version new-version) . _)
-          (maybe-replace module-path
-                         (list new-module-path new-version)))))))
+         (('with ('file-path _))
+          ;; Superseded by a module in a local path, so let's delete it.
+          (alist-delete old-module-path requirements))
+         (('with ('module-path new-module-path) ('version new-version))
+          (maybe-replace old-module-path old-version
+                         new-module-path new-version))))))
 
   (define (require directive requirements)
     (match directive
@@ -389,8 +406,10 @@ (define (require directive requirements)
 
   (let* ((requires (go.mod-directives go.mod 'require))
          (replaces (go.mod-directives go.mod 'replace))
-         (requirements (fold require '() requires)))
-    (fold replace requirements replaces)))
+         (requirements (fold require '() requires))
+         (result (fold replace requirements replaces)))
+    (log.debug "requires:~% ~S~%replaces:~% ~S~%result:~% ~S" requires replaces result)
+    result))
 
 ;; Prevent inlining of this procedure, which is accessed by unit tests.
 (set! go.mod-requirements go.mod-requirements)
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:42 +0200
[PATCH 07/10] guix: import: go: More resilience wrt network errors; add logging.
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-7-attila@lendvai.name
This also adds logging statements to various parts of the go importer.

* guix/import/go.scm (go-module->guix-package*): Catch exceptions and retry in
case of network errors. Add ability to enable printing a full backtrace when
debugging.
(go-module->guix-package): Tolerate the inability to fetch the synopsis and
description.
---
guix/import/go.scm | 104 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 86 insertions(+), 18 deletions(-)

Toggle diff (141 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index a08005d090..6871132a70 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -23,7 +23,15 @@
 ;;; You should have received a copy of the GNU General Public License
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
+;;; goproxy protocol:
+;;;   https://golang.org/ref/mod#module-proxy
+;;;   https://docs.gomods.io/intro/protocol/
+;;;   https://roberto.selbach.ca/go-proxies/
+
 (define-module (guix import go)
+  #:use-module (git)
+  #:use-module (git structs)
+  #:use-module (git errors)
   #:use-module (guix build-system go)
   #:use-module (guix git)
   #:use-module (guix hash)
@@ -51,6 +59,8 @@ (define-module (guix import go)
   #:autoload   (guix build utils) (mkdir-p dump-port)
   #:autoload   (gcrypt hash) (hash-algorithm sha256)
   #:use-module (ice-9 format)
+  #:use-module (ice-9 control)
+  #:use-module (ice-9 exceptions)
   #:use-module (ice-9 match)
   #:use-module (ice-9 peg)
   #:use-module (ice-9 rdelim)
@@ -629,10 +639,14 @@ (define (go-import->module-meta content-text)
        (make-module-meta root-path (string->symbol vcs)
                          (strip-.git-suffix/maybe repo-url)))))
   ;; <meta name="go-import" content="import-prefix vcs repo-root">
-  (let* ((meta-data (http-fetch* (format #f "https://~a?go-get=1" module-path)))
+  (let* ((url (format #f "https://~a?go-get=1" module-path))
+         (meta-data (http-fetch* url #:accept-all-response-codes? #true))
          (select (sxpath `(// (meta (@ (equal? (name "go-import"))))
-                              // content))))
-    (match (select (html->sxml meta-data #:strict? #t))
+                              // content)))
+         (sxml (html->sxml meta-data #:strict? #t))
+         (selected (select sxml)))
+    (log.debug "The fetched meta-data from ~A is:~%~S" url selected)
+    (match selected
       (() #f)                           ;nothing selected
       ((('content content-text) ..1)
        (or
@@ -809,22 +823,76 @@ (define* (go-module->guix-package module-path #:key
          dependencies+versions
          dependencies))))
 
-(define go-module->guix-package*
-  (lambda args
-    ;; Disable output buffering so that the following warning gets printed
-    ;; consistently.
-    (setvbuf (current-error-port) 'none)
-    (let ((package-name (match args ((name _ ...) name))))
-      (guard (c ((http-get-error? c)
-                 (warning (G_ "Failed to import package ~s.
-reason: ~s could not be fetched: HTTP error ~a (~s).
+(define (go-module->guix-package* . args)
+  ;; Disable output buffering so that the following warning gets printed
+  ;; consistently.
+  (setvbuf (current-error-port) 'none)
+  (setvbuf (current-warning-port) 'none)
+  (let* ((package-name (match args ((name _ ...) name)))
+         ;; Use report-all-errors? for debugging purposes only, because
+         ;; e.g. getaddrinfo is not reentrant and therefore we must unwind
+         ;; before retrying.
+         (report-all-errors? #false)
+         (report-network-error
+          (lambda (reason)
+            (warning (G_ "Failing to import package ~S.
+reason: ~A.~%")
+                     package-name reason))))
+    (let loop ((attempts 0))
+      (when (> attempts 0)
+        (sleep 3)
+        (log.info "~%Retrying, attempt ~s." attempts))
+      (cond
+       ((> attempts 60)
+        (warning (G_ "Giving up on importing package ~s.
 This package and its dependencies won't be imported.~%")
-                          package-name
-                          (uri->string (http-get-error-uri c))
-                          (http-get-error-code c)
-                          (http-get-error-reason c))
-                 (values #f '())))
-        (apply go-module->guix-package args)))))
+                 package-name)
+        (values #f '()))
+       (else
+        (guard (c ((http-get-error? c)
+                   (report-network-error
+                    (format #f "~s could not be fetched: HTTP error ~a (~s)"
+                            (uri->string (http-get-error-uri c))
+                            (http-get-error-code c)
+                            (http-get-error-reason c)))
+                   (loop (+ 1 attempts)))
+                  ((eq? (exception-kind c)
+                        'getaddrinfo-error)
+                   (report-network-error "DNS lookup failed")
+                   (loop (+ 1 attempts)))
+                  ((and (eq? (exception-kind c)
+                             'git-error)
+                        (eq? (git-error-class (first (exception-args c)))
+                             GITERR_NET))
+                   (report-network-error "network error coming from git")
+                   (loop (+ 1 attempts)))
+                  (else
+                   (let ((port (current-warning-port)))
+                     (format port "Unexpected error, will skip ~S.~%reason: "
+                             package-name)
+                     ;; Printing a backtrace here is not very useful: it is
+                     ;; cut off because GUARD unwinds.
+                     (print-exception port (stack-ref (make-stack #t) 1)
+                                      c (exception-args c))
+                     (display-backtrace (make-stack #t) port))
+                   ;; give up on this entry
+                   (values #f '())))
+          (with-exception-handler
+              (lambda (c)
+                (when report-all-errors?
+                  (let ((port (current-warning-port)))
+                    (format port "*** exception while importing:~%")
+                    (print-exception port (stack-ref (make-stack #t) 1)
+                                     c (exception-args c))
+                    (format port "*** printing backtrace:~%")
+                    (display-backtrace (make-stack #t) port)
+                    ;; DISPLAY-BACKTRACE can fail, so it's better to make its
+                    ;; exit also visible.
+                    (format port "*** done printing backtrace~%")))
+                (raise-continuable c))
+            (lambda ()
+              (apply go-module->guix-package args))
+            #:unwind? (not report-all-errors?))))))))
 
 (define* (go-module-recursive-import package-name
                                      #:key (goproxy "https://proxy.golang.org")
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:42 +0200
[PATCH 08/10] guix: import: go: Modules in a subdir and prefixed tags.
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-8-attila@lendvai.name
Handle golang projects that are located in a subdir relative to the repo's
root directory, and whose tags are also prefixed accordingly with the relative
path.

An example: guix import go github.com/aws/aws-sdk-go-v2/service/route53@v1.1.1

Also be more resilient while dealing with licences.

* guix/import/go.scm (go-module->guix-package): Attempt to split comma
separated lists of licences, and tolerate the inability to fetch any license
information. Handle go modules that are a subdirectory of a vcs repo,
including the special naming of their tags (it includes the subdir as a prefix).
(list->licenses): Warn when the conversion fails.
(+known-vcs+): Renamed from known-vcs to use naming convention for constants.
---
guix/import/go.scm | 130 +++++++++++++++++++++++++++++++++++----------
1 file changed, 102 insertions(+), 28 deletions(-)

Toggle diff (243 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 6871132a70..ce6463cc51 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -346,6 +346,7 @@ (define (go-package-synopsis module-name)
 the https://pkg.go.dev/ web site."
   ;; Note: Only the *module* (rather than package) page has the README title
   ;; used as a synopsis on the https://pkg.go.dev web site.
+  (log.debug "Getting synopsis for ~S" module-name)
   (let* ((body (pkg.go.dev-info module-name))
          ;; Extract the text contained in a h2 child node of any
          ;; element marked with a "License" class attribute.
@@ -377,7 +378,12 @@ (define (list->licenses licenses)
                             ("GPL3" "GPL-3.0")
                             ("NIST" "NIST-PD")
                             (_ license)))
-                         'unknown-license!)))
+                         (begin
+                           (warning (G_ "Failed to identify license ~S.~%")
+                                    license)
+                           ;; This will put the license there as a string that
+                           ;; will error at compilation.
+                           license))))
               licenses))
 
 (define (fetch-go.mod goproxy module-path version)
@@ -550,7 +556,8 @@ (define-record-type <vcs>
 (define (make-vcs prefix regexp type)
   (%make-vcs prefix (make-regexp regexp) type))
 
-(define known-vcs
+;; TODO use define-constant
+(define +known-vcs+
   ;; See the following URL for the official Go equivalent:
   ;; https://github.com/golang/go/blob/846dce9d05f19a1f53465e62a304dea21b99f910/src/cmd/go/internal/vcs/vcs.go#L1026-L1087
   (list
@@ -587,16 +594,17 @@ (define vcs-qualifiers '(".bzr" ".fossil" ".git" ".hg" ".svn"))
 
   (define (vcs-qualified-module-path->root-repo-url module-path)
     (let* ((vcs-qualifiers-group (string-join vcs-qualifiers "|"))
-           (pattern (format #f "^(.*(~a))(/|$)" vcs-qualifiers-group))
-           (m (string-match pattern module-path)))
-      (and=> m (cut match:substring <> 1))))
+           (pattern (format #f "^(.*(~a))(/|$)" vcs-qualifiers-group)))
+      (and=> (string-match pattern module-path)
+             (cut match:substring <> 1))))
 
   (or (and=> (find (lambda (vcs)
                      (string-prefix? (vcs-url-prefix vcs) module-path))
-                   known-vcs)
+                   +known-vcs+)
              (lambda (vcs)
                (match:substring (regexp-exec (vcs-root-regex vcs)
-                                             module-path) 1)))
+                                             module-path)
+                                1)))
       (vcs-qualified-module-path->root-repo-url module-path)
       module-path))
 
@@ -666,6 +674,7 @@ (define (module-meta-data-repo-url meta-data goproxy)
 (define* (git-checkout-hash url reference algorithm)
   "Return the ALGORITHM hash of the checkout of URL at REFERENCE, a commit or
 tag."
+  (log.info "Fetching git repo at ~S, reference ~S" url reference)
   (define cache
     (string-append (or (getenv "TMPDIR") "/tmp")
                    "/guix-import-go-"
@@ -681,6 +690,7 @@ (define cache
                   (update-cached-checkout url
                                           #:ref
                                           `(tag-or-commit . ,reference)))))
+    (log.debug " hashing at checkout ~S, commit ~S, reference ~S" checkout commit reference)
     (file-hash* checkout #:algorithm algorithm #:recursive? #true)))
 
 (define (vcs->origin vcs-type vcs-repo-url version)
@@ -688,8 +698,10 @@ (define (vcs->origin vcs-type vcs-repo-url version)
 control system is being used."
   (case vcs-type
     ((git)
-     (let ((plain-version? (string=? version (go-version->git-ref version)))
-           (v-prefixed?    (string-prefix? "v" version)))
+     (let* ((git-ref        (go-version->git-ref version))
+            (plain-version? (string=? version git-ref))
+            (v-prefixed?    (string-prefix? "v" version)))
+       (log.debug "Version ~S converted to git reference ~S" version git-ref)
        `(origin
           (method git-fetch)
           (uri (git-reference
@@ -699,13 +711,12 @@ (define (vcs->origin vcs-type vcs-repo-url version)
                 ;; stripped of any 'v' prefixed.
                 (commit ,(if (and plain-version? v-prefixed?)
                              '(string-append "v" version)
-                             '(go-version->git-ref version)))))
+                             git-ref))))
           (file-name (git-file-name name version))
           (sha256
            (base32
             ,(bytevector->nix-base32-string
-              (git-checkout-hash vcs-repo-url (go-version->git-ref version)
-                                 (hash-algorithm sha256))))))))
+              (git-checkout-hash vcs-repo-url git-ref (hash-algorithm sha256))))))))
     ((hg)
      `(origin
         (method hg-fetch)
@@ -768,41 +779,104 @@ (define* (go-module->guix-package module-path #:key
   "Return the package S-expression corresponding to MODULE-PATH at VERSION, a Go package.
 The meta-data is fetched from the GOPROXY server and https://pkg.go.dev/.
 When VERSION is unspecified, the latest version available is used."
+  (log.info "~%Processing go module ~A@~A" module-path version)
   (let* ((available-versions (go-module-available-versions goproxy module-path))
          (version* (validate-version
                     (or (and version (ensure-v-prefix version))
                         (go-module-version-string goproxy module-path)) ;latest
                     available-versions
                     module-path))
-         (content (fetch-go.mod goproxy module-path version*))
-         (dependencies+versions (go.mod-requirements (parse-go.mod content)))
+         (go.mod (fetch-go.mod goproxy module-path version*))
+         (dependencies+versions (go.mod-requirements (parse-go.mod go.mod)))
          (dependencies (if pin-versions?
                            dependencies+versions
                            (map car dependencies+versions)))
-         (module-path-sans-suffix
-          (match:prefix (string-match "([\\./]v[0-9]+)?$" module-path)))
          (guix-name (go-module->guix-package-name module-path))
-         (root-module-path (module-path->repository-root module-path))
+         (repo-root (module-path->repository-root module-path))
          ;; The VCS type and URL are not included in goproxy information. For
          ;; this we need to fetch it from the official module page.
-         (meta-data (fetch-module-meta-data root-module-path))
+         (meta-data (fetch-module-meta-data repo-root))
+         (module-root-path (module-meta-import-prefix meta-data))
          (vcs-type (module-meta-vcs meta-data))
          (vcs-repo-url (module-meta-data-repo-url meta-data goproxy))
-         (synopsis (go-package-synopsis module-path))
-         (description (go-package-description module-path))
-         (licenses (go-package-licenses module-path)))
+         (raw-subdir (if (< (string-length module-root-path)
+                            (string-length module-path))
+                         (substring module-path
+                                    (+ 1 (string-length module-root-path)))
+                         #false))
+         (module-subdirectory raw-subdir)
+         (major-version (and=>
+                         (and raw-subdir
+                              (string-match ".*v([0-9]+)$" raw-subdir))
+                         (lambda (m)
+                           (let* ((v-postfix (match:substring m 1))
+                                  (ver (string->number v-postfix)))
+                             (log.debug " ~A matched as having a version-postfix: ~A"
+                                        raw-subdir v-postfix)
+                             (set! module-subdirectory
+                                   (substring raw-subdir 0
+                                              ;; Don't go negative when
+                                              ;; raw-subdir is a simple "v2".
+                                              (max 0
+                                                   (- (string-length raw-subdir)
+                                                      (string-length v-postfix)
+                                                      ;; Drop two slashes.
+                                                      2))))
+                             (when (string-null? module-subdirectory)
+                               (set! module-subdirectory #false))
+                             (unless (integer? ver)
+                               (raise
+                                (formatted-message (G_ "failed to parse version postfix from '~a' for package '~a'")
+                                                   raw-subdir module-path)))
+                             ;; TODO assert that major-version matches the first number in version
+                             ;; TODO assert that only version 1.x.y is allowed without a /v[major-version] postfix
+                             ver))))
+         (vcs-tag (if module-subdirectory
+                      (string-append module-subdirectory "/" version*)
+                      version*))
+         (synopsis (or (false-if-exception
+                        (go-package-synopsis module-path))
+                       (begin
+                         (warning (G_ "Failed to fetch synopsis for ~S.~%")
+                                  module-path)
+                         "TODO FIXME")))
+         (description (or (false-if-exception
+                           (go-package-description module-path))
+                          (begin
+                            (warning (G_ "Failed to fetch description for ~S.~%")
+                                     module-path)
+                            "TODO FIXME")))
+         (licenses (or (false-if-exception
+                        (go-package-licenses module-path))
+                       (begin
+                         (warning (G_ "Failed to fetch license for ~S.~%")
+                                  module-path)
+                         '("unknown-license!")))))
+    ;; Maybe split comma separated list of licenses in a single string
+    (when (and (= 1 (length licenses))
+               (string? (first licenses)))
+      (let ((pieces (map string-trim-both
+                         (remove! string-null?
+                                  (string-split (first licenses) #\,)))))
+        (when (< 1 (length pieces))
+          (set! licenses pieces))))
+    (log.debug " meta-data: ~S~% raw-subdir: ~S, module-subdirectory: ~S, \
+major-version: ~S"
+               meta-data raw-subdir module-subdirectory major-version)
+    (log.debug " dependencies:~%~S" dependencies+versions)
     (values
      `(package
         (name ,guix-name)
         (version ,(strip-v-prefix version*))
         (source
-         ,(vcs->origin vcs-type vcs-repo-url version*))
+         ,(vcs->origin vcs-type vcs-repo-url vcs-tag))
         (build-system go-build-system)
         (arguments
-         '(#:import-path ,module-path
-           ,@(if (string=? module-path-sans-suffix root-module-path)
-                 '()
-                 `(#:unpack-path ,root-module-path))))
+         '(#:tests? #false ; some packages have unrecorded dependencies needed only by their tests
+           #:import-path ,module-path
+           ,@(if module-subdirectory
+                 `(#:unpack-path ,module-root-path)
+                 '())))
         ,@(maybe-propagated-inputs
            (map (match-lambda
                   ((name version)
@@ -810,7 +884,7 @@ (define* (go-module->guix-package module-path #:key
                   (name
                    (go-module->guix-package-name name)))
                 dependencies))
-        (home-page ,(format #f "https://~a" root-module-path))
+        (home-page ,(format #f "https://~a" module-root-path))
         (synopsis ,synopsis)
         (description ,(and=> description beautify-description))
         (license ,(match (list->licenses licenses)
@@ -898,7 +972,7 @@ (define* (go-module-recursive-import package-name
                                      #:key (goproxy "https://proxy.golang.org")
                                      version
                                      pin-versions?)
-
+  (log.info "Initiating recursive import of ~a, version ~a" package-name version)
   (recursive-import
    package-name
    #:repo->guix-package
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:43 +0200
[PATCH 09/10] guix: import: go: module-name -> module-path to be consistent
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-9-attila@lendvai.name
* guix/import/go.scm (go-package-description): Renamed first parameter.
---
guix/import/go.scm | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

Toggle diff (25 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index ce6463cc51..25d424c1ac 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -340,14 +340,14 @@ (define (go-package-description name)
       ((p elements ...)
        (apply string-append (filter string? (map sxml->texi elements)))))))
 
-(define (go-package-synopsis module-name)
-  "Retrieve a short synopsis for a Go module named MODULE-NAME,
+(define (go-package-synopsis module-path)
+  "Retrieve a short synopsis for a Go module named MODULE-PATH,
 e.g. \"google.golang.org/protobuf\".  The data is scraped from
 the https://pkg.go.dev/ web site."
   ;; Note: Only the *module* (rather than package) page has the README title
   ;; used as a synopsis on the https://pkg.go.dev web site.
-  (log.debug "Getting synopsis for ~S" module-name)
-  (let* ((body (pkg.go.dev-info module-name))
+  (log.debug "Getting synopsis for ~S" module-path)
+  (let* ((body (pkg.go.dev-info module-path))
          ;; Extract the text contained in a h2 child node of any
          ;; element marked with a "License" class attribute.
          (select-title (sxpath
-- 
2.35.1
A
A
Attila Lendvai wrote on 3 May 13:43 +0200
[PATCH 10/10] guix: import: go: Better handling of /v2 in the module path.
(address . 55242@debbugs.gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220503114301.9524-10-attila@lendvai.name
* guix/import/go.scm (module-path->repository-root): Delete version param.
(*module-path->import-path*): New mapping, initialized with a small db of
projects.
(go-module->guix-package): Return packages with #:unpack-path when needed.
---
guix/import/go.scm | 93 +++++++++++++++++++++++++++-------------------
1 file changed, 54 insertions(+), 39 deletions(-)

Toggle diff (133 lines)
diff --git a/guix/import/go.scm b/guix/import/go.scm
index 25d424c1ac..fbed3ca88b 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -75,6 +75,7 @@ (define-module (guix import go)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-71) ; let*
   #:use-module (sxml match)
   #:use-module ((sxml xpath) #:renamer (lambda (s)
                                          (if (eq? 'filter s)
@@ -608,19 +609,15 @@ (define (vcs-qualified-module-path->root-repo-url module-path)
       (vcs-qualified-module-path->root-repo-url module-path)
       module-path))
 
-(define* (go-module->guix-package-name module-path #:optional version)
-  "Converts a module's path to the canonical Guix format for Go packages.
-Optionally include a VERSION string to append to the name."
+(define* (go-module->guix-package-name module-path)
+  "Converts a module's path to the canonical Guix format for Go packages."
   ;; Map dot, slash, underscore and tilde characters to hyphens.
   (let ((module-path* (string-map (lambda (c)
                                     (if (member c '(#\. #\/ #\_ #\~))
                                         #\-
                                         c))
                                   module-path)))
-    (string-downcase (string-append "go-" module-path*
-                                    (if version
-                                        (string-append "-" version)
-                                        "")))))
+    (string-downcase (string-append "go-" module-path*))))
 
 (define (strip-.git-suffix/maybe repo-url)
   "Strip a repository URL '.git' suffix from REPO-URL if hosted at GitHub."
@@ -771,6 +768,14 @@ (define (validate-version version available-versions module-path)
 available versions:~{ ~a~}.")
                                   (map strip-v-prefix
                                        available-versions)))))))))
+(define *module-path->import-path*
+  ;; There's no other way to derive this information, at least that I know of.
+  '(("github.com/google/go-cmp" . "github.com/google/go-cmp/cmp")
+    ("github.com/sergi/go-diff" . "github.com/sergi/go-diff/diffmatchpatch")
+    ("github.com/davecgh/go-spew" . "github.com/davecgh/go-spew/spew")
+    ("github.com/beorn7/perks" . "github.com/beorn7/perks/quantile")
+    ("github.com/census-instrumentation/opencensus-proto" .
+     "github.com/census-instrumentation/opencensus-proto/gen-go")))
 
 (define* (go-module->guix-package module-path #:key
                                   (goproxy "https://proxy.golang.org")
@@ -864,38 +869,48 @@ (define* (go-module->guix-package module-path #:key
 major-version: ~S"
                meta-data raw-subdir module-subdirectory major-version)
     (log.debug " dependencies:~%~S" dependencies+versions)
-    (values
-     `(package
-        (name ,guix-name)
-        (version ,(strip-v-prefix version*))
-        (source
-         ,(vcs->origin vcs-type vcs-repo-url vcs-tag))
-        (build-system go-build-system)
-        (arguments
-         '(#:tests? #false ; some packages have unrecorded dependencies needed only by their tests
-           #:import-path ,module-path
-           ,@(if module-subdirectory
-                 `(#:unpack-path ,module-root-path)
-                 '())))
-        ,@(maybe-propagated-inputs
-           (map (match-lambda
-                  ((name version)
-                   (go-module->guix-package-name name (strip-v-prefix version)))
-                  (name
-                   (go-module->guix-package-name name)))
-                dependencies))
-        (home-page ,(format #f "https://~a" module-root-path))
-        (synopsis ,synopsis)
-        (description ,(and=> description beautify-description))
-        (license ,(match (list->licenses licenses)
-                    (() #f)                       ;unknown license
-                    ((license)                    ;a single license
-                     license)
-                    ((license ...)                ;a list of licenses
-                     `(list ,@license)))))
-     (if pin-versions?
-         dependencies+versions
-         dependencies))))
+    (let* ((import-path unpack-path
+            (if module-subdirectory
+                (values module-path module-root-path)
+                (let ((import-path (assoc-ref *module-path->import-path*
+                                              module-path)))
+                  (if import-path
+                      (begin
+                        (log.debug "matched as an import-path exception: ~S" import-path)
+                        (values import-path module-path))
+                      (values module-path #f))))))
+      (values
+       `(package
+          (name ,guix-name)
+          (version ,(strip-v-prefix version*))
+          (source
+           ,(vcs->origin vcs-type vcs-repo-url vcs-tag))
+          (build-system go-build-system)
+          (arguments
+           '(#:import-path ,import-path
+             ,@(if unpack-path
+                   `(#:unpack-path ,unpack-path)
+                   '())))
+          ,@(maybe-propagated-inputs
+             (map (match-lambda
+                    ((name version)
+                     (list (go-module->guix-package-name name)
+                           (strip-v-prefix version)))
+                    (name
+                     (go-module->guix-package-name name)))
+                  dependencies))
+          (home-page ,(format #f "https://~a" module-root-path))
+          (synopsis ,synopsis)
+          (description ,(and=> description beautify-description))
+          (license ,(match (list->licenses licenses)
+                      (() #f)                       ;unknown license
+                      ((license)                    ;a single license
+                       license)
+                      ((license ...)                ;a list of licenses
+                       `(list ,@license)))))
+       (if pin-versions?
+           dependencies+versions
+           dependencies)))))
 
 (define (go-module->guix-package* . args)
   ;; Disable output buffering so that the following warning gets printed
-- 
2.35.1
M
M
Maxime Devos wrote on 3 May 18:12 +0200
Re: [bug#55242] [PATCH 07/10] guix: import: go: More resilience wrt network errors; add logging.
7974a00256e3ee153507f77d6b0a405473e9bbc4.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (9 lines)
> +                   (let ((port (current-warning-port)))
> +                     (format port "Unexpected error, will skip ~S.~%reason: "
> +                             package-name)
> +                     ;; Printing a backtrace here is not very useful: it is
> +                     ;; cut off because GUARD unwinds.
> +                     (print-exception port (stack-ref (make-stack #t) 1)
> +                                      c (exception-args c))
> +                     (display-backtrace (make-stack #t) port))

Why are unknown errors being catched here?

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFUgRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7mhkAQDw7mBIcVzVc1I07e0G/oj+ffOE
Bs3nRgbLD24cFdY0tQD/QP2UwSVoxTLOy9K+Sij2pqPRu5CyxhGDJUDy+L0BMws=
=vGcf
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:17 +0200
ff2f84b74ecddae9e4513abc9782b544323c3501.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (14 lines)
> +          (with-exception-handler
> +              (lambda (c)
> +                (when report-all-errors?
> +                  (let ((port (current-warning-port)))
> +                    (format port "*** exception while importing:~%")
> +                    (print-exception port (stack-ref (make-stack #t) 1)
> +                                     c (exception-args c))
> +                    (format port "*** printing backtrace:~%")
> +                    (display-backtrace (make-stack #t) port)
> +                    ;; DISPLAY-BACKTRACE can fail, so it's better to make its
> +                    ;; exit also visible.
> +                    (format port "*** done printing backtrace~%")))
> +                (raise-continuable c))

What's the '-continuable' for here? Is it to avoid extra 'raise-
exception' entries in the backtrace, or does this actually make use of
Scheme's continuable exceptions?

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFVfhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7rwBAP9LEPY+FcRn5fv6BJAwvFmZUTHP
qk9SXSHRA3XriMrkBwD/dSJ7gTrvKmH9nOqAvnRFSaAihVAkulZtFXNGWfoBEQ4=
=DbsO
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:21 +0200
Re: [bug#55242] [PATCH 01/10] guix: import: Print the number of packages at the end.
15b2a5f3da980087b0a3f84910563b13a929076f.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:16 [+0200]:
Toggle quote (5 lines)
> [bug#55242] [PATCH 01/10] guix: import: Print the number of packages
> at the end.
> +                (format (current-warning-port)
> +                        (G_ "Imported ~a packages~%") count)))

What's the use case for counting the number of packages?

Why only for the Go importer, and why a warning? 

The 'info' macro from (guix diagnostics) seems to fit here (it also
uses 'current-warning-port', but only internally).

The plural form of 'packages' is incorrect when count=1.

Greetings,
Maxime
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFWeBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7smFAP9xcPIDwYxVWxn1g4eWCczdJkDL
oX4NmITfM829JsaebgEA6E2nWhoAFICcV5UnhywfCnYfddPprhnvDjv66kqvnAo=
=Yu5W
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:22 +0200
Re: [bug#55242] [PATCH 02/10] guix: import: go: Rename go.pkg.dev-info to pkg.go.dev-info.
afc2411679e7d6a34bdac1b0c8aca84c82409c3f.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (2 lines)
> github

Standard capitalisation is "GitHub" here, not that it really matters
much.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iIwEABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFWxBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7iLeAPkB2KJiq2xgmDaME+zdtGWmxfNS
Wvdle6e4tChs34JNdgD1H8KVOyL1du+H87NhAOtNBp7w1yGAStoSUkpdYOQCDA==
=aZgn
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:23 +0200
Re: [bug#55242] [PATCH 03/10] guix: import: go: Add mockup logging facility.
5322f2d5dbe9e5b442c17084279d188f9c6c6f1d.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (11 lines)
> +;; FIXME set up logging for the entire project, and replace this poor man's
> +;; logger with the proper one.
> +(define-syntax-rule (log.info format-string ...)
> +  (let ((port (current-warning-port)))
> +    (format port format-string ...)
> +    (newline port)))
> +
> +(define-syntax-rule (log.debug format-string ...)
> +  ;;(log.info format-string ...)
> +  '())

We already have a logging functionality: 'info' from
guix/diagnostics.scm. No 'debug' yet though ...

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFXEhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7pTbAQCwVTyNG23CikuzpGUeBSogPU7o
be+F8X+mTVTMTN5X/gEA1PByz5LIVERSANNoSpGtSn2gp/5C50nTNiyusnR6/AA=
=yBDF
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:25 +0200
Re: [bug#55242] [PATCH 04/10] guix: import: go: Fix the interpretation of the replace directive.
a73f177d4e7d8081c2b0bc0a3584ed164dde0ea6.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (5 lines)
> +    ;; Interesting test cases:
> +    ;;   github.com/influxdata/influxdb-client-go/v2@v2.4.0
> +    (log.debug "inside replace, directive ~S, requirements ~S" directive requirements)

You can use 'pk' for this:

#;(pk "inside replace, directive" directive "requirements" requirements)

(delete 'pk' when debugging).

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFXbRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7oRlAQDMlFXyvv5xEzLB9C7d5S8en0Ym
ty720Ip5MhVTEw6omwD/TBPvThevt5iBFTY7dYUzhf1ReVB67Gcih4DCwy81VwQ=
=6SDX
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:26 +0200
5252c8ff3e4373ab50553dd686fb6b9c7e9aed95.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (18 lines)
> +    (define (maybe-replace old-module-path old-version new-module-path new-version)
> +      (let ((matched-entry (assoc-ref requirements old-module-path)))
> +        (log.debug "inside maybe-replace, ~S ~S => ~S ~S, matched-entry ~S"
> +                   old-module-path old-version new-module-path new-version matched-entry)
> +        (cond ((and (equal? old-module-path new-module-path)
> +                    (not matched-entry))
> +               ;; "A replace directive has no effect if the module version on the left
> +               ;; side is not required."
> +               ;; Do not allow version updates for indirect dependencies.
> +               requirements)
> +              ((and matched-entry
> +                    (or (not old-version)
> +                        (equal? old-version new-version)))
> +               (cons (list new-module-path new-version)
> +                     (alist-delete old-module-path requirements)))
> +              (else
> +               requirements))))

There's some extra complexity here, so I recommend some corresponding
extra tests, to avoid it breaking in the future.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFXpxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7nYXAQDT+BC9OyC5u60sbdg4X+OLmxaJ
jEZNyJXUcCqZCpGiAgD/Y5amn1yDxlLxcYnAvt+lA4yat+RG4nd2OKvd2g9KYQE=
=0F8r
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:27 +0200
46d5aa1aa8868e449e62a528c286d6803aa7c0e9.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (2 lines)
> +       (unless (null? old-version)

How can a version be a list?

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFX5BccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7vJHAQD7oPar7Qdjmg6196hP8CcljWle
ofiHgaZkUsTNyDSN+AD+NooHsEyRlz9BucJA1k+r2CefVlMAXabPafEqtiAI/ww=
=Io3b
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:28 +0200
619bfc4fb252b256b34af2305e1282c63a3c5d4c.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (3 lines)
> +       (unless (null? old-version)
> +         (set! old-version (first old-version)))

Nevermind, there's some ...

Still, why choose the first old-version instead of the latest old-
version? How can we know which one is correct?.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFYJRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7sVYAP0Q1IiuxcKNwPM1OmL0yloR2lRf
ctZP5JN0uoDbYLu2UwEA8ep0ZmGKSZMgjexPcP9rwE7DVfd6jGiyFn6ns1Te4Qs=
=vFx0
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:32 +0200
Re: [bug#55242] [PATCH 05/10] guix: import: go: Harden sxml->texi conversion.
646875155b87dbd5bff76f1f3ddbbdb50abd627f.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (11 lines)
>    (sxml-match sxml-node
> -              ((strong ,text)
> -               (format #f "@strong{~a}" text))
> -              ((a (@ (href ,url)) ,text)
> -               (format #f "@url{~a,~a}" url text))
> -              ((code ,text)
> -               (format #f "@code{~a}" text))
> -              (,something-else something-else)))
> +    ((strong ,text)
> +     (format #f "@strong{~a}" text))

Do we know for sure here that 'text' is actually a string? What if the
input was (strong (em (a (href "http://super") "Super") emphasis")?

Toggle quote (10 lines)
> +    ((a (@ (href ,url)) ,body)
> +     ;; Examples: image in the url: github.com/go-openapi/jsonpointer
> +     ;; (code ...) in the URL body: github.com/mwitkow/go-conntrack
> +     (if (string? body)
> +         (format #f "@url{~a,~a}" url body)
> +         (sxml-match body
> +                     ((code ,text)
> +                      (format #f "@url{~a,~a}" url (sxml->texi body)))
> +                     (,_

I'm not familiar enough with sxml to be sure, but maybe the , can be
removed here.

Toggle quote (10 lines)
> +                      (format #f "@url{~a}" url)))))
> +    ((code ,text)
> +     (format #f "@code{~a}" text))
> +    (,something-else
> +     ;; Example: @ in the description: github.com/ethersphere/langos
> +     (if (string? something-else)
> +         (string-replace-substring something-else
> +                                   "@" "@@")
> +         something-else))))

Anyway, more cases are nice, but I recommend tests.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFZOhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7nccAPkBhs4PVtSOOIlR7jlsXFm+KMgY
Lv4H8UGMrsMjVliT7QEA27Fpxebn2NKs2CWvCsEP7wgHVyrbyDC1bAIggkJfCgo=
=4yaz
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:36 +0200
Re: [bug#55242] [PATCH 07/10] guix: import: go: More resilience wrt network errors; add logging.
724a0beb49e2854b5efe1a5069f11911b616cad9.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (6 lines)
> * guix/import/go.scm (go-module->guix-package*): Catch exceptions and retry in
> case of network errors. Add ability to enable printing a full backtrace when
> debugging.
> (go-module->guix-package): Tolerate the inability to fetch the synopsis and
> description.

Ok, but couldn't this be done more generally, for all importers using
HTTP? E.g., could we have a variant 'http-fetch/retrying' of 'http-
fetch' that automatically retries a number of times? Then the retrying
functionality could almost trivially be used in other HTTP-using
importers as well.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFaDxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7jaAAQC3g7zni1ahy4ss9v4jnTwxHhYU
JckLdSa6gKTTY+BsywD5ATKyWslMVKLID30o/t4mRGv6VxO1N+oVwxa1QN4BUgI=
=8yVk
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:37 +0200
ff7e50d5ec0e717a3ac78178e61290faf0719b4d.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (3 lines)
> +            (warning (G_ "Failing to import package ~S.
> +reason: ~A.~%")

Why a warning instead of a 'report-error'?

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFaMBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7mmEAP9ZKs569BMvh6GKZExnx0b5HtkY
WqbfQ9YXxq9hC6VTJQEAi/O9egV1S5LQlukIW+sL8Hwwm9GduRwflpWCmiUAJQI=
=Cf8y
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:42 +0200
Re: [bug#55242] [PATCH 08/10] guix: import: go: Modules in a subdir and prefixed tags.
4a6bd86252c50d3302ae3bd4818071f26112ef6b.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (5 lines)
> -                         'unknown-license!)))
> +                         (begin
> +                           (warning (G_ "Failed to identify license ~S.~%")
> +                                    license)

Why only for the Go importer and not other importers?
Maybe the behaviour of other importers in presence of unidentifiable
licenses could be unified, with some common support procedure or
something. Then only a single location needs to be modified to change
things, and all importers would benefit.

Toggle quote (4 lines)
> +                           ;; This will put the license there as a string that
> +                           ;; will error at compilation.
> +                           license))))

I don't think this is correct, in the past I've put '(foobar) and
#false in 'license' fields of packages and those packages could be
compiled, without any compilation issues. I assume the same holds for
strings as well.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFbahccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7mIlAQCph1mw1I+bQ41s51ekfzUNFC2n
8t7+O2CIYavUwVg+KQEA1QNhG15Apod4z8ug64DQhoJ2LX7N/0PjwLmkmBO8rQ8=
=RqKc
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:48 +0200
a1b3837274257314f2fa2349d15de3ca3329a9da.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (3 lines)
> +;; TODO use define-constant
> +(define +known-vcs+

The naming convention in (Guile) Scheme is '%known-vcs' or 'known-vcs'.
Also, 'define-constant' doesn't exist in Guile, and the Lisp macro
'defconstant' and 'defmacro' appears to have some complications w.r.t.
phasing. What would be the benefit of 'define-constant' here?


Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFcxxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7p37AQDyFVsLc+LIrqt5B+brsUrNxnm4
8JMR4KglAfass9qiDQD+Nh5WnToK3VGSZgS9o7VUngx2Wo+f4IDaPKgyTbR9fQY=
=v7AX
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:50 +0200
849ca73cf56763c56faa2a98ea744f678f233d1b.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (2 lines)
> +  (log.info "~%Processing go module ~A@~A" module-path version)

Why a newline at the beginning instead of the end? Also, why the
logging? If I do "guix import go foobar", wouldn't I know in advance
it will process the foobar module? And in case of --recursive,
wouldn't I know after the fact which modules were processed?

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFdaRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7p0lAQCyMFsDN9ov/MBB0iYC479jKpFy
sOtIZ/7EWzdeX+wGeAD+KYeLc3jPyKU+ITmYgTmhDEljwgFgO6XSM5g0RkXSIQ8=
=HEzh
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:51 +0200
6a532228dd7e6d8aada6b36df29241172aa5f3cd.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (6 lines)
> +         (raw-subdir (if (< (string-length module-root-path)
> +                            (string-length module-path))
> +                         (substring module-path
> +                                    (+ 1 (string-length module-root-path)))
> +                         #false))

You can use (and COND (substring ...)) here.
Also, the usual extra complexity --> extra tests.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFdlhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7jTpAP4kiUY7Gawrvw41IYNMtObS+xcx
MRs0Js7Rqq5wrOx6RgD+NySB8s87bcpEesZa99kaRQ9Eq1RfHbcRMlbiFfTsmgQ=
=56/P
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:53 +0200
27abb4025a581b0fda4cfe8b7e71f9a6e6ca6dbb.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (2 lines)
> +                              (string-match ".*v([0-9]+)$" raw-subdir))

Why a $ at the end but no "^" at the beginning? What's the point of
the leading ".*"? Could the regexp be constructed in advance, or only
once (maybe 'delay/force' and a global variable) -- turning a regexp
into a NFA into a DFA can be expensive.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFeGhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7og8AQCYqIFAC13fe33qxn9vUYrMJEAJ
N+rrrm5iz2LgFUiM4wEAiCFxzi/t1G5eNqJvH6ywOqUioNRJiDIOpls8M7LZ2Q0=
=GIIz
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:55 +0200
140b35404dfd03c0a341c4c49a9b5d700c7f841d.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (3 lines)
> +         (synopsis (or (false-if-exception
> +                        (go-package-synopsis module-path))

Are you sure you want to ignore _all_ exceptions? Including stack
overflows, out-of-memories, unbound variable errors, type errors ...?

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFechccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7s2uAP4uQmKj6l9KUX7Q/9sPUl2PlW1w
1bS7wIEnHjmhISTALAEAxUrb+8KrQ3zARIzZM5/UmpIO8XKRbb0MglWf4qTUrgk=
=91ab
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:56 +0200
d0cf883672065bfc028f395bd1267c902877a931.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (5 lines)
> +                         (warning (G_ "Failed to fetch license for ~S.~%")
> +                                  module-path)
> +                         '("unknown-license!")))))


In some other location, the symbol 'unknonw-license!' is used. Here, a
string is used. Why the inconsistency? Also, given the "unknown-
license!", the warning seems mostly redundant to me.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFexBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7oS9AP4jqGMi+Ay5a8Cq8oZBsCJzvFRW
2TIQD3Kyhwh+x6PUyQD8DFNNg+BgM8yGqMJxcaRIeWSuqu3CEtw2QXTcCRC56g0=
=YqtN
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 18:59 +0200
b5e7c69e909edb952782545a60170e5cbca69952.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (6 lines)
> +    ;; Maybe split comma separated list of licenses in a single string
> +    (when (and (= 1 (length licenses))
> +               (string? (first licenses)))
> +      (let ((pieces (map string-trim-both
> +                         (remove! string-null?

When can it be null? Also, tests.

Toggle quote (2 lines)
> +                                  (string-split (first licenses) #\,)))))

Is there a need for speed here? If not, I'd keep things simple by
using the non-mutating version (I expect the speed gain to be neglible
here). If there is, I would also use the mutating version of map --
'map!' from srfi-1.


Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFfZxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7u2uAP9qhbYln/s2x2ZiOqZqp6JphqRf
RkuprghXBjQ6xOYs+QD/UUNypxbRV86m/1FECjm3z5580fiv73CO9o+eTJIHBwY=
=UH/D
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 19:00 +0200
a18b191e875fc8a8ac597e27200bb99d45b9c562.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (2 lines)
> +         '(#:tests? #false ; some packages have unrecorded dependencies needed only by their tests

Then they need to be recorded, and the reviewer will ask why tests are
being skipped here.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFfpRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7owHAP99Xtin+0OA0OshN8mC1N7HBuiA
7awbWkALEEQ3WjnVOgEArkl9zWVtYNP4pepriAdUvjDwJ8g1NeiF8vGPgYEb9g8=
=V0GW
-----END PGP SIGNATURE-----


A
A
Attila Lendvai wrote on 3 May 19:00 +0200
Re: [bug#55242] [PATCH 07/10] guix: import: go: More resilience wrt network errors; add logging.
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 55242@debbugs.gnu.org)
O9w-304SD4fvo6wWmpf2MalIwuxcoKjwxTx1eKYl9jkla_-7NUaoSc8ld3R01g-9GRQHCHeMuNvHQsPMpF0rSR8HnG3KTrf_oPiCKY2OR5Y=@lendvai.name
Toggle quote (12 lines)
> > +                   (let ((port (current-warning-port)))
> > +                     (format port "Unexpected error, will skip ~S.~%reason: "
> > +                             package-name)
> > +                     ;; Printing a backtrace here is not very useful: it is
> > +                     ;; cut off because GUARD unwinds.
> > +                     (print-exception port (stack-ref (make-stack #t) 1)
> > +                                      c (exception-args c))
> > +                     (display-backtrace (make-stack #t) port))
>
>
> Why are unknown errors being catched here?

*all* errors are caught, displayed, and swallowed, so that the importing can proceed to the rest of the packages.

a possible scenario: a large import can take several minutes. if there's a transient network error, then this way it may finish with 99% of the packages, and the rest can be restarted by hand after inspecting the log output.

another scenario is that the importer is simply buggy, and the user gets 99% of the packages imported, and has to do only one package by hand.


Toggle quote (5 lines)
> What's the '-continuable' for here? Is it to avoid extra 'raise-
> exception' entries in the backtrace, or does this actually make use
> of Scheme's continuable exceptions?


honestly, i can't remember. which translates to: i should have
commented on that!

*shakes head and makes a mental note*

from guile's manual:

"If continuable? is true, the handler is invoked in tail position
relative to the raise-exception call. Otherwise if the handler
returns, a non-continuable exception of type &non-continuable is
raised in the same dynamic environment as the handler."

i.e. it should rather be called raise-continuably not
raise-continuable.

and i think the reason is that the stack is not unwound that way, so
that the handlers in the guard can print a backtrace showing the
entire stack.

does this make sense? i'm still learning scheme's stack/exception
handling...

or maybe what i wanted is that the handlers in the guard can return
with (values #f '())? but i think that should work with vanilla RAISE,
too.


Toggle quote (3 lines)
> Why a warning instead of a 'report-error'?


because i want the importer to be able to deal with a transitive
closures of dependencies with > 400 entries. if it fails at the first
error, then a `guix import go -r` becomes a hopeless endeavor for
larger go modules.

thanks for the review Maxime! i'll address the rest, too.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“'Emergencies' have always been the pretext on which the safeguards of individual liberty have been eroded.”
— F. A. Hayek (1899–1992)
M
M
Maxime Devos wrote on 3 May 19:01 +0200
Re: [bug#55242] [PATCH 08/10] guix: import: go: Modules in a subdir and prefixed tags.
c9c1de9095244c938d41fea488933af8b4bcc7b1.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:42 [+0200]:
Toggle quote (2 lines)
> +  (log.info "Initiating recursive import of ~a, version ~a" package-name version)

This seems implied by typing "guix import go --recursive foo@bar"?

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFf1BccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7kbFAP9tCRkgScGTQ6x7zQHnC3oVy7Pe
thCJx3hrFlOrtSHtCAD+KDBAygT8bYzLNPHIDDXbcJbQNEVm6HQVbwEBUBOLdQM=
=e3Zu
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 19:29 +0200
Re: [bug#55242] [PATCH 01/10] guix: import: Print the number of packages at the end.
703cf3f8136272d5134bb61488a3f8c6f7234f93.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:16 [+0200]:
Toggle quote (8 lines)
> note that i only have a mediocre knowledge of the golang
> infrastructure, so this should be reviewed by someone who
> more deeply understands the golang build process, and its
> implementation within guix.
>
> i think most of it is not very controversial, maybe except
> the last commit.

FWIW (given that I don't know the Go packaging system well), the last
patch (10/10) seems the least controversial to me. I'm not seeing why
it would be controversial.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFmYBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7ncsAP4/OHBbakg6GvVZKIkAb34NfV9v
CByVzezxm6EdMCNC9AD8C/pSlHq1nOrVSOQWmDacjom1/fL8h40SeSQRV1VjFAw=
=XXra
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 3 May 19:38 +0200
Re: [bug#55242] [PATCH 07/10] guix: import: go: More resilience wrt network errors; add logging.
(name . Attila Lendvai)(address . attila@lendvai.name)(address . 55242@debbugs.gnu.org)
76510182d505978ad41f5bff88bf7eee7c289558.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 17:00 [+0000]:
Toggle quote (5 lines)
> a possible scenario: a large import can take several minutes.
> if there's a transient network error, then this way it may finish
> with 99% of the packages, and the rest can be restarted by hand after
> inspecting the log output.

Catching (presumably) transient network errors and reporting them with
'report-network-error' and the like: no problem.

Toggle quote (2 lines)
> another scenario is that the importer is simply buggy,

... but if there's some kind of bug, why not just report and fix it?

Toggle quote (3 lines)
> and the user gets 99% of the packages imported, and has to do only
> one package by hand.

If the bug is fixed, then no packages need to be done by hand and other
people will benefit as well. Whereas if it's only for 1% and the
backtrace only happens for recursive import and not for non-recursive
or manual import, then I'd think that the chance that the user actually
reports or fixes the bug decreases a lot.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnFoixccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7oCtAP44EseNJfpxRQyMFCDiGdUGPdKS
d+kjKCQOEvB0VdLomAD/QPt1XZ8+ZdIPgOBkZf77VpdWu8ZbEtIOzS4foNy31g0=
=xsbZ
-----END PGP SIGNATURE-----


A
A
Attila Lendvai wrote on 9 May 14:34 +0200
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 55242@debbugs.gnu.org)
Dfnmonrp5k3MvgDgMmpqYG9xADUrEjWTHYy5fxgOQDRbWpt_iCvsdhufGBUnh21iE0kzSoKeRbF-DQYVI2IJwYWlx5canDahL47_IESTZoc=@lendvai.name
Toggle quote (3 lines)
> ... but if there's some kind of bug, why not just report and fix it?


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

i think an importer, that does proper logging, and finishes with a 99%
correct result, is worth much more than one that barks at the first
error, and then refuses to continue processing the 100+ remaining
entries.

this was true even in my own case, when i also had the maintainer hat
on, i.e. i was already neck deep into fixing the importer.


Toggle quote (11 lines)
> > and the user gets 99% of the packages imported, and has to do only
> > one package by hand.
>
>
> If the bug is fixed, then no packages need to be done by hand and other
> people will benefit as well. Whereas if it's only for 1% and the
> backtrace only happens for recursive import and not for non-recursive
> or manual import, then I'd think that the chance that the user actually
> reports or fixes the bug decreases a lot.


the tradeoff is this: what leads to a lower level of overall annoyance of all the parties involved:

1) have a tool that is less useful, and through that it annoys the
users to report issues, which then (hopefully) thrusts some people
towards improving the tool... or

2) have a tool that does its best to produce the most useful output.

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.

does it make enough sense to justify my proposed behavior?

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Communism doesn’t work because people like to own stuff.”
— Frank Zappa (1940–1993)
A
A
Attila Lendvai wrote on 9 May 14:37 +0200
Re: [bug#55242] [PATCH 01/10] guix: import: Print the number of packages at the end.
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 55242@debbugs.gnu.org)
9Wta2NGLHf5YNvjwLMVfMMswsybhgLVGU7XrCPnWU4VDVOwUgvrPEAGeT0vikTuryjXHFcoI7Dmg84J5TTP98HQuzlycp51oDPlDKsokgMI=@lendvai.name
Toggle quote (14 lines)
> > note that i only have a mediocre knowledge of the golang
> > infrastructure, so this should be reviewed by someone who
> > more deeply understands the golang build process, and its
> > implementation within guix.
> >
> > i think most of it is not very controversial, maybe except
> > the last commit.
>
>
> FWIW (given that I don't know the Go packaging system well), the last
> patch (10/10) seems the least controversial to me. I'm not seeing why
> it would be controversial.


because it introduces an in-source database of golang projects that
require special-casing (*module-path->import-path*).

i suspect that whatever info i'm storing in that db of exceptions is
probably available somewhere, somehow.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Nobody in the world, nobody in history, has ever gotten their freedom by appealing to the moral sense of the people who were oppressing them.”
— Assata Shakur (1947–)
A
A
Attila Lendvai wrote on 9 May 14:50 +0200
Re: [bug#55242] [PATCH 08/10] guix: import: go: Modules in a subdir and prefixed tags.
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 55242@debbugs.gnu.org)
SgOuXNceIVqDv4sq7aYOQT841SBGJzrZSe1J-Y196mu8qjuj_6blHft1CvcnRpYs7hvU5Pv_JUwgPxSzbdyZsoPklLz0_zU5CRFuNogRS9o=@lendvai.name
Toggle quote (6 lines)
> > +  (log.info "~%Processing go module ~A@~A" module-path version)
>
>
> Why a newline at the beginning instead of the end? Also, why the


it makes the log more readable.


Toggle quote (5 lines)
> logging? If I do "guix import go foobar", wouldn't I know in advance
> it will process the foobar module? And in case of --recursive,
> wouldn't I know after the fact which modules were processed?


here's how i was working on this codebase:

RUN=12
clear && ./pre-inst-env guix import go -r --pin-versions github.com/ethersphere/bee@v1.5.1 > >(tee -a ~/workspace/guix/importing/bee-run-${RUN}.scm) 2> >(tee -a ~/workspace/guix/importing/bee-run-${RUN}.log >&2) && beep

RUN=36
clear && ./pre-inst-env guix import go -r --pin-versions github.com/ethereum/go-ethereum@v1.10.17 > >(tee -a ~/workspace/guix/importing/go-ethereum-run-${RUN}.scm) 2> >(tee -a ~/workspace/guix/importing/go-ethereum-run-${RUN}.log >&2) && beep


IOW, i was repeatedly firing long-running imports whose output i stored in versioned files. when a run was finished, i inspected the log and the output file (containing hundreds of packages).

whenever i had to understand what went wrong -- which often manifested in successfull runs producing broken output (i.e. no errors/backtraces) --, i added log statements to see what's happening. i could have deleted the printfs, and then re-add them again a gazillion times, or just leave them there for the upcoming programmers (which includes me), conditionally compiled in with the help of a macro.

background: in my lisp codebase emacs colors the log statement gray, and the logging lib i'm using in CL has a separate compile-time and a runtime log level. IOW, i can eliminate the entire runtime logging overhead, and most of the code readability overhead.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Associate yourself with people of good quality, for it is better to be alone than in bad company.”
— Booker T. Washington (1856–1915)
M
M
Maxime Devos wrote on 9 May 19:45 +0200
Re: [bug#55242] [PATCH 07/10] guix: import: go: More resilience wrt network errors; add logging.
(name . Attila Lendvai)(address . attila@lendvai.name)(address . 55242@debbugs.gnu.org)
91eb2af9b86d2491c9e9f9010b9e3cb2d05dd253.camel@telenet.be
Attila Lendvai schreef op ma 09-05-2022 om 12:34 [+0000]:
Toggle quote (2 lines)
> [...] 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

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

in this case.

Toggle quote (5 lines)
> 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’

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.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnlTIxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7hqmAP9/mbFb5ISVnEYRAFT1pTTsYOBZ
37FeGSrC4Vavb0A3AAD9G+Zo2FvqafGGH6HQwbRqQjJgNADS5dmoO0zgkMcAKQs=
=9ani
-----END PGP SIGNATURE-----


A
A
Attila Lendvai wrote on 9 May 22:02 +0200
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 55242@debbugs.gnu.org)
5BVwkhUhKpe_HOgy_US9e5QdB8dwxw3ED31P1VUezGdevXB90Ea2-eh6s2iaiTfMpwITHe5Z-2BVNn5vHGcejoNCe3pkWP2bRWA9QRFmEhY=@lendvai.name
Toggle quote (6 lines)
> 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


note that the list of people *using* the go importer is probably not all that much larger.


Toggle quote (4 lines)
> old Go importer bugs/patches on issues.guix.gnu.org, I don't think
> we have to worry about


the fact that the go importer bugs were not even reported into the issue tracker doesn't change much about the fact that i have experienced several problems when i first tried to recursively import a non-trivial go project (which even included issues with http-fetch). it means that all those bug were there, not hidden by exception handling, and yet they have *not* been reported into the issue tracker.

or to put it another way, i think there are just too many unverified assumptions in this discussion about actual user behavior. my strategy in those situations is to expose as much info as feasible, and let the users respond intelligently to the situation.


Toggle quote (10 lines)
> 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).


note that this patch does not just catch errors, but handles them by emitting useful information about them, and continues only after that.

i consider silently swallowing errors a major sin (read: code that will certainly eat up some portion of someone's life in the future).

the only place where this patch silently swallows errors is in the extraction of descriprions and such, i.e. data that the user must review anyway.


Toggle quote (6 lines)
> 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.


this patch does not hide any bugs, only enables the code to continue by skipping a package.

and as a bonus: i wouldn't be surprised if it emitted *more* useful information than when allowing the exception to pass through to the caller, because error handling in the guix codebase could use a bit more love.


Toggle quote (5 lines)
> 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.


ok, i'll look into putting this feature behind a CLI argument, and generalize it to all importers. as time allows i'll get back to the rest of the questions/suggestions, too.

thanks for the feedback Maxime!

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“A great civilization is not conquered from without until it destroys itself from within.”
— Will Durant
M
M
Maxime Devos wrote on 9 May 22:08 +0200
(name . Attila Lendvai)(address . attila@lendvai.name)(address . 55242@debbugs.gnu.org)
280a5bda16d7f580b6b8c46a00bc81101bcabb53.camel@telenet.be
Attila Lendvai schreef op ma 09-05-2022 om 20:02 [+0000]:
Toggle quote (2 lines)
> note that this patch does not just catch errors, but handles them by emitting useful information about them, and continues only after that.

I think such errors are easily overlooked, but yes, and I don't have
any actual data on that.

Toggle quote (5 lines)
> i consider silently swallowing errors a major sin (read: code that will certainly eat up some portion of someone's life in the future).
>
> the only place where this patch silently swallows errors is in the extraction of descriprions and such, i.e. data that the user must review anyway.
> [...]

I think I've overstated the ‘swallows errors etc.’ a bit, given that
if "guix import go --recursive <foo>" skips a package <bar>, then the
user will have to package its dependency <bar> anyway, and for that
the user could first try "guix import go <bar>" (reporting errors on
bug-guix@ or #guix) before fixing/extending the importer or doing
things manually.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnl0qxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7kE+AP9odJkKelOQoXG/LUrX1wmWq+Y+
ZlYW1gXLuAI+B5EzQQD/fazvlplYuXCk2xIFz0YsQCU2MENe0Vr5p0GajoJJRwE=
=y67O
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 9 May 22:39 +0200
Re: [bug#55242] [PATCH 10/10] guix: import: go: Better handling of /v2 in the module path.
f43fe0ea01c729cdac794cb2ff6c47f394bf4945.camel@telenet.be
Attila Lendvai schreef op di 03-05-2022 om 13:43 [+0200]:
Toggle quote (2 lines)
> +(define *module-path->import-path*

In (Guile) Scheme (at least as used in the Scheme code I've seen),
*earmuffs* are typically used for mutable variables, not for global
constants. Though guix/self.scm uses *system-modules* for not-really-
constant-but-not-mutated lexical variables so maybe ...

FWIW, I would write (define %module-path->import-path ...) here, not
that it really matters.

Toggle quote (8 lines)
> +  ;; There's no other way to derive this information, at least that I know of.
> +  '(("github.com/google/go-cmp" . "github.com/google/go-cmp/cmp")
> +    ("github.com/sergi/go-diff" . "github.com/sergi/go-diff/diffmatchpatch")
> +    ("github.com/davecgh/go-spew" . "github.com/davecgh/go-spew/spew")
> +    ("github.com/beorn7/perks" . "github.com/beorn7/perks/quantile")
> +    ("github.com/census-instrumentation/opencensus-proto" .
> +     "github.com/census-instrumentation/opencensus-proto/gen-go")))

About the ‘is this controversional’ bit: these kind of mappings and
sets have precedent, see e.g. 'known-vcs' in (guix import cran),
%builtin-mods in (guix import minetest), default-r-packages in (guix
import cran).

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYnl8ChccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7jjvAP9QIc+zxtNflCWJ016k5MLpkFG2
kADgmIrdtMFXaX9wEwD/apAf+KB+UWzgCRtepf7n2sCmXnaJwG0HTDyPZ2AW+wk=
=j4Jt
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 14 May 09:09 +0200
Re: [bug#55242] [PATCH 01/10] guix: import: Print the number of packages at the end.
0bd2cbbbca8758572a906796fa1819b0c3d7e945.camel@telenet.be
Hi,

I forgot that "guix build" already does some debug logging when passed
"--verbose" or "--debug". Maybe "guix import ..." can gain a similar 
system. I don't know if the internals are reusable though ...
(I'm not convinced that we need logging for "guix import" but I'm not
convinced of the contrary either, and there is already some kind of
debug logging system elsewhere.)

Toggle quote (9 lines)
> here's how i was working on this codebase:
>
> RUN=12
> clear && ./pre-inst-env guix import go -r --pin-versions
> github.com/ethersphere/bee@v1.5.1 > >(tee -a
> ~/workspace/guix/importing/bee-run-${RUN}.scm) 2> >(tee -a
> ~/workspace/guix/importing/bee-run-${RUN}.log >&2) && beep
>

FWIW, for these kind of things I do

while ./pre-inst-env guix some-command; do echo 'Next'; done

and for your case (broken output without errors), maybe

while true; do echo 'Next run' >> the-log-file && ./pre-inst-env guix some-command >> the-log-file; done

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYn9VjhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7gELAP936Cyd3oOg6VZlpzAzmsjxK9PH
Kc8j07HJYpSVepmGfwD/aSFI7+wiWBHcAfVePqPrmRTggATJ2yfHExkmKEAPrwI=
=5Jil
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 14 May 09:15 +0200
d6af3680c53a120a1dc2d42b34a7e844b67d9bb4.camel@telenet.be
Toggle quote (11 lines)
> > 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.
>
>
> ok, i'll look into putting this feature behind a CLI argument, and
> generalize it to all importers. as time allows i'll get back to the
> rest of the questions/suggestions, too.

I'm not sure if I mentioned it previously, but it looks like some
patches are uncontroversial and usable mostly as-is (e.g. 04/10 without
the logging, 02/10, 05/10 with verifying types, 06/10 without the
overly-general false-if-exception and logging, 9/10, 10/10, 04/10). So
you could try the non-controversial bits first and then see if some
kind of consensus or such could be found for the remainder.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYn9XFhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7k2mAP4tPdEIDwNDqPB0N8qLdRXd9g7/
i8VjI+ePAgCzgkTJKQD+O0XoEXn2YP2YZgqJR1SVUrMLmThehuKB3idFpn6CJAs=
=YNOJ
-----END PGP SIGNATURE-----


M
M
Maxim Cournoyer wrote on 14 Jun 21:46 +0200
Re: bug#55242: [PATCH 01/10] guix: import: Print the number of packages at the end.
(name . Attila Lendvai)(address . attila@lendvai.name)(address . 55242@debbugs.gnu.org)
877d5jdo4u.fsf_-_@gmail.com
Hello Attila,

Attila Lendvai <attila@lendvai.name> writes:

Toggle quote (28 lines)
> Introduce a (local) mockup logger, so that we don't need to keep adding and
> deleting format's when debugging the codebase.
>
> * guix/import/go.scm (log.info) (log.debug): New macros.
> ---
> guix/import/go.scm | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/guix/import/go.scm b/guix/import/go.scm
> index bb4bb7bb7b..0af5e4b5e2 100644
> --- a/guix/import/go.scm
> +++ b/guix/import/go.scm
> @@ -100,6 +100,17 @@ (define-module (guix import go)
>
> ;;; Code:
>
> +;; FIXME set up logging for the entire project, and replace this poor man's
> +;; logger with the proper one.
> +(define-syntax-rule (log.info format-string ...)
> + (let ((port (current-warning-port)))
> + (format port format-string ...)
> + (newline port)))
> +
> +(define-syntax-rule (log.debug format-string ...)
> + ;;(log.info format-string ...)
> + '())
> +

We alreading have 'warning' and 'info' in (guix diagnostics); perhaps we
could also have 'debug', enabled when GUIX_DEBUG is set or something.
Alternatively, there's a full blown logging library available in the
Guile-Lib project, as (logging logger) [0].


Maxim
?