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

[Ricardo Wurmus <rekado@elephly.net>]

Roel Janssen <roel@gnu.org> writes:

> Ricardo Wurmus writes:
>
>> Roel Janssen <roel@gnu.org> writes:
>>
>>> * gnu/packages/bioinformatics.scm (r-bioccheck): New variable.
>>> ---
>>>  gnu/packages/bioinformatics.scm | 29 +++++++++++++++++++++++++++++
>>>  1 file changed, 29 insertions(+)
>>>
>>> diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
>>> index 902d1d2..00a4161 100644
>>> --- a/gnu/packages/bioinformatics.scm
>>> +++ b/gnu/packages/bioinformatics.scm
>>> @@ -5426,6 +5426,35 @@ like automatically generated help option and usage, support for default
>>>  values, positional argument support, etc.")                                                                     
>>>      (license license:gpl2+)))
>>>  
>>> +(define-public r-bioccheck
>>> +  (package
>>> +    (name "r-bioccheck")
>>> +    (version "1.8.2")
>>> +    (source (origin
>>> +              (method url-fetch)
>>> +              (uri (bioconductor-uri "BiocCheck" version))
>>> +              (sha256
>>> +               (base32
>>> +                "0bvkhxmr25nb0gkncifbc3x48i5zgls3737g3xais6jx16wd4q35"))))
>>> +    (properties
>>> +     `((upstream-name . "BiocCheck")))
>>> +    (build-system r-build-system)
>>> +    (inputs
>>> +     `(("which" ,which)))
>>
>> Is this a native input?
>
> Ah, yes it is.
>
>>> +    (propagated-inputs
>>> +     `(("r-graph" ,r-graph)
>>> +       ("r-knitr" ,r-knitr)
>>> +       ("r-httr" ,r-httr)
>>> +       ("r-optparse" ,r-optparse)
>>> +       ("r-devtools" ,r-devtools)
>>> +       ("r-biocinstaller" ,r-biocinstaller)
>>> +       ("r-biocviews" ,r-biocviews)))
>>> +    (home-page "http://bioconductor.org/packages/BiocCheck")
>>> +    (synopsis "Executes Bioconductor-specific package checks")
>>> +    (description "This package executes Bioconductor-specific
>>> +package checks.")
>>
>> Well… okay. :)  It would be nice if it was a little more specific.
>>
>> Maybe:
>>
>>    This package contains tools to perform quality checks on R packages
>>    that are to be submitted to the Bioconductor repository.
>>
>> WDYT?
>
> I used your description, but added "additional" to it.  I also added a
> custom build phase to make "R CMD BiocCheck" work.
>
> Here's the new patch:
>
>From 1d9248098fc507876d77cddc890ea906aef7b83d Mon Sep 17 00:00:00 2001
> From: Roel Janssen <roel@gnu.org>
> Date: Wed, 19 Oct 2016 16:43:04 +0200
> Subject: [PATCH] gnu: Add r-bioccheck.
>
> * gnu/packages/bioinformatics.scm (r-bioccheck): New variable.
> ---
>  gnu/packages/bioinformatics.scm | 48 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
> index 3fe7b5f..d21781f 100644
> --- a/gnu/packages/bioinformatics.scm
> +++ b/gnu/packages/bioinformatics.scm
> @@ -5936,6 +5936,54 @@ 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 _
> +             (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”.


> +               (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?

~~ Ricardo