Merge lp:~jml/pkgme/clean-package-name into lp:pkgme

[Jonathan Lange]

Enforce Debian package name policy in PackageName.

Encodes http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Source into the PackageName info element.

This takes some of the burden that lp:pkgme-binary has in guessing the package name when it's not provided. The general method is to remove offending characters and produce a clean result if we can, only raising an exception when it becomes impossible to clean by removing (i.e. when the package name is too short).

[James Westby (james-w)]

Hi,

This looks good.

I'm not entirely sure that dropping invalid chars rather than
substituting them or erroring is the right thing, but I'm not
entirely unsure either.

Thanks,

James

review: Approve

[Jonathan Lange (jml)]

Likewise. I guess this is part & parcel of doing guesswork.

=== modified file 'pkgme/info_elements.py'
--- pkgme/info_elements.py	2012-01-10 15:02:34 +0000
+++ pkgme/info_elements.py	2012-02-04 18:33:20 +0000
@@ -1,5 +1,9 @@
 import json
-
+import re
+
+
+class InvalidInfoError(Exception):
+    pass
 
 class MissingInfoError(Exception):
     pass
@@ -65,6 +69,23 @@
     description = "The name of the package."
     required = True
 
+    @classmethod
+    def clean(cls, value):
+        # Package names (both source and binary, see Package, Section 5.6.7)
+        # must consist only of lower case letters (a-z), digits (0-9), plus
+        # (+) and minus (-) signs, and periods (.). They must be at least two
+        # characters long and must start with an alphanumeric character.
+        #
+        # http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Source
+        cleaned = value.lower()
+        cleaned = re.sub(r'[^a-z0-9+.-]', '', cleaned)
+        cleaned = cleaned.lstrip('.+-')
+        if len(cleaned) < 2:
+            raise InvalidInfoError(
+                "Package name must be at least two characters long: %r"
+                % (value,))
+        return cleaned
+
 
 class Section(InfoElement):
 

=== modified file 'pkgme/tests/test_info_elements.py'
--- pkgme/tests/test_info_elements.py	2012-01-10 15:02:34 +0000
+++ pkgme/tests/test_info_elements.py	2012-02-04 18:33:20 +0000
@@ -9,7 +9,9 @@
     ExtraFiles,
     ExtraFilesFromPaths,
     InfoElement,
+    InvalidInfoError,
     MissingInfoError,
+    PackageName,
     )
 from pkgme.project_info import DictInfo
 
@@ -261,3 +263,39 @@
         self.assertEqual("""initial synopsis
  follow-up line
  more information""", cleaned)
+
+
+class TestPackageName(TestCase):
+
+    def test_valid(self):
+        cleaned = PackageName.clean('foo')
+        self.assertEqual('foo', cleaned)
+
+    def test_capitals(self):
+        cleaned = PackageName.clean('Foo')
+        self.assertEqual('foo', cleaned)
+
+    def test_too_short(self):
+        e = self.assertRaises(InvalidInfoError, PackageName.clean, 'f')
+        self.assertEqual(
+            "Package name must be at least two characters long: 'f'", str(e))
+
+    def test_invalid_characters(self):
+        cleaned = PackageName.clean('foo&ba@r')
+        self.assertEqual('foobar', cleaned)
+
+    def test_too_short_after_stripping(self):
+        e = self.assertRaises(InvalidInfoError, PackageName.clean, 'f&&&')
+        self.assertEqual(
+            "Package name must be at least two characters long: 'f&&&'",
+            str(e))
+        e = self.assertRaises(InvalidInfoError, PackageName.clean, '+++f')
+        self.assertEqual(
+            "Package name must be at least two characters long: '+++f'",
+            str(e))
+
+    def test_invalid_start_character(self):
+        self.assertEqual('foobar', PackageName.clean('+foobar'))
+        self.assertEqual('foobar', PackageName.clean('-foobar'))
+        self.assertEqual('foobar', PackageName.clean('.foobar'))
+

=== modified file 'pkgme/tests/test_package_files.py'
--- pkgme/tests/test_package_files.py	2011-12-22 22:05:20 +0000
+++ pkgme/tests/test_package_files.py	2012-02-04 18:33:20 +0000
@@ -78,6 +78,9 @@
             args.update(other_args)
         return self.cls.from_info(DictInfo(args))
 
+    def get_package_name(self):
+        return PackageName.clean(self.getUniqueString())
+
 
 class BasicFileTests(TestCase):
 
@@ -281,7 +284,7 @@
         self.assertEqual(True, package_file.overwrite)
 
     def test_get_contents_sets_package_name(self):
-        package_name = self.getUniqueString()
+        package_name = self.get_package_name()
         args = {PackageName.name: package_name}
         package_file = self.get_object(args)
         self.assertThat(
@@ -289,7 +292,7 @@
             ControlSourceStanzaHasField("Source", package_name))
 
     def test_get_contents_sets_binary_package_name(self):
-        package_name = self.getUniqueString()
+        package_name = self.get_package_name()
         args = {PackageName.name: package_name}
         package_file = self.get_object(args)
         self.assertThat(
@@ -399,7 +402,7 @@
             }
 
     def test_path(self):
-        package_name = self.getUniqueString()
+        package_name = self.get_package_name()
         package_file = self.get_object({PackageName.name: package_name})
         self.assertEqual('%s.desktop' % package_name, package_file.path)
 

=== modified file 'pkgme/tests/test_write_packaging.py'
--- pkgme/tests/test_write_packaging.py	2011-11-01 18:51:24 +0000
+++ pkgme/tests/test_write_packaging.py	2012-02-04 18:33:20 +0000
@@ -43,7 +43,7 @@
         return info
 
     def test_write_packaging(self):
-        name = self.getUniqueString()
+        name = PackageName.clean(self.getUniqueString())
         info = self.make_info(name)
         tempdir = self.useFixture(TempdirFixture())
         backend = StaticBackend(
@@ -53,7 +53,7 @@
         self.assert_control_has_source_name(name, tempdir)
 
     def test_write_packaging_passes_allowed_backend_names(self):
-        name = self.getUniqueString()
+        name = PackageName.clean(self.getUniqueString())
         other_name = name+"WRONG"
         info1 = self.make_info(name)
         info2 = self.make_info(other_name)