[PATCH] git-version: Handle invalid arguments gracefully

DoneSubmitted by Jakub Kądziołka.
Details
2 participants
  • Jakub Kądziołka
  • Ludovic Courtès
Owner
unassigned
Severity
normal
J
J
Jakub Kądziołka wrote on 15 Apr 17:18 +0200
(address . guix-patches@gnu.org)
20200415151824.22988-1-kuba@kadziolka.net
* guix/git-download.scm (git-version): Add a check for commit ID length.---If you're curious for the motivation, see [1]. This took a while todebug, so I'm hoping to ease this for the next person who inevitablystumbles upon this. Is a change like this okay?
[1]: http://logs.guix.gnu.org/guix/2020-04-14.log#162809
guix/git-download.scm | 9 +++++++++ 1 file changed, 9 insertions(+)
Toggle diff (36 lines)diff --git a/guix/git-download.scm b/guix/git-download.scmindex 1eae035fc4..40d81c72b9 100644--- a/guix/git-download.scm+++ b/guix/git-download.scm@@ -2,6 +2,7 @@ ;;; Copyright © 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2017 Mathieu Lirzin <mthl@gnu.org> ;;; Copyright © 2017 Christopher Baines <mail@cbaines.net>+;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net> ;;; ;;; This file is part of GNU Guix. ;;;@@ -30,6 +31,7 @@ #:use-module (ice-9 match) #:use-module (ice-9 vlist) #:use-module (srfi srfi-1)+ #:use-module (srfi srfi-35) #:export (git-reference git-reference? git-reference-url@@ -170,6 +172,13 @@ HASH-ALGO (a symbol). Use NAME as the file name, or a generic name if #f." (define (git-version version revision commit) "Return the version string for packages using git-download."+ ;; git-version is almost exclusively executed while modules are being loaded.+ ;; This makes any errors hide their backtrace. Avoid the mysterious error+ ;; "Value out of range 0 to N: 7" when the commit ID is too short, which+ ;; can happen, for example, when the user swapped the revision and commit+ ;; arguments by mistake.+ (when (< (string-length commit) 7)+ (error "git-version: commit ID unexpectedly short")) (string-append version "-" revision "." (string-take commit 7))) (define (git-file-name name version)-- 2.26.0
L
L
Ludovic Courtès wrote on 17 Apr 23:16 +0200
(name . Jakub Kądziołka)(address . kuba@kadziolka.net)(address . 40643@debbugs.gnu.org)
87v9lx95j7.fsf@gnu.org
Hi Jakub,
Jakub Kądziołka <kuba@kadziolka.net> skribis:
Toggle quote (6 lines)> * guix/git-download.scm (git-version): Add a check for commit ID length.> ---> If you're curious for the motivation, see [1]. This took a while to> debug, so I'm hoping to ease this for the next person who inevitably> stumbles upon this. Is a change like this okay?
Yes, I think so. The ‘error’ procedure is not great, we would ratheruse ‘raise’ with a ‘&message’ condition (which additionally allows fori18n) but it’s no big deal here.
Thanks,Ludo’.
J
J
Jakub Kądziołka wrote on 22 Apr 00:45 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 40643@debbugs.gnu.org)
20200421224545.ztr7beu3mqaufyo3@gravity
On Fri, Apr 17, 2020 at 11:16:44PM +0200, Ludovic Courtès wrote:
Toggle quote (14 lines)> Hi Jakub,> > Jakub Kądziołka <kuba@kadziolka.net> skribis:> > > * guix/git-download.scm (git-version): Add a check for commit ID length.> > ---> > If you're curious for the motivation, see [1]. This took a while to> > debug, so I'm hoping to ease this for the next person who inevitably> > stumbles upon this. Is a change like this okay?> > Yes, I think so. The ‘error’ procedure is not great, we would rather> use ‘raise’ with a ‘&message’ condition (which additionally allows for> i18n) but it’s no big deal here.
I considered using raise instead, but I couldn't get it to workproperly. I was getting a "Wrong type (expecting exact integer)" errorinstead:
(raise (condition (&message (message "git-version: commit ID unexpectedly short"))))
Do you know why that might be?
Regards,Jakub Kądziołka
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE5Xa/ss9usT31cTO54xWnWEYTFWQFAl6fd5MACgkQ4xWnWEYTFWRU1BAAjPJ6kRmZHq80VzCFfpao+YGAcQoOo/jX4RrYl9SOwvMfTKTV1rf1jv/27QV4Z/q6jg7xPcBkuPL2M+BZB8P3ELlRnTQPZcPhojYEb7R4u+C+7r3otSb8L26VsNEHXCjbUbgHdKMBgAU48oAHtT6cFgFOo5h7Lnkz6JwIUU1K65VOAuh7O30UI9bmdfgNnIUh0RRT8jLSkcUqsCC90XdwEs/jxTihIzMwoFIWRBdWj17lgUtoQWSQjd00pX2QidfKaYr7fxJgY7qiG/0hTFwLBkpduIRmFB48Btoc+eeEEWAvmcV4ShBz2pjE1RQQsP3u73x0SOOAWKBjegA3xXmtbZwzVrmJgXYeyXFxukGMhYOIpSEQDQYQE45ju6A6mE3NbzaPUJGtaA6xOBr4P7YOccg35C2ZsdtKHNopRDaLgcKnol8vaitADsNBu8mO3F1V32TpGXovtifZz992INEzuym2zR/SrgAC3OS9DbKQpbVLqA78PxfIJDT0QEuXF5oAdpDzvyLkZPpTxKd2hIECTM37r/FPl8TH5FFzLlpAUZFXJAeXFilqrbJDim/Te1lQ0bTPpYmSIXBdcat3ZdBnjYAC5WzwYuSkhPl3w7iaj168MT243e8umH5XHT1Q+/nZlfA2mkLadpA+rxRTxvO1PLbz0B2HAOxSSLG2upH7OA8==81vp-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 22 Apr 17:10 +0200
(name . Jakub Kądziołka)(address . kuba@kadziolka.net)(address . 40643@debbugs.gnu.org)
87pnbzwo85.fsf@gnu.org
Hi,
Jakub Kądziołka <kuba@kadziolka.net> skribis:
Toggle quote (25 lines)> On Fri, Apr 17, 2020 at 11:16:44PM +0200, Ludovic Courtès wrote:>> Hi Jakub,>> >> Jakub Kądziołka <kuba@kadziolka.net> skribis:>> >> > * guix/git-download.scm (git-version): Add a check for commit ID length.>> > --->> > If you're curious for the motivation, see [1]. This took a while to>> > debug, so I'm hoping to ease this for the next person who inevitably>> > stumbles upon this. Is a change like this okay?>> >> Yes, I think so. The ‘error’ procedure is not great, we would rather>> use ‘raise’ with a ‘&message’ condition (which additionally allows for>> i18n) but it’s no big deal here.>> I considered using raise instead, but I couldn't get it to work> properly. I was getting a "Wrong type (expecting exact integer)" error> instead:>> (raise> (condition (&message> (message "git-version: commit ID unexpectedly short"))))>> Do you know why that might be?
It must be because you forgot to include (srfi srfi-34): there are two‘raise’ procedure, and in core Guile ‘raise’ is about signals, not errorconditions.
HTH!
Ludo’.
J
J
Jakub Kądziołka wrote on 23 Apr 15:32 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 40643-done@debbugs.gnu.org)
20200423133201.7cif3mwfdotazedt@gravity
On Wed, Apr 22, 2020 at 05:10:18PM +0200, Ludovic Courtès wrote:
Toggle quote (4 lines)> It must be because you forgot to include (srfi srfi-34): there are two> ‘raise’ procedure, and in core Guile ‘raise’ is about signals, not error> conditions.
Thanks! That did it! I pushed an updated patch.
Regards,Jakub Kądziołka
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE5Xa/ss9usT31cTO54xWnWEYTFWQFAl6hmM0ACgkQ4xWnWEYTFWQsBA//TB4DOC0ZBqNdfhdS1Z0T8U0AfgJP40NhPwNMoEIpt3CE+rPvuOY5weAqPpJkQraWo0icPYLvvxqwT9s1b7AVqhrkMm+EVjhY3ggmBvl4dzT3NAS5M84qBQQ83xcPTClVo5sHFeo/6Q7cR3SevA/fNR764QKlQN++ie4bW+k48snDy+ZnB2qIzp9xHpUFuWtN4ljV17Fw7sLSQIZ+vzBL/p/FL67dntvOOtqFn2lD5k1qZJ7xh7yHI8iwTZnBXsTNAqGmpaR8xlZzyoXTgZt0PylzLPFODFK4oTK4+R3Dqb3mamTI6zk6G1wxSjwVTUypqsIvFYpAHyeFfThFO2CxOHNIpfv/DaquH1Suiksf6+9098AUZFdoDvB4zBugRWjOA/zt0iS7iRFtKKHV7Q3vsqirrcy6QvD9wwiYVIaClbK4Gz0qM5yag27+XLkqSbO3D4Tps46ewvEQomVh/6yqCodyhZg3dxikUUAkN5s3t5rA0H3ta6RpW1Oc5AfdbV5Mg/YXh8Ur9eBS11PRzGB54owxzClg3d7EsZ0uJwrZGWGKvNF6Y8vjoQb4/hXDS5p5n6wkAN/TuftbjvHAKwmHetyT35UvIsZXNBjHSGfYQLCbbGcMDXEbO8sDjXqek54w2vIgOvy4lZ1CKjJA0A5kygqNxNDi+G3t0y+AWPRZNLQ==3eo/-----END PGP SIGNATURE-----

Closed
?