python-debian, wrap-and-sort, paragraphs separators and comments

[Stuart Prescott]

Hi John,

thanks for your feedback on this patch series too. I've dealt with all your 
comments except the two below and updated the git repo accordingly.

[ pasting the diffs in afresh since successive quoting makes a mess of it ]


                with open_utf8(filename) as fh:
+                    count = len(list(deb822.Packages.iter_paragraphs(fh)))
+                    # this time the apt_pkg parser should be used and this
+                    # *should* elide the paragraphs and make a mess
+                    self.assertEqual(count, 1,
+                            "Wrong number paragraphs were found in file: 1 != 
%d" % count)


> Does it really make sense to test for this behavior?


     def test_iter_paragraphs_comments_use_apt_pkg(self):
-        paragraphs = list(deb822.Deb822.iter_paragraphs(
-            UNPARSED_PARAGRAPHS_WITH_COMMENTS.splitlines(), 
use_apt_pkg=True))
-        self._test_iter_paragraphs_comments(paragraphs)
+        """ apt_pkg does not support comments within multiline fields
+
+        This test actually checks that that the file is *incorrectly* parsed
+        to ensure that this behaviour doesn't unknowingly and accidentally
+        change in the future.
+
+        See also https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=750247#35
+        """
+        fd, filename = tempfile.mkstemp()
+        fp = os.fdopen(fd, 'wb')
+        fp.write(UNPARSED_PARAGRAPHS_WITH_COMMENTS.encode('utf-8'))
+        fp.close()
+
+        try:
+            with open_utf8(filename) as fh:
+                paragraphs = list(deb822.Deb822.iter_paragraphs(
+                    fh, use_apt_pkg=True))
+                self.assertEqual(paragraphs[0]['Build-Depends'], 
'debhelper,')
+        finally:
+            os.remove(filename)


> As above, I don't quite understand the point of this.  What action would
> we reasonably take if apt_pkg changed its behavior to be more
> permissive?


I share your concern with these bits of code. I actually started by testing 
that TagFile was *correctly* parsing the file with comments when I was 
investigating this issue and I guess to an extent the tests were then adjusted 
to reflect reality. I figured leaving them in was useful mostly as a counter-
example of how *not* to use iter_paragraphs and what will happen if you do. 
The test does ensure that the remaining paragraphs are identified correctly and 
perhaps that is useful information in itself. Were TagFile to change (highly 
unlikely I'd say), then we would be able to switch the default implementation 
back to it.

I don't know whether this really makes sense to do or not.

cheers
Stuart


-- 
Stuart Prescott    http://www.nanonanonano.net/   stuart at nanonanonano.net
Debian Developer   http://www.debian.org/         stuart at debian.org
GPG fingerprint    90E2 D2C1 AD14 6A1B 7EBB 891D BBC1 7EBB 1396 F2F7