Bug 16099 - Version checks (sometimes) missed on Import-ed or Depend-ed packages
Summary: Version checks (sometimes) missed on Import-ed or Depend-ed packages
Status: NEW
Alias: None
Product: R
Classification: Unclassified
Component: Low-level (show other bugs)
Version: R 3.1.2
Hardware: Other Other
: P5 enhancement
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2014-12-06 05:18 UTC by Geoff
Modified: 2014-12-07 01:53 UTC (History)
1 user (show)

See Also:


Attachments
Console output from the 40 tests (showing funA running under a number of scenarios) (42.54 KB, text/plain)
2014-12-06 05:18 UTC, Geoff
Details
Zipped copy of R code for simulating user with an outdated packge installed (80.66 KB, application/x-zip-compressed)
2014-12-06 05:32 UTC, Geoff
Details
Short summary table showing when outdated package version is detected (1.44 KB, text/plain)
2014-12-07 01:34 UTC, Geoff
Details
Runs the 40 'tests' of version checking in a more sensible order (6.54 KB, text/plain)
2014-12-07 01:53 UTC, Geoff
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Geoff 2014-12-06 05:18:36 UTC
Created attachment 1695 [details]
Console output from the 40 tests (showing funA running under a number of scenarios)

This is a follow on from / expansion of bug 16094, now that I have explored the issue more comprehensively.

The issue:
=========

I believe there are some inconsistencies (bugs) in how version checks are 
carried out when packages are loaded or attached.  The problem does not relate 
to explicit checks on the primary package itself ~ call it mypackage, 
or packageB.  They work fine.

Rather it relates to the packages which packageB specifies in its DESCRIPTION
file as either Depends or Imports ~ I've call that depended upon package
packageA (>= 2.0) in my examples.

An overview of the examples:
===========================

In the attached examples I have simulated what happens if a user of packageB has an outdated version of packageA installed, and tries to load packageB and call funB , which in turn calls funA from packageA.

There are 40 tests in all (2 ways for packageB to signal it requires version 2.0
of packageA, by 5 ways of loading packageB and calling funB, by 4 ways that funB finds and calls funA)

A summary of the results:
========================

require('packageB') and library('packageB') work most of the time 

~ they only 'fail' when packageB has 'Imports: packageA (>= 2.0)' in its
DESCRIPTION file AND then uses the packageA::funA() syntax rather than an 
@import(package) in the NAMESPACE file, 
but

loadNamespace('packageB'), attachNamespace('packageB'), and an implicit load (as in packageB::funB() ) 'fail' most of the time 

~ they only work when packageB has 'Imports : packageA (>= 2.0)' in its 
DESCRIPTION file, AND also mentions packageA in it's NAMESPACE file - either as @import(packageA), or as @importfrom(packageA, funA).  
In particular it appears that loadNamespace does not apply any version checking 
when packageB has 'Depends: packageA (>= 2.0)' in its DESCRIPTION file.

'Fail' means loads packageA and runs funA, when an outdated packageA
(ie version 1.0) is installed. In 22 out of the 40 different combinations I  tried, the outdated funA was loaded and run.

The details of the testing:
==========================

The tests try 5 different ways of loading packageB and calling funB, namely

1)  ns <- loadNamespace('packageB'); ns$funB()
2)  attachNamespace('packageB'); funB()
3)  packageB::funB()
4)  pkgB_loaded <- require('packageB'); if (pkgB_loaded) funB() and
5)  library('packageB'); funB()

There are 8 versions of packageB tested.  The main difference between versions is how packageB mentions packageA in its NAMESPACE file, and then calls funA

version 1) @import(packageA); and then funA()
version 2) @importFrom(packageA, funA); and then funA()
version 3) nothing in NAMESPACE about packageA; and then packageA::funA()
version 4) as per version 3 but with requireNamespace('packageA', 
           versionCheck = list(name=, etc)) in a .onLoad function for packageB

Subversions of packageB differ in terms of their DESCRIPTION file

subversion x.0) has Imports: packageA (>= 2.0), while
subversion x.1) has Depends: packageA (>= 2.0)

The attached zip file contains 2 versions of packageA ~ version 2.0 which is 'latest' version, and  version 1.0 which is used to simulate an out of date package.

