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

Hardcode htslib and samtools versions in pysam #1041

Merged
merged 2 commits into from
Mar 14, 2016

Conversation

kyleabeauchamp
Copy link
Contributor

So pysam dynamically links to htslib and also calls samtools. For these reasons, we should probably hard-pin the versions to ensure compatibility.

@sebastian-luna-valero what do you think? I worry that people could get strange results if we don't force the recipe to use all the same versions.

@kyleabeauchamp
Copy link
Contributor Author

I think this is actually a must-have. For example, many of the pysam tests will hard-fail upon detection of different versions of bcftools, for example.

@kyleabeauchamp
Copy link
Contributor Author

Feel free to merge if you agree.

@sebastian-luna-valero
Copy link
Member

You are right, many thanks Kyle.

sebastian-luna-valero added a commit that referenced this pull request Mar 14, 2016
Hardcode htslib and samtools versions in pysam
@sebastian-luna-valero sebastian-luna-valero merged commit 9a4f4cf into bioconda:master Mar 14, 2016
@kyleabeauchamp
Copy link
Contributor Author

I think we should also delete the first two builds of the pysam 0.9 conda package. Basically, the user can still force a broken installation by manually selecting incompatible versions. See below:

conda install -c bioconda -c R pysam=0.9 htslib=1.2
Using Anaconda Cloud api site https://api.anaconda.org
Fetching package metadata: ............
Solving package specifications: ..........

Package plan for installation in environment ~/anaconda2:

The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    htslib-1.2.1               |                0         1.5 MB

The following packages will be DOWNGRADED:

    htslib: 1.3-0        --> 1.2.1-0     
    pysam:  0.9.0-py27_2 --> 0.9.0-py27_1

@kyleabeauchamp
Copy link
Contributor Author

@johanneskoester I think we should delete the builds 0 and 1 for pysam 0.9 packages, as they may lead to incompatible software versions.

We should SAVE build 2 of these recipes.

cc @sebastian-luna-valero

@johanneskoester
Copy link
Contributor

Using versions like that is not the right way in conda. There is actually support for jinja templates for this. We can set e.g. a htslib version as an environment variable, and refer to that version from the recipe, see here.

All I need to do is to find some time to implement a mechanism to read the fixed versions from some central file in our build script. I guess as we are experiencing the first issues in this direction, I should do that soon :-).

@johanneskoester
Copy link
Contributor

@bioconda/all, please see above post: strict dependencies are not the intended way to solve dynamic linking issues in conda. The "right" solution is to infer versions via jinja templates. I will set up a mechanism as soon as possible.

@kyleabeauchamp
Copy link
Contributor Author

But each version of pysam is actually hard-coded to work for only one version of htslib. This is not just a matter of dynamic linking. Pysam actually calls executables from the other packages and specifically depends on having versions 1.3 of htslib, samtools, and bcftools. Having another version installed is essentially a bug IMHO.

@johanneskoester
Copy link
Contributor

Oh, I see. In that case, your way of specifying the exact version is completely right, sorry for the noise.

@johanneskoester
Copy link
Contributor

Feel free to merge and delete the old builds. Looks good to me.

@brentp
Copy link
Contributor

brentp commented Mar 14, 2016

Is there an example in bioconda of using jinja?

@johanneskoester
Copy link
Contributor

Not yet, because we need to implement the environment variable infrastructure (or use the travis build matrix for this).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants