Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

samtools: 1.10 -> 1.11 , lumpy: 0.3.0 -> 0.3.1 #100814

Merged
merged 3 commits into from Oct 30, 2020
Merged

samtools: 1.10 -> 1.11 , lumpy: 0.3.0 -> 0.3.1 #100814

merged 3 commits into from Oct 30, 2020

Conversation

@unode
Copy link
Member

@unode unode commented Oct 17, 2020

Motivation for this change

Needs #100812 to be merged first or the wrong version of htslib will be used.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 25, 2020

Fails to build for me on NixOS:

gcc -Wall -g -O2 -I. -I/nix/store/yykmsrs5gnzhcihlrpawbhwamyvhha13-htslib-1.10.2/include -I./lz4  -c -o sample.o sample.c
bam_fastq.c: In function 'copy_tag':
bam_fastq.c:267:15: warning: implicit declaration of function 'bam_aux_get_str'; did you mean 'bam_aux_get'? [-Wimplicit-function-declaration]
  267 |     int ret = bam_aux_get_str(rec, tag, linebuf);
      |               ^~~~~~~~~~~~~~~
      |               bam_aux_get
sam_view.c: In function 'main_samview':
sam_view.c:468:47: warning: implicit declaration of function 'fai_path'; did you mean 'fai_fetch'? [-Wimplicit-function-declaration]
  468 |     if (fn_fai == 0 && ga.reference) fn_fai = fai_path(ga.reference);
      |                                               ^~~~~~~~
      |                                               fai_fetch
sam_view.c:468:45: warning: assignment to 'char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  468 |     if (fn_fai == 0 && ga.reference) fn_fai = fai_path(ga.reference);
      |                                             ^
sam_view.c:682:13: error: 'errno' undeclared (first use in this function)
  682 |             errno = 0;
      |             ^~~~~
sam_view.c:43:1: note: 'errno' is defined in header '<errno.h>'; did you forget to '#include <errno.h>'?
   42 | #include "bedidx.h"
  +++ |+#include <errno.h>
   43 |
sam_view.c:682:13: note: each undeclared identifier is reported only once for each function it appears in
  682 |             errno = 0;
      |             ^~~~~
bam_sort.c: In function 'seq_to_bam':
bam_sort.c:2163:9: warning: implicit declaration of function 'bam_set_seqi'; did you mean 'bam_get_seq'? [-Wimplicit-function-declaration]
 2163 |         bam_set_seqi(seq, i, seq_nt16_table[(unsigned char)buf[i]]);
      |         ^~~~~~~~~~~~
      |         bam_get_seq
make: *** [Makefile:135: sam_view.o] Error 1
make: *** Waiting for unfinished jobs....
builder for '/nix/store/9zppbpar10asqd4iv7fa4pj5vfb46s8l-samtools-1.11.drv' failed with exit code 2
error: build of '/nix/store/9zppbpar10asqd4iv7fa4pj5vfb46s8l-samtools-1.11.drv' failed

@unode
Copy link
Member Author

@unode unode commented Oct 25, 2020

@SuperSandro2000 can you try this after #100812 ? I suspect that's the cause of this error.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 26, 2020

@unode ping me if it is merged

@unode unode mentioned this pull request Oct 26, 2020
10 tasks
@unode
Copy link
Member Author

@unode unode commented Oct 29, 2020

@SuperSandro2000 #100812 was just merged, whenever possible can you give this one another try? Thanks!

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 29, 2020

Result of nixpkgs-review pr 100814 run on x86_64-linux 1

13 packages failed to build:
  • deeptools
  • lumpy
  • python27Packages.HTSeq
  • python27Packages.pysam
  • python37Packages.HTSeq
  • python37Packages.cnvkit
  • python37Packages.pysam
  • python38Packages.HTSeq
  • python38Packages.cnvkit
  • python38Packages.pysam
  • samtools
  • tebreak
  • truvari

@unode
Copy link
Member Author

@unode unode commented Oct 29, 2020

Thanks! Will work on getting the other packages fixed.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 29, 2020

Thanks! Will work on getting the other packages fixed.

I think you need to rebase this PR and to pull in the other PR.

configuring
configure flags: --prefix=/nix/store/g68qf9njbc9zyiwn12y8xg6sw058nkkw-samtools-1.11 --with-htslib=/nix/store/7xwhkjqjindjay9f6qiazmig87dxjc2l-htslib-1.11
/nix/store/q1zjp9grl4w92qalkdqjs2bj5d0pf8ih-stdenv-linux/setup: ./configure: /bin/sh: bad interpreter: No such file or directory

