diff mbox

[2/3] gnu: Add python-pyxb.

Message ID 87eg4cyni4.fsf@ike.i-did-not-set--mail-host-address--so-tickle-me
State New
Headers show

Commit Message

Marius Bakke Sept. 22, 2016, 3:15 p.m. UTC
Ben Woodcroft <b.woodcroft@uq.edu.au> writes:

>> Subject: [PATCH 1/3] gnu: python-pysam: Update to 0.9.1.4.
> I'm not sure whether this is a product of the upgrade or not, but I 
> notice this in the build log. I think it is harmless though, WDYT?
>
> starting phase `validate-runpath'
> validating RUNPATH of 10 binaries in 
> "/gnu/store/bpiq3lm6b1kpf54i1vj2dl09ff293wic-python-pysam-0.9.1.4/lib"...
> /gnu/store/bpiq3lm6b1kpf54i1vj2dl09ff293wic-python-pysam-0.9.1.4/lib/python3.4/site-packages/pysam-0.9.1.4-py3.4-linux-x86_64.egg/pysam/libchtslib.cpython-34m.so: 
> warning: RUNPATH contains bogus entries: ("pysam" "." 
> "build/lib.linux-x86_64-3.4/pysam")

I don't see this in the previous version, so it is a regression.
However, it should be mostly harmless. Readelf reports (when compiled
with external htslib, see below):

 0x000000000000001d (RUNPATH)            Library runpath: [/gnu/store/m4gc2wx4q9if1vrhgclpspdil7rqsn21-python-3.4.3/lib:/gnu/store/ba22myqvxccwmmjwwq665rc43hanycxy-htslib-1.3.1/lib:build/lib.linux-x86_64-3.4/pysam:$ORIGIN:/gnu/store/m9vxvhdj691bq1f85lpflvnhcvrdilih-glibc-2.23/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib:/gnu/store/xl19qrfzga52vrvp4ncccwjlnrjqwj95-ncurses-6.0/lib:/gnu/store/5992iq1v7arqa14ym3di58n4la0893nv-zlib-1.2.8/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.3/../../..]

Compared to the runpath of the same file currently in Guix:

 0x000000000000001d (RUNPATH)            Library runpath: [/gnu/store/m4gc2wx4q9if1vrhgclpspdil7rqsn21-python-3.4.3/lib:/gnu/store/m9vxvhdj691bq1f85lpflvnhcvrdilih-glibc-2.23/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib:/gnu/store/xl19qrfzga52vrvp4ncccwjlnrjqwj95-ncurses-6.0/lib:/gnu/store/5992iq1v7arqa14ym3di58n4la0893nv-zlib-1.2.8/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.3/../../..]

If a folder named "$CWD/build/lib.linux-x86_64-3.4/pysam exists, it
could potentially allow for code injection, which is troubling.

I opened an issue on their tracker, but don't think it's worth holding
the patch: https://github.com/pysam-developers/pysam/issues/347

>
> Also, I notice that pysam bundles htslib, bcftools and samtools C code. 
> Hopefully it should be straightforward enough to remove htslib as there 
> are install instructions, I'm not sure about the other two. This 
> shouldn't block the patch here, but would you mind taking a look?
> http://pysam.readthedocs.io/en/latest/installation.html#installation

I had a go at this, and also enabled tests since I was reading the build
system anyway. Samtools and bcftools does not seem possible to un-bundle
at this time, but htslib was straightforward.

Enabling tests required packaging bcftools as well. I've attached the
full patch series below. The first three commits are unchanged.

Thanks!

Comments

Ben Woodcroft Sept. 23, 2016, 1:28 a.m. UTC | #1
On 09/23/2016 01:15 AM, Marius Bakke wrote:
> Ben Woodcroft <b.woodcroft@uq.edu.au> writes:
>
>>> Subject: [PATCH 1/3] gnu: python-pysam: Update to 0.9.1.4.
>> I'm not sure whether this is a product of the upgrade or not, but I
>> notice this in the build log. I think it is harmless though, WDYT?
>>
>> starting phase `validate-runpath'
>> validating RUNPATH of 10 binaries in
>> "/gnu/store/bpiq3lm6b1kpf54i1vj2dl09ff293wic-python-pysam-0.9.1.4/lib"...
>> /gnu/store/bpiq3lm6b1kpf54i1vj2dl09ff293wic-python-pysam-0.9.1.4/lib/python3.4/site-packages/pysam-0.9.1.4-py3.4-linux-x86_64.egg/pysam/libchtslib.cpython-34m.so:
>> warning: RUNPATH contains bogus entries: ("pysam" "."
>> "build/lib.linux-x86_64-3.4/pysam")
> I don't see this in the previous version, so it is a regression.
> However, it should be mostly harmless. Readelf reports (when compiled
> with external htslib, see below):
>
>   0x000000000000001d (RUNPATH)            Library runpath: [/gnu/store/m4gc2wx4q9if1vrhgclpspdil7rqsn21-python-3.4.3/lib:/gnu/store/ba22myqvxccwmmjwwq665rc43hanycxy-htslib-1.3.1/lib:build/lib.linux-x86_64-3.4/pysam:$ORIGIN:/gnu/store/m9vxvhdj691bq1f85lpflvnhcvrdilih-glibc-2.23/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib:/gnu/store/xl19qrfzga52vrvp4ncccwjlnrjqwj95-ncurses-6.0/lib:/gnu/store/5992iq1v7arqa14ym3di58n4la0893nv-zlib-1.2.8/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.3/../../..]
>
> Compared to the runpath of the same file currently in Guix:
>
>   0x000000000000001d (RUNPATH)            Library runpath: [/gnu/store/m4gc2wx4q9if1vrhgclpspdil7rqsn21-python-3.4.3/lib:/gnu/store/m9vxvhdj691bq1f85lpflvnhcvrdilih-glibc-2.23/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib:/gnu/store/xl19qrfzga52vrvp4ncccwjlnrjqwj95-ncurses-6.0/lib:/gnu/store/5992iq1v7arqa14ym3di58n4la0893nv-zlib-1.2.8/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.3/../../..]
>
> If a folder named "$CWD/build/lib.linux-x86_64-3.4/pysam exists, it
> could potentially allow for code injection, which is troubling.
>
> I opened an issue on their tracker, but don't think it's worth holding
> the patch: https://github.com/pysam-developers/pysam/issues/347
Thanks, I agree.

