Mikael Jagan
2022-Sep-13 21:45 UTC
[Rd] Should package version requirements assume installation from sources?
[Arguably also appropriate for R-package-devel, but posted to R-devel as the discussion is aimed primarily at "experts" ... ] We, the authors of Matrix, have encountered a somewhat subtle issue induced by caching of S4 classes and methods in package namespaces. The namespaces of three reverse dependent packages (SeuratObject, conText, mcmcsae) cache the formal definition of our virtual class Matrix (and some subclasses). For example: > ns <- asNamespace("SeuratObject") > grep("^[.]__C__.*Matrix$", names(ns), value = TRUE) [1] ".__C__dMatrix" ".__C__compMatrix" ".__C__AnyMatrix" [4] ".__C__generalMatrix" ".__C__CsparseMatrix" ".__C__sparseMatrix" [7] ".__C__dsparseMatrix" ".__C__Matrix" The cached definition (which includes a _validity method_) is obtained from the version of Matrix available when the reverse dependent package was built from sources. For example, if SeuratObject was built under Matrix 1.4-1, then we get: > getValidity(ns$.__C__Matrix) function (object) { if (!isTRUE(r <- .Call(Dim_validate, object, "Matrix"))) r else .Call(dimNames_validate, object) } <bytecode: 0x11e7ca508> <environment: namespace:Matrix> whereas if SeuratObject was built under Matrix >= 1.5-0, then we get: > getValidity(ns$.__C__Matrix) function (object) .Call(Matrix_validate, object) <bytecode: 0x107dc1698> <environment: namespace:Matrix> There are two "questions" here: 1. The symbol 'Matrix_validate' is not defined until Matrix 1.5-0. Is it necessary, for this reason alone, for SeuratObject to have 'Imports: Matrix (>= 1.5-0)'? Or can SeuratObject continue using 'Imports: Matrix (>= 1.3-3)', at the risk of errors like > Error: object 'Matrix_validate' not found (as already seen here: https://stackoverflow.com/questions/73700130)? Note that this error would not occur for anyone installing SeuratObject from sources, unless they decide to _downgrade_ Matrix after doing so. Hence this primarily concerns Windows and macOS where R users would typically install a binary built by CRAN (i.e., not on their system). We are aware that package TMB tests in .onLoad() that the current Matrix version is equal to or greater than the version available at build time, thus avoiding a "strict" version requirement, but do not want this practice to spread ... 2. For how long should Matrix retain the superceded 'Dim_validate' and 'dimNames_validate', in order to ensure that "stale" cached validity methods continue to work? We hope that this discussion will highlight the potential ramifications of importing classes and methods from other packages, and having one's classes and methods imported _by_ other packages, especially for version requirements. Mikael, Martin, and Doug
Hervé Pagès
2022-Sep-13 23:11 UTC
[Rd] Should package version requirements assume installation from sources?
FWIW this happens all the time in Bioconductor, where S4 is used very intensively, when we make that kind of change to a core infrastructure package. Then a bunch of Windows and Mac binary packages need to be rebuilt because they contain cached stuff that is now out-of-sync with the new state of affairs. The way we deal with this is by bumping the version numbers of all the affected packages. This triggers automatic rebuilt and propagation of the new binaries. Note that these problems also affect packages that were already installed on the user machines _prior_ to the changes to the upstream package. Because the caching we are talking about is actually in the package installation folder (and the fact that it ends up in the binaries is just a consequence of that). So even on a system where packages are installed from source, the affected packages need to be reinstalled. Bumping the version numbers of all the affected packages also solves that. I don't see how this could be handled or mitigated via package requirements. Best, H. On 13/09/2022 14:45, Mikael Jagan wrote:> [Arguably also appropriate for R-package-devel, but posted to R-devel > ??? as the discussion is aimed primarily at "experts" ... ] > > We, the authors of Matrix, have encountered a somewhat subtle issue > induced by caching of S4 classes and methods in package namespaces. > > The namespaces of three reverse dependent packages (SeuratObject, > conText, > mcmcsae) cache the formal definition of our virtual class Matrix (and > some > subclasses).? For example: > > > ns <- asNamespace("SeuratObject") > > grep("^[.]__C__.*Matrix$", names(ns), value = TRUE) > [1] ".__C__dMatrix"?????? ".__C__compMatrix"??? ".__C__AnyMatrix" > [4] ".__C__generalMatrix" ".__C__CsparseMatrix" ".__C__sparseMatrix" > [7] ".__C__dsparseMatrix" ".__C__Matrix" > > The cached definition (which includes a _validity method_) is obtained > from > the version of Matrix available when the reverse dependent package was > built > from sources.? For example, if SeuratObject was built under Matrix 1.4-1, > then we get: > > > getValidity(ns$.__C__Matrix) > function (object) > { > ??? if (!isTRUE(r <- .Call(Dim_validate, object, "Matrix"))) > ??????? r > ??? else .Call(dimNames_validate, object) > } > <bytecode: 0x11e7ca508> > <environment: namespace:Matrix> > > whereas if SeuratObject was built under Matrix >= 1.5-0, then we get: > > > getValidity(ns$.__C__Matrix) > function (object) > .Call(Matrix_validate, object) > <bytecode: 0x107dc1698> > <environment: namespace:Matrix> > > There are two "questions" here: > > 1.? The symbol 'Matrix_validate' is not defined until Matrix 1.5-0. > ??? Is it necessary, for this reason alone, for SeuratObject to have > ??? 'Imports: Matrix (>= 1.5-0)'?? Or can SeuratObject continue using > ??? 'Imports: Matrix (>= 1.3-3)', at the risk of errors like > > ??? > Error: object 'Matrix_validate' not found > > ??? (as already seen here: https://stackoverflow.com/questions/73700130)? > > ??? Note that this error would not occur for anyone installing > SeuratObject > ??? from sources, unless they decide to _downgrade_ Matrix after doing > so. > ??? Hence this primarily concerns Windows and macOS where R users would > ??? typically install a binary built by CRAN (i.e., not on their system). > > ??? We are aware that package TMB tests in .onLoad() that the current > Matrix > ??? version is equal to or greater than the version available at build > time, > ??? thus avoiding a "strict" version requirement, but do not want this > practice > ??? to spread ... > > 2.? For how long should Matrix retain the superceded 'Dim_validate' and > ??? 'dimNames_validate', in order to ensure that "stale" cached validity > ??? methods continue to work? > > We hope that this discussion will highlight the potential ramifications > of importing classes and methods from other packages, and having one's > classes and methods imported _by_ other packages, especially for version > requirements. > > Mikael, Martin, and Doug > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel-- Herv? Pag?s Bioconductor Core Team hpages.on.github at gmail.com
Simon Urbanek
2022-Sep-14 01:27 UTC
[Rd] Should package version requirements assume installation from sources?
Mikael, first about the macOS part of the issue: the first step is that you can tell me and I can trigger a full re-build. For performance reasons the macOS build system does not do full reverse builds, because they take too long as small updates in popular packages can trigger large portions of CRAN needing a re-build - in most cases with no benefit. Now, that still only addresses one part where the users can upgrade to re-built versions __if they are aware__. However, they would have to know and do it explicitly, because a regular update.packages() would not re-install such reverse-dependent packages since their version has not changed. Therefore I would strongly suggest making sure that updates are backward-compatible, because there is simply no way around that problem as users have no way of knowing that an upgrade of Matrix requires re-installation of many other packages. In fact for Matrix I would argue that such breaking changes should only be allowed with major R release since at that point all packages have to be reinstalled by definition anyway. So to answer your second part of the question: until next major (=annual) release. Note that this problem can exist in both directions - even if you upgrade Matrix, if a dependent package is build against newer Matrix __but does not require the newer version__ then the user doesn't know that the installed version is incompatible, so you have to guard in both directions. Cheers, Simon> On 14/09/2022, at 9:45 AM, Mikael Jagan <jaganmn2 at gmail.com> wrote: > > [Arguably also appropriate for R-package-devel, but posted to R-devel > as the discussion is aimed primarily at "experts" ... ] > > We, the authors of Matrix, have encountered a somewhat subtle issue > induced by caching of S4 classes and methods in package namespaces. > > The namespaces of three reverse dependent packages (SeuratObject, conText, > mcmcsae) cache the formal definition of our virtual class Matrix (and some > subclasses). For example: > > > ns <- asNamespace("SeuratObject") > > grep("^[.]__C__.*Matrix$", names(ns), value = TRUE) > [1] ".__C__dMatrix" ".__C__compMatrix" ".__C__AnyMatrix" > [4] ".__C__generalMatrix" ".__C__CsparseMatrix" ".__C__sparseMatrix" > [7] ".__C__dsparseMatrix" ".__C__Matrix" > > The cached definition (which includes a _validity method_) is obtained from > the version of Matrix available when the reverse dependent package was built > from sources. For example, if SeuratObject was built under Matrix 1.4-1, > then we get: > > > getValidity(ns$.__C__Matrix) > function (object) > { > if (!isTRUE(r <- .Call(Dim_validate, object, "Matrix"))) > r > else .Call(dimNames_validate, object) > } > <bytecode: 0x11e7ca508> > <environment: namespace:Matrix> > > whereas if SeuratObject was built under Matrix >= 1.5-0, then we get: > > > getValidity(ns$.__C__Matrix) > function (object) > .Call(Matrix_validate, object) > <bytecode: 0x107dc1698> > <environment: namespace:Matrix> > > There are two "questions" here: > > 1. The symbol 'Matrix_validate' is not defined until Matrix 1.5-0. > Is it necessary, for this reason alone, for SeuratObject to have > 'Imports: Matrix (>= 1.5-0)'? Or can SeuratObject continue using > 'Imports: Matrix (>= 1.3-3)', at the risk of errors like > > > Error: object 'Matrix_validate' not found > > (as already seen here: https://stackoverflow.com/questions/73700130)? > > Note that this error would not occur for anyone installing SeuratObject > from sources, unless they decide to _downgrade_ Matrix after doing so. > Hence this primarily concerns Windows and macOS where R users would > typically install a binary built by CRAN (i.e., not on their system). > > We are aware that package TMB tests in .onLoad() that the current Matrix > version is equal to or greater than the version available at build time, > thus avoiding a "strict" version requirement, but do not want this practice > to spread ... > > 2. For how long should Matrix retain the superceded 'Dim_validate' and > 'dimNames_validate', in order to ensure that "stale" cached validity > methods continue to work? > > We hope that this discussion will highlight the potential ramifications > of importing classes and methods from other packages, and having one's > classes and methods imported _by_ other packages, especially for version > requirements. > > Mikael, Martin, and Doug > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel >
Duncan Murdoch
2022-Sep-14 10:04 UTC
[Rd] Should package version requirements assume installation from sources?
On 13/09/2022 5:45 p.m., Mikael Jagan wrote:> [Arguably also appropriate for R-package-devel, but posted to R-devel > as the discussion is aimed primarily at "experts" ... ] > > We, the authors of Matrix, have encountered a somewhat subtle issue > induced by caching of S4 classes and methods in package namespaces. > > The namespaces of three reverse dependent packages (SeuratObject, conText, > mcmcsae) cache the formal definition of our virtual class Matrix (and some > subclasses). For example: > > > ns <- asNamespace("SeuratObject") > > grep("^[.]__C__.*Matrix$", names(ns), value = TRUE) > [1] ".__C__dMatrix" ".__C__compMatrix" ".__C__AnyMatrix" > [4] ".__C__generalMatrix" ".__C__CsparseMatrix" ".__C__sparseMatrix" > [7] ".__C__dsparseMatrix" ".__C__Matrix" > > The cached definition (which includes a _validity method_) is obtained from > the version of Matrix available when the reverse dependent package was built > from sources. For example, if SeuratObject was built under Matrix 1.4-1, > then we get: > > > getValidity(ns$.__C__Matrix) > function (object) > { > if (!isTRUE(r <- .Call(Dim_validate, object, "Matrix"))) > r > else .Call(dimNames_validate, object) > } > <bytecode: 0x11e7ca508> > <environment: namespace:Matrix> > > whereas if SeuratObject was built under Matrix >= 1.5-0, then we get: > > > getValidity(ns$.__C__Matrix) > function (object) > .Call(Matrix_validate, object) > <bytecode: 0x107dc1698> > <environment: namespace:Matrix> > > There are two "questions" here: > > 1. The symbol 'Matrix_validate' is not defined until Matrix 1.5-0. > Is it necessary, for this reason alone, for SeuratObject to have > 'Imports: Matrix (>= 1.5-0)'? Or can SeuratObject continue using > 'Imports: Matrix (>= 1.3-3)', at the risk of errors like > > > Error: object 'Matrix_validate' not found > > (as already seen here: https://stackoverflow.com/questions/73700130)? > > Note that this error would not occur for anyone installing SeuratObject > from sources, unless they decide to _downgrade_ Matrix after doing so. > Hence this primarily concerns Windows and macOS where R users would > typically install a binary built by CRAN (i.e., not on their system). > > We are aware that package TMB tests in .onLoad() that the current Matrix > version is equal to or greater than the version available at build time, > thus avoiding a "strict" version requirement, but do not want this practice > to spread ... > > 2. For how long should Matrix retain the superceded 'Dim_validate' and > 'dimNames_validate', in order to ensure that "stale" cached validity > methods continue to work? > > We hope that this discussion will highlight the potential ramifications > of importing classes and methods from other packages, and having one's > classes and methods imported _by_ other packages, especially for version > requirements.This sounds like a bug or bad design in the S4 system, i.e. caching things without a way to detect or update when the cache becomes stale. Is it really necessary to cache things as part of the binary package, or could they be put in place when needed using lazy loading, getting a copy from the loaded copy of Matrix? Duncan Murdoch