Merge lp:~jml/pkgme/binary-all-info-slpit into lp:pkgme

[Jonathan Lange]

This splits up the all_info binary backend script into multiple scripts. The price is a little bit of duplication, but the gain is less weird-ass boilerplate/iffery and easier debugging when experimenting from the command line.

Main interesting bit of this change is correctly using ExtraFiles itself to get itself out of the info, and then giving it a proper clean() method so we can turn a JSON dump into a dictionary of files.

[James Westby (james-w)]

review: Approve

=== removed file 'pkgme/backends/binary/all_info'
--- pkgme/backends/binary/all_info	2011-07-15 16:30:18 +0000
+++ pkgme/backends/binary/all_info	1970-01-01 00:00:00 +0000
@@ -1,61 +0,0 @@
-#!/usr/bin/python
-
-import json
-import os
-import sys
-
-from pkgme.binary import (
-    CATEGORIES,
-    DEPENDS,
-    get_desktop_file,
-    get_install_file,
-    guess_dependencies,
-    METADATA_FILE,
-    PACKAGE_NAME,
-    TAGLINE,
-    )
-from pkgme.info_elements import (
-    BuildDepends,
-    Depends,
-    ExtraFiles,
-    PackageName,
-    )
-
-# XXX: Split this into multiple files. all_info is the wrong fit. Also,
-# multiple files would allow for easier experimentation.
-
-
-def get_all_info(wanted, metadata):
-    package_name = metadata[PACKAGE_NAME]
-    output = {}
-    path = os.getcwd()
-    if PackageName.name in wanted:
-        output[PackageName.name] = package_name
-    if ExtraFiles.name in wanted:
-        install_file = get_install_file(package_name, path, True)
-        desktop_file = get_desktop_file(
-            package_name, path,
-            tagline=metadata.get(TAGLINE, ''),
-            categories=metadata.get(CATEGORIES, ''))
-        output[ExtraFiles.name] = {
-            # XXX: Hardcoded literal attack!
-            'debian/install': install_file,
-            'debian/%s.desktop' % (package_name,): desktop_file.get_contents(),
-        }
-    if BuildDepends.name in wanted:
-        output[BuildDepends.name] = list(guess_dependencies(path))
-    if Depends.name in wanted:
-        output[Depends.name] = DEPENDS
-    return output
-
-
-def main():
-    wanted = json.load(sys.stdin)
-    with open(METADATA_FILE) as f:
-        metadata = json.load(f)
-    output = get_all_info(wanted, metadata)
-    json.dump(output, sys.stdout)
-
-
-if __name__ == '__main__':
-    main()

=== added file 'pkgme/backends/binary/build_depends'
--- pkgme/backends/binary/build_depends	1970-01-01 00:00:00 +0000
+++ pkgme/backends/binary/build_depends	2011-07-18 11:33:32 +0000
@@ -0,0 +1,14 @@
+#!/usr/bin/python
+
+import os
+
+from pkgme.binary import guess_dependencies
+
+
+def main():
+    path = os.getcwd()
+    print ', '.join(guess_dependencies(path))
+
+
+if __name__ == '__main__':
+    main()

=== added file 'pkgme/backends/binary/depends'
--- pkgme/backends/binary/depends	1970-01-01 00:00:00 +0000
+++ pkgme/backends/binary/depends	2011-07-18 11:33:32 +0000
@@ -0,0 +1,11 @@
+#!/usr/bin/python
+
+from pkgme.binary import DEPENDS
+
+
+def main():
+    print DEPENDS
+
+
+if __name__ == '__main__':
+    main()

=== added file 'pkgme/backends/binary/extra_files'
--- pkgme/backends/binary/extra_files	1970-01-01 00:00:00 +0000
+++ pkgme/backends/binary/extra_files	2011-07-18 11:33:32 +0000
@@ -0,0 +1,36 @@
+#!/usr/bin/python
+
+import json
+import os
+import sys
+
+from pkgme.binary import (
+    CATEGORIES,
+    get_desktop_file,
+    get_install_file,
+    METADATA_FILE,
+    PACKAGE_NAME,
+    TAGLINE,
+    )
+
+
+def main():
+    with open(METADATA_FILE) as f:
+        metadata = json.load(f)
+    package_name = metadata[PACKAGE_NAME]
+    path = os.getcwd()
+    install_file = get_install_file(package_name, path, True)
+    desktop_file = get_desktop_file(
+        package_name, path,
+        tagline=metadata.get(TAGLINE, ''),
+        categories=metadata.get(CATEGORIES, ''))
+    json.dump(
+        {
+            # XXX: Hardcoded literal attack!
+            'debian/install': install_file,
+            'debian/%s.desktop' % (package_name,): desktop_file.get_contents(),
+        }, sys.stdout)
+
+
+if __name__ == '__main__':
+    main()

=== added file 'pkgme/backends/binary/package_name'
--- pkgme/backends/binary/package_name	1970-01-01 00:00:00 +0000
+++ pkgme/backends/binary/package_name	2011-07-18 11:33:32 +0000
@@ -0,0 +1,17 @@
+#!/usr/bin/python
+
+import json
+from pkgme.binary import (
+    METADATA_FILE,
+    PACKAGE_NAME,
+    )
+
+
+def main():
+    with open(METADATA_FILE) as f:
+        metadata = json.load(f)
+    print metadata[PACKAGE_NAME]
+
+
+if __name__ == '__main__':
+    main()

=== modified file 'pkgme/info_elements.py'
--- pkgme/info_elements.py	2011-07-15 16:04:55 +0000
+++ pkgme/info_elements.py	2011-07-18 11:33:32 +0000
@@ -1,3 +1,6 @@
+import json
+
+
 class MissingInfoError(Exception):
     pass
 
