Bug 16094 - Version constraints on imported package are avoided if the importing package only uses "packageA::funA"
Summary: Version constraints on imported package are avoided if the importing package ...
Status: NEW
Alias: None
Product: R
Classification: Unclassified
Component: Low-level (show other bugs)
Version: R 3.1.2
Hardware: x86_64/x64/amd64 (64-bit) Windows 64-bit
: P5 minor
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2014-11-29 01:02 UTC by Geoff
Modified: 2014-12-03 00:34 UTC (History)
1 user (show)

See Also:


Attachments
Zip file containing the toy packages and a script to demonstrate the behaviour (10.54 KB, application/x-zip-compressed)
2014-11-29 01:02 UTC, Geoff
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Geoff 2014-11-29 01:02:33 UTC
Created attachment 1694 [details]
Zip file containing the toy packages and a script to demonstrate the behaviour

This probably qualifies as 'a correct behaviour but not what I want' rather than a bug, but I will mention it in case others should be aware of it.

It seems that the version constraints specified in the DESCRIPTION field of a package (say packageB specifies Imports: packageA (>= 2.0)) are not checked if the code in packageB only uses the "packageA::funA" idiom to refer to functions from the imported packageA.

The relevance is that I understand the "packageA::funA" idiom is a recommended way of referring to functions which are imported from another package, but not used many times.  Using an outdated version with no warnings is not what would be desired in that scenario.

I have attached a zip file containing some toy packages which demonstrate this behaviour.  There is a .r file ('demonstrate_toy_package_behaviour.r') which is best to execute one line at a time, and a directory called package_versions, containing toy packages which the .r file copies, renames, builds local binary versions of, and then experiments with loading and running functions from them.

packageA is the package to be imported.  It exists as version 1.0 and version 2.0  There is little difference in the toy examples except for the version in the DESCRIPTION file, and the text of some messages emitted at load, attach etc time.

packageB is the package which wishes to import a function (funA) from packageA, version 2.0.  The DESCRIPTION file of packageB has an Import: field with packageA (>= 2.0).

There are 4 versions of packageB which I have trialled.  They differ in the detail of how funA is discovered and called.
version 1.0 has a NAMESPACE file with @import(packageA) and code which calls funA()
version 2.0 has a NAMESPACE file with @importFrom(packageA, funA) and code which calls funA()
version 3.0 does not mention packageA in the NAMESPACE file, instead its code calls packageA::funA
version4.0 is similar to version 3.0, except that a .onLoad functions uses requireNamespace call to try and force packageB to load a 'correct' version of packageA.

The trials consist of building the 4 versions of packageB using the correct (version 2.0) of packageA, and then simulating a user with an outdated (version 1.0) copy of packageA installed and seeing what happens when packageB is installed and a function (funB) is executed.

version 1.0 and 2.0 of packageB behaved as I expected (ie complained) when run with an outdated packageA installed .

But version 3.0 and 4.0 of packageB loaded and ran the outdated version of funA from packageA, which is not what I expected (although now I know what happens it sort of make sense - why should the `::` function know it is being called from within a package?)

Here is a excerpt from the trials I have run, including a devtools::session_info() (full details of the trials are available in the attachment)

d> # Now simulate a packageB user who has an outdated version of packageA installed
d> cleanUpToyNamespaces()
Package from libpath  C:/Users/Geoff/Documents/R-dev/packageA  has been unloaded  (version 2.0)
d> install.packages(pA1, repos = NULL)
Installing package into ‘C:/Users/Geoff/Documents/R-dev’
(as ‘lib’ is unspecified)
package ‘packageA’ successfully unpacked and MD5 sums checked
d> packageVersion('packageA')
[1] ‘1.0’
d> # packageB version 1 is OK (it complains about the outdated packageA)
d> cleanUpToyNamespaces()
d> install.packages(pB1, repos = NULL)
Installing package into ‘C:/Users/Geoff/Documents/R-dev’
(as ‘lib’ is unspecified)
package ‘packageB’ successfully unpacked and MD5 sums checked
d> packageVersion('packageB')
[1] ‘1.0’
d> packageB::funB()
Error in loadNamespace(i, c(lib.loc, .libPaths()), versionCheck = vI[[i]]) : 
  namespace ‘packageA’ 1.0 is being loaded, but >= 2.0 is required
d> # packageB version 2 is OK (it complains about the outdated packageA)
d> cleanUpToyNamespaces()
d> install.packages(pB2, repos = NULL)
Installing package into ‘C:/Users/Geoff/Documents/R-dev’
(as ‘lib’ is unspecified)
package ‘packageB’ successfully unpacked and MD5 sums checked
d> packageVersion('packageB')
[1] ‘2.0’
d> packageB::funB()
Error in loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]) : 
  namespace ‘packageA’ 1.0 is being loaded, but >= 2.0 is required
d> # packageB version 3 surprised me (it accepted the outdated packageA)
d> cleanUpToyNamespaces()
d> install.packages(pB3, repos = NULL)
Installing package into ‘C:/Users/Geoff/Documents/R-dev’
(as ‘lib’ is unspecified)
package ‘packageB’ successfully unpacked and MD5 sums checked
d> packageVersion('packageB')
[1] ‘3.0’
d> packageB::funB()
packageB version3 funB has been called.It calls packageA::funA()
 NB. The packageB DESCRIPTION file contains Imports: packageA(>= 2.0)
 NB. The packageB NAMESPACE file does not mention packageA
Package  packageA  from libname  C:/Users/Geoff/Documents/R-dev has been loaded  (version 1.0)
funA from packageA version 1.0 has been called
Session info---------------------------------------------------------------------
 setting  value                       
 version  R version 3.1.2 (2014-10-31)
 system   x86_64, mingw32             
 ui       RStudio (0.98.953)          
 language (EN)                        
 collate  English_Australia.1252      
 tz       Australia/Sydney            

Packages-------------------------------------------------------------------------
 package    * version date       source        
 devtools   * 1.6.1   2014-10-07 CRAN (R 3.1.2)
 digest       0.6.4   2013-12-03 CRAN (R 3.1.1)
 packageA     1.0     2014-11-29 local         
 packageB     3.0     2014-11-29 local         
 Rcpp         0.11.2  2014-06-08 CRAN (R 3.1.1)
 roxygen2   * 4.0.2   2014-09-02 CRAN (R 3.1.2)
 rstudioapi   0.1     2014-03-27 CRAN (R 3.1.1)
 stringr      0.6.2   2012-12-06 CRAN (R 3.1.1)
d> # packageB version 4 is not OK (it complains about the outdated packageA - but then '::' loads it anyway)
d> cleanUpToyNamespaces()
Package from libpath  C:/Users/Geoff/Documents/R-dev/packageA  has been unloaded  (version 1.0)
d> install.packages(pB4, repos = NULL)
Installing package into ‘C:/Users/Geoff/Documents/R-dev’
(as ‘lib’ is unspecified)
package ‘packageB’ successfully unpacked and MD5 sums checked
d> packageVersion('packageB')
[1] ‘4.0’
d> packageB::funB()
Package  packageB  from libname  C:/Users/Geoff/Documents/R-dev has been loaded  (version 4.0)
Loading required namespace: packageA
Failed with error:  ‘namespace ‘packageA’ 1.0 is being loaded, but >= 2.0 is required’
packageB version4 funB has been called.It calls packageA::funA()
 NB. The packageB DESCRIPTION file contains Imports: packageA(>= 2.0)
 NB. The packageB NAMESPACE file does not mention packageA
 NB. packageB has an .onload function which checks the version of packageA
Package  packageA  from libname  C:/Users/Geoff/Documents/R-dev has been loaded  (version 1.0)
funA from packageA version 1.0 has been called
Session info---------------------------------------------------------------------
 setting  value                       
 version  R version 3.1.2 (2014-10-31)
 system   x86_64, mingw32             
 ui       RStudio (0.98.953)          
 language (EN)                        
 collate  English_Australia.1252      
 tz       Australia/Sydney            

Packages-------------------------------------------------------------------------
 package    * version date       source        
 devtools   * 1.6.1   2014-10-07 CRAN (R 3.1.2)
 digest       0.6.4   2013-12-03 CRAN (R 3.1.1)
 packageA     1.0     2014-11-29 local         
 packageB     4.0     2014-11-29 local         
 Rcpp         0.11.2  2014-06-08 CRAN (R 3.1.1)
 roxygen2   * 4.0.2   2014-09-02 CRAN (R 3.1.2)
 rstudioapi   0.1     2014-03-27 CRAN (R 3.1.1)
 stringr      0.6.2   2012-12-06 CRAN (R 3.1.1)
Comment 1 Geoff 2014-12-03 00:33:00 UTC
Short version : 

Nudged by an observation by Hadley Wickham in an email, I have read a bit more of the loadNamespace code, and I now believe this does qualify as a bug (or at least an undesirable quirk) in loadNamespace.  

At the moment R (loadNamespace) appears to delegate the version checking of Imports field packages to later calls to loadNamespace which occur while the NAMESPACE directives are being processed, in the course of creating the imports:mypackage namespace / environment.

I think R could check the version dependencies in Imports when loading the package directly, instead.  Hence this update to the bug report.

========================================================================

My reasoning in more more detail : 

loadNamespace on 'mypackage' checks the version of 'mypackage' in two places ( depending on whether 'mypackage' is already loaded, or 'mypackage' really does have to be loaded).

In the second branch of the code (when 'mypackage' is actually being loaded) it obtains a full list of all the imported packages that were originally specified in the mypackage DESCRIPTION file Imports field and stores them in a local variable called vI  (vI <- pkgInfo$Imports)  - roughly around line 180 in my dumped copy of the loadNamespace code.

But vI is only used in loadNamespace later on, when the NAMESPACE directives are being processed - inside loops like 

for (i in nsInfo$imports) { ...},
for (imp in nsInfo$importClasses) ...  ,
for (imp in nsInfo$importMethods) ...

In these loops the version checking is effectively delegated to another call to loadNamespace (on the imported package mentioned in the particular 'mypackage' NAMESPACE directive being processed at the time).  vI is used to obtain the versionCheck parameter for that call (since NAMESPACE directives do not include version constraints in their syntax)

That's why imported packages only referred to by mypackage code using the importedpackage::fun syntax don't get checked by loadNamespace(mypackage, ...) - there is no need to find them and place them into the imports::mypackage environment (or should I call it a namespace?) when mypackage is being loaded.

Led by this strategy, I had been thinking it would be necessary to do the version check on the imported package when the importedpackage::fun code was executed (ie when the `::` function was called), and I could see that would be *really really* messy.

But on reflection there is no reason I can see why a small loop or function in loadNamespace couldn't just do a version check on all the imported packages mentioned in vI (ie effectively all the imported packages in named in pkgInfo$Imports), whether or not the imported packages subsequently appear in a NAMESPACE directive.  The loop/function would have to separate the checks for 
a)importedpackages that were in fact already loaded, from 
b)imported packages that were not loaded (but were installed and available for loading) 
but that's not *all* that hard (theoretically).


Geoff