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

Make BiocManager work for a version of Bioconductor that doesn't "officially exist" yet #46

Open
hpages opened this issue May 4, 2019 · 7 comments

Comments

@hpages
Copy link
Contributor

hpages commented May 4, 2019

So we have a bunch of failures on the BioC 3.10 report today because BiocManager is complaining with:

unknown Bioconductor version '3.10'; see https://bioconductor.org/install`

That's because the build system installed the new BiocVersion (3.10.0) at the beginning of the builds yesterday. Then later during the R CMD build and R CMD check stages of the builds BiocManager detected that BioC 3.10 was being used and didn't like it.

This affected 19 packages on all platforms:

affyILM
BiocCheck
BSgenome
cellscape
CEMiTool
ELBOW
enrichTF
maskBAD
motifbreakR
MutationalPatterns
OmicsLonDA
OPWeight
OrganismDbi
pipeFrame
pkgDepTools
QSutils
quantro
QuasR
Ularcirc

This could be avoided by having BiocManager::version() and BiocManager::repositories() "work" as long as BiocVersion is installed. This would also address other problems we've seen earlier like Marcel's inability to use BioC 3.10 yesterday, or the problem with the BBS/utils/createPropagationDB.R script that broke this afternoon when trying to generate the propagation DB at the end of the BioC 3.10 builds.

Even if the config files at bioconductor.org didn't officially acknowledge its existence until today, BioC 3.10 started to "exist" when we branched yesterday. Furthermore, the 3.10 repos have been available (and populated) for the last 2 weeks and there is no reason why BiocManager couldn't have been used to install stuff from these repos. When the repos are not yet available, BiocManager::install() would emit a warning (it would be the warning coming from install.packages()) and wouldn't be able to install anything, but BiocManager::version() and BiocManager::repositories() would still work.

Is that a reasonable request?

Thanks!

@hpages
Copy link
Contributor Author

hpages commented Oct 30, 2019

Running into this again (using R 4.0):

library(BiocManager)
BiocManager::version()
# [1] ‘3.11’
BiocManager::repositories()
# Error: unknown Bioconductor version '3.11'; see https://bioconductor.org/install

If BiocVersion says this is BioC 3.11 and BiocManager::version() reports that, I still don't understand why BiocManager::repositories() would fail. Especially since the 3.11 repositories already exist and are populated.

This is breaking the scripts we use for generating the 3.11 build report and propagating packages. The scripts need to access the 3.11 repositories to see what's already there in order to decide what to propagate or not. Seems natural to use BiocManager::repositories() for that.

Thanks!
H.

@hpages
Copy link
Contributor Author

hpages commented Jan 6, 2020

It would be nice to get this out of the way before the next Bioconductor release. I hope it's a reasonable request. Thanks!

@mtmorgan
Copy link
Collaborator

mtmorgan commented Jan 7, 2020

I'm concerned that this would allow the user to have an invalid installation where BIocVersion implies an unsupported version.

@hpages
Copy link
Contributor Author

hpages commented Jan 7, 2020

Let's see if we are on the same page:

  1. The change I'm proposing would only affect a user that manages to install the new BiocVersion between the time we bump its version (in master, the day before the release) and the time it propagates (to the repos of the new BioC devel, the day of the release). This is a 24h window.

  2. The only way a user can install this new BiocVersion during the 24h window is directly from git.bioconductor.org. Very very few users might want to do this. Why would they?

  3. The very very few users who might actually want to do 2. are:

    • The devel builds (i.e. the build machines that build the master branch).
    • Maybe some core team members who want to use BiocManager::repos() or BiocManager::install() with this new BiocVersion (so they can test the soon-to-become-the-new-BioC-devel-version a few hours before the release).

    Unfortunately with the current state of affairs, BiocManager no longer works after they install this new BiocVersion. This breaks the devel builds and prevents anybody from doing any kind of early testing of the soon-to-become-the-new-BioC-devel-version.

The change I'm proposing will only have an effect during the 24h window during which it's important that BiocManager works properly with the new BiocVersion. It won't change anything outside this 24h window.

@mtmorgan
Copy link
Collaborator

mtmorgan commented Jan 7, 2020

I understand the intention of the issue.

The implementation would either make a general exception (once BiocVersion is found, the version is assumed to be valid) or require elaborate conditional code to know that the installed version is leading the version in config.yaml in a valid way.

If the build system has special requirements, shouldn't the build system implement the solution?

@hpages
Copy link
Contributor Author

hpages commented Jan 7, 2020

If the build system has special requirements, shouldn't the build system implement the solution?

The proposed change would benefit not only the build system but anybody who installs the new BiocVersion during the 24h window.

once BiocVersion is found, the version is assumed to be valid

The easier way to go is to replace the error by a warning. So a 1-word change only. The change would only be noticeable during the 24h window.

@hpages
Copy link
Contributor Author

hpages commented Jan 7, 2020

Also it's important to understand that all the package failures we see during the last run of the devel builds that precedes a release are because these packages contain code that uses BiocManager but BiocManager refuses to work. Not sure how the build system could implement a solution for that. To make things even worse, these failures cannot even be reproduced because the 24h window ends soon after the report is published and BiocManager typically starts to work normally again at the end of the window (it starts to work again when the new devel branch becomes official).

FWIW the BBS code itself was using BiocManager::repositories() in one place (in the script that builds the propagation DB) and I already replaced this with BiocManager:::.repositories() a couple of months ago. This doesn't address all the package failures due to BiocManager not working though.

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

No branches or pull requests

2 participants