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

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • zimoun
Owner
unassigned
Submitted by
zimoun
Severity
normal
Z
Z
zimoun wrote on 19 Jan 2021 16:25
(address . guix-patches@gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210119152518.3464-1-zimon.toutoune@gmail.com
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:

Toggle snippet (7 lines)
$ ./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

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
Z
Z
zimoun wrote on 19 Jan 2021 16:27
[PATCH 1/3] import: gnu: Add internationalized messages.
(address . 45983@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210119152737.4344-1-zimon.toutoune@gmail.com
* guix/import/gnu.scm (gnu->guix-package): Add 'G_' to messages.
---
guix/import/gnu.scm | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

Toggle diff (42 lines)
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
Z
Z
zimoun wrote on 19 Jan 2021 16:27
[PATCH 2/3] guix: gnu-maintenance: Fix error handling.
(address . 45983@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210119152737.4344-2-zimon.toutoune@gmail.com

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

Toggle diff (25 lines)
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
Z
Z
zimoun wrote on 19 Jan 2021 16:27
[PATCH 3/3] scripts: import: json: Fix error handling.
(address . 45983@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210119152737.4344-3-zimon.toutoune@gmail.com

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

Toggle diff (27 lines)
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
L
L
Ludovic Courtès wrote on 26 Jan 2021 23:14
Re: bug#45983: [PATCH 0/3] Fix non-recursive importers gnu and json
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45983@debbugs.gnu.org)
87sg6nwdk3.fsf_-_@gnu.org
Hi,

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

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

[...]

Toggle quote (15 lines)
> - (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’.
L
L
Ludovic Courtès wrote on 26 Jan 2021 23:17
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45983@debbugs.gnu.org)
87o8hbwdez.fsf_-_@gnu.org
zimoun <zimon.toutoune@gmail.com> skribis:

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

[...]

Toggle quote (7 lines)
> - (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’.
Z
Z
zimoun wrote on 28 Jan 2021 00:46
[PATCH v2 2/3] import: gnu: Add internationalized messages.
(address . 45983@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210127234603.24077-2-zimon.toutoune@gmail.com
* guix/import/gnu.scm (gnu->guix-package): Add 'G_' to messages.
---
guix/import/gnu.scm | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)

Toggle diff (48 lines)
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
Z
Z
zimoun wrote on 28 Jan 2021 00:46
[PATCH v2 1/3] guix: gnu-maintenance: Fix error handling.
(address . 45983@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210127234603.24077-1-zimon.toutoune@gmail.com

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

Toggle diff (27 lines)
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
Z
Z
zimoun wrote on 28 Jan 2021 00:46
[PATCH v2 3/3] scripts: import: json: Fix error handling.
(address . 45983@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20210127234603.24077-3-zimon.toutoune@gmail.com

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

Toggle diff (30 lines)
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
L
L
Ludovic Courtès wrote on 31 Jan 2021 21:37
Re: bug#45983: [PATCH 0/3] Fix non-recursive importers gnu and json
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 45983-done@debbugs.gnu.org)
87pn1klu61.fsf_-_@gnu.org
Hi!

I applied the whole series.

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

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

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

Toggle quote (5 lines)
> + (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’.
Closed
?