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

Add Reseek #51882

Merged
merged 12 commits into from
Nov 5, 2024
Merged

Add Reseek #51882

merged 12 commits into from
Nov 5, 2024

Conversation

apcamargo
Copy link
Contributor

This PR adds reseek.


Please read the guidelines for Bioconda recipes before opening a pull request (PR).

General instructions

  • If this PR adds or updates a recipe, use "Add" or "Update" appropriately as the first word in its title.
  • New recipes not directly relevant to the biological sciences need to be submitted to the conda-forge channel instead of Bioconda.
  • PRs require reviews prior to being merged. Once your PR is passing tests and ready to be merged, please issue the @BiocondaBot please add label command.
  • Please post questions on Gitter or ping @bioconda/core in a comment.

Instructions for avoiding API, ABI, and CLI breakage issues

Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify run_exports (see here for the rationale and comprehensive explanation).
Add a run_exports section like this:

build:
  run_exports:
    - ...

with ... being one of:

Case run_exports statement
semantic versioning {{ pin_subpackage("myrecipe", max_pin="x") }}
semantic versioning (0.x.x) {{ pin_subpackage("myrecipe", max_pin="x.x") }}
known breakage in minor versions {{ pin_subpackage("myrecipe", max_pin="x.x") }} (in such a case, please add a note that shortly mentions your evidence for that)
known breakage in patch versions {{ pin_subpackage("myrecipe", max_pin="x.x.x") }} (in such a case, please add a note that shortly mentions your evidence for that)
calendar versioning {{ pin_subpackage("myrecipe", max_pin=None) }}

while replacing "myrecipe" with either name if a name|lower variable is defined in your recipe or with the lowercase name of the package in quotes.

Bot commands for PR management

Please use the following BiocondaBot commands:

Everyone has access to the following BiocondaBot commands, which can be given in a comment:

@BiocondaBot please update Merge the master branch into a PR.
@BiocondaBot please add label Add the please review & merge label.
@BiocondaBot please fetch artifacts Post links to CI-built packages/containers.
You can use this to test packages locally.

Note that the @BiocondaBot please merge command is now depreciated. Please just squash and merge instead.

Also, the bot watches for comments from non-members that include @bioconda/<team> and will automatically re-post them to notify the addressed <team>.

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 60fbea9 and f4ba7a7.

📒 Files selected for processing (2)
  • recipes/reseek/build.sh (1 hunks)
  • recipes/reseek/meta.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • recipes/reseek/build.sh
🧰 Additional context used
🪛 yamllint
recipes/reseek/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (3)
recipes/reseek/meta.yaml (3)

19-24: ⚠️ Potential issue

Add OpenMP runtime dependencies.

The package requires OpenMP support, but the runtime dependencies are missing. This was previously flagged in earlier reviews and needs to be addressed.

Apply this diff:

 requirements:
   build:
     - {{ compiler("cxx") }}
     - {{ compiler("c") }}
     - make
+    - llvm-openmp  # [osx]
+  run:
+    - llvm-openmp  # [osx]
+    - libgomp      # [linux]

32-32: ⚠️ Potential issue

Add maintainer and platform information.

The recipe is missing the extra section with maintainer information and additional platform support, which is required for Bioconda packages.

Add after the about section:

extra:
  recipe-maintainers:
    - apcamargo
  additional-platforms:
    - linux-aarch64
    - osx-arm64

20-22: ⚠️ Potential issue

Configure GCC compiler for builds.

As mentioned in previous reviews, reseek requires GCC for compilation. The compiler configuration needs to be explicitly set.

Create a new file conda_build_config.yaml with:

c_compiler_version:         # [osx]
  - 12                     # [osx]
cxx_compiler_version:      # [osx]
  - 12                     # [osx]
c_compiler:                # [osx]
  - gcc                    # [osx]
cxx_compiler:              # [osx]
  - gxx                    # [osx]

@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
@bioconda bioconda deleted a comment from coderabbitai bot Nov 3, 2024
@apcamargo
Copy link
Contributor Author

@BiocondaBot please add label

@BiocondaBot BiocondaBot added the please review & merge set to ask for merge label Nov 3, 2024
@@ -0,0 +1,225 @@
#!/usr/bin/env python
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this coming from? Maybe add a comment here, that this file needs to be kept in sync?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the author's own script. In his repository he downloads it at build time, but I decided to have it within the recipe to have more control over it. I'll add a comment.

@apcamargo
Copy link
Contributor Author

Thanks, @bgruening
@bioconda/osx This build is failing for macOS with the error below:

 07:53:49 BIOCONDA INFO (ERR) parasail.cpp:56:11: error: reference to unresolved using declaration
07:53:49 BIOCONDA INFO (ERR)    56 |     ptr = aligned_alloc(alignment, size);
07:53:49 BIOCONDA INFO (ERR)       |           ^
07:53:49 BIOCONDA INFO (ERR) /opt/mambaforge/envs/bioconda/conda-bld/reseek_1730620228398/_build_env/bin/../include/c++/v1/cstdlib:150:1: note: using declaration annotated with 'using_if_exists' here
07:53:49 BIOCONDA INFO (ERR)   150 | using ::aligned_alloc _LIBCPP_USING_IF_EXISTS;
07:53:49 BIOCONDA INFO (ERR)       | ^
07:53:49 BIOCONDA INFO (ERR) parasail.cpp:56:11: error: excess elements in scalar initializer
07:53:49 BIOCONDA INFO (ERR)    56 |     ptr = aligned_alloc(alignment, size);
07:53:49 BIOCONDA INFO (ERR)       |           ^                      ~~~~~~
07:53:49 BIOCONDA INFO (ERR) 2 errors generated.
07:53:49 BIOCONDA INFO (ERR) make: *** [Makefile:270: o/parasail.o] Error 1

I tried several approaches to use a newer version of Clang, but they failed. The author of the package compiles it for macOS using GCC, but this is not an option for us (as far as I know). Is there a solution that I'm missing?

@mencian mencian merged commit 5e9b930 into bioconda:master Nov 5, 2024
6 checks passed
This was referenced Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please review & merge set to ask for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants