Merge lp:~jml/pkgme/build-source-package into lp:pkgme

[Jonathan Lange]

Does what it says on the packet.

[James Westby (james-w)]

6 -test_command=python -m subunit.run $IDLIST
7 +test_command=PYTHONPATH=`pwd` python -m subunit.run $IDLIST

Is that not implicit?

41 + stdout, stderr = proc.communicate()

Perhaps we want to do something with the returncode? Also reporting the
stdout, stderr if it fails?

Also, I think it should use that new logging framework that some bright spark
wrote to log a message about what it is doing.

We're going to want to do something with GPG keys as well, but I'm fine deferring
that for now.

Thanks,

James

review: Needs Fixing

[Jonathan Lange (jml)]

On Wed, Jul 27, 2011 at 5:42 PM, James Westby <email address hidden> wrote:
> Review: Needs Fixing
> 6       -test_command=python -m subunit.run $IDLIST
> 7       +test_command=PYTHONPATH=`pwd` python -m subunit.run $IDLIST
>
> Is that not implicit?
>

Well, sort of. It's there so that the spawned backend scripts will
have the top-level of the tree in their PYTHONPATH, so that they will
use the development version of pkgme libs instead of the system
installed one.

> 41      + stdout, stderr = proc.communicate()
>
> Perhaps we want to do something with the returncode? Also reporting the
> stdout, stderr if it fails?
>
> Also, I think it should use that new logging framework that some bright spark
> wrote to log a message about what it is doing.
>

Good calls both. Will address.

> We're going to want to do something with GPG keys as well, but I'm fine deferring
> that for now.
>

Agreed. Was thinking that. Should have said it.

jml

[James Westby (james-w)]

> On Wed, Jul 27, 2011 at 5:42 PM, James Westby <email address hidden>
> wrote:
> > Review: Needs Fixing
> > 6       -test_command=python -m subunit.run $IDLIST
> > 7       +test_command=PYTHONPATH=`pwd` python -m subunit.run $IDLIST
> >
> > Is that not implicit?
> >
>
> Well, sort of. It's there so that the spawned backend scripts will
> have the top-level of the tree in their PYTHONPATH, so that they will
> use the development version of pkgme libs instead of the system
> installed one.

Does that only apply if not using virtualenv + setup.py develop?

Thanks,

James

[Jonathan Lange (jml)]

 * debug stuff done in another branch, merged to trunk
 * don't prompt for gpg passphrase during test run
 * actual tests to show we create these files
 * don't run lintian, because, you know, why bother?
 * raise an error if retcode is non-zero
 * basic infrastructure for showing nice errors, rather than stack traces
 * lots of debugging stuff

[James Westby (james-w)]

37 + except PkgmeError, e:
38 + sys.stderr.write("ERROR: %s" % (e,))
39 + return 3

Don't we have a --debug option now? Could it turn this in to a "raise" or something?

78 + if stdout:
79 + trace.debug('D: %s' % (stdout.rstrip('\n'),))

Why the prefix on the message there?

Otherwise this looks great, thanks. It's certainly a valuable feature to have, even
if there are some more details to work out later.

Thanks,

James

review: Approve

[Jonathan Lange (jml)]

Changed the error to raise when debug is set.

The prefix is to make the output more readable. I started without a prefix, then added one and thought the output was more useful.

[lp:~jml/pkgme/build-source-package]

93. By Jonathan Lange on 2011-08-01

Raise on debug.

=== modified file 'pkgme/bin/main.py'
--- pkgme/bin/main.py	2011-07-29 14:40:05 +0000
+++ pkgme/bin/main.py	2011-08-01 09:02:27 +0000
@@ -5,10 +5,22 @@
 import sys
 
 from pkgme import __version__, write_packaging
+from pkgme.debuild import build_source_package
+from pkgme.errors import PkgmeError
 from pkgme import trace
 
 
