Message ID | a82f869c91b6b33a5d063b505cab01e19b53b84d.camel@gnu.org |
---|---|
State | Accepted |
Headers | show |
Series | [bug#44613] Fix build for bedtools | expand |
Context | Check | Description |
---|---|---|
cbaines/submitting builds | success | |
cbaines/issue | success | View issue |
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | fail | View Laminar job |
Hi Roel, On Fri, 13 Nov 2020 at 12:01, Roel Janssen <roel@gnu.org> wrote: > With the following patches, I'd like to add samtools-1.9, htslib-1.9 > (samtools depends on that) to fix this problem with bedtools. Recently, investigating why the substitute of ’python-pysam’ was not available, I decided then to give a try at fixing the TODO: --8<---------------cut here---------------start------------->8--- (snippet '(begin ;; Drop bundled htslib. TODO: Also remove samtools ;; and bcftools. (delete-file-recursively "htslib") #t)))) --8<---------------cut here---------------end--------------->8--- And the bundled version is 1.9 (if I remember correctly), therefore because of: --8<---------------cut here---------------start------------->8--- (native-inputs `(("python-cython" ,python-cython) ;; Dependencies below are are for tests only. ("samtools" ,samtools) ("bcftools" ,bcftools) ("python-nose" ,python-nose))) --8<---------------cut here---------------end--------------->8--- some tests are unhappy. That’s said, these additions seem fine with me. :-) All the best, simon
Hi Simon, On Fri, 2020-11-13 at 13:39 +0100, zimoun wrote: > Hi Roel, > > On Fri, 13 Nov 2020 at 12:01, Roel Janssen <roel@gnu.org> wrote: > > > With the following patches, I'd like to add samtools-1.9, htslib- > > 1.9 > > (samtools depends on that) to fix this problem with bedtools. > > Recently, investigating why the substitute of ’python-pysam’ was not > available, I decided then to give a try at fixing the TODO: > > --8<---------------cut here---------------start------------->8--- > (snippet '(begin > ;; Drop bundled htslib. TODO: Also remove > samtools > ;; and bcftools. > (delete-file-recursively "htslib") > #t)))) > --8<---------------cut here---------------end--------------->8--- > > And the bundled version is 1.9 (if I remember correctly), therefore > because of: > > --8<---------------cut here---------------start------------->8--- > (native-inputs > `(("python-cython" ,python-cython) > ;; Dependencies below are are for tests only. > ("samtools" ,samtools) > ("bcftools" ,bcftools) > ("python-nose" ,python-nose))) > --8<---------------cut here---------------end--------------->8--- > > some tests are unhappy. > > That’s said, these additions seem fine with me. :-) > I also tried removing the bundled htslib for bedtools, but didn't go this route for two reasons: - The bundled htslib for bedtools seems "slightly modified" (I didn't investigate further) - Replacing the references to libhts.a with $(pkg-config htslib -- cflags --libs) produced various linker errors. So I stopped right there. I'm sure more tools will likely have failed because of the htslib upgrade (sorry about this!), so having htslib-1.9 around for some time may be a good fallback for now. Just to double-check: Is it OK to push the proposed patches? Kind regards, Roel Janssen
Hi Roel, On Fri, 13 Nov 2020 at 13:55, Roel Janssen <roel@gnu.org> wrote: > I also tried removing the bundled htslib for bedtools, but didn't go > this route for two reasons: > - The bundled htslib for bedtools seems "slightly modified" (I didn't > investigate further) > - Replacing the references to libhts.a with $(pkg-config htslib -- > cflags --libs) produced various linker errors. So I stopped right > there. > > I'm sure more tools will likely have failed because of the htslib > upgrade (sorry about this!), so having htslib-1.9 around for some time > may be a good fallback for now. Thank for your explanations. > Just to double-check: Is it OK to push the proposed patches? I have not tried them but they LGTM. All the best, simon PS: I am always confused if the removal should be done in ’origin’ or in the ’add-after 'unpack’ phase; especially when the bundle is free software. Other said, what should an user expect when fetching with “guix build -S”? Anyway! :-)
Hi Simon, On Fri, 2020-11-13 at 14:34 +0100, zimoun wrote: > Hi Roel, > > On Fri, 13 Nov 2020 at 13:55, Roel Janssen <roel@gnu.org> wrote: > > > I also tried removing the bundled htslib for bedtools, but didn't > > go > > this route for two reasons: > > - The bundled htslib for bedtools seems "slightly modified" (I > > didn't > > investigate further) > > - Replacing the references to libhts.a with $(pkg-config htslib -- > > cflags --libs) produced various linker errors. So I stopped right > > there. > > > > I'm sure more tools will likely have failed because of the htslib > > upgrade (sorry about this!), so having htslib-1.9 around for some > > time > > may be a good fallback for now. > > Thank for your explanations. > > > > Just to double-check: Is it OK to push the proposed patches? > > I have not tried them but they LGTM. > Thanks for the quick review. I pushed the patches in c3232fcc7785abc1057a0d4b5b1832f1e39c9c1b, da4a38edad52f7bb5a8d41465d09f3f0197fd0b7, and 3ede804f6d4c38ff0b9a5705544a8c35f6827ff1. Kind regards, Roel Janssen
zimoun <zimon.toutoune@gmail.com> writes: > PS: > I am always confused if the removal should be done in ’origin’ or in the > ’add-after 'unpack’ phase; especially when the bundle is free software. > Other said, what should an user expect when fetching with “guix build -S”? > Anyway! :-) Unbundling is always better to do in a snippet. It leads to less bandwidth usage, and users can more easily inspect the (actual) code. For other kinds of patching the boundary is less clear. Generally, Guix-specific tweaks should be in a phase, but "universal" bug fixes may well be in a snippet. I sometimes imagine a downstream distribution that use Guix sources, but not the build scripts, to draw the line.
Hi Marius, Thank you for the explanations. On Fri, 13 Nov 2020 at 16:08, Marius Bakke <marius@gnu.org> wrote: > zimoun <zimon.toutoune@gmail.com> writes: > >> PS: >> I am always confused if the removal should be done in ’origin’ or in the >> ’add-after 'unpack’ phase; especially when the bundle is free software. >> Other said, what should an user expect when fetching with “guix build -S”? >> Anyway! :-) > > Unbundling is always better to do in a snippet. It leads to less > bandwidth usage, and users can more easily inspect the (actual) code. Well, I do not know. For example, I could do this workflow: guix environment bedtools tar -xvf $(guix build -S bedtools) make which probably fails because removing the bundles often needs some extra tweaks. Concretely, see python-pysam for instance: --8<---------------cut here---------------start------------->8--- (snippet '(begin ;; Drop bundled htslib. TODO: Also remove samtools ;; and bcftools. (delete-file-recursively "htslib") #t)))) [...] #:phases (modify-phases %standard-phases (add-before 'build 'set-flags (lambda* (#:key inputs #:allow-other-keys) (setenv "HTSLIB_MODE" "external") (setenv "HTSLIB_LIBRARY_DIR" (string-append (assoc-ref inputs "htslib") "/lib")) (setenv "HTSLIB_INCLUDE_DIR" (string-append (assoc-ref inputs "htslib") "/include")) --8<---------------cut here---------------end--------------->8--- Then, I am not convince that: guix build bedtools --with-git-url=http://example.org works too. Or ’--with-source=’ as well. I remember a discussion initiated by Mark and Maxim about this: snippet vs phases but I am not able to reach it. > For other kinds of patching the boundary is less clear. Generally, > Guix-specific tweaks should be in a phase, but "universal" bug fixes may > well be in a snippet. I agree that non-free and bug fixes should go to snippet. Then I am still confused and my feelings are mixed about Guix specific tweaks. > I sometimes imagine a downstream distribution that use Guix sources, but > not the build scripts, to draw the line. It seems a good criteria to draw the line. And in the case of bedtools or python-pysam or many others, ’snippet’ removes (free software) bundles because of an implicit and non-uniform Guix policy that a downstream distribution could choose differently. Well, my mind is not clear about this topic. :-) All the best, simon
From 220604159383b09e11a7b0bed51237257c7c0442 Mon Sep 17 00:00:00 2001 From: Roel Janssen <roel@gnu.org> Date: Fri, 13 Nov 2020 11:28:43 +0100 Subject: [PATCH 1/3] gnu: Add htslib-1.9. * gnu/packages/bioinformatics.scm (htslib-1.9): New variable. --- gnu/packages/bioinformatics.scm | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm index 06972dee51..ba86333fc3 100644 --- a/gnu/packages/bioinformatics.scm +++ b/gnu/packages/bioinformatics.scm @@ -4250,6 +4250,19 @@ data. It also provides the @command{bgzip}, @command{htsfile}, and ;; the rest is released under the Expat license (license (list license:expat license:bsd-3)))) +(define-public htslib-1.9 + (package (inherit htslib) + (name "htslib") + (version "1.9") + (source (origin + (method url-fetch) + (uri (string-append + "https://github.com/samtools/htslib/releases/download/" + version "/htslib-" version ".tar.bz2")) + (sha256 + (base32 + "16ljv43sc3fxmv63w7b2ff8m1s7h89xhazwmbm1bicz8axq8fjz0")))))) + ;; This package should be removed once no packages rely upon it. (define htslib-1.3 (package -- 2.29.2