Re: [PATCH 7/7] gnu: Add r-bioccheck.

[Ricardo Wurmus]

Roel Janssen <address@hidden> writes:

>>> +(define-public r-bioccheck
>>> +  (package
>>> +    (name "r-bioccheck")
>>> +    (version "1.10.0")
>>> +    (source (origin
>>> +              (method url-fetch)
>>> +              (uri (bioconductor-uri "BiocCheck" version))
>>> +              (sha256
>>> +               (base32
>>> +                "1rfy37xg1nc2cmgbclvzsi7sgmdcdjiahsx9crgx3yaw7kxgiack"))))
>>> +    (properties
>>> +     `((upstream-name . "BiocCheck")))
>>> +    (build-system r-build-system)
>>> +    (arguments
>>> +     '(#:phases
>>> +       (modify-phases %standard-phases
>>> +         ;; This package can be used by calling BiocCheck(<package>) from
>>> +         ;; within R, or by running R CMD BiocCheck <package>.  This phase
>>> +         ;; makes sure the latter works.  For this to work, the BiocCheck
>>> +         ;; script must be somewhere on the PATH (not the R bin directory).
>>> +         (add-after 'install 'install-bioccheck-subcommand
>>> +           (lambda _
>>> +             (let ((dest-dir (string-append %output "/bin"))
>>> +                   (script-dir
>>> +                    (string-append %output 
>>> "/site-library/BiocCheck/script/")))
>>
>> We usually prefer to do this
>>
>>     (lambda* (#:key outputs #:allow-other-keys)
>>       (let* ((out (assoc-ref outputs "out"))
>>              … out …)))
>>
>> instead of using the magical “%output”.
>
> Ofcourse!  Sorry, I should've known this.
>
>
>>> +               (mkdir-p dest-dir)
>>> +               (zero? (system* "ln" "--symbolic"
>>> +                               (string-append script-dir "/checkBadDeps.R")
>>> +                               (string-append dest-dir "/checkBadDeps.R")))
>>> +               (zero? (system* "ln" "--symbolic"
>>> +                               (string-append script-dir "/BiocCheck")
>>> +                               (string-append dest-dir 
>>> "/BiocCheck")))))))))
>>
>> Would it work to use “install-file” here (or to use Guile’s “symlink
>> oldpath newpath”) instead of shelling out?
>
> I prefer symlinking.  Sorry for being a shell-out.  I updated the patch
> to use Guile's @code{symlink} instead (I wasn't aware of this function!).
>
> Here's the new patch:
>
>>From cce8a9580645011cf8f6b259296a4779633aa2af Mon Sep 17 00:00:00 2001
> From: Roel Janssen <address@hidden>
> Date: Thu, 20 Oct 2016 00:00:33 +0200
> Subject: [PATCH] gnu: Add r-bioccheck.
>
> * gnu/packages/bioinformatics.scm (r-bioccheck): New variable.
> ---
>  gnu/packages/bioinformatics.scm | 46 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
> index 3fe7b5f..8e426d0 100644
> --- a/gnu/packages/bioinformatics.scm
> +++ b/gnu/packages/bioinformatics.scm
> @@ -5936,6 +5936,52 @@ Bioconductor PDF and HTML documents.  Package 
> vignettes illustrate use and
>  functionality.")
>      (license license:artistic2.0)))
>  
> +(define-public r-bioccheck
> +  (package
> +    (name "r-bioccheck")
> +    (version "1.10.0")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (bioconductor-uri "BiocCheck" version))
> +              (sha256
> +               (base32
> +                "1rfy37xg1nc2cmgbclvzsi7sgmdcdjiahsx9crgx3yaw7kxgiack"))))
> +    (properties
> +     `((upstream-name . "BiocCheck")))
> +    (build-system r-build-system)
> +    (arguments
> +     '(#:phases
> +       (modify-phases %standard-phases
> +         ;; This package can be used by calling BiocCheck(<package>) from
> +         ;; within R, or by running R CMD BiocCheck <package>.  This phase
> +         ;; makes sure the latter works.  For this to work, the BiocCheck
> +         ;; script must be somewhere on the PATH (not the R bin directory).
> +         (add-after 'install 'install-bioccheck-subcommand
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let ((dest-dir (string-append (assoc-ref outputs "out") 
> "/bin"))
> +                   (script-dir
> +                    (string-append %output 
> "/site-library/BiocCheck/script/")))

You missed one “%output” :)
I’d bind “out” it in the “let*” so that you don’t need to “assoc-ref” twice.

> +               (mkdir-p dest-dir)
> +               (symlink (string-append script-dir "/checkBadDeps.R")
> +                        (string-append dest-dir "/checkBadDeps.R"))
> +               (symlink (string-append script-dir "/BiocCheck")
> +                        (string-append dest-dir "/BiocCheck"))))))))

One last thing: please let the lambda end with #t.
Other than that it’s all good.

Thanks!

~~ Ricardo