It also contains 8 versions of packageB.  All versions of packageB specify packageA (>= 2.0).  The versions of packageB differ in how they attempt to load packageA and run funA (from packageA). A synopsis of the key features of each version of packageB follows:

   Synopsis of packageB version 1.0
   DESCRIPTION~ Imports: packageA (>= 2.0)
   NAMESPACE  ~ @import(packageA)
   funB calls ~ funA()

   Synopsis of packageB version 2.0
   DESCRIPTION~ Imports: packageA (>= 2.0)
   NAMESPACE  ~ @importFrom(packageA, funA)
   funB calls ~ funA()

   Synopsis of packageB version 3.0
   DESCRIPTION~ Imports: packageA (>= 2.0)
   NAMESPACE  ~ no mention of packageA
   funB calls ~ packageA::funA()

   Synopsis of packageB version 4.0
   DESCRIPTION~ Imports: packageA (>= 2.0)
   NAMESPACE  ~ no mention of packageA
   .onLoad    ~ requireNamespace('packageA', versionCheck = list(name='packageA,
   op = '>=', version = package_version('2.0')), quietly = FALSE)
   funB calls ~ packageA::funA()

   Synopsis of packageB version 1.1
   DESCRIPTION~ Depends: packageA (>= 2.0)
   NAMESPACE  ~ @import(packageA)
   funB calls ~ funA()

   Synopsis of packageB version 2.1
   DESCRIPTION~ Depends: packageA (>= 2.0)
   NAMESPACE  ~ @importFrom(packageA, funA)
   funB calls ~ funA()

   Synopsis of packageB version 3.1
   DESCRIPTION~ Depends: packageA (>= 2.0)
   NAMESPACE  ~ no mention of packageA
   funB calls ~ packageA::funA()

   Synopsis of packageB version 4.1
   DESCRIPTION~ Depends: packageA (>= 2.0)
   NAMESPACE  ~ no mention of packageA
   .onLoad    ~ requireNamespace('packageA', versionCheck = list(name='packageA'
   , op = '>=', version = package_version('2.0')), quietly = FALSE)
   funB calls ~ packageA::funA()


To replicate my results:
=======================

Some R code (in demonstrate_toy_package_behaviour.r) :
- copies source versions of each package from directory package_versions to the working directory,
- uses devtools to build binary versions in a zip file (I am running on Windows),
- then systematically uses a function I have named runTests which installs combinations of packageA and packageB (using install.packages) and then 'try's  various ways of loading packageB and running funB (cleaning up the 
loadedNamespaces between each test).

In case you do not want to use devtools and roxygen2 to build the binary versions, I will also include the 10 zip files in the attachment.

Aside : I have also used devtools in packageA::funA to echo session_info whenever funA runs (it shouldn't run but ...) so it is necessary to have devtools installed to replicate these results.
Comment 1 Geoff 2014-12-06 05:32:34 UTC
Created attachment 1696 [details]
Zipped copy of R code for simulating user with an outdated packge installed

Apologies for the length of this code - a consequence of trialling a number of approaches systematically.

If this should have been divided into separate components (eg tests using loadNamespace, tests using library, etc) pls let me know an I will amend accordingly.
Comment 2 Geoff 2014-12-07 01:34:24 UTC
Created attachment 1697 [details]
Short summary table showing when outdated package version is detected

I felt the original bug report was far too verbose, so I have produced this summary table with a row for each way my code attempts to load 'mypackage' (aka packageB), and a column for each version of packageB.  Different versions of packageB try to find and access funA from an Import-ed or Depend-ed packageA in a different way.  Y entries in the table indicate where the outdated packageA was detected and rejected, N entries where the outdated funA was loaded and run.

I hope this is clearer.

Geoff
Comment 3 Geoff 2014-12-07 01:53:52 UTC
Created attachment 1698 [details]
Runs the 40 'tests' of version checking in a more sensible order

As well the report itself being too verbose, I felt the original code (attachment 1696 [details]) ran the tests in a less than useful order.

This R code its a supplement to the first code in attachment 1696 [details].  Instead of grouping the tests in the order of each of my 8 toy packages (ie with all ways of loading a package trialled on version 1.0 of mypackage, then all ways of loading trialled on version 2.0 etc) this code groups the tests in the order of the R function used to load a package (ie loadNamespace is trialled on all 8 versions of mypackage, then attachNamespace is trialled on all 8 versions etc).

I have also tried to reduce the number of messages appearing when the tests run, and focus more on expressing the results as assertions (using assertError, since the main premise of the tests is that packageB should not be loaded, since packageA is out of date (too low a version number).

I originally began this code to explore for myself what happens when packages are loaded - if I had in mind a bug report or a test suite when I began, I could probably have written it much more concisely, and nicely.  If it would be useful to rewrite the tests as if they had actually been designed as unit tests in the first place, pls let me know and I'll have a go at doing that.

Geoff

PS This code is quite slow, since to get a better testing order I reinstall the different versions of packageB lots of time - again something that could probably be improved.  But I'll wait for feedback (if any) on this report before doing any more.