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 BioConductor packages to BioC 3.20 #51889

Open
wants to merge 6 commits into
base: bulk
Choose a base branch
from
Open

Update BioConductor packages to BioC 3.20 #51889

wants to merge 6 commits into from

Conversation

bgruening
Copy link
Member

This PR updates all existing packages to BioC 3.20. It also adds a few new packages from the new release.

The PR is targeting the bulk branch. Please provide feedback, so that we can merge it into bulk.

@bgruening
Copy link
Member Author

Ok, I'm not planning to add new packages. Please review and if you feel brave merge it into bulk. I'm not sure how much time I will have next week.

@danielnachun
Copy link

There are few recurring errors I'll post below

Most failures are due to this error:

 bioconductor-data-packages >=20241103  does not exist (perhaps a typo or a missing channel).

I'm not sure why this package isn't coming up as available.

bioconductor-rawrr can't find mono despite it being included as a dependency:

error: The cross platform, open source .NET framework (mono) is not available.

Several packages now say they need R 4.4:

  • bioconductor-hubmapr
  • bioconductor-biocversion

bioconductor-iclusterplus can't find a compatible Fortran compiler:

Attempting to detect how R was configured for Fortran 90....
Unsupported Fortran 90 compiler or Fortran 90
compilers unavailable! Stop!

@bgruening
Copy link
Member Author

For the R 4.4, we might want to bump r_base in conda-forge https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/main/recipe/conda_build_config.yaml#L829 at first?

The data packages are its own thing, I need to check the json files and need to bump the data-package recipes, however, this should not stop us to proceed IMHO.

The R 4.4 is a show-stopper - we should migrate to 4.4. before we merge I think.

@bgruening
Copy link
Member Author

If conda-forge has updated the pins, we can update our dependencies on those pins here https://github.com/bioconda/bioconda-utils/blob/master/bioconda_utils/bioconda_utils-requirements.txt and proceed.

@danielnachun do you want to propose the 4.4. pins in conda-forge?

@danielnachun
Copy link

I'll propose the pin change, though the potential issue at the moment is that the R 4.4 migration in conda-forge is not totally finished: https://conda-forge.org/status/migration/?name=r-base44_and_m2w64-ucrt. However, assuming #49778 did not miss any dependencies, all of the conda-forge R packages needed for Bioconductor have been migrated.

The migration is nearly done, but in the event that conda-forge does not want to move the pinnings yet, would we be able to raise the r-base version to 4.4 in bioconda_utils-requirements.txt? I think we'd be okay since all of our dependencies have been migrated but I'm not sure if there are other ways it would break things.

@bgruening
Copy link
Member Author

@danielnachun thanks for looking at this.

I don't think the migration needs to be completely finished isn't it? We could start here already in parallel?

@danielnachun
Copy link

Yes, I think we can start here without waiting for the conda-forge migration to finish. I will try to ping other Conda Forge R maintainers about the last hold-outs for R 4.4 there, but everything we need from Conda Forge is ready to go.

@aliciaaevans
Copy link
Contributor

@bgruening @danielnachun I think we'd need to update r-base pinning here: bioconda_utils-conda_build_config.yaml. Looks like we'd need to do that anyway, even if conda-forge updates theirs.

I think bioconda_utils-requirements.txt is for installing bioconda-utils itself.

@bgruening
Copy link
Member Author

That looks right to me.

@bgruening
Copy link
Member Author

I released a new version of bioconda-utils. I increased the r-base version, as discussed, and also bumped the conda-forge-pinning version.

Anything else you can think of?

@martin-g
Copy link
Contributor

martin-g commented Dec 2, 2024

The Linux64 tests fail with 17:37:44 BIOCONDA INFO (ERR) [Nov 3 17:37:44] SERR [Errno 28] No space left on device
The Linux ARM64 tests fail with timeout (more than 1h).

How this bulk build is going to be fully executed ?

@aliciaaevans
Copy link
Contributor