This is what nixpkgs-review logged. You probably need to use patchShebangs.

@@ -2,11 +2,11 @@

stdenv.mkDerivation rec {
pname = "samtools";
version = "1.10";
version = "1.11";

src = fetchurl {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Oct 29, 2020

Can you use fetchGithub please?

Copy link
Member Author

@unode unode Oct 29, 2020

Unfortunately not in this case. The automatic package generated by GitHub is incomplete as stated by the authors on the releases page.

Copy link
Member

@SuperSandro2000 SuperSandro2000 Oct 30, 2020

We are not using the bundled library. They generated files are used and would be pointless to regenerate, right?

@unode
Copy link
Member Author

@unode unode commented Oct 29, 2020

After rebasing I was still getting test failures from pysam which are related to #100823 . Since I maintain that package too, I included the fix here.
While checking why lumpy was failing I also ended up updating it here too.

@unode unode changed the title samtools: 1.10 -> 1.11 samtools: 1.10 -> 1.11 , lumpy: 0.3.0 -> 0.3.1 Oct 29, 2020
@ofborg ofborg bot requested a review from jbedo Oct 29, 2020
@unode
Copy link
Member Author

@unode unode commented Oct 29, 2020

nixpkgs-review now reports 13 successes on my end.
@SuperSandro2000 would you mind checking again? thanks!

Copy link
Contributor

@jbedo jbedo left a comment

Lumpy compiles but does not function:
/nix/store/a3zwdl2m606mynzxcqa77ymk6a3j9nnf-python-2.7.18/bin/python: can't open file '/nix/store/wpzji1i2wgnn9b9y6jkqwpclp15w5bvf-lumpy-0.3.1//scripts/bamkit/bamlibs.py': [Errno 2] No such file or directory

Error is caused by the new bamkit submodule, so fix is to fetch submodules as well.

Samtools tested fine for me.

unode added 2 commits Oct 30, 2020
These will be fixed upstream and are due to format changes introduced
in samtools and htslib 1.11
@ofborg ofborg bot requested a review from jbedo Oct 30, 2020
@unode
Copy link
Member Author

@unode unode commented Oct 30, 2020

Had to include an additional sed for htslib includes. @jbedo can you check on your end?

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 30, 2020

The following report might be wrong as I hit some strange error:

Result of nixpkgs-review pr 100814 run on x86_64-linux 1

10 packages failed to build:
  • deeptools
  • lumpy
  • python37Packages.HTSeq
  • python37Packages.cnvkit
  • python37Packages.pysam
  • python38Packages.HTSeq
  • python38Packages.cnvkit
  • python38Packages.pysam
  • tebreak
  • truvari
3 packages built:
  • python27Packages.HTSeq
  • python27Packages.pysam
  • samtools

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 30, 2020

We are getting there. Only 9 more builds and we are green.

Result of nixpkgs-review pr 100814 run on x86_64-linux 1

9 packages failed to build:
  • deeptools
  • python37Packages.HTSeq
  • python37Packages.cnvkit
  • python37Packages.pysam
  • python38Packages.HTSeq
  • python38Packages.cnvkit
  • python38Packages.pysam
  • tebreak
  • truvari
4 packages built:
  • lumpy
  • python27Packages.HTSeq
  • python27Packages.pysam
  • samtools
ls/vcfisec.c.pysam.o build/temp.linux-x86_64-3.8/bcftools/mpileup.c.pysam.o build/temp.linux-x86_64-3.8/bcftools/vcfsom.c.pysam.o -L/build/source/pysam -L/nix/store/7xwhkjqjindjay9f6qiazmig87dxjc2l-htslib-1.11/lib -Lbuild/lib.linux-x86_64-3.8/pysam -Lbuild/lib.linux-x86_64-3.8/pysam -Lbuild/lib.linux-x86_64-3.8/pysam -L/nix/store/346skv0d24rqnf4npknbp9h5bs14j8zy-python3-3.8.6/lib -lz -lhts -lchtslib.cpython-38-x86_64-linux-gnu -o build/lib.linux-x86_64-3.8/pysam/libcbcftools.cpython-38-x86_64-linux-gnu.so -Wl,-rpath,$ORIGIN
building 'pysam.libcutils' extension
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/nix/store/7xwhkjqjindjay9f6qiazmig87dxjc2l-htslib-1.11/include -I/build/source/samtools -I/build/source/samtools/lz4 -I/build/source/bcftools -I/build/source/pysam -I/build/source -I/nix/store/346skv0d24rqnf4npknbp9h5bs14j8zy-python3-3.8.6/include/python3.8 -c pysam/libcutils.c -o build/temp.linux-x86_64-3.8/pysam/libcutils.o -Wno-unused -Wno-strict-prototypes -Wno-sign-compare -Wno-error=declaration-after-statement
pysam/libcutils.c: In function ‘__pyx_pw_5pysam_9libcutils_9_pysam_dispatch’:
pysam/libcutils.c:524:40: warning: ‘__pyx_v_retval’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  524 |   #define PyInt_FromLong               PyLong_FromLong
      |                                        ^~~~~~~~~~~~~~~
pysam/libcutils.c:5972:7: note: ‘__pyx_v_retval’ was declared here
 5972 |   int __pyx_v_retval;
      |       ^~~~~~~~~~~~~~
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/nix/store/7xwhkjqjindjay9f6qiazmig87dxjc2l-htslib-1.11/include -I/build/source/samtools -I/build/source/samtools/lz4 -I/build/source/bcftools -I/build/source/pysam -I/build/source -I/nix/store/346skv0d24rqnf4npknbp9h5bs14j8zy-python3-3.8.6/include/python3.8 -c pysam/pysam_util.c -o build/temp.linux-x86_64-3.8/pysam/pysam_util.o -Wno-unused -Wno-strict-prototypes -Wno-sign-compare -Wno-error=declaration-after-statement
gcc -shared -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-zlib-1.2.11/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-bzip2-1.0.6.0.1/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-expat-2.2.8/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-xz-5.2.5/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-libffi-3.3/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-gdbm-1.18.1/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-sqlite-3.33.0/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-readline-6.3p08/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-ncurses-6.2/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-openssl-1.1.1g/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-zlib-1.2.11/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-bzip2-1.0.6.0.1/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-expat-2.2.8/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-xz-5.2.5/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-libffi-3.3/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-gdbm-1.18.1/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-sqlite-3.33.0/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-readline-6.3p08/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-ncurses-6.2/lib -L/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-openssl-1.1.1g/lib build/temp.linux-x86_64-3.8/pysam/libcutils.o build/temp.linux-x86_64-3.8/pysam/pysam_util.o -L/build/source/pysam -L/nix/store/7xwhkjqjindjay9f6qiazmig87dxjc2l-htslib-1.11/lib -Lbuild/lib.linux-x86_64-3.8/pysam -Lbuild/lib.linux-x86_64-3.8/pysam -Lbuild/lib.linux-x86_64-3.8/pysam -Lbuild/lib.linux-x86_64-3.8/pysam -L/nix/store/346skv0d24rqnf4npknbp9h5bs14j8zy-python3-3.8.6/lib -lz -lhts -lchtslib.cpython-38-x86_64-linux-gnu -lcsamtools.cpython-38-x86_64-linux-gnu -lcbcftools.cpython-38-x86_64-linux-gnu -o build/lib.linux-x86_64-3.8/pysam/libcutils.cpython-38-x86_64-linux-gnu.so -Wl,-rpath,$ORIGIN
building 'pysam.libcalignmentfile' extension
gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/nix/store/7xwhkjqjindjay9f6qiazmig87dxjc2l-htslib-1.11/include -I/build/source/samtools -I/build/source/samtools/lz4 -I/build/source/bcftools -I/build/source/pysam -I/build/source -I/nix/store/346skv0d24rqnf4npknbp9h5bs14j8zy-python3-3.8.6/include/python3.8 -c pysam/libcalignmentfile.c -o build/temp.linux-x86_64-3.8/pysam/libcalignmentfile.o -Wno-unused -Wno-strict-prototypes -Wno-sign-compare -Wno-error=declaration-after-statement
pysam/libcalignmentfile.c: In function ‘__pyx_f_5pysam_17libcalignmentfile_14IteratorColumn_cnext’:
pysam/libcalignmentfile.c:30113:127: warning: passing argument 5 of ‘bam_mplp_auto’ from incompatible pointer type [-Wincompatible-pointer-types]
30113 |   __pyx_v_ret = bam_mplp_auto(__pyx_v_self->pileup_iter, (&__pyx_v_self->tid), (&__pyx_v_self->pos), (&__pyx_v_self->n_plp), (&__pyx_v_self->plp));
      |                                                                                                                              ~^~~~~~~~~~~~~~~~~~~
      |                                                                                                                               |
      |                                                                                                                               bam_pileup1_t ** {aka struct bam_pileup1_t **}
In file included from pysam/htslib_util.h:4,
                 from pysam/libcalignmentfile.c:616:
/nix/store/7xwhkjqjindjay9f6qiazmig87dxjc2l-htslib-1.11/include/htslib/sam.h:1939:96: note: expected ‘const bam_pileup1_t **’ {aka ‘const struct bam_pileup1_t **’} but argument is of type ‘bam_pileup1_t **’ {aka ‘struct bam_pileup1_t **’}
 1939 |     int bam_mplp_auto(bam_mplp_t iter, int *_tid, int *_pos, int *n_plp, const bam_pileup1_t **plp);
      |                                                                          ~~~~~~~~~~~~~~~~~~~~~~^~

@jbedo
Copy link
Contributor

@jbedo jbedo commented Oct 30, 2020

Had to include an additional sed for htslib includes. @jbedo can you check on your end?

Lumpy works perfectly now, thanks!

jbedo
jbedo approved these changes Oct 30, 2020
Copy link
Contributor

@jbedo jbedo left a comment

LGTM. FWIW nix-review is building all packages successfully for me on x86_64-linux.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 30, 2020

Result of nixpkgs-review pr 100814 run on x86_64-darwin 1

8 packages marked as broken and skipped:
  • deeptools
  • python27Packages.HTSeq
  • python37Packages.HTSeq
  • python37Packages.cnvkit
  • python38Packages.HTSeq
  • python38Packages.cnvkit
  • tebreak
  • truvari
1 package built:
  • samtools

@unode
Copy link
Member Author

@unode unode commented Oct 30, 2020

Hum... I don't have a darwin system to test on. @SuperSandro2000 can you pull any logs out?

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 30, 2020

@unode those packages have a meta.broken = true for darwin so they are not build. I can try to build them if you want but I would move this to a separate PR.

@unode
Copy link
Member Author

@unode unode commented Oct 30, 2020

@SuperSandro2000 ah thanks! Give them a try in case the updates fixed some of them but I agree we should move to a different PR.

Then any objections or do we have green light here? If so, I'll ask for a merge.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 30, 2020

I am not sure why the linux nixpkgs-review is so flaky. I build all packages manually and they succeeded. Maybe something about machine is not quite right.

Edit:

platforms = platforms.x86_64; shouldn't not mean the package is broken on darwin. I am confused.

@unode
Copy link
Member Author

@unode unode commented Oct 30, 2020

I am not sure why the linux nixpkgs-review is so flaky. I build all packages manually and they succeeded. Maybe something about machine is not quite right.

I had similar issues here. Still getting used to it. In a few cases had to modify the code to trigger a rebuild. It might just be the way it works with the nixpkgs cache...

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 30, 2020

I just build the packages on darwin:

python27Packages.pysam
python38Packages.pysam
lumpy
deeptools
python27Packages.HTSeq
python38Packages.HTSeq
python38Packages.cnvkit

But please mark this in a separate PR.

@Mic92 Mic92 merged commit 46731b8 into NixOS:master Oct 30, 2020
19 checks passed
19 checks passed
@github-actions[bot]
tests
Details
@github-actions[bot]
action
Details
@ofborg[bot]
Evaluation Performance Report Evaluator Performance Report
Details
@github-actions[bot]
Wait for ofborg
Details
@ofborg[bot]
grahamcofborg-eval ^.^!
Details
@ofborg[bot]
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
@ofborg[bot]
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
@ofborg[bot]
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="459c5ec"; rev="459c5ec3b19767b96ef11225d60b0964c467f3b1"; } ./pkgs/t
Details
@ofborg[bot]
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
@ofborg[bot]
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="459c5ec"; rev="459c5ec3b19767b96ef11225d60b0964c467f3b1"; } ./nixos/
Details
@ofborg[bot]
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="459c5ec"; rev="459c5ec3b19767b96ef11225d60b0964c467f3b1"; } ./nixos/
Details
@ofborg[bot]
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="459c5ec"; rev="459c5ec3b19767b96ef11225d60b0964c467f3b1"; } ./nixos/
Details
@ofborg[bot]
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="459c5ec"; rev="459c5ec3b19767b96ef11225d60b0964c467f3b1"; } ./pkgs/t
Details
@ofborg[bot]
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="459c5ec"; rev="459c5ec3b19767b96ef11225d60b0964c467f3b1"; } ./pkgs/t
Details
@ofborg[bot]
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="459c5ec"; rev="459c5ec3b19767b96ef11225d60b0964c467f3b1"; } ./pkgs/t
Details
@ofborg[bot]
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
@ofborg[bot]
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@ofborg[bot]
lumpy, lumpy.passthru.tests, samtools, samtools.passthru.tests on aarch64-linux Success
Details
@ofborg[bot]
lumpy, lumpy.passthru.tests, samtools, samtools.passthru.tests on x86_64-linux Success
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants