GNU bug report logs - #44613
[PATCH] Fix build for bedtools

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 44613 in the body.
You can then email your comments to 44613 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Roel Janssen <roel <at> gnu.org>
To: guix-patches <at> gnu.org
Subject: [PATCH] Fix build for bedtools
Date: Fri, 13 Nov 2020 12:01:04 +0100

[Message part 1 (text/plain, inline)]

Dear Guix,

By updating samtools to 1.11, I introduced a build failure for
bedtools. More precisely, the tests for intersect break in precisely
this way:
https://github.com/arq5x/bedtools2/issues/814

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.

Alternatively we could add a patch to disable the failing bedtools
tests.  I manually inspected the test results, and seem to match
perfectly (indicating that there's no problem with bedtools).

Kind regards,
Roel Janssen

[0003-gnu-bedtools-Use-samtools-1.9.patch (text/x-patch, attachment)]

[0002-gnu-Add-samtools-1.9.patch (text/x-patch, attachment)]

[0001-gnu-Add-htslib-1.9.patch (text/x-patch, attachment)]

Message #8 received at 44613 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Roel Janssen <roel <at> gnu.org>, 44613 <at> debbugs.gnu.org
Subject: Re: [bug#44613] [PATCH] Fix build for bedtools
Date: Fri, 13 Nov 2020 13:39:56 +0100

Hi Roel,

On Fri, 13 Nov 2020 at 12:01, Roel Janssen <roel <at> 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

Message #11 received at 44613 <at> debbugs.gnu.org (full text, mbox):

From: Roel Janssen <roel <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>, 44613 <at> debbugs.gnu.org
Subject: Re: [bug#44613] [PATCH] Fix build for bedtools
Date: Fri, 13 Nov 2020 13:55:29 +0100

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 <at> 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

Message #14 received at 44613 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Roel Janssen <roel <at> gnu.org>, 44613 <at> debbugs.gnu.org
Subject: Re: [bug#44613] [PATCH] Fix build for bedtools
Date: Fri, 13 Nov 2020 14:34:06 +0100

Hi Roel,

On Fri, 13 Nov 2020 at 13:55, Roel Janssen <roel <at> 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! :-)

Message #19 received at 44613-done <at> debbugs.gnu.org (full text, mbox):

From: Roel Janssen <roel <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 44613-done <at> debbugs.gnu.org
Subject: Re: [bug#44613] [PATCH] Fix build for bedtools
Date: Fri, 13 Nov 2020 15:00:57 +0100

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 <at> 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

Message #22 received at 44613 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <marius <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>, Roel Janssen <roel <at> gnu.org>,
 44613 <at> debbugs.gnu.org
Subject: Re: [bug#44613] [PATCH] Fix build for bedtools
Date: Fri, 13 Nov 2020 16:08:27 +0100

[Message part 1 (text/plain, inline)]

zimoun <zimon.toutoune <at> 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.

[signature.asc (application/pgp-signature, inline)]

Message #25 received at 44613 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Marius Bakke <marius <at> gnu.org>, Roel Janssen <roel <at> gnu.org>,
 44613 <at> debbugs.gnu.org
Subject: what “guix build -S” should
 return?
Date: Fri, 13 Nov 2020 16:51:46 +0100

Hi Marius,

Thank you for the explanations.


On Fri, 13 Nov 2020 at 16:08, Marius Bakke <marius <at> gnu.org> wrote:
> zimoun <zimon.toutoune <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.