From: Daira Hopwood <daira@jacaranda.org>
Date: Tue, 28 Jul 2015 16:35:11 +0000 (+0100)
Subject: Revert "Don't show scary diagnostic warnings from --version[-and-path]"
X-Git-Url: https://git.rkrishnan.org/reliability?a=commitdiff_plain;h=9df6b82796fdfbce772e6bab89c14d7da0b83910;p=tahoe-lafs%2Ftahoe-lafs.git

Revert "Don't show scary diagnostic warnings from --version[-and-path]"

This reverts commit 431728f8f854e02d9ab2f731675f12ce18cda122.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
---

diff --git a/src/allmydata/__init__.py b/src/allmydata/__init__.py
index 5262773a..e66538f7 100644
--- a/src/allmydata/__init__.py
+++ b/src/allmydata/__init__.py
@@ -380,9 +380,10 @@ def cross_check(pkg_resources_vers_and_locs, imported_vers_and_locs_list):
     extra_vers_and_locs_list = []
     for pr_name, (pr_ver, pr_loc) in pkg_resources_vers_and_locs.iteritems():
         if pr_name not in imported_packages and pr_name not in ignorable:
-            extra_vers_and_locs_list.append( (pr_name, (pr_ver, pr_loc, "according to pkg_resources")) )
+            errors.append("Warning: dependency %r (version %r) found by pkg_resources not found by import."
+                          % (pr_name, pr_ver))
 
-    return errors, extra_vers_and_locs_list
+    return errors
 
 
 def get_error_string(errors, debug=False):
@@ -437,12 +438,6 @@ def get_package_locations():
     return dict([(k, l) for k, (v, l, c) in _vers_and_locs_list])
 
 def get_package_versions_string(show_paths=False, debug=False):
-    errors = []
-    if not hasattr(sys, 'frozen'):
-        global _vers_and_locs_list
-        errors, extra_vers_and_locs_list = cross_check_pkg_resources_versus_import()
-        _vers_and_locs_list += extra_vers_and_locs_list
-
     res = []
     for p, (v, loc, comment) in _vers_and_locs_list:
         info = str(p) + ": " + str(v)
@@ -454,7 +449,9 @@ def get_package_versions_string(show_paths=False, debug=False):
 
     output = "\n".join(res) + "\n"
 
-    if errors:
-        output += get_error_string(errors, debug=debug)
+    if not hasattr(sys, 'frozen'):
+        errors = cross_check_pkg_resources_versus_import()
+        if errors:
+            output += get_error_string(errors, debug=debug)
 
     return output
diff --git a/src/allmydata/test/test_version.py b/src/allmydata/test/test_version.py
index 076e6189..9433e365 100644
--- a/src/allmydata/test/test_version.py
+++ b/src/allmydata/test/test_version.py
@@ -73,71 +73,65 @@ class CheckRequirement(unittest.TestCase):
         # The bug in #1355 is triggered when a version string from either pkg_resources or import
         # is not parseable at all by normalized_version.
 
