bioconda / bioconda-recipes

Conda recipes for the bioconda channel.
https://bioconda.github.io
MIT License
1.64k stars 3.27k forks source link

New bioconda build system feature: recipe linting #4038

Closed daler closed 7 years ago

daler commented 7 years ago

@bioconda/all,

We just rolled out some changes to the build system that will reduce workload for the core team, reduce build times on travis-ci, and ensure high-quality recipes. New recipes will now be automatically examined for common issues ("linted"). If an issue is found, the build will fail immediately and the travis-ci log will indicate the issues found. If this happens, please see the new docs at https://bioconda.github.io/linting.html for what they mean and help on fixing them.

Please report back on this issue with any problems you think might be related to these changes.

Thanks!

(xrefs: https://github.com/bioconda/bioconda-recipes/pull/3933, https://github.com/bioconda/bioconda-utils/pull/57)

bgruening commented 7 years ago

Such a great work, thanks a ton @daler!

chapmanb commented 7 years ago

Ryan -- thanks for doing this. It's great to have consistent approaches and enforce them. I've accidentally been moving to use git_url more because it seemed cleaner than the tarballs, so glad to have this direction to swap back. Thanks again.

bgruening commented 7 years ago

@chapmanb we do advertise to use tarballs because we try to backup the tarballs. Git commits can simply disappear and creating backups of this is more complicated. Moreover, git_url is usually more expensive than downloading tarballs.

nsoranzo commented 7 years ago

I think something's broken, new recipes are linted but not tested now with simulate_travis.py . On my workstation:

$ ./simulate-travis.py 
+ bioconda-utils lint recipes config.yml --git-range master HEAD
INFO bioconda_utils.cli:lint(98): Recipes modified according to git: recipes/onto2nx/meta.yaml
 recipes/py2neo/meta.yaml
 recipes/pybel/meta.yaml
 recipes/pyfiglet/meta.yaml
+ set +x
+ bioconda-utils build recipes config.yml --docker --loglevel=info --mulled-test --git-range master HEAD
INFO bioconda_utils.docker_utils:_pull_image(317): BIOCONDA DOCKER: Pulling docker image condaforge/linux-anvil
INFO bioconda_utils.docker_utils:_pull_image(323): BIOCONDA DOCKER: Done pulling image
INFO bioconda_utils.docker_utils:_build_image(363): BIOCONDA DOCKER: Built docker image tag=tmp-bioconda-builder
INFO bioconda_utils.cli:build(220): Recipes modified according to git: .. .. .. ..
INFO bioconda_utils.build:build_recipes(233): blacklist: bioconductor-bubbletree, bioconductor-cexor, bioconductor-deseq2/1.10.1, bioconductor-diffbind, bioconductor-genelendatabase/1.6.0, bioconductor-iranges/2.4.6, bioconductor-iranges/2.4.7, bioconductor-limma/3.26.7, bioconductor-limma/3.28.2, bioconductor-limma/3.28.6, bioconductor-mmdiff, bioconductor-s4vectors/0.8.7, bioconductor-summarizedexperiment/1.0.2, bioconductor-systempiper, blast/2.2.21, cap-mirseq, denovogear, detonate, ensembl-vep, mothur/1.36.1, mysqlclient, perl-font-afm, r-hdrcde, r-knitr, r-ks, r-mutoss, r-phonr, r-rainbow, r-readr, r-sartools/1.2.0, r-spp, triform2
INFO bioconda_utils.build:build_recipes(246): Nothing to be done.
+ set +x

Note that there's a .. for each recipe that should have been tested in the line INFO bioconda_utils.cli:build(220): Recipes modified according to git:

That's happening also on Travis, see e.g. https://travis-ci.org/bioconda/bioconda-recipes/jobs/208298123

daler commented 7 years ago

Thanks @nsoranzo, checking into it now . . .

daler commented 7 years ago

Thanks @nsoranzo; found the problem and made a few improvements along the way in https://github.com/bioconda/bioconda-utils/issues/74. @johanneskoester and @acaprez, this affects your earlier merges today. I will take care of those once the bioconda-utils PR is merged into master.

acaprez commented 7 years ago

Thanks @nsoranzo for catching it and @daler for the fix. Guess I should've looked at the build logs in more detail before merging my PR...

johanneskoester commented 7 years ago

@daler, I see, that clarifies the behavior I did not understand in PR #4042.

johanneskoester commented 7 years ago

@daler, there are currently cases where master branch wants to investigate all recipes, e.g. if triggered by a cron job (this is intentional), or due to some PR that does not change a recipe (this is not intentional and we should fix it). However, in any case we should ensure that linting only runs if specific recipes are targeted. Otherwise, the following happens: https://travis-ci.org/bioconda/bioconda-recipes/jobs/208513936

daler commented 7 years ago

Thanks @johanneskoester, looks like that bug only happened on master when on bioconda/bioconda-recipes and when on travis. Should now be fixed in #4052.

Also I realized we don't have OS-specific version detection when checking if a package is already available. This means if the linux build on master finishes and uploads the complete package before OSX does (a common occurrence) then the OSX linting fails (example here). The temporary fix is to disable the "already_in_bioconda" lint, which we were already considering (https://github.com/bioconda/bioconda-utils/pull/75/commits/b3ee6dea16ae9940bc19c459a0f3c4463439b01e).

Last, there was an issue of not using the correct relative directory name for nested version recipes, which should be fixed in https://github.com/bioconda/bioconda-utils/pull/75.

Still waiting on OSX builds on travis to be able to merge these fixes.

acaprez commented 7 years ago

Looks like the same thing happened with my PR yesterday for beagle-lib, #4015. @daler - should I wait until all the fixes are merged to trigger a rebuild?

daler commented 7 years ago

Yep, I'll reply here once the changes have been merged and notify folks who have recently opened/updated PRs or merges.

daler commented 7 years ago

Update: builds are working now.

If anyone had merged a recipe that didn't get uploaded to the channel (here) due to build issues, there is a cron job that runs every night to double-check recipes that need to be built. That should have fixed the problem automatically (@acaprez, #4015 is one of these cases).

However, anyone else with open PRs, please update them to get the new changes.

Please continue to report any issues you find, thanks!

hainm commented 7 years ago

@daler: can linting on travis explicitly say "why" rather "The following recipes failed linting. See https://bioconda.github.io/linting.html for details:"? thx.

daler commented 7 years ago

Actually right after that statement there's a pandas dataframe printed out that lists the recipes and the reasons for failure. For example here, https://travis-ci.org/bioconda/bioconda-recipes/jobs/212276596#L472, the reason is in_other_channels.

The link is for more information on what failed, so in this case specifically https://bioconda.github.io/linting.html#in-other-channels.

When I do a conda info nglview, it looks like this version is already in conda-forge:

nglview 0.6.2.3 py35_0
----------------------
file name   : nglview-0.6.2.3-py35_0.tar.bz2
name        : nglview
version     : 0.6.2.3
build string: py35_0
build number: 0
channel     : conda-forge
size        : 5.2 MB
arch        : x86_64
has_prefix  : True
license     : MIT
md5         : 74e26fb0c7e71dfa7e1bcd13c0592b2a
platform    : linux
requires    : ()
subdir      : linux-64
url         : https://conda.anaconda.org/conda-forge/linux-64/nglview-0.6.2.3-py35_0.tar.bz2
dependencies:
    ipywidgets >=5.2.2
    notebook
    numpy
    python 3.5*

Here's a more extreme case with a lot of recipes failing linting, many with multiple failures: https://travis-ci.org/bioconda/bioconda-recipes/jobs/212029418#L475

hainm commented 7 years ago

reason is in_other_channels.

Hi @dakl

thanks for the explanation. I see why now.

But for nglview, I (lead developer of this project) really want to have it here (bioconda, which is more bio-related than other channels). I can not "not allow others" submit build to other channels anyway. Is there any way to overcome this?

daler commented 7 years ago

Your package is just too popular then :). You can skip linting on a per-recipe, per-lint basis as described here: https://bioconda.github.io/linting.html#skipping-a-linting-test. You might want to talk to the conda-forge folks and have that recipe be deprecated so that the next version of nglview is only on bioconda. Then you wouldn't have to use the lint-skipping mechanism any more.

