[bug#45983] [PATCH 0/3] Fix non-recursive importers gnu and json

[bug#45983] [PATCH 0/3] Fix non-recursive importers gnu and json

[zimoun]

Dear,

The first patch is cosmetic.  Even, the file guix/import/gnu.scm should not
contain UI messages, IMHO.  Another story.

The two other patches fix the ugly backtrace throw by:

    guix import gnu do-not-exist
    guix import json do-no-exist

with something like:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix import gnu do-not-exist
guix import: error: failed to determine latest release of GNU do-not-exist

$ ./pre-inst-env guix import json do-not-exist
guix import: error: invalid file name
--8<---------------cut here---------------end--------------->8---

See bug#44115 <http://issues.guix.gnu.org/44115#3> for details about the bug.

All the best,
simon


zimoun (3):
  import: gnu: Add internationalized messages.
  guix: gnu-maintenance: Fix error handling.
  scripts: import: json: Fix error handling.

 guix/gnu-maintenance.scm     |  5 ++++-
 guix/import/gnu.scm          | 11 +++++++++--
 guix/scripts/import/json.scm |  7 +++++--
 3 files changed, 18 insertions(+), 5 deletions(-)


base-commit: 2d9c6542c804eb2ef3d8934e1e3ab8b24e9bbafb
-- 
2.29.2

[bug#45983] [PATCH 1/3] import: gnu: Add internationalized messages.

[zimoun]

* guix/import/gnu.scm (gnu->guix-package): Add 'G_' to messages.
---
 guix/import/gnu.scm | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/guix/import/gnu.scm b/guix/import/gnu.scm
index 29324d7554..fe7a2f1e54 100644
--- a/guix/import/gnu.scm
+++ b/guix/import/gnu.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +20,7 @@
 (define-module (guix import gnu)
   #:use-module (guix gnu-maintenance)
   #:use-module (guix import utils)
+  #:use-module (guix i18n)
   #:use-module (guix utils)
   #:use-module (guix store)
   #:use-module (gcrypt hash)
@@ -115,13 +117,18 @@ details.)"
          (#f
           (raise (condition
                   (&message
-                   (message "couldn't find meta-data for GNU package")))))
+                   (message
+                    (format #f
+                            (G_ "couldn't find meta-data for GNU ~a")
+                            name))))))
          (info
           (gnu-package->sexp info release #:key-download key-download)))))
     (_
      (raise (condition
              (&message
               (message
-               "failed to determine latest release of GNU package")))))))
+               (format #f
+                       (G_ "failed to determine latest release of GNU ~a")
+                       name))))))))
 
 ;;; gnu.scm ends here
-- 
2.29.2

[bug#45983] [PATCH 2/3] guix: gnu-maintenance: Fix error handling.

[zimoun]

Fixes partially <https://bugs.gnu.org/44115>.

* guix/gnu-maintenance.scm (latest-release): Handle 'ftp-error'.
---
 guix/gnu-maintenance.scm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/guix/gnu-maintenance.scm b/guix/gnu-maintenance.scm
index 08b2bcf758..0da6fc19b6 100644
--- a/guix/gnu-maintenance.scm
+++ b/guix/gnu-maintenance.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2012, 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -361,7 +362,9 @@ return the corresponding signature URL, or #f it signatures are unavailable."
 
   (let loop ((directory directory)
              (result    #f))
-    (let* ((entries (ftp-list conn directory))
+    (let* ((entries (catch 'ftp-error
+                      (lambda _ (ftp-list conn directory))
+                      (const '())))
 
            ;; Filter out things like /gnupg/patches.  Filter out "w32"
            ;; directories as found on ftp.gnutls.org.
-- 
2.29.2

[bug#45983] [PATCH 3/3] scripts: import: json: Fix error handling.

[zimoun]

Fixes partially <https://bugs.gnu.org/44115>.

* guix/scripts/import/json.scm (guix-import-json): Handle error.
---
 guix/scripts/import/json.scm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/import/json.scm b/guix/scripts/import/json.scm
index 778e5f4bc5..63fba260ae 100644
--- a/guix/scripts/import/json.scm
+++ b/guix/scripts/import/json.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014 Eric Bavier <bavier@member.fsf.org>
 ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -88,8 +89,10 @@ Import and convert the JSON package definition in PACKAGE-FILE.\n"))
                            (reverse opts))))
     (match args
       ((file-name)
-       (or (json->code file-name)
-           (leave (G_ "invalid JSON in file '~a'~%") file-name)))
+       (if (file-exists? file-name)
+         (or (json->code file-name)
+             (leave (G_ "invalid JSON in file '~a'~%") file-name))
+         (leave (G_ "invalid file name~%"))))
       (()
        (leave (G_ "too few arguments~%")))
       ((many ...)
-- 
2.29.2

[bug#45983] [PATCH 0/3] Fix non-recursive importers gnu and json

[Ludovic Courtès]

zimoun <zimon.toutoune@gmail.com> skribis:

> Fixes partially <https://bugs.gnu.org/44115>.
>
> * guix/scripts/import/json.scm (guix-import-json): Handle error.

[...]

> -       (or (json->code file-name)
> -           (leave (G_ "invalid JSON in file '~a'~%") file-name)))
> +       (if (file-exists? file-name)
> +         (or (json->code file-name)
> +             (leave (G_ "invalid JSON in file '~a'~%") file-name))
> +         (leave (G_ "invalid file name~%"))))

I’d suggest this:

  (catch 'system-error
    (lambda ()
      (or (json->code …) …))
    (lambda args
      (leave (G_ "failed to access '~a': ~a~%")
             file-name (strerror (system-error-errno args)))))

This avoids TOCTTOU and gives details about the failure.

Could you send updated patches?

Thanks,
Ludo’.

[bug#45983] [PATCH 0/3] Fix non-recursive importers gnu and json

[Ludovic Courtès]

Hi,

zimoun <zimon.toutoune@gmail.com> skribis:

> * guix/import/gnu.scm (gnu->guix-package): Add 'G_' to messages.

[...]

> -                   (message "couldn't find meta-data for GNU package")))))
> +                   (message
> +                    (format #f
> +                            (G_ "couldn't find meta-data for GNU ~a")
> +                            name))))))
>           (info
>            (gnu-package->sexp info release #:key-download key-download)))))
>      (_
>       (raise (condition
>               (&message
>                (message
> -               "failed to determine latest release of GNU package")))))))
> +               (format #f
> +                       (G_ "failed to determine latest release of GNU ~a")

Please use ‘formatted-message’ instead.

Note that (message "foo") is subject to translation just like (G_
"foo").  See the ‘--keyword’ options in po/guix/Makevars.

Thanks,
Ludo’.

[bug#45983] [PATCH v2 1/3] guix: gnu-maintenance: Fix error handling.

[zimoun]

Fixes partially <https://bugs.gnu.org/44115>.

* guix/gnu-maintenance.scm (latest-release): Handle 'ftp-error'.
---
 guix/gnu-maintenance.scm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/guix/gnu-maintenance.scm b/guix/gnu-maintenance.scm
index 08b2bcf758..0da6fc19b6 100644
--- a/guix/gnu-maintenance.scm
+++ b/guix/gnu-maintenance.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2012, 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -361,7 +362,9 @@ return the corresponding signature URL, or #f it signatures are unavailable."
 
   (let loop ((directory directory)
              (result    #f))
-    (let* ((entries (ftp-list conn directory))
+    (let* ((entries (catch 'ftp-error
+                      (lambda _ (ftp-list conn directory))
+                      (const '())))
 
            ;; Filter out things like /gnupg/patches.  Filter out "w32"
            ;; directories as found on ftp.gnutls.org.

base-commit: d265809b782293eb42dd663b4611ca19dd2bf1b3
-- 
2.29.2

[bug#45983] [PATCH v2 2/3] import: gnu: Add internationalized messages.

[zimoun]

* guix/import/gnu.scm (gnu->guix-package): Add 'G_' to messages.
---
 guix/import/gnu.scm | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/guix/import/gnu.scm b/guix/import/gnu.scm
index 29324d7554..d307bbbaba 100644
--- a/guix/import/gnu.scm
+++ b/guix/import/gnu.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -17,8 +18,10 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix import gnu)
+  #:use-module ((guix diagnostics) #:select (formatted-message))
   #:use-module (guix gnu-maintenance)
   #:use-module (guix import utils)
+  #:use-module (guix i18n)
   #:use-module (guix utils)
   #:use-module (guix store)
   #:use-module (gcrypt hash)
@@ -113,15 +116,16 @@ details.)"
      (let ((version (upstream-source-version release)))
        (match (find-package name)
          (#f
-          (raise (condition
-                  (&message
-                   (message "couldn't find meta-data for GNU package")))))
+          (raise (make-compound-condition
+                  (formatted-message
+                   (G_ "couldn't find meta-data for GNU ~a")
+                   name))))
          (info
           (gnu-package->sexp info release #:key-download key-download)))))
     (_
-     (raise (condition
-             (&message
-              (message
-               "failed to determine latest release of GNU package")))))))
+     (raise (make-compound-condition
+             (formatted-message
+              (G_ "failed to determine latest release of GNU ~a")
+              name))))))
 
 ;;; gnu.scm ends here
-- 
2.29.2

bug#45983: [PATCH 0/3] Fix non-recursive importers gnu and json

[Ludovic Courtès]

Hi!

I applied the whole series.

zimoun <zimon.toutoune@gmail.com> skribis:

> * guix/import/gnu.scm (gnu->guix-package): Add 'G_' to messages.

Two comments: (1) things like (message "string") are already i18n’d, and
(2):

> +     (raise (make-compound-condition
> +             (formatted-message
> +              (G_ "failed to determine latest release of GNU ~a")
> +              name))))))

I removed ‘make-compound-condition’, which is unnecessary here.

Thanks!

Ludo’.

[bug#45983] [PATCH v2 3/3] scripts: import: json: Fix error handling.

[zimoun]

Fixes partially <https://bugs.gnu.org/44115>.

* guix/scripts/import/json.scm (guix-import-json): Handle error.
---
 guix/scripts/import/json.scm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/import/json.scm b/guix/scripts/import/json.scm
index 778e5f4bc5..d8d5c3a4af 100644
--- a/guix/scripts/import/json.scm
+++ b/guix/scripts/import/json.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014 Eric Bavier <bavier@member.fsf.org>
 ;;; Copyright © 2015, 2017 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -88,8 +89,13 @@ Import and convert the JSON package definition in PACKAGE-FILE.\n"))
                            (reverse opts))))
     (match args
       ((file-name)
-       (or (json->code file-name)
-           (leave (G_ "invalid JSON in file '~a'~%") file-name)))
+       (catch 'system-error
+         (lambda ()
+           (or (json->code file-name)
+               (leave (G_ "invalid JSON in file '~a'~%") file-name)))
+         (lambda args
+           (leave (G_ "failed to access '~a': ~a~%")
+                  file-name (strerror (system-error-errno args))))))
       (()
        (leave (G_ "too few arguments~%")))
       ((many ...)
-- 
2.29.2