-        (errors, extras) = cross_check({"foo": ("unparseable", "")}, [("foo", ("1.0", "", None))])
-        self.failUnlessEqual(extras, [])
-        self.failUnlessEqual(len(errors), 1)
-        self.failUnlessIn("by pkg_resources could not be parsed", errors[0])
+        res = cross_check({"foo": ("unparseable", "")}, [("foo", ("1.0", "", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("by pkg_resources could not be parsed", res[0])
 
-        (errors, extras) = cross_check({"foo": ("1.0", "")}, [("foo", ("unparseable", "", None))])
-        self.failUnlessEqual(extras, [])
-        self.failUnlessEqual(len(errors), 1)
-        self.failUnlessIn(") could not be parsed", errors[0])
+        res = cross_check({"foo": ("1.0", "")}, [("foo", ("unparseable", "", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn(") could not be parsed", res[0])
 
     def test_cross_check(self):
         res = cross_check({}, [])
-        self.failUnlessEqual(res, ([], []))
+        self.failUnlessEqual(res, [])
 
         res = cross_check({}, [("allmydata-tahoe", ("1.0", "", "blah"))])
-        self.failUnlessEqual(res, ([], []))
+        self.failUnlessEqual(res, [])
 
         res = cross_check({"foo": ("unparseable", "")}, [])
-        self.failUnlessEqual(res, ([], [("foo", ("unparseable", "", "according to pkg_resources"))]))
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("not found by import", res[0])
 
         res = cross_check({"argparse": ("unparseable", "")}, [])
-        self.failUnlessEqual(res, ([], []))
+        self.failUnlessEqual(len(res), 0)
 
-        (errors, extras) = cross_check({}, [("foo", ("unparseable", "", None))])
-        self.failUnlessEqual(extras, [])
-        self.failUnlessEqual(len(errors), 1)
-        self.failUnlessIn("was not found by pkg_resources", errors[0])
+        res = cross_check({}, [("foo", ("unparseable", "", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("not found by pkg_resources", res[0])
 
         res = cross_check({"distribute": ("1.0", "/somewhere")}, [("setuptools", ("2.0", "/somewhere", "distribute"))])
-        self.failUnlessEqual(res, ([], []))
+        self.failUnlessEqual(len(res), 0)
 
-        (errors, extras) = cross_check({"distribute": ("1.0", "/somewhere")}, [("setuptools", ("2.0", "/somewhere", None))])
-        self.failUnlessEqual(extras, [])
-        self.failUnlessEqual(len(errors), 1)
-        self.failUnlessIn("location mismatch", errors[0])
+        res = cross_check({"distribute": ("1.0", "/somewhere")}, [("setuptools", ("2.0", "/somewhere", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("location mismatch", res[0])
 
-        (errors, extras) = cross_check({"distribute": ("1.0", "/somewhere")}, [("setuptools", ("2.0", "/somewhere_different", None))])
-        self.failUnlessEqual(extras, [])
-        self.failUnlessEqual(len(errors), 1)
-        self.failUnlessIn("location mismatch", errors[0])
+        res = cross_check({"distribute": ("1.0", "/somewhere")}, [("setuptools", ("2.0", "/somewhere_different", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("location mismatch", res[0])
 
         res = cross_check({"zope.interface": ("1.0", "")}, [("zope.interface", ("unknown", "", None))])
-        self.failUnlessEqual(res, ([], []))
+        self.failUnlessEqual(len(res), 0)
 
-        (errors, extras) = cross_check({"foo": ("1.0", "")}, [("foo", ("unknown", "", None))])
-        self.failUnlessEqual(extras, [])
-        self.failUnlessEqual(len(errors), 1)
-        self.failUnlessIn("could not find a version number", errors[0])
+        res = cross_check({"foo": ("1.0", "")}, [("foo", ("unknown", "", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("could not find a version number", res[0])
 
         # When pkg_resources and import both find a package, there is only a warning if both
         # the version and the path fail to match.
 
         res = cross_check({"foo": ("1.0", "/somewhere")}, [("foo", ("2.0", "/somewhere", None))])
-        self.failUnlessEqual(res, ([], []))
+        self.failUnlessEqual(len(res), 0)
 
         res = cross_check({"foo": ("1.0", "/somewhere")}, [("foo", ("1.0", "/somewhere_different", None))])
-        self.failUnlessEqual(res, ([], []))
+        self.failUnlessEqual(len(res), 0)
 
         res = cross_check({"foo": ("1.0-r123", "/somewhere")}, [("foo", ("1.0.post123", "/somewhere_different", None))])
-        self.failUnlessEqual(res, ([], []))
+        self.failUnlessEqual(len(res), 0)
 
-        (errors, extras) = cross_check({"foo": ("1.0", "/somewhere")}, [("foo", ("2.0", "/somewhere_different", None))])
-        self.failUnlessEqual(extras, [])
-        self.failUnlessEqual(len(errors), 1)
-        self.failUnlessIn("but version '2.0'", errors[0])
+        res = cross_check({"foo": ("1.0", "/somewhere")}, [("foo", ("2.0", "/somewhere_different", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("but version '2.0'", res[0])
 
     def test_extract_openssl_version(self):
         self.failUnlessEqual(extract_openssl_version(MockSSL("")),