sebastian-luna-valero commented 7 years ago

Hello,

I think this is the appropriate thread to discuss the following problem.

I am adding a new recipe here https://github.com/bioconda/bioconda-recipes/commit/9dbb70427b41654d7d589e1bdb9c1bcf128420a3

Locally I get:

INFO bioconda_utils.cli:build(221): No recipe modified according to git, exiting.

but this is a new recipe. Additionally, on Travis nothing runs but I cannot find the same INFO message: https://travis-ci.org/bioconda/bioconda-recipes/jobs/223514623

I must be missing something obvious here, could you please help?

Best regards, Sebastian

johanneskoester commented 7 years ago

The answer is in this message: https://travis-ci.org/bioconda/bioconda-recipes/jobs/223514623#L302

We do this in order to save build time. If you want to test before your actual PR, you have to do that in your own fork. However, I am currently working on a replacement of Travis (using buildkite.org) that makes such tricks unnecessary.

sebastian-luna-valero commented 7 years ago

Thanks, will do.

But why am I getting the INFO message locally?

johanneskoester commented 7 years ago

I don't know, could be a bug in simulate-travis. I always run bioconda-utils (or conda build) directly. @daler, any comment?

sebastian-luna-valero commented 7 years ago

Thanks!

Using directly bioconda-utils (or conda build) works. Can I ask what does simulate-travis.py expect from git? is it require that you run simulate-travis.py before committing changes with git?

johanneskoester commented 7 years ago

No. As I said, it is optional. It just tries to ensure that your environment is as similar as possible to what we have in travis (e.g. channel order).

johanneskoester commented 7 years ago

One thing that simulate-travis does is running bioconda-utils with --docker and --mulled-test. I suggest to do the same when you run bioconda-utils yourself. In general, if it works for you without simulate-travis, feel free to submit a PR and see what happens.

sebastian-luna-valero commented 7 years ago

Thanks!

I think the problem is with the --git-range HEAD option in bioconda-utils, either called directly or via simulate-travis.py.

If I have committed the changes to the new recipe, calling simulate-travis.py will print out: No recipe modified according to git, exiting.

If I want my new recipe to be tested after the commit, I have to call bioconda-utils directly but just removing --git-range HEAD.

Just my 2p ;)

johanneskoester commented 7 years ago

If you do --git-range master HEAD, does that help in both cases?

sebastian-luna-valero commented 7 years ago

Confirmed:

After committing the changes with git.

johanneskoester commented 7 years ago

And does it also work in the case where changes are not yet committed?

sebastian-luna-valero commented 7 years ago

Indeed.

johanneskoester commented 7 years ago

@sebastian-luna-valero see PR #4495. Thank you!

sebastian-luna-valero commented 7 years ago

Great, that's working for me!

bgruening commented 7 years ago

Working smoothly in the last month. Will close it. Thanks again @daler!