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

Update the scripts used to generate the ucsc-XYZ recipes #50325

Merged
merged 9 commits into from
Aug 27, 2024

Conversation

martin-g
Copy link
Contributor

This PR updates the scripts used to generate the ucsc-xyz recipes.
See #50323 and #50324 for examples of the changes in the recipes.

Once this PR is merged the other recipes may be pushed in bulk ?!


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>.

@martin-g
Copy link
Contributor Author

martin-g commented Aug 26, 2024

@bgruening This is the result of your suggestion at #50106 (comment)

What would be the best way to rebuild the recipes ?

  • should I open PRs against master in mini-bulks (i.e. 5-6 recipes in one PR) ?
  • should someone (@aliciaaevans ?!) do it in the bulk branch ? (I don't know how exactly the bulk branch changes work...)

@aliciaaevans
Copy link
Contributor

Oh cool, I didn't realize these scripts existed. I could run this on bulk once it's merged. Should be relatively quick since these aren't interdependent like bioconductor recipes.

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot!

@@ -23,14 +26,14 @@ requirements:
- libpng
- libuuid
- mysql-connector-c
- openssl
- libopenssl-static
Copy link
Member

Choose a reason for hiding this comment

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

interesting why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tools require libssl.a instead of libssl.so. I am not sure since when though.
I have seen some recipes using includes.patch to change .a to .so. I decided to keep the original build, i.e. to not patch it, and use the new dependency instead.

scripts/ucsc/template-meta-with-python.yaml Show resolved Hide resolved
doc_url: "https://github.com/ucscGenomeBrowser/kent/blob/master/README"

extra:
additional-platforms:
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a doi as reference here: https://genome.ucsc.edu/cite.html#ref

Maybe the bigwig, bigbed paper? https://doi.org/10.1093/bioinformatics/btq351

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please give me the YAML snippet that I should add? Or please push it yourself to this PR!

mkdir -p "$PREFIX/bin"
cp {program_source_dir}/{program} "$PREFIX/bin"
chmod +x "$PREFIX/bin/{program}"
cp -f {program_source_dir}/{program} "${{PREFIX}}/bin"
Copy link
Member

Choose a reason for hiding this comment

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

Is the force needed, or are we masking potential error here?

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 an improvement by @mencian to one of my previous PRs.
I could remove it if there is a chance to mask some problem!

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the reason, so I would rather remove.

@bgruening
Copy link
Member

This is super cool, thanks for updating them. We should use the bulk branch here, that why we have it. Mini-bulks are just wasting your time.

@martin-g
Copy link
Contributor Author

Thanks, @aliciaaevans !

There are ~200 disabled ucsc- recipes in build-fail-blacklist that should be re-enabled before running the bulk build!
There will be (hopefully few) recipes like #49297 which will fail on linux-aarch64 - for them we will need to remove the additional-platforms entry. Actually I could add a new array in the Python script for the known ones, so they are skipped automatically. Let me update the PR!

@martin-g
Copy link
Contributor Author

Actually I could add a new array in the Python script for the known ones, so they are skipped automatically. Let me update the PR!

Done!

mkdir -p "$PREFIX/bin"
cp {program_source_dir}/{program} "$PREFIX/bin"
chmod +x "$PREFIX/bin/{program}"
cp -f {program_source_dir}/{program} "${{PREFIX}}/bin"
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the reason, so I would rather remove.

scripts/ucsc/template-meta.yaml Show resolved Hide resolved
martin-g added a commit to martin-g/bioconda-recipes that referenced this pull request Aug 27, 2024
Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Found a better one, sorry.

scripts/ucsc/template-meta-with-python.yaml Outdated Show resolved Hide resolved
scripts/ucsc/template-meta.yaml Outdated Show resolved Hide resolved
martin-g and others added 6 commits August 27, 2024 10:24
…h known build issues on aarch64

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Co-authored-by: Björn Grüning <[email protected]>
Co-authored-by: Björn Grüning <[email protected]>
@martin-g martin-g force-pushed the update-ucsc-recipe-generator branch from 19753d4 to 12d77ef Compare August 27, 2024 07:24
Set -x for easier debugging

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@martin-g
Copy link
Contributor Author

@BiocondaBot please add label

@BiocondaBot BiocondaBot added the please review & merge set to ask for merge label Aug 27, 2024
martin-g and others added 2 commits August 27, 2024 10:54
To be consistent with the SKIP list

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Thanks a lot @martin-g

@@ -198,6 +197,12 @@ def program_subdir(program, names):
'gfServer',
]

# A list of programs which have problems to build on linux-aarch64
SKIP_AARCH64 = [
Copy link
Member

Choose a reason for hiding this comment

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

I think at some point we should have a global list, that script like this one can consume.

@aliciaaevans aliciaaevans merged commit f5257d5 into bioconda:master Aug 27, 2024
6 checks passed
@aliciaaevans
Copy link
Contributor

@martin-g I've got time today to get this started on bulk.

@aliciaaevans
Copy link
Contributor

Still working though timeouts and disk space limits, but here are failures so far: https://github.com/bioconda/bioconda-recipes/wiki/build-failures-bulk

I think the aarch failures are all that knet_init_alt so we can remove the platform for those after they've all been built or failed. I think the linux-64 are mostly one error: multiple definition of htmlRecover.

@aliciaaevans
Copy link
Contributor

(I'm still using this PR for notes about this bulk update for ucsc.)

I've removed linux-aarch64 from the failures with the knet_init_alt error, removed the build_failure files, and added them to the SKIP_AARCH64 in the python script (all on the bulk branch). Now https://github.com/bioconda/bioconda-recipes/wiki/build-failures-bulk only has other failures. I will probably merge bulk back to master sooner rather than later (tomorrow?). We can always do another bulk run to fix the knet_init_alt issue and the remaining failures can be individual PRs.

@martin-g martin-g deleted the update-ucsc-recipe-generator branch August 28, 2024 07:21
@martin-g
Copy link
Contributor Author

Thank you, @aliciaaevans !

In #50368 @mencian added support for osx-64 to one of the ucsc recipes.
After running the scripts in bulk this change will be overwritten and lost.
@mencian We can add the OSX specific lines to the templates once @aliciaaevans merges the current improvements back to master.

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