The Linux64 tests fail with 17:37:44 BIOCONDA INFO (ERR) [Nov 3 17:37:44] SERR [Errno 28] No space left on device The Linux ARM64 tests fail with timeout (more than 1h).

How this bulk build is going to be fully executed ?

There is a special bulk branch workflow that distributes the recipes to be built across multiple runners and immediately pushes successful builds to the channel. If we run into space issues or it times out, we can rerun it and it picks up after it leaves off. We can ignore the PR checks for this.

@daler
Copy link
Member

daler commented Dec 9, 2024

I'm blocking out time this week to work on this.

Some notes for myself...

  1. Pin R 4.4 in bioconda-utils (as @bgruening pointed out this is done).
  2. Evaluate current state of bulk branch; make bulk as consistent with master as possible
  3. On bulk, run bioconda-utils update-pinnings. This will, for example build other R dependencies in R 4.4
  4. Merge this in to bulk and play the "who failed this time" game, eventually merging into master.

May need to edit https://bioconda.github.io/developer/bulk.html, because in cases like this with a bumped R version, I think we need the bioconda-utils update-pinnings step. Also in those docs, referring to just numbered steps seems error-prone.

Throughout the process, will need to be careful about runner allocation, since we now have a pretty convoluted set of CI platforms, OSs, and architectures. We need to keep one osx-arm64 available for main PRs.

@daler
Copy link
Member

daler commented Dec 9, 2024

Current status: merged master into bulk and have been working through the fixes. A LOT of recipes needed build number bumps due to recent pinning changes. Latest batch of updates is running on bulk; once that's error free I'll merge bulk into master and then merge this PR into bulk.

Running update-pinning crashed the VM I was working on, but I'm unconvinced it actually needs to be run in this case. I believe bulk is doing the full inspection, which is what causes it to detect that build numbers need be bumped. It's just that I manually am making changes rather than automatically as update-pinning would do (I believe).

@daler
Copy link
Member

daler commented Dec 10, 2024

Updates and some clarification:

In the end, everything needs to make its way into bulk. The question is in the order of operations.

Option 1: bump non-BioC packages on bulk, finish building those, then merge this

Option 2: merge this immediately, but simultaneously deal with BioC and all other packages. Going directly to BioC 3.20 means we will probably never get around to adding BioC 3.19 which came out in May.

In an attempt to make things less confusing when fixing packages, especially when we know BioC can get tricky with all of the "DAG management" required to get dependencies to build in a timely manner, I went for option 1.

That also buys us a little time for deciding whether 3.19 should ever go into bioconda. I am leaning towards no, in the interest of getting these updates in. However @bgruening, not sure if the requests for updates you're getting are for 3.19 or 3.20 versions.

@danielnachun
Copy link

For the use cases I'm dealing with, the main issue is just needing R 4.4 support for Bioconductor. I can't imagine there will be many use cases where someone needs the specific version of a package provided by Bioconductor 3.19.

@bgruening
Copy link
Member Author

We should skip 3.19 and go with the latest stable version imho.

@martin-g
Copy link
Contributor

+1 to skip 3.19 from me too

@daler
Copy link
Member

daler commented Dec 11, 2024

OK -- on it.

@daler
Copy link
Member

daler commented Dec 13, 2024

Update: trying to resolve the "No space left on device" errors that are failing many builds. This is happening on the relatively smaller set of recipes that need to be rebuilt for the new conda-forge pinning updates. I'm hesitant to merge the thousands of recipe updates in this PR when these known infrastructure problems still exist. @aliciaaevans is adding some additional reporting to bioconda-utils to diagnose, just to see what we're up against. Worst case is if there's nothing we can do about it, in which case we merge this in and then babysit and kill/restart individual workers as soon as they run out of space.

@aliciaaevans
Copy link
Contributor

I'm going to release the version of bioconda-utils with the additional logging. I'll test it out on bulk by updating the matching branch in bioconda-common, so it won't be used for master yet.

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.

5 participants