-def main(argv=None, target_dir=None):
+def build_package(target_dir, interactive):
+    """Where the magic happens."""
+    trace.debug("Building packaging for %s" % (target_dir,))
+    write_packaging(target_dir)
+    trace.debug("Built packaging for %s" % (target_dir,))
+    trace.debug("Building source package for %s" % (target_dir,))
+    build_source_package(target_dir, sign=interactive)
+    trace.debug("Built source package for %s" % (target_dir,))
+
+
+def main(argv=None, target_dir=None, interactive=True):
     if argv is None:
         argv = sys.argv[1:]
     parser = argparse.ArgumentParser(
@@ -23,10 +35,16 @@
         trace.set_debug()
     if target_dir is None:
         target_dir = os.getcwd()
-    trace.debug("Building packaging for %s" % (target_dir,))
-    write_packaging(target_dir)
-    trace.debug("Built packaging for %s" % (target_dir,))
+    try:
+        build_package(target_dir, interactive=interactive)
+    except PkgmeError, e:
+        if options.debug:
+            raise
+        else:
+            sys.stderr.write("ERROR: %s" % (e,))
+            return 3
+    return 0
 
 
 if __name__ == '__main__':
-    main()
+    sys.exit(main())

=== added file 'pkgme/debuild.py'
--- pkgme/debuild.py	1970-01-01 00:00:00 +0000
+++ pkgme/debuild.py	2011-08-01 09:02:27 +0000
@@ -0,0 +1,34 @@
+import subprocess
+
+from pkgme.errors import PkgmeError
+from pkgme import trace
+
+
+class DebuildError(PkgmeError):
+
+    def __init__(self, command, retcode, output_lines):
+        super(DebuildError, self).__init__(
+            "%s failed with %s. Output:\n%s" % (
+                ' '.join(command),
+                retcode,
+                ''.join(' | %s' % (line,) for line in output_lines if line)))
+
+
+def build_source_package(directory, sign=True):
+    cmd = ['debuild', '--no-lintian', '-S']
+    if not sign:
+        cmd.extend(['-uc', '-us'])
+    trace.debug("Running %s" % ' '.join(cmd))
+    proc = subprocess.Popen(
+        cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=directory)
+    output = []
+    while True:
+        stdout = proc.stdout.readline()
+        output.append(stdout)
+        if stdout:
+            trace.debug('D: %s' % (stdout.rstrip('\n'),))
+        else:
+            break
+    retcode = proc.wait()
+    if retcode != 0:
+        raise DebuildError(cmd, retcode, output)

=== added file 'pkgme/errors.py'
--- pkgme/errors.py	1970-01-01 00:00:00 +0000
+++ pkgme/errors.py	2011-08-01 09:02:27 +0000
@@ -0,0 +1,5 @@
+"""Exceptions raised by pkgme."""
+
+
+class PkgmeError(Exception):
+    """Base pkgme error."""

=== modified file 'pkgme/testing.py'
--- pkgme/testing.py	2011-07-15 09:22:59 +0000
+++ pkgme/testing.py	2011-08-01 09:02:27 +0000
@@ -64,6 +64,21 @@
         return "Path exists and is a directory"
 
 
+class DirContains(Matcher):
+
+    def __init__(self, filenames):
+        self.filenames = filenames
+
+    def match(self, path):
+        mismatch = PathExists().match(path)
+        if mismatch is not None:
+            return mismatch
+        mismatch = IsDirectory().match(path)
+        if mismatch is not None:
+            return mismatch
+        return Equals(sorted(self.filenames)).match(sorted(os.listdir(path)))
+
+
 class FileContains(Matcher):
 
     def __init__(self, contents):

=== modified file 'pkgme/tests/test_script.py'
--- pkgme/tests/test_script.py	2011-07-15 09:22:47 +0000
+++ pkgme/tests/test_script.py	2011-08-01 09:02:27 +0000
@@ -1,23 +1,26 @@
-import os
-
 import pkg_resources
+from StringIO import StringIO
 
-from fixtures import TestWithFixtures
+from fixtures import MonkeyPatch
 from testtools import TestCase
 
 from pkgme.testing import (
+    DirContains,
     DirExists,
     TempdirFixture,
     )
 
 
-class ScriptTests(TestCase, TestWithFixtures):
+class ScriptTests(TestCase):
     """Smoke tests for the top-level pkgme script."""
 
     def run_script(self, cwd):
         ep = pkg_resources.load_entry_point(
             "pkgme", "console_scripts", "pkgme")
-        ep(argv=[], target_dir=cwd)
+        stderr = StringIO()
+        with MonkeyPatch('sys.stderr', stderr):
+            ep(argv=[], target_dir=cwd, interactive=False)
+        self.assertEqual('', stderr.getvalue())
 
     def test_writes_files(self):
         tempdir = self.useFixture(TempdirFixture())
@@ -28,3 +31,19 @@
 """)
         self.run_script(tempdir.path)
         self.assertThat(tempdir.abspath("debian"), DirExists())
+
+    def test_builds_source_package(self):
+        tempdir = self.useFixture(TempdirFixture())
+        tempdir.create_file(
+            "foo/setup.py", """from distutils.core import setup
+
+setup(name="foo")
+""")
+        self.run_script(tempdir.abspath('foo'))
+        self.assertThat(tempdir.path, DirContains(
+            ["foo_0.0.0.tar.gz",
+             "foo_0.0.0.dsc",
+             "foo_0.0.0_source.build",
+             "foo_0.0.0_source.changes",
+             "foo",
+             ]))