GNU bug report logs - #40643
[PATCH] git-version: Handle invalid arguments gracefully

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 40643 in the body.
You can then email your comments to 40643 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: guix-patches <at> gnu.org
Subject: [PATCH] git-version: Handle invalid arguments gracefully
Date: Wed, 15 Apr 2020 17:18:23 +0200

* 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?

[1]: http://logs.guix.gnu.org/guix/2020-04-14.log#162809

 guix/git-download.scm | 9 +++++++++
 1 file changed, 9 insertions(+)

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 <at> gnu.org>
 ;;; Copyright © 2017 Mathieu Lirzin <mthl <at> gnu.org>
 ;;; Copyright © 2017 Christopher Baines <mail <at> cbaines.net>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> 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

Message #8 received at 40643 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Jakub Kądziołka <kuba <at> kadziolka.net>
Cc: 40643 <at> debbugs.gnu.org
Subject: Re: [bug#40643] [PATCH] git-version: Handle invalid arguments
 gracefully
Date: Fri, 17 Apr 2020 23:16:44 +0200

Hi Jakub,

Jakub Kądziołka <kuba <at> 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’.

Message #11 received at 40643 <at> debbugs.gnu.org (full text, mbox):

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 40643 <at> debbugs.gnu.org
Subject: Re: [bug#40643] [PATCH] git-version: Handle invalid arguments
 gracefully
Date: Wed, 22 Apr 2020 00:45:45 +0200

[Message part 1 (text/plain, inline)]

On Fri, Apr 17, 2020 at 11:16:44PM +0200, Ludovic Courtès wrote:
> Hi Jakub,
> 
> Jakub Kądziołka <kuba <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 40643 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Jakub Kądziołka <kuba <at> kadziolka.net>
Cc: 40643 <at> debbugs.gnu.org
Subject: Re: [bug#40643] [PATCH] git-version: Handle invalid arguments
 gracefully
Date: Wed, 22 Apr 2020 17:10:18 +0200

Hi,

Jakub Kądziołka <kuba <at> kadziolka.net> skribis:

> On Fri, Apr 17, 2020 at 11:16:44PM +0200, Ludovic Courtès wrote:
>> Hi Jakub,
>> 
>> Jakub Kądziołka <kuba <at> 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’.

Message #19 received at 40643-done <at> debbugs.gnu.org (full text, mbox):

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 40643-done <at> debbugs.gnu.org
Subject: Re: [bug#40643] [PATCH] git-version: Handle invalid arguments
 gracefully
Date: Thu, 23 Apr 2020 15:32:01 +0200

[Message part 1 (text/plain, inline)]

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

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.