| Message ID | 20200415151824.22988-1-kuba@kadziolka.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#40643] git-version: Handle invalid arguments gracefully | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
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. Thanks, Ludo’.
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? Regards, Jakub Kądziołka
Hi, Jakub Kądziołka <kuba@kadziolka.net> skribis: > 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 error conditions. HTH! Ludo’.
On Wed, Apr 22, 2020 at 05:10:18PM +0200, Ludovic Courtès wrote: > 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
diff --git a/guix/git-download.scm b/guix/git-download.scm index 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)