-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add longer form tutorials on how to add, update, and build recipes #31
base: main
Are you sure you want to change the base?
Conversation
@jfy133 this is fantastic, thank you for this! Please ping me here if/when you're ready for review/editing or need more info. |
Will do! Just the markdown to rst conversion taking time (doing in 20m gaps while waiting for things to finish) |
FYI, |
Why do I ALWAYS forget Ok, I think it's a state for reviewing now @daler :) I'm not precious about this, so feel free to directly make edits when easier :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @jfy133
I added a few small comments
host: | ||
- zlib | ||
run: | ||
- zlib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zlib does not need to be here, it has its own run_export defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've copied exactly from the recipe: https://github.com/bioconda/bioconda-recipes/blob/b95f209b980339300b2fd84514a4912f6ad495e9/recipes/centrifuge/meta.yaml
If you have a better example following best practises I would be happy to replace it, this was just one I was familiar with :)
I was looking for something not too simple and not too big, and for something non-python
source/tutorials/2024-adding-bioinformatic-software-to-bioconda.rst
Outdated
Show resolved
Hide resolved
## Option 2: bioconda-utils | ||
docker run -t --net host --rm -v /tmp/tmp<randomletters>/build_script.bash:/opt/build_script.bash -v /<path>/<to>/<conda-install>/envs/<toolname>/conda-bld/:/opt/host-conda-bld -v /<path>/<to>/<recipes_local_clone>/recipes/<toolname>:/opt/recipe -e LC_ADDRESS=en_GB.UTF-8 -e LC_NAME=en_GB.UTF-8 -e LC_MONETARY=en_GB.UTF-8 -e LC_PAPER=en_GB.UTF-8 -e LANG=en_GB.UTF-8 -e LC_IDENTIFICATION=en_GB.UTF-8 -e LC_TELEPHONE=en_GB.UTF-8 -e LC_MEASUREMENT=en_GB.UTF-8 -e LC_TIME=en_GB.UTF-8 -e LC_NUMERIC=en_GB.UTF-8 -e HOST_USER_ID=1000 quay.io/bioconda/bioconda-utils-build-env-cos7:2.11.1 bash | ||
|
||
conda mambabuild -c file:///opt/host-conda-bld --override-channels --no-anaconda-upload -c conda-forge -c bioconda -c defaults -e /opt/host-conda-bld/conda_build_config_0_-e_conda_build_config.yaml -e /opt/host-conda-bld/conda_build_config_1_-e_bioconda_utils-conda_build_config.yaml /opt/recipe/meta.yaml 2>&1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still need mambabuild or can we just use conda build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, I took this command from what the Azure bioconda runners were executing at the time I made my own notes, has this been changed?
additional-platforms: | ||
- linux-aarch64 | ||
identifiers: | ||
- biotools:Centrifuge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- biotools:Centrifuge | |
- biotools:centrifuge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
For some tools we may have to copy other files into other directories, such as databases [6]_, but this is less common. | ||
|
||
Another tricky thing is compiling of C++ code, which can be a bit of a pain. | ||
For reasons [7]_, we need to use specific variables that point to the non-standard (it seems) places that conda stores its libraries and headers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what those problems are? Can we just link to the c help?
In general it might be also good to link to the conda-forge documentation: https://conda-forge.org/docs/maintainer/adding_pkgs/#the-recipe-metayaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be one of the hardest type of compilation, based both on my experience (although that was a year or so ago now) and when I've looked for help in other PRs
The issues are often due to hardcoded varibles and paths, which conflict with bioconda's own compliler locations etc. It's also the one that I'm asked above more often at least.
The link to the C help is already in the next sentence :)
And for the conda-forge recipe yaml: I hadn't seen that before! it's very nice, I've added it to the top of the meta.yaml section.
Thanks @bgruening ! Addressed some of the comments, others I've left responses for your feedback. |
WalkthroughThe changes in this pull request involve updates to the documentation for the Bioconda project, specifically enhancing user guidance on local building, debugging, and contributing to Bioconda packages. The document The New tutorial documents have been added: 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (13)
source/tutorials/index.rst (1)
8-11
: Consider reordering entries chronologically.Given the year-based naming convention, consider placing
gcb2020
before the 2024 tutorials for consistent chronological ordering.:maxdepth: 2 + gcb2020 2024-adding-bioinformatic-software-to-bioconda 2024-updating-bioinformatic-software-to-bioconda 2024-debugging-bioinformatic-software-to-bioconda - gcb2020source/contributor/workflow.rst (2)
41-44
: Fix typo in the introductory sentence.There's a grammatical error in the sentence structure.
-Typically this involves creating within a in `recipes/<tool name>` two files: +Typically this involves creating within `recipes/<tool name>` two files:
46-49
: Remove duplicate word in the introductory sentence.-For more detailed guides, see the the following tutorials +For more detailed guides, see the following tutorialssource/tutorials/2024-updating-bioinformatic-software-to-bioconda.rst (2)
22-27
: Fix RST formatting for the note block.The note about GitHub releases automation should use proper RST note directive formatting.
Here's the suggested fix:
-*Note that if we use GitHub releases for our tool/package, Bioconda -tries to *automatically* update Bioconda recipes for us, so we may not -need to do many of the steps this manually. -*Of course, this works if -there are no changes to the dependencies or tests that can cause the -tests and thus the recipe building to fail.* +.. note:: + If we use GitHub releases for our tool/package, Bioconda + tries to *automatically* update Bioconda recipes for us, so we may not + need to do many of the steps manually. + + Of course, this works if there are no changes to the dependencies or tests + that can cause the tests and thus the recipe building to fail.
134-136
: Enhance link text for better accessibility.Consider making the link text more descriptive for better accessibility and user understanding.
-on the `Bioconda gitter/matrix -channel <https://gitter.im/bioconda/Lobby>`__. +on the `Bioconda Community Chat (Gitter/Matrix) +<https://gitter.im/bioconda/Lobby>`__.source/tutorials/2024-debugging-bioinformatic-software-to-bioconda.rst (3)
1-15
: Consider enhancing the introduction with additional metadata.The header and attribution are well-structured, but consider adding:
- A subtitle describing the scope and target audience
- Estimated time or difficulty level
- Prerequisites summary
Debugging a Bioconda build - a reference guide ============================================== + +A comprehensive guide for troubleshooting Bioconda package builds +-------------------------------------------------------------- + +| Difficulty: Intermediate +| Time: 30-45 minutes +| Prerequisites: Basic knowledge of conda, Docker, and command line *This guide was originally written by James A. Fellows Yates and reviewed by the µbinfie community for the* `µbinfie blog <https://ubinfie.github.io/>`_.
67-68
: Clarify path examples and add verification steps.The path example could be confusing. Consider:
- Using more realistic examples
- Adding commands to help users locate their conda environment path
- Including verification steps
- cd /<path>/<to>/<conda-install>/envs/<toolname>/conda-bld/linux-64 - conda create -n <debugging-env-name> -c ./ <toolname_as_in_recipe> + # Find your conda environment path + conda info --base + + # Navigate to the build directory (example path shown) + cd ~/miniconda3/envs/bioconda-build/conda-bld/linux-64 + + # Create debugging environment + conda create -n debug-mypackage -c ./ mypackage
181-183
: Update and enhance community support information.The Gitter/Matrix reference could be enhanced with additional community resources.
Consider adding:
- GitHub Discussions link
- Stack Overflow tags
- Bioconda documentation contributing guidelines
- Expected response times
Also, verify if Gitter is still the primary communication channel, as many projects have moved to Discord or GitHub Discussions.
source/tutorials/2024-adding-bioinformatic-software-to-bioconda.rst (5)
61-62
: Consider using a more maintainable command structure.The long docker command and conda build command are difficult to read. Consider breaking them into multiple lines using line continuation (
\
) for better readability.- docker run -t --net host --rm -v /tmp/tmp<randomletters>/build_script.bash:/opt/build_script.bash -v /<path>/<to>/<conda-install>/envs/<toolname>/conda-bld/:/opt/host-conda-bld -v /<path>/<to>/<recipes_local_clone>/recipes/<toolname>:/opt/recipe -e LC_ADDRESS=en_GB.UTF-8 -e LC_NAME=en_GB.UTF-8 -e LC_MONETARY=en_GB.UTF-8 -e LC_PAPER=en_GB.UTF-8 -e LANG=en_GB.UTF-8 -e LC_IDENTIFICATION=en_GB.UTF-8 -e LC_TELEPHONE=en_GB.UTF-8 -e LC_MEASUREMENT=en_GB.UTF-8 -e LC_TIME=en_GB.UTF-8 -e LC_NUMERIC=en_GB.UTF-8 -e HOST_USER_ID=1000 quay.io/bioconda/bioconda-utils-build-env-cos7:2.11.1 bash + docker run -t --net host --rm \ + -v /tmp/tmp<randomletters>/build_script.bash:/opt/build_script.bash \ + -v /<path>/<to>/<conda-install>/envs/<toolname>/conda-bld/:/opt/host-conda-bld \ + -v /<path>/<to>/<recipes_local_clone>/recipes/<toolname>:/opt/recipe \ + -e LC_ADDRESS=en_GB.UTF-8 \ + -e LC_NAME=en_GB.UTF-8 \ + -e LC_MONETARY=en_GB.UTF-8 \ + -e LC_PAPER=en_GB.UTF-8 \ + -e LANG=en_GB.UTF-8 \ + -e LC_IDENTIFICATION=en_GB.UTF-8 \ + -e LC_TELEPHONE=en_GB.UTF-8 \ + -e LC_MEASUREMENT=en_GB.UTF-8 \ + -e LC_TIME=en_GB.UTF-8 \ + -e LC_NUMERIC=en_GB.UTF-8 \ + -e HOST_USER_ID=1000 \ + quay.io/bioconda/bioconda-utils-build-env-cos7:2.11.1 bash - conda mambabuild -c file:///opt/host-conda-bld --override-channels --no-anaconda-upload -c conda-forge -c bioconda -c defaults -e /opt/host-conda-bld/conda_build_config_0_-e_conda_build_config.yaml -e /opt/host-conda-bld/conda_build_config_1_-e_bioconda_utils-conda_build_config.yaml /opt/recipe/meta.yaml 2>&1 + conda mambabuild \ + -c file:///opt/host-conda-bld \ + --override-channels \ + --no-anaconda-upload \ + -c conda-forge \ + -c bioconda \ + -c defaults \ + -e /opt/host-conda-bld/conda_build_config_0_-e_conda_build_config.yaml \ + -e /opt/host-conda-bld/conda_build_config_1_-e_bioconda_utils-conda_build_config.yaml \ + /opt/recipe/meta.yaml 2>&1
197-209
: Consider adding comments to explain the Jinja2 templating.The meta.yaml example uses Jinja2 templating but doesn't explain what the syntax means. Consider adding comments to help users understand the templating syntax.
+ # Use Jinja2 templating to define variables that can be reused throughout the recipe {% set name = "centrifuge" %} {% set version = "1.0.4.1" %} package: + # Use the 'lower' filter to ensure the package name is lowercase name: {{ name|lower }} version: {{ version }} build: number: 2 skip: true # [osx] run_exports: + # Use pin_subpackage to ensure ABI compatibility - {{ pin_subpackage("centrifuge", max_pin="x.x") }}
407-408
: Add code block for BiocondaBot command.The BiocondaBot command should be in a code block for consistency with other commands in the document.
-If we let the automated Bioconda CI do the testing on Azure, we can leave a comment with '@BiocondaBot please fetch artifacts' and this will generate a comment on the PR with two tables. +If we let the automated Bioconda CI do the testing on Azure, we can leave a comment with: + +.. code-block:: text + + @BiocondaBot please fetch artifacts + +This will generate a comment on the PR with two tables.
426-427
: Add code block for BiocondaBot label command.The BiocondaBot label command should be in a code block for consistency with other commands in the document.
-If the CI on Microsoft Azure passes, then back on GitHub we can leave a comment in our PR saying '@BiocondaBot please add label'. +If the CI on Microsoft Azure passes, then back on GitHub we can leave a comment in our PR: + +.. code-block:: text + + @BiocondaBot please add label
455-461
: Consider improving footnote formatting and content.The footnotes could be improved by:
- Adding more descriptive content for footnote 4 about license formatting
- Adding a reference to conda-forge documentation for footnote 7
- Using consistent punctuation at the end of each footnote
.. [1] Note that conda-forge has a different system for adding packages! .. [2] You can do a shallow clone ``git clone --depth 1``, to make the size of the cloned repo smaller on your machine. Thanks to @Wytamma for the tip! .. [3] Various Bioconda documentation pages say we should use ``mamba``, but recent versions of conda include ``lib-mamba`` by default, so generally we can use standard ``conda``. But if you're having problems with things being very slow, try switching to ``mamba``. -.. [4] Possibly from a fixed list, and how to format these, I don't know... I just copy and paste from other recipes. +.. [4] Licenses should follow the SPDX license identifiers. For a complete list, see https://spdx.org/licenses/. .. [5] I've noticed in a few more recent recipes that these commands can go within the ``meta.yaml`` itself `in an entry <https://docs.conda.io/projects/conda-build/en/stable/resources/define-metadata.html#script>`_ called ``script:`` under ``build:``, but I guess this only works for very simple commands... .. [6] Even though I absolutely HATE this, as often it leads to gigantic multi-gigabyte conda environments which we can't use on small CI runners. Give me the choice where to store my databases already! Don't force me to place them in a specific place /rant. -.. [7] That I've never found a good explanation or documentation for. +.. [7] For more information about conda-forge's compiler configuration, see https://conda-forge.org/docs/maintainer/knowledge_base.html#compilers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
source/contributor/building-locally.rst
(1 hunks)source/contributor/workflow.rst
(3 hunks)source/index.rst
(2 hunks)source/tutorials/2024-adding-bioinformatic-software-to-bioconda.rst
(1 hunks)source/tutorials/2024-debugging-bioinformatic-software-to-bioconda.rst
(1 hunks)source/tutorials/2024-updating-bioinformatic-software-to-bioconda.rst
(1 hunks)source/tutorials/index.rst
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- source/contributor/building-locally.rst
- source/index.rst
🔇 Additional comments (8)
source/tutorials/index.rst (2)
6-6
: LGTM! Good documentation structure.
The :maxdepth: 2
directive is appropriate for tutorial documentation, providing a balanced navigation depth.
8-10
: Verify the existence of referenced tutorial files.
The new tutorial entries follow a clear naming convention and logical sequence. Let's ensure all referenced files exist in the repository.
✅ Verification successful
All referenced tutorial files are present and correctly located
All three tutorial files exist in the expected location under source/tutorials/
:
- 2024-adding-bioinformatic-software-to-bioconda.rst
- 2024-updating-bioinformatic-software-to-bioconda.rst
- 2024-debugging-bioinformatic-software-to-bioconda.rst
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new tutorial files exist in the source directory
echo "Checking for tutorial files..."
for file in "2024-adding-bioinformatic-software-to-bioconda.rst" \
"2024-updating-bioinformatic-software-to-bioconda.rst" \
"2024-debugging-bioinformatic-software-to-bioconda.rst"; do
if fd "$file" source/; then
echo "✓ Found: $file"
else
echo "✗ Missing: $file"
fi
done
Length of output: 1407
source/contributor/workflow.rst (3)
35-36
: LGTM! Title change improves clarity.
The expanded section title better reflects the full range of contribution types, making it more welcoming for new contributors.
65-66
: LGTM! Helpful additions to the workflow.
The new guidance about copying from existing recipes and local building provides valuable practical advice for contributors. The link to the debugging tutorial is particularly useful for troubleshooting common issues.
Also applies to: 75-76
48-49
: Verify the existence of referenced tutorial files.
Let's ensure the new tutorial files exist in the correct location.
✅ Verification successful
Tutorial files exist and are correctly referenced
All the referenced tutorial files are present in the correct location within the repository:
source/tutorials/2024-adding-bioinformatic-software-to-bioconda.rst
source/tutorials/2024-updating-bioinformatic-software-to-bioconda.rst
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the existence of referenced tutorial files
# List of tutorials to check
tutorials=(
"source/tutorials/2024-adding-bioinformatic-software-to-bioconda.rst"
"source/tutorials/2024-updating-bioinformatic-software-to-bioconda.rst"
"source/tutorials/2024-debugging-bioinformatic-software-to-bioconda.rst"
)
# Check each tutorial file
for tutorial in "${tutorials[@]}"; do
if [ -f "$tutorial" ]; then
echo "✓ Found: $tutorial"
else
echo "✗ Missing: $tutorial"
fi
done
Length of output: 1083
source/tutorials/2024-updating-bioinformatic-software-to-bioconda.rst (2)
1-8
: LGTM! Clear title and proper attribution.
The document header and attribution section are well-structured, with appropriate RST formatting and clear source acknowledgment.
138-139
: LGTM! Well-formatted footnotes with helpful information.
The footnotes provide valuable additional context and are properly formatted in RST.
source/tutorials/2024-debugging-bioinformatic-software-to-bioconda.rst (1)
9-9
: Verify cross-references to other tutorials.
Let's verify that the referenced tutorial documents exist and are properly linked.
Also applies to: 158-158
✅ Verification successful
Let me try a different approach to verify the RST references:
Cross-references are valid and properly linked
The verification shows that:
- Both referenced tutorial files exist in the correct locations
- Both files contain valid content
- All
:doc:
references in the debugging tutorial correctly point to these existing files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that referenced tutorial documents exist
# Check for referenced tutorial files
echo "Checking for referenced tutorial files..."
for tutorial in \
"source/tutorials/2024-adding-bioinformatic-software-to-bioconda.rst" \
"source/tutorials/2024-updating-bioinformatic-software-to-bioconda.rst"
do
if [ -f "$tutorial" ]; then
echo "✓ Found: $tutorial"
else
echo "✗ Missing: $tutorial"
fi
done
# Check for broken RST references
echo -e "\nChecking for broken RST references..."
rg -l ":doc:" | xargs rg ":doc:\`[^`]+\`"
Length of output: 1092
Script:
#!/bin/bash
# Check for RST references in the specific file
echo "Checking RST references in the debugging tutorial..."
rg ":doc:" "source/tutorials/2024-debugging-bioinformatic-software-to-bioconda.rst"
# Also check content of the referenced files to ensure they exist and have content
echo -e "\nVerifying content of referenced tutorials..."
echo "=== Content check: adding tutorial ==="
head -n 5 "source/tutorials/2024-adding-bioinformatic-software-to-bioconda.rst"
echo -e "\n=== Content check: updating tutorial ==="
head -n 5 "source/tutorials/2024-updating-bioinformatic-software-to-bioconda.rst"
Length of output: 1722
- Bioconda configured as a source channel (see `bioconda documentation <https://bioconda.github.io/#usage>`_) | ||
- The following conda packages: | ||
|
||
- ``conda-utils`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix package name: conda-utils
should be conda-build
The package name appears to be incorrect. The correct package name is conda-build
, not conda-utils
.
- - ``conda-utils``
+ - ``conda-build``
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- ``conda-utils`` | |
- ``conda-build`` |
|
||
6. Open the PR on ``bioconda-recipes``, wait for the CI to to complete | ||
successfully, and tag for review with '@BiocondaBot please add label' | ||
as above. For more details see :doc:`/tutorials/2024-updating-bioinformatic-software-to-bioconda`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect cross-reference in documentation link.
The cross-reference appears to be referencing itself, which creates a circular reference.
- as above. For more details see :doc:`/tutorials/2024-updating-bioinformatic-software-to-bioconda`.
+ as above. For more details see :doc:`/tutorials/2024-adding-bioinformatic-software-to-bioconda`.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
as above. For more details see :doc:`/tutorials/2024-updating-bioinformatic-software-to-bioconda`. | |
as above. For more details see :doc:`/tutorials/2024-adding-bioinformatic-software-to-bioconda`. |
If we want to just let the Bioconda CI do the testing, skip to the [next section](#opening-the-pull-request). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix broken internal reference link.
The reference to the next section using Markdown syntax [next section](#opening-the-pull-request)
won't work in reStructuredText. Use the correct reST syntax instead.
-If we want to just let the Bioconda CI do the testing, skip to the [next section](#opening-the-pull-request).
+If we want to just let the Bioconda CI do the testing, skip to the `Opening the Pull Request`_ section.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
If we want to just let the Bioconda CI do the testing, skip to the [next section](#opening-the-pull-request). | |
If we want to just let the Bioconda CI do the testing, skip to the `Opening the Pull Request`_ section. | |
Based on my original tutorials from the µbinfie blog: https://ubinfie.github.io/
I'm still in the process of converting it to rst