[tahoe-lafs-trac-stream] [tahoe-lafs] #1258: having Tahoe or any dependency installed in site-packages (or any other shared directory) can cause us to run or test the wrong code

[tahoe-lafs]

#1258: having Tahoe or any dependency installed in site-packages (or any other
shared directory) can cause us to run or test the wrong code
------------------------------------+---------------------------------------
     Reporter:  davidsarah          |       Owner:  zooko                             
         Type:  defect              |      Status:  assigned                          
     Priority:  major               |   Milestone:  1.9.0                             
    Component:  dev-infrastructure  |     Version:  1.8.0                             
   Resolution:                      |    Keywords:  setuptools test docs review-needed
Launchpad Bug:                      |  
------------------------------------+---------------------------------------

Comment (by davidsarah):

 Replying to [comment:18 zooko]:
 > Replying to [comment:8 davidsarah]
 > >
 > > It turned out that the {{{--version-and-path}}} output was wrong. At
 [source:src/allmydata/__init__.py at 4796#L227], the versions and paths
 obtained from {{{pkg_resources.require}}} take precedence over those
 obtained by importing modules. But in a case such as this bug where the
 imported code is different from what {{{pkg_resources}}} ''attempted'' to
 put on the {{{sys.path}}}, the result of {{{pkg_resources.require}}} is
 the wrong thing to use.
 >
 > Two questions about this:
 >
 > 1. Could we start writing a unit test for this by taking current trunk
 and adding an assertion in [source:src/allmydata/__init__.py at 4796#L227
 allmydata/__init__.py get_package_versions_and_locations()] that the
 {{{pkg_resources.require()}}} version and the specific-attribute-of-this-
 module version have to match or else exit noisily? Then we could hopefully
 produce a minimal test case which triggered that assertion.
 >
 > 2. If we "solve" this bug by removing the {{{pkg_resources.require()}}},
 as is done by
 [20110101110141-93fa1-3557bc2136f970fae05c1d20e336c32fec8e3d6d]/ticket1306,
 is it really a full solution? Could it be then that we are relying on the
 {{{zetuptoolz}}}-based dependency-resolution to make sure we have deps of
 the right version number, and there is a bug in {{{zetuptoolz}}}'s
 {{{pkg_resources}}} so that we're actually getting a package of the wrong
 version number, and
 [20110101110141-93fa1-3557bc2136f970fae05c1d20e336c32fec8e3d6d] causes us
 to ignore this fact?

 The calls to {{{pkg_resources.require()}}} that were removed in
 [4936/ticket1306], were never relied on to perform version checking. It
 was the calls in {{{_auto_deps.require_auto_deps()}}} that were supposed
 to be doing that. But they weren't working either.

 In [4942/ticket1306], we replace {{{require_auto_deps}}} with
 {{{check_all_requirements}}}, which compares the requirement with the
 imported version as obtained directly from each package module. Then in
 [4959/ticket1306], we change that check to use verlib and add tests for
 both verlib and {{{check_requirement}}}.

 If we did as you suggest in 1, we'd be using {{{pkg_resources.require}}}
 ''only'' in order to detect the case where it's giving the wrong result.
 (As it happens, that corresponds to a case where {{{__requires__ = ...;
 import pkg_resources}}} in the support script failed to do the right
 thing. However, that's almost a coincidence! We shouldn't rely on the fact
 that two setuptools bugs happen to coincide. The [4942/ticket1306] change
 already adds a reliable test for what we actually want to know, which is
 that the imported dependencies satisfy the requirements.)

 Caveat: {{{check_all_requirements}}} doesn't currently check indirect
 dependencies, e.g. pyOpenSSL, pyutil, argparse, and zbase32.
 {{{require_auto_deps}}} didn't check those either.

-- 
Ticket URL: <http://tahoe-lafs.org/trac/tahoe-lafs/ticket/1258#comment:20>
tahoe-lafs <http://tahoe-lafs.org>
secure decentralized storage