[PATCH] gnu: kallisto: Update to 0.50.1

  • Done
  • quality assurance status badge
Details
2 participants
  • guix
  • Ricardo Wurmus
Owner
unassigned
Submitted by
guix
Severity
normal
G
(address . guix-patches@gnu.org)(name . Ricardo Wurmus)(address . rekado@elephly.net)
171645804083.6.7508412953776945309.337355746@mawumag.com
Updated kallisto to 0.50.1, it now supports more single-cell technologies (e.g. SPLiT-seq).

I had to modify the 'do-not-use-bundled-htslib snippet to make one regex more selective---in this new version it the previous regex was mistakenly modifying parts of CMakeLists.txt other than the intended one.
From 9bec1c5bf14b644ef7147b1bc66acaea6a903830 Mon Sep 17 00:00:00 2001
Message-ID: <9bec1c5bf14b644ef7147b1bc66acaea6a903830.1716457241.git.guix@mawumag.com>
From: Marco Baggio <guix@mawumag.com>
Date: Thu, 23 May 2024 08:52:06 +0200
Subject: [PATCH] gnu: kallisto: Update to 0.50.1

Change-Id: I1b048be328f6d1d5034dfe29688a44f2af0b026a
---
gnu/packages/bioinformatics.scm | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

Toggle diff (48 lines)
diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
index b7e8e54de2..8dee300320 100644
--- a/gnu/packages/bioinformatics.scm
+++ b/gnu/packages/bioinformatics.scm
@@ -14820,7 +14820,7 @@ (define-public segemehl
(define-public kallisto
(package
(name "kallisto")
- (version "0.48.0")
+ (version "0.50.1")
(source (origin
(method git-fetch)
(uri (git-reference
@@ -14829,7 +14829,7 @@ (define-public kallisto
(file-name (git-file-name name version))
(sha256
(base32
- "0wx1ndmhndsd60952piaa925lk8bjr59d2yr1m2hcsqcb6cdjwpn"))
+ "0zfs79mv75599cf9d7d3c5a3s8idgz9qvl4qfzhvhbd87y3dv7p0"))
(modules '((guix build utils)))
(snippet
'(delete-file-recursively "ext/htslib/"))))
@@ -14841,16 +14841,16 @@ (define-public kallisto
(add-after 'unpack 'do-not-use-bundled-htslib
(lambda _
(substitute* "CMakeLists.txt"
- (("^ExternalProject_Add" m)
+ (("^ExternalProject_Add\\(htslib[^\\)]*\\)" m)
(string-append "if (NEVER)\n" m))
- (("^\\)")
- (string-append ")\nendif(NEVER)"))
(("include_directories\\(\\$\\{htslib_PREFIX.*" m)
(string-append "# " m)))
(substitute* "src/CMakeLists.txt"
(("target_link_libraries\\(kallisto kallisto_core pthread \
-\\$\\{CMAKE_CURRENT_SOURCE_DIR\\}/../ext/htslib/libhts.a\\)")
- "target_link_libraries(kallisto kallisto_core pthread hts)")
+\\$\\{CMAKE_CURRENT_SOURCE_DIR\\}/../ext/htslib/libhts.a \
+\\$\\{install_dir\\}/build/src/libbifrost.a\\)")
+ "target_link_libraries(kallisto kallisto_core pthread hts \
+${install_dir}/build/src/libbifrost.a)")
(("include_directories\\(\\.\\./ext/htslib\\)") "")))))))
(inputs
(list hdf5 htslib-1.9 zlib))

base-commit: 3597c736588c45efde3c22d533ea8774c3fdd235
--
2.41.0
R
R
Ricardo Wurmus wrote on 1 Jun 21:13 +0200
(address . guix@mawumag.com)(address . 71146@debbugs.gnu.org)
87le3ojx2x.fsf@elephly.net
Ricardo Wurmus <rekado@elephly.net> writes:

Toggle quote (12 lines)
>> I had to modify the 'do-not-use-bundled-htslib snippet to make one
>> regex more selective---in this new version it the previous regex was
>> mistakenly modifying parts of CMakeLists.txt other than the intended
>> one.
>
> It looks like this might not be working as intended. I used:
>
> guix gc -R $(./pre-inst-env guix build kallisto)
>
> and it shows me that the newly built kallisto does not link with our
> htslib. Perhaps it used the static library?

Turns out it's not actually using htslib at all, because USE_BAM is not
set. It uses a bundled copy of bifrost instead. If we want to build
kallisto with bifrost only then we should remove htslib from the inputs,
package bifrost (https://github.com/pmelsted/bifrost)and unbundle it
from kallisto, linking with the shared library.

Does this sound like a good plan or should we use htslib instead?

--
Ricardo
R
R
Ricardo Wurmus wrote on 1 Jun 21:13 +0200
(address . guix@mawumag.com)(address . 71146@debbugs.gnu.org)
87jzj8jx2d.fsf@elephly.net
Thank you for the patch!

Toggle quote (5 lines)
> I had to modify the 'do-not-use-bundled-htslib snippet to make one
> regex more selective---in this new version it the previous regex was
> mistakenly modifying parts of CMakeLists.txt other than the intended
> one.

It looks like this might not be working as intended. I used:

guix gc -R $(./pre-inst-env guix build kallisto)

and it shows me that the newly built kallisto does not link with our
htslib. Perhaps it used the static library?

--
Ricardo
G
(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 71146@debbugs.gnu.org)
171751540946.7.6431104630771377265.346935814@mawumag.com
Toggle quote (10 lines)
> Ricardo Wurmus rekado@elephly.net writes:
>
> Turns out it's not actually using htslib at all, because USE_BAM is not
> set. It uses a bundled copy of bifrost instead. If we want to build
> kallisto with bifrost only then we should remove htslib from the inputs,
> package bifrost (https://github.com/pmelsted/bifrost) and unbundle it
> from kallisto, linking with the shared library.
>
> Does this sound like a good plan or should we use htslib instead?

My initial attempt was to unbundle bifrost from kallisto, but I had two concerns:

- I was not able to determine the exact version of bifrost that was bundled with kallisto (it is definitely not the latest one, as the relevant ext/bifrost directory in kallisto is older).
This is also a problem with htslib, I am sure whoever packaged kallisto 0.48.0 was careful to choose the correct version, but I would not know how to verify this.
This has the potential to lead to mismatches (or even bugs) between the guix version and the official one compiled from their sources.

- Some header files of bifrost appear to be included in kallisto source code directly, but it was not clear to me how to include bifrost in guix in a way that exports its libraries and headers.

Especially for the first reason, I would actually propose to use the bundled htslib and bifrost for kallisto. Both are bundled as source code and are compiled during the build process, so this should not pose a concern (bifrost is released under BSD 2-Clause License, like kallisto, while htslib is released under expat).

Please let me know what you think.

Cheers,
Marco
R
R
Ricardo Wurmus wrote on 4 Nov 15:51 +0100
[PATCH] gnu: kallisto: Update to 0.50.1
(address . 71146-done@debbugs.gnu.org)(address . guix@mawumag.com)
87msifgihh.fsf@elephly.net
Hi,

I'm back to work now and just reviewed our discussion on kallisto.
We
may want to revisit the decision to not unbundle libbifrost and
htslib
at a later point, but at the moment I agree to keep the bundled
libraries until we've attained more clarity.

I removed the now obsolete build phase completely and I disabled
parallel building (see comment).

Thanks for the patch and my apologies for not being able to merge
this
sooner!

--
Ricardo
Closed
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 71146@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 71146
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch