Merge lp:~maxb/bzr-builddeb/dch-fail-exception into lp:bzr-builddeb

[Max Bowsher]

Currently when bzr-builddeb's invocation of dch fails, it eats the cause. This tries to reveal it a bit more.

I'm pretty sure trace.note(exception) isn't quite the right thing to be doing - advice gratefully accepted.

[James Westby (james-w)]

review: Approve

=== modified file 'cmds.py'
--- cmds.py	2011-04-29 22:32:59 +0000
+++ cmds.py	2011-05-15 17:24:32 +0000
@@ -67,6 +67,7 @@
     )
 from bzrlib.plugins.builddeb.errors import (
     BuildFailedError,
+    DchError,
     MissingChangelogError,
     NoPreviousUpload,
     PackageVersionNotPresent,
@@ -541,8 +542,11 @@
             changelog):
         from bzrlib.plugins.builddeb.merge_upstream import (
             changelog_add_new_version)
-        if not changelog_add_new_version(tree, version, distribution_name,
-                changelog, package):
+        try:
+            changelog_add_new_version(tree, version, distribution_name,
+                changelog, package)
+        except DchError, e:
+            note(e)
             raise BzrCommandError('Adding a new changelog stanza after the '
                     'merge had completed failed. Add the new changelog '
                     'entry yourself, review the merge, and then commit.')

=== modified file 'errors.py'
--- errors.py	2011-04-08 18:47:10 +0000
+++ errors.py	2011-05-15 17:24:32 +0000
@@ -224,3 +224,10 @@
 
     _fmt = ("Build refused because there are unknown files in the tree. "
             "To list all known files, run 'bzr unknowns'.")
+
+
+class DchError(BzrError):
+    _fmt = 'There was an error using dch: %(error)s.'
+
+    def __init__(self, error):
+        BzrError.__init__(self, error=error)

=== modified file 'merge_upstream.py'
--- merge_upstream.py	2011-04-29 12:54:58 +0000
+++ merge_upstream.py	2011-05-15 17:24:32 +0000
@@ -34,6 +34,11 @@
     # Prior to 0.1.15 the debian module was called debian_bundle
     from debian_bundle.changelog import Version
 
+from bzrlib.plugins.builddeb.errors import (
+    DchError,
+    )
+
+
 TAG_PREFIX = "upstream-"
 
 
@@ -83,7 +88,6 @@
     :param distribution_name: Distribution name (debian, ubuntu, ...)
     :param changelog: Changelog object
     :param package: Package name
-    :return: Whether an entry was successfully added
     """
     assert isinstance(upstream_version, str), \
          "upstream_version should be a str, not %s" % str(
@@ -99,8 +103,7 @@
             "--package", package, entry_description]
     if not tree.has_filename("debian/changelog"):
         argv.append("--create")
-    proc = subprocess.Popen(argv, cwd=tree.basedir)
-    proc.wait()
-    # FIXME: Raise insightful exception here rather than just checking
-    # return code.
-    return proc.returncode == 0
+    proc = subprocess.Popen(argv, cwd=tree.basedir, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    (stdout, stderr) = proc.communicate()
+    if proc.returncode != 0:
+        raise DchError("Adding changelog entry failed: %s" % stderr)