launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

Данило Шеган has proposed merging lp:~danilo/launchpad/bug-682186 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #682186 X-Generator: Launchpad (build Unknown)
  https://bugs.launchpad.net/bugs/682186


= Bug 682186 =

Make sure versioninfo works correctly even when called from a script ran outside the LP root directory. Since all of our cronscripts are run from a different path with absolute command names (eg. $LP_PY $LP_ROOT/cronscripts/blahblah), nothing that uses versioninfo gets correct information in scripts.

This includes X-Generated-By headers in all LP emails and PO files generated by Launchpad.

I chatted extensively with Gary about this, and the actual fix is really his code (we've gone through a few variants through pastebins).  I just triaged a bug, provided a test and worried about the bug fix. :)

I am also doing a follow-up branch which moves the versioninfo to lp.app, but that would make this branch more confusing so I split it up.

I am not really sure what can I do about the lint problems (other than make linter ignore them).

== Tests ==

bin/test -cvvt versioninfo

== Launchpad lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/launchpadversioninfo.py
  lib/canonical/launchpad/versioninfo.py
  lib/canonical/launchpad/tests/test_versioninfo.py

./lib/launchpadversioninfo.py
      20: E303 too many blank lines (3)
./lib/canonical/launchpad/versioninfo.py
      42: undefined name 'InputError'

-- 
https://code.launchpad.net/~danilo/launchpad/bug-682186/+merge/43549
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-682186 into lp:launchpad.

=== added file 'lib/canonical/launchpad/tests/test_versioninfo.py'
--- lib/canonical/launchpad/tests/test_versioninfo.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/tests/test_versioninfo.py	2010-12-13 18:19:04 +0000
@@ -0,0 +1,24 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import os.path
+import subprocess
+import unittest
+
+from canonical.config import TREE_ROOT
+from canonical.launchpad.versioninfo import revno
+
+
+class TestVersionInfo(unittest.TestCase):
+
+    def test_out_of_tree_versioninfo_access(self):
+        # Our cronscripts are executed with cwd != LP root.
+        # Getting version info should still work in them.
+        args = [os.path.join(TREE_ROOT, "bin/py"), "-c",
+                "from canonical.launchpad.versioninfo import revno;"
+                "print revno"]
+        process = subprocess.Popen(args, cwd='/tmp', stdout=subprocess.PIPE)
+        (output, errors) = process.communicate(None)
+        self.assertEquals(revno, int(output))

=== modified file 'lib/canonical/launchpad/versioninfo.py'
--- lib/canonical/launchpad/versioninfo.py	2009-09-02 19:11:01 +0000
+++ lib/canonical/launchpad/versioninfo.py	2010-12-13 18:19:04 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Give access to bzr and other version info, if available.
@@ -36,17 +36,13 @@
     ]
 
 
-import imp
-
-
 def read_version_info():
     try:
-        infomodule = imp.load_source(
-            'launchpadversioninfo', 'bzr-version-info.py')
-    except IOError:
+        import launchpadversioninfo
+    except InputError:
         return None
     else:
-        return getattr(infomodule, 'version_info', None)
+        return getattr(launchpadversioninfo, 'version_info', None)
 
 
 versioninfo = read_version_info()

=== added symlink 'lib/launchpadversioninfo.py'
=== target is u'../bzr-version-info.py'