GNU bug report logs - #26373
Various improvements to the CRAN importer

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 26373 in the body.
You can then email your comments to 26373 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: Ricardo Wurmus <rekado <at> elephly.net>
To: guix-patches <at> gnu.org
Subject: Various improvements to the CRAN importer
Date: Wed, 05 Apr 2017 18:40:02 +0200

The following patch series improves the CRAN importer.

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 26373 <at> debbugs.gnu.org
Cc: Ricardo Wurmus <rekado <at> elephly.net>
Subject: [PATCH 1/6] import cran: Exclude experiment packages in predicate
 "bioconductor-package?".
Date: Wed,  5 Apr 2017 18:42:05 +0200

* guix/import/cran.scm (bioconductor-package?): Exclude experiment packages,
because they cannot be updated with the default bioconductor updater.
---
 guix/import/cran.scm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index 8e24f6e17..f63d23972 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -429,7 +429,9 @@ dependencies."
                           ;; the Github mirror, so we have to exclude them
                           ;; from the set of bioconductor packages that can be
                           ;; updated automatically.
-                          (not (string-contains uri "/data/annotation/"))))))
+                          (not (string-contains uri "/data/annotation/"))
+                          ;; Experiment packages are in a separate repository.
+                          (not (string-contains uri "/data/experiment/"))))))
     (and (string-prefix? "r-" (package-name package))
          (match (and=> (package-source package) origin-uri)
            ((? string? uri)
-- 
2.12.2

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 26373 <at> debbugs.gnu.org
Cc: Ricardo Wurmus <rekado <at> elephly.net>
Subject: [PATCH 2/6] import cran: Add predicate for Bioconductor experiment
 packages.
Date: Wed,  5 Apr 2017 18:42:06 +0200

* guix/import/cran.scm (bioconductor-experiment-package?): New variable.
---
 guix/import/cran.scm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index f63d23972..48ab7355d 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -453,6 +453,19 @@ dependencies."
             (any predicate uris))
            (_ #f)))))
 
+(define (bioconductor-experiment-package? package)
+  "Return true if PACKAGE is an R experiment package from Bioconductor."
+  (let ((predicate (lambda (uri)
+                     (and (string-prefix? "http://bioconductor.org" uri)
+                          (string-contains uri "/data/experiment/")))))
+    (and (string-prefix? "r-" (package-name package))
+         (match (and=> (package-source package) origin-uri)
+           ((? string? uri)
+            (predicate uri))
+           ((? list? uris)
+            (any predicate uris))
+           (_ #f)))))
+
 (define %cran-updater
   (upstream-updater
    (name 'cran)
-- 
2.12.2

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 26373 <at> debbugs.gnu.org
Cc: Ricardo Wurmus <rekado <at> elephly.net>
Subject: [PATCH 3/6] import cran: Refactor "needs-zlib?".
Date: Wed,  5 Apr 2017 18:42:07 +0200

* guix/import/cran.scm (tarball-files-match-pattern?): New procedure.
(needs-zlib?): Implement in terms of "tarball-files-match-pattern?".
---
 guix/import/cran.scm | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index 48ab7355d..be3b678cd 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -201,17 +201,16 @@ empty list when the FIELD cannot be found."
       (check "*.f95")
       (check "*.f")))
 
-(define (needs-zlib? tarball)
-  "Return #T if any of the Makevars files in the src directory of the TARBALL
-contain a zlib linker flag."
+(define (tarball-files-match-pattern? tarball regexp . file-patterns)
+  "Return #T if any of the files represented by FILE-PATTERNS in the TARBALL
+match the given REGEXP."
   (call-with-temporary-directory
    (lambda (dir)
-     (let ((pattern (make-regexp "-lz")))
+     (let ((pattern (make-regexp regexp)))
        (parameterize ((current-error-port (%make-void-port "rw+")))
-         (system* "tar"
-                  "xf" tarball "-C" dir
-                  "--wildcards"
-                  "*/src/Makevars*" "*/src/configure*" "*/configure*"))
+         (apply system* "tar"
+                "xf" tarball "-C" dir
+                `("--wildcards" ,@file-patterns)))
        (any (lambda (file)
               (call-with-input-file file
                 (lambda (port)
@@ -220,10 +219,16 @@ contain a zlib linker flag."
                       (cond
                        ((eof-object? line) #f)
                        ((regexp-exec pattern line) #t)
-                       (else (loop)))))))
-              #t)
+                       (else (loop))))))))
             (find-files dir))))))
 
+(define (needs-zlib? tarball)
+  "Return #T if any of the Makevars files in the src directory of the TARBALL
+contain a zlib linker flag."
+  (tarball-files-match-pattern?
+   tarball "-lz"
+   "*/src/Makevars*" "*/src/configure*" "*/configure*"))
+
 (define (description->package repository meta)
   "Return the `package' s-expression for an R package published on REPOSITORY
 from the alist META, which was derived from the R package's DESCRIPTION file."
-- 
2.12.2

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 26373 <at> debbugs.gnu.org
Cc: Ricardo Wurmus <rekado <at> elephly.net>
Subject: [PATCH 4/6] import cran: Check if pkg-config is needed.
Date: Wed,  5 Apr 2017 18:42:08 +0200

* guix/import/cran.scm (needs-pkg-config?): New procedure.
(description->package): Use it.
---
 guix/import/cran.scm | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index be3b678cd..423835637 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -229,6 +229,13 @@ contain a zlib linker flag."
    tarball "-lz"
    "*/src/Makevars*" "*/src/configure*" "*/configure*"))
 
+(define (needs-pkg-config? tarball)
+  "Return #T if any of the Makevars files in the src directory of the TARBALL
+reference the pkg-config tool."
+  (tarball-files-match-pattern?
+   tarball "pkg-config"
+   "*/src/Makevars*" "*/src/configure*" "*/configure*"))
+
 (define (description->package repository meta)
   "Return the `package' s-expression for an R package published on REPOSITORY
 from the alist META, which was derived from the R package's DESCRIPTION file."
@@ -278,11 +285,12 @@ from the alist META, which was derived from the R package's DESCRIPTION file."
         (build-system r-build-system)
         ,@(maybe-inputs sysdepends)
         ,@(maybe-inputs (map guix-name propagate) 'propagated-inputs)
-        ,@(if (needs-fortran? tarball)
-              `((native-inputs (,'quasiquote
-                                ,(list "gfortran"
-                                       (list 'unquote 'gfortran)))))
-              '())
+        ,@(maybe-inputs
+           `(,@(if (needs-fortran? tarball)
+                   '("gfortran") '())
+             ,@(if (needs-pkg-config? tarball)
+                   '("pkg-config") '()))
+           'native-inputs)
         (home-page ,(if (string-null? home-page)
                         (string-append base-url name)
                         home-page))
-- 
2.12.2

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 26373 <at> debbugs.gnu.org
Cc: Ricardo Wurmus <rekado <at> elephly.net>
Subject: [PATCH 5/6] import cran: Ensure substring indices are valid.
Date: Wed,  5 Apr 2017 18:42:09 +0200

* guix/import/cran.scm (package->upstream-name): Check that "start" and "end"
are valid before using them as substring indices.
---
 guix/import/cran.scm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index 423835637..557d694ad 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -384,9 +384,10 @@ dependencies."
              ((or (? string? url) (url _ ...))
               (let ((end   (string-rindex url #\_))
                     (start (string-rindex url #\/)))
-                ;; The URL ends on
-                ;; (string-append "/" name "_" version ".tar.gz")
-                (substring url (+ start 1) end)))
+                (and start end
+                     ;; The URL ends on
+                     ;; (string-append "/" name "_" version ".tar.gz")
+                     (substring url (+ start 1) end))))
              (_ #f)))
           (_ #f)))))
 
-- 
2.12.2

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 26373 <at> debbugs.gnu.org
Cc: Ricardo Wurmus <rekado <at> elephly.net>
Subject: [PATCH 6/6] import cran: Skip updating when meta data cannot be
 downloaded.
Date: Wed,  5 Apr 2017 18:42:10 +0200

* gnu/packages/bioinformatics.scm (latest-cran-release,
latest-bioconductor-release): Abort early when meta data cannot be downloaded.
---
 guix/import/cran.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index 557d694ad..fc7a1ed84 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -398,7 +398,8 @@ dependencies."
     (package->upstream-name package))
 
   (define meta
-    (fetch-description 'cran upstream-name))
+    (false-if-exception
+     (fetch-description 'cran upstream-name)))
 
   (and meta
        (let ((version (assoc-ref meta "Version")))
@@ -415,7 +416,8 @@ dependencies."
     (package->upstream-name package))
 
   (define meta
-    (fetch-description 'bioconductor upstream-name))
+    (false-if-exception
+     (fetch-description 'bioconductor upstream-name)))
 
   (and meta
        (let ((version (assoc-ref meta "Version")))
-- 
2.12.2

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 1/6] import cran: Exclude experiment packages
 in predicate "bioconductor-package?".
Date: Mon, 10 Apr 2017 11:50:37 +0200

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> * guix/import/cran.scm (bioconductor-package?): Exclude experiment packages,
> because they cannot be updated with the default bioconductor updater.

OK.

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 2/6] import cran: Add predicate for
 Bioconductor experiment packages.
Date: Mon, 10 Apr 2017 11:51:36 +0200

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> * guix/import/cran.scm (bioconductor-experiment-package?): New variable.

It seems to be a private and unused procedure.  Is it really needed?
:-)

Thanks,
Ludo’.

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 3/6] import cran: Refactor "needs-zlib?".
Date: Mon, 10 Apr 2017 11:52:02 +0200

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> * guix/import/cran.scm (tarball-files-match-pattern?): New procedure.
> (needs-zlib?): Implement in terms of "tarball-files-match-pattern?".

LGTM!

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 4/6] import cran: Check if pkg-config is needed.
Date: Mon, 10 Apr 2017 11:52:19 +0200

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> * guix/import/cran.scm (needs-pkg-config?): New procedure.
> (description->package): Use it.

LGTM!

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 5/6] import cran: Ensure substring indices are
 valid.
Date: Mon, 10 Apr 2017 11:52:59 +0200

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> * guix/import/cran.scm (package->upstream-name): Check that "start" and "end"
> are valid before using them as substring indices.

LGTM!

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 6/6] import cran: Skip updating when meta data
 cannot be downloaded.
Date: Mon, 10 Apr 2017 11:55:49 +0200

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> * gnu/packages/bioinformatics.scm (latest-cran-release,
> latest-bioconductor-release): Abort early when meta data cannot be downloaded.
> ---
>  guix/import/cran.scm | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/guix/import/cran.scm b/guix/import/cran.scm
> index 557d694ad..fc7a1ed84 100644
> --- a/guix/import/cran.scm
> +++ b/guix/import/cran.scm
> @@ -398,7 +398,8 @@ dependencies."
>      (package->upstream-name package))
>  
>    (define meta
> -    (fetch-description 'cran upstream-name))
> +    (false-if-exception
> +     (fetch-description 'cran upstream-name)))

I would prefer catching only the relevant exception.  So I suppose
something like:

  (guard (c ((http-get-error? c)
             (if (= 404 (http-get-error-code c))
                 #f
                 (raise c))))
    (fetch-description 'cran upstream-name))

However I see that ‘fetch-description’ already does that, so what
exceptions are we protecting against?

Thanks,
Ludo’.

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 2/6] import cran: Add predicate for
 Bioconductor experiment packages.
Date: Mon, 10 Apr 2017 15:50:33 +0200

Ludovic Courtès <ludo <at> gnu.org> writes:

> Ricardo Wurmus <rekado <at> elephly.net> skribis:
>
>> * guix/import/cran.scm (bioconductor-experiment-package?): New variable.
>
> It seems to be a private and unused procedure.  Is it really needed?
> :-)

Oh, it wasn’t supposed to be private.  There are three kinds of
bioconductor packages: the normal ones, experiment packages, and
annotation packages.

The latter two cannot be updated the usual way, so they are excluded
from “bioconductor-package?”, but I thought that one may still want to
be able to distinguish between them.  It’s true, though, that there’s
nothing that can use “bioconductor-experiment-package?” yet, but it’s
possible to add an updater for just the experiment packages.  (They use
a different SVN repository.)

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 6/6] import cran: Skip updating when meta data
 cannot be downloaded.
Date: Mon, 10 Apr 2017 15:55:29 +0200

Ludovic Courtès <ludo <at> gnu.org> writes:

> Ricardo Wurmus <rekado <at> elephly.net> skribis:
>
>> * gnu/packages/bioinformatics.scm (latest-cran-release,
>> latest-bioconductor-release): Abort early when meta data cannot be downloaded.
>> ---
>>  guix/import/cran.scm | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/guix/import/cran.scm b/guix/import/cran.scm
>> index 557d694ad..fc7a1ed84 100644
>> --- a/guix/import/cran.scm
>> +++ b/guix/import/cran.scm
>> @@ -398,7 +398,8 @@ dependencies."
>>      (package->upstream-name package))
>>
>>    (define meta
>> -    (fetch-description 'cran upstream-name))
>> +    (false-if-exception
>> +     (fetch-description 'cran upstream-name)))
>
> I would prefer catching only the relevant exception.  So I suppose
> something like:
>
>   (guard (c ((http-get-error? c)
>              (if (= 404 (http-get-error-code c))
>                  #f
>                  (raise c))))
>     (fetch-description 'cran upstream-name))
>
> However I see that ‘fetch-description’ already does that, so what
> exceptions are we protecting against?

I don’t know what the desired behaviour here is.  When updating
packages, I think it’s good to keep going.  If there’s an error
downloading the package meta data I want “meta” to be “#f”, which leads
to skipping the update.

Without turning errors to “#f” I wasn’t able to just update all packages
with “guix refresh -t cran,bioconductor -u”.  That said, I don’t like
this.  I feel that it would be better to add a “--keep-going” option for
“refresh” and implement it in the shared updater code rather than here.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 2/6] import cran: Add predicate for
 Bioconductor experiment packages.
Date: Mon, 10 Apr 2017 23:34:51 +0200

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Ricardo Wurmus <rekado <at> elephly.net> skribis:
>>
>>> * guix/import/cran.scm (bioconductor-experiment-package?): New variable.
>>
>> It seems to be a private and unused procedure.  Is it really needed?
>> :-)
>
> Oh, it wasn’t supposed to be private.  There are three kinds of
> bioconductor packages: the normal ones, experiment packages, and
> annotation packages.
>
> The latter two cannot be updated the usual way, so they are excluded
> from “bioconductor-package?”, but I thought that one may still want to
> be able to distinguish between them.  It’s true, though, that there’s
> nothing that can use “bioconductor-experiment-package?” yet, but it’s
> possible to add an updater for just the experiment packages.  (They use
> a different SVN repository.)

OK, I see.  Make sure to export it, then!  :-)

Thanks for the explanation,
Ludo’.

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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26373 <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 6/6] import cran: Skip updating when meta data
 cannot be downloaded.
Date: Mon, 10 Apr 2017 23:37:29 +0200

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Ricardo Wurmus <rekado <at> elephly.net> skribis:
>>
>>> * gnu/packages/bioinformatics.scm (latest-cran-release,
>>> latest-bioconductor-release): Abort early when meta data cannot be downloaded.
>>> ---
>>>  guix/import/cran.scm | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/guix/import/cran.scm b/guix/import/cran.scm
>>> index 557d694ad..fc7a1ed84 100644
>>> --- a/guix/import/cran.scm
>>> +++ b/guix/import/cran.scm
>>> @@ -398,7 +398,8 @@ dependencies."
>>>      (package->upstream-name package))
>>>
>>>    (define meta
>>> -    (fetch-description 'cran upstream-name))
>>> +    (false-if-exception
>>> +     (fetch-description 'cran upstream-name)))
>>
>> I would prefer catching only the relevant exception.  So I suppose
>> something like:
>>
>>   (guard (c ((http-get-error? c)
>>              (if (= 404 (http-get-error-code c))
>>                  #f
>>                  (raise c))))
>>     (fetch-description 'cran upstream-name))
>>
>> However I see that ‘fetch-description’ already does that, so what
>> exceptions are we protecting against?
>
> I don’t know what the desired behaviour here is.  When updating
> packages, I think it’s good to keep going.  If there’s an error
> downloading the package meta data I want “meta” to be “#f”, which leads
> to skipping the update.
>
> Without turning errors to “#f” I wasn’t able to just update all packages
> with “guix refresh -t cran,bioconductor -u”.

What was the exception?

I think a good approach is to catch precisely the kind of error that we
don’t want to see.  ‘false-if-exception’ catches everything and could
thus hide genuine errors/bugs (including unbound variables and similar),
which sounds undesirable.

WDYT?

Ludo’.

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 26373-done <at> debbugs.gnu.org
Subject: Re: bug#26373: [PATCH 6/6] import cran: Skip updating when meta data
 cannot be downloaded.
Date: Tue, 16 May 2017 21:48:16 +0200

Ludovic Courtès <ludo <at> gnu.org> writes:

> Ricardo Wurmus <rekado <at> elephly.net> skribis:
>
>> Ludovic Courtès <ludo <at> gnu.org> writes:
>>
>>> Ricardo Wurmus <rekado <at> elephly.net> skribis:
>>>
>>>> * gnu/packages/bioinformatics.scm (latest-cran-release,
>>>> latest-bioconductor-release): Abort early when meta data cannot be downloaded.
>>>> ---
>>>>  guix/import/cran.scm | 6 ++++--
>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/guix/import/cran.scm b/guix/import/cran.scm
>>>> index 557d694ad..fc7a1ed84 100644
>>>> --- a/guix/import/cran.scm
>>>> +++ b/guix/import/cran.scm
>>>> @@ -398,7 +398,8 @@ dependencies."
>>>>      (package->upstream-name package))
>>>>
>>>>    (define meta
>>>> -    (fetch-description 'cran upstream-name))
>>>> +    (false-if-exception
>>>> +     (fetch-description 'cran upstream-name)))
>>>
>>> I would prefer catching only the relevant exception.  So I suppose
>>> something like:
>>>
>>>   (guard (c ((http-get-error? c)
>>>              (if (= 404 (http-get-error-code c))
>>>                  #f
>>>                  (raise c))))
>>>     (fetch-description 'cran upstream-name))
>>>
>>> However I see that ‘fetch-description’ already does that, so what
>>> exceptions are we protecting against?
>>
>> I don’t know what the desired behaviour here is.  When updating
>> packages, I think it’s good to keep going.  If there’s an error
>> downloading the package meta data I want “meta” to be “#f”, which leads
>> to skipping the update.
>>
>> Without turning errors to “#f” I wasn’t able to just update all packages
>> with “guix refresh -t cran,bioconductor -u”.
>
> What was the exception?
>
> I think a good approach is to catch precisely the kind of error that we
> don’t want to see.  ‘false-if-exception’ catches everything and could
> thus hide genuine errors/bugs (including unbound variables and similar),
> which sounds undesirable.
>
> WDYT?

I don’t remember why I wanted this, so I didn’t push this patch.  I
agree that it’s much better to catch errors precisely.

Thanks for the review!  I pushed all the other patches (with the
exception of one that Mathieu had already implemented independently) to
master.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

Previous Next

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