>
>> Also, I notice that pysam bundles htslib, bcftools and samtools C code.
>> Hopefully it should be straightforward enough to remove htslib as there
>> are install instructions, I'm not sure about the other two. This
>> shouldn't block the patch here, but would you mind taking a look?
>> http://pysam.readthedocs.io/en/latest/installation.html#installation
> I had a go at this, and also enabled tests since I was reading the build
> system anyway. Samtools and bcftools does not seem possible to un-bundle
> at this time, but htslib was straightforward.
OK. I don't think it needs to be propagated though, right? Also, would 
you mind separating the change to modify-phases syntax and unbundling of 
htslib into two patches please? Other than that this whole series LGTM.

Sorry, I keep asking one more thing..
ben
diff mbox

Patch

From ab4f0a5174554a7f2b7b243b5488654de7d81ac1 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Thu, 22 Sep 2016 14:11:59 +0100
Subject: [PATCH 6/6] gnu: python-pysam: Enable tests.

* gnu/packages/bioinformatics.scm (python-pysam)[source]: Change from
PyPi to source archive due to missing test data.
[arguments]: Add check phase after install.
[native-inputs]: Add python-nose, samtools and bcftools.
---
 gnu/packages/bioinformatics.scm | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index edfb943..abbe601 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -1353,18 +1353,21 @@  multiple sequence alignments.")
     (version "0.9.1.4")
     (source (origin
               (method url-fetch)
-              (uri (pypi-uri "pysam" version))
+              ;; Test data is missing on PyPi.
+              (uri (string-append
+                    "https://github.com/pysam-developers/pysam/archive/v"
+                    version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
               (sha256
                (base32
-                "1i1djacqbr88y7w18b4aa78zxnsyr4sz7yqdq2spi7gs0y6pzvjn"))
+                "0y41ssbg6nvn2jgcbnrvkzblpjcwszaiv1rgyd8dwzjkrbfsgsmc"))
               (modules '((guix build utils)))
               (snippet
                ;; Drop bundled htslib. TODO: Also remove samtools and bcftools.
                '(delete-file-recursively "htslib"))))
     (build-system python-build-system)
     (arguments
-     `(#:tests? #f ; tests are excluded in the manifest
-       #:phases
+     `(#:phases
        (modify-phases %standard-phases
          (add-before 'build 'set-flags
            (lambda* (#:key inputs #:allow-other-keys)
@@ -1374,7 +1377,24 @@  multiple sequence alignments.")
                                            (assoc-ref inputs "htslib") "/lib"))
              (setenv "HTSLIB_INCLUDE_DIR" (string-append
                                            (assoc-ref inputs "htslib") "/include"))
-             (setenv "CFLAGS" "-D_CURSES_LIB=1"))))))
+             (setenv "CFLAGS" "-D_CURSES_LIB=1")))
+         (delete 'check)
+         (add-after 'install 'check
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (setenv "PYTHONPATH"
+                     (string-append
+                      (getenv "PYTHONPATH")
+                      ":" (assoc-ref outputs "out")
+                      "/lib/python"
+                      (string-take (string-take-right
+                                    (assoc-ref inputs "python") 5) 3)
+                      "/site-packages"))
+             ;; Step out of source dir so python does not import from CWD.
+             (chdir "tests")
+             (setenv "HOME" "/tmp")
+             (and (zero? (system* "make" "-C" "pysam_data"))
+                  (zero? (system* "make" "-C" "cbcf_data"))
+                  (zero? (system* "nosetests" "-v"))))))))
     (propagated-inputs
      `(("htslib"            ,htslib)))
     (inputs
@@ -1382,6 +1402,9 @@  multiple sequence alignments.")
        ("zlib"              ,zlib)))
     (native-inputs
      `(("python-cython"     ,python-cython)
+       ("python-nose"       ,python-nose)
+       ("samtools"          ,samtools)
+       ("bcftools"          ,bcftools)
        ("python-setuptools" ,python-setuptools)))
     (home-page "https://github.com/pysam-developers/pysam")
     (synopsis "Python bindings to the SAMtools C API")
-- 
2.10.0