@@ -159,6 +162,12 @@
     """
     default = {}
 
+    @classmethod
+    def clean(cls, value):
+        if isinstance(value, dict):
+            return value
+        return json.loads(value)
+
 
 # The following are for 'binary'.
 

=== modified file 'pkgme/package_files.py'
--- pkgme/package_files.py	2011-07-14 16:03:47 +0000
+++ pkgme/package_files.py	2011-07-18 11:33:32 +0000
@@ -94,7 +94,7 @@
         files = []
         for file_cls in self.files_cls:
             files.append(file_cls.from_info(new_info))
-        extra_files = values.get(ExtraFiles.name, {})
+        extra_files = ExtraFiles.get_value(project_info)
         for path, contents in extra_files.items():
             files.append(BasicFile(path, contents))
         return files

=== modified file 'pkgme/tests/test_binary_backend.py'
--- pkgme/tests/test_binary_backend.py	2011-07-15 16:30:18 +0000
+++ pkgme/tests/test_binary_backend.py	2011-07-18 11:33:32 +0000
@@ -49,7 +49,7 @@
         backend = ExternalHelpersBackend(BACKEND_NAME, backend_dir)
         self.assertEqual(0, backend.want(tempdir.path))
 
-    def test_all_info_package_name(self):
+    def test_package_name(self):
         # The binary backend gets the package name from the metadata file.
         tempdir = self.useFixture(TempdirFixture())
         tempdir.create_file(METADATA_FILE, json.dumps({PACKAGE_NAME: 'foo'}))
@@ -58,7 +58,7 @@
         self.assertEqual(
             {PackageName.name: "foo"}, info.get_all([PackageName.name]))
 
-    def test_all_info_build_depends(self):
+    def test_build_depends(self):
         tempdir = self.useFixture(TempdirFixture())
         tempdir.create_file(METADATA_FILE, json.dumps({PACKAGE_NAME: 'foo'}))
         shutil.copy(
@@ -66,18 +66,18 @@
             tempdir.path)
         backend = ExternalHelpersBackend(BACKEND_NAME, backend_dir)
         info = backend.get_info(tempdir.path)
-        deps = list(guess_dependencies(tempdir.path))
+        deps = ', '.join(guess_dependencies(tempdir.path))
         self.assertEqual(
             {BuildDepends.name: deps}, info.get_all([BuildDepends.name]))
 
-    def test_all_info_depends(self):
+    def test_depends(self):
         tempdir = self.useFixture(TempdirFixture())
         tempdir.create_file(METADATA_FILE, json.dumps({PACKAGE_NAME: 'foo'}))
         backend = ExternalHelpersBackend(BACKEND_NAME, backend_dir)
         info = backend.get_info(tempdir.path)
         self.assertEqual({Depends.name: DEPENDS}, info.get_all([Depends.name]))
 
-    def test_all_info_extra_files_install_file(self):
+    def test_extra_files_install_file(self):
         # We create an 'install' file that tells debhelper to just copy
         # everything to opt.
         tempdir = self.useFixture(TempdirFixture())
@@ -86,12 +86,12 @@
         backend = ExternalHelpersBackend(BACKEND_NAME, backend_dir)
         info = backend.get_info(tempdir.path)
         extra_files = info.get_all([ExtraFiles.name])[ExtraFiles.name]
-        install_file = extra_files['debian/install']
+        install_file = json.loads(extra_files)['debian/install']
         self.assertEqual(
             get_install_file(package_name, tempdir.path, True),
             install_file)
 
-    def test_all_info_extra_files_desktop_file(self):
+    def test_extra_files_desktop_file(self):
         # We create an 'install' file that tells debhelper to just copy
         # everything to opt.
         tempdir = self.useFixture(TempdirFixture())
@@ -113,5 +113,5 @@
         backend = ExternalHelpersBackend(BACKEND_NAME, backend_dir)
         info = backend.get_info(tempdir.path)
         extra_files = info.get_all([ExtraFiles.name])[ExtraFiles.name]
-        desktop = extra_files['debian/%s.desktop' % (package_name,)]
+        desktop = json.loads(extra_files)['debian/%s.desktop' % (package_name,)]
         self.assertEqual(expected_desktop_file, desktop)

=== modified file 'pkgme/tests/test_info_elements.py'
--- pkgme/tests/test_info_elements.py	2011-07-15 16:04:55 +0000
+++ pkgme/tests/test_info_elements.py	2011-07-18 11:33:32 +0000
@@ -1,7 +1,10 @@
+import json
+
 from testtools import TestCase
 
 from pkgme.info_elements import (
     BuildDepends,
+    ExtraFiles,
     InfoElement,
     MissingInfoError,
     )
@@ -105,3 +108,16 @@
         # `all_info`.
         value = BuildDepends.clean(["python", "gtk2"])
         self.assertEqual("python, gtk2, debhelper (>= 7)", value)
+
+
+class ExtraFilesTestCase(TestCase):
+
+    def test_clean_preserves_dict(self):
+        in_value = {'foo': 'bar'}
+        out_value = ExtraFiles.clean(in_value)
+        self.assertEqual(out_value, in_value)
+
+    def test_clean_loads_json(self):
+        in_value = {'foo': 'bar'}
+        out_value = ExtraFiles.clean(json.dumps(in_value))
+        self.assertEqual(out_value, in_value)

=== modified file 'pkgme/tests/test_package_files.py'
--- pkgme/tests/test_package_files.py	2011-07-14 16:03:47 +0000
+++ pkgme/tests/test_package_files.py	2011-07-18 11:33:32 +0000
@@ -1,3 +1,4 @@
+import json
 import os
 
 from debian import changelog
@@ -470,3 +471,16 @@
         self.assertEqual('foo.txt', foo.path)
         self.assertEqual("Hello world!\n", foo.get_contents())
         self.assertEqual(True, foo.overwrite)
+
+    def test_extra_files_single_info(self):
+        group = PackageFileGroup()
+        project_info = DictInfo({
+            ExtraFiles.name: json.dumps({
+                'foo.txt': "Hello world!\n",
+                'debian/install': "Bwawahahaha\n",
+                })})
+        files = group.get_files(project_info)
+        [foo, install] = files
+        self.assertEqual('foo.txt', foo.path)
+        self.assertEqual("Hello world!\n", foo.get_contents())
+        self.assertEqual(True, foo.overwrite)

=== modified file 'pkgme/tests/test_project_info.py'
--- pkgme/tests/test_project_info.py	2011-07-15 09:22:47 +0000
+++ pkgme/tests/test_project_info.py	2011-07-18 11:33:32 +0000
@@ -1,5 +1,4 @@
 import json
-import os
 import sys
 
 from fixtures import TestWithFixtures