Merge lp:~danilo/launchpad/bug-682186 into lp:launchpad

Proposed by Данило Шеган on 2010-12-13
Status: Merged
Approved by: Данило Шеган on 2010-12-14
Approved revision: no longer in the source branch.
Merged at revision: 12066
Proposed branch: lp:~danilo/launchpad/bug-682186
Merge into: lp:launchpad
Diff against target: 62 lines (+28/-8)
2 files modified
lib/canonical/launchpad/tests/test_versioninfo.py (+24/-0)
lib/canonical/launchpad/versioninfo.py (+4/-8)
To merge this branch: bzr merge lp:~danilo/launchpad/bug-682186
Reviewer Review Type Date Requested Status
Henning Eggers (community) code Approve on 2010-12-14
Aaron Bentley (community) 2010-12-13 Needs Fixing on 2010-12-13
Review via email: mp+43549@code.launchpad.net

Commit Message

[no-qa] [rollback=12041] [r=henninge][ui=none][bug=682186] Make LP versioninfo work for scripts as well (when CWD is not LP root).

Description of the Change

= 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'

To post a comment you must log in.
Aaron Bentley (abentley) wrote :

I think you mean ImportError-- the linter is quite right to complain about InputError.

review: Needs Fixing
Данило Шеган (danilo) wrote :

Indeed, fixed.

Данило Шеган (danilo) wrote :

Lint is now clean.

Henning Eggers (henninge) wrote :

Looks good, thanks. ;)

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'lib/canonical/launchpad/tests/test_versioninfo.py'
2--- lib/canonical/launchpad/tests/test_versioninfo.py 1970-01-01 00:00:00 +0000
3+++ lib/canonical/launchpad/tests/test_versioninfo.py 2010-12-14 09:39:18 +0000
4@@ -0,0 +1,24 @@
5+# Copyright 2010 Canonical Ltd. This software is licensed under the
6+# GNU Affero General Public License version 3 (see the file LICENSE).
7+
8+__metaclass__ = type
9+
10+import os.path
11+import subprocess
12+import unittest
13+
14+from canonical.config import TREE_ROOT
15+from canonical.launchpad.versioninfo import revno
16+
17+
18+class TestVersionInfo(unittest.TestCase):
19+
20+ def test_out_of_tree_versioninfo_access(self):
21+ # Our cronscripts are executed with cwd != LP root.
22+ # Getting version info should still work in them.
23+ args = [os.path.join(TREE_ROOT, "bin/py"), "-c",
24+ "from canonical.launchpad.versioninfo import revno;"
25+ "print revno"]
26+ process = subprocess.Popen(args, cwd='/tmp', stdout=subprocess.PIPE)
27+ (output, errors) = process.communicate(None)
28+ self.assertEquals(revno, int(output))
29
30=== modified file 'lib/canonical/launchpad/versioninfo.py'
31--- lib/canonical/launchpad/versioninfo.py 2009-09-02 19:11:01 +0000
32+++ lib/canonical/launchpad/versioninfo.py 2010-12-14 09:39:18 +0000
33@@ -1,4 +1,4 @@
34-# Copyright 2009 Canonical Ltd. This software is licensed under the
35+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
36 # GNU Affero General Public License version 3 (see the file LICENSE).
37
38 """Give access to bzr and other version info, if available.
39@@ -36,17 +36,13 @@
40 ]
41
42
43-import imp
44-
45-
46 def read_version_info():
47 try:
48- infomodule = imp.load_source(
49- 'launchpadversioninfo', 'bzr-version-info.py')
50- except IOError:
51+ import launchpadversioninfo
52+ except ImportError:
53 return None
54 else:
55- return getattr(infomodule, 'version_info', None)
56+ return getattr(launchpadversioninfo, 'version_info', None)
57
58
59 versioninfo = read_version_info()
60
61=== added symlink 'lib/launchpadversioninfo.py'
62=== target is u'../bzr-version-info.py'