From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [PATCH] gnu: Add freebayes. Date: Wed, 9 Mar 2016 11:17:33 +0100 Message-ID: References: <874mchc6uz.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:59232) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adbBZ-0000Zd-Ds for guix-devel@gnu.org; Wed, 09 Mar 2016 05:17:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adbBW-0000Op-55 for guix-devel@gnu.org; Wed, 09 Mar 2016 05:17:45 -0500 In-Reply-To: <874mchc6uz.fsf@gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Roel Janssen Cc: guix-devel@gnu.org Roel Janssen writes: > One of the problems with the patch is probably the bulk of dependencies > dragged in (for example, vcflib). They use specific versions so they > are tied to this package (so that's why I cannot package them separatel= y). Yeah, this is worrying. > From 38302e8cac77275694c8793933be414ec26906ec Mon Sep 17 00:00:00 2001 > From: Roel Janssen > Date: Tue, 8 Mar 2016 16:38:46 +0100 > Subject: [PATCH] gnu: Add freebayes. > * gnu/packages/bioinformatics.scm (freebayes): New variable. [...] > +(define-public freebayes > + (let ((commit "3ce827d8ebf89bb3bdc097ee0fe7f46f9f30d5fb")) > + (package > + (name "freebayes") > + (version (string-append "v1.0.2-" (string-take commit 7))) The version should not start with =E2=80=9Cv=E2=80=9D and it should inclu= de a numeric revision because later git commits may be sorted lower than the current commit. (let ((commit ....) (revision "1")) ... (version (string-append "1.0.2-" revision "." commit)) ...) Why does it have to be a git clone, though? I see that only six commits were made to master since release 1.0.2. If fetching from git must happen it would be good to have a reason in a comment. > + (native-inputs > + `(("cmake" ,cmake) > + ("htslib" ,htslib) > + ("zlib" ,zlib) =E2=80=9Chtslib=E2=80=9D and =E2=80=9Czlib=E2=80=9D sound like regular in= puts. > + ("python" ,python-2) > + ("perl" ,perl) These maybe as well. > + ("bamtools-src" > + ,(origin > + (method url-fetch) > + (uri (string-append "https://github.com/ekg/bamtools/arch= ive/" > + "e77a43f5097ea7eee432ee765049c6b246d49baa" ".tar.gz"= )) > + (file-name "bamtools-src.tar.gz") > + (sha256 > + (base32 "0rqymka21g6lfjfgxzr40pxz4c4fcl77jpy1np1li70pnc7= h2cs1")))) We already have bamtools, I think. Is there no way to link with that version? Does it have to be this arbitrary-looking commit? > + ("vcflib-src" > + ,(origin > + (method url-fetch) > + (uri (string-append "https://github.com/vcflib/vcflib/arc= hive/" > + "5ac091365fdc716cc47cc5410bb97ee5dc2a2c92" ".tar.gz"= )) > + (file-name "vcflib-5ac0913.tar.gz") > + (sha256 > + (base32 "0ywshwpif059z5h0g7zzrdfzzdj2gr8xvwlwcsdxrms3p9i= y35h8")))) > + ;; These are submodules for the vcflib version used in freeba= yes > + ("tabixpp-src" > + ("intervaltree-src" > + ("smithwaterman-src" > + ("multichoose-src" > + ("fsom-src" > + ("filevercmp-src" > + ("fastahack-src" If these are submodules of this particular version of vcflib I think it would be better to create a separate vcflib package where these submodules are included. If ever possible you would then coerce freebayes to link with that version of vcflib. Note that vcflib doesn=E2=80=99t *have* to be exported via define-public.= It would be nice, though, if we could get a regular vcflib package as a side-effect of all this. > + (arguments > + `(#:tests? #f Please quickly comment on why the tests are disabled. > + #:phases > + (modify-phases %standard-phases > + (delete 'configure) > + (delete 'check) You won=E2=80=99t need this when tests are disabled already. > + (add-after 'unpack 'unpack-submodule-sources > + (lambda* (#:key inputs #:allow-other-keys) > + (let ((unpack (lambda (source target) > + (with-directory-excursion target > + (zero? (system* "tar" "xvf" > + (assoc-ref inputs sou= rce) > + "--strip-components=3D= 1")))))) > + (and > + (unpack "bamtools-src" "bamtools") > + (unpack "vcflib-src" "vcflib") > + (unpack "intervaltree-src" "intervaltree") > + (unpack "fastahack-src" "vcflib/fastahack") > + (unpack "filevercmp-src" "vcflib/filevercmp") > + (unpack "fsom-src" "vcflib/fsom") > + (unpack "intervaltree-src" "vcflib/intervaltree") > + (unpack "multichoose-src" "vcflib/multichoose") > + (unpack "smithwaterman-src" "vcflib/smithwaterman") > + (unpack "tabixpp-src" "vcflib/tabixpp"))))) > + (add-after 'unpack-submodule-sources 'fix-makefile > + (lambda* (#:key inputs #:allow-other-keys) > + ;; We don't have the .git folder to get the version tag= from. > + ;; For this checkout of the code, it's v1.0.0. > + (substitute* '("vcflib/Makefile") > + (("^GIT_VERSION.*") "GIT_VERSION =3D v1.0.0")))) > + (replace > + 'build > + (lambda* (#:key inputs make-flags #:allow-other-keys) > + (and > + ;; Compile Bamtools before compiling the main project. > + (with-directory-excursion "bamtools" > + (system* "mkdir" "build") > + (with-directory-excursion "build" > + (and (zero? (system* "cmake" "../")) > + (zero? (system* "make"))))) > + ;; Compile vcflib before we compiling the main project. > + (with-directory-excursion "vcflib" > + (with-directory-excursion "tabixpp" > + (zero? (system* "make"))) > + (zero? (system* "make" "CC=3Dgcc" > + (string-append "CFLAGS=3D\"" "-Itabixpp " > + "-I" (assoc-ref inputs "htslib") "/include " "\""= ) "all"))) > + (with-directory-excursion "src" > + (zero? (system* "make")))))) This seems too hackish for my taste. It would be so much nicer if bamtools and vcflib were built in separate packages. This would make it much clearer what hacks are required for what package and you could use the cmake-build-system for bamtools and the gnu-build-system for vcflib. You might be able to force freebayes to use those separate packages by overriding BAMTOOLS_ROOT and VCFLIB_ROOT in =E2=80=9Csrc/Makefile=E2=80=9D= or by replacing =E2=80=9C$(BAMTOOLS_ROOT)/lib/libbamtools.a=E2=80=9D with the p= ath to the actual =E2=80=9Clibbamtools.a=E2=80=9D in the store. Looking at =E2=80=9C= src/Makefile=E2=80=9D the entanglement isn=E2=80=99t that bad and we should be able to resolve it. If you need assistance with this I could help you. > + (replace > + 'install Please leave =E2=80=9C'install=E2=80=9D on the same line. ~~ Ricardo