Merge lp:~cjwatson/cupstream2distro/fix-symbols-replacement into lp:cupstream2distro

[Colin Watson]

Don't try to substitute 0replaceme in directories or symlinks.

r629 broke building any package containing a directory under debian/, such as debian/source/ (so including but not limited to all 3.0 (quilt) packages). See https://ci-train.ubuntu.com/job/landing-009-1-build/119/console for an example.

Make sure only to attempt substitutions in regular files, not directories or symlinks.

[Colin Watson (cjwatson)]

Incidentally, I would have written a unit test for this, but the relevant tests (tests.unit.test_packagemanager.PackageManagerOfflineTests) currently fail in flames. Perhaps somebody could fix things up a bit there ...

[lp:~cjwatson/cupstream2distro/fix-symbols-replacement]

631. By Colin Watson on 2014-07-20

Test the directories-under-debian/ case.

632. By Colin Watson on 2014-07-20

Make test actually be meaningful.

[Colin Watson (cjwatson)]

Oh look, turns out I can write a test after all, though the other ones still need fixing.

[lp:~cjwatson/cupstream2distro/fix-symbols-replacement]

633. By Colin Watson on 2014-07-20

Avoid applying replacements to symlinks too.

634. By Colin Watson on 2014-07-20

Tweak test name.

[John Lenton (chipaca)]

LGTM, FWIW :-)

review: Approve

[Robert Bruce Park (robru)]

lgtm, deploying!

review: Approve

=== modified file 'cupstream2distro/packagemanager.py'
--- cupstream2distro/packagemanager.py	2014-07-18 22:11:43 +0000
+++ cupstream2distro/packagemanager.py	2014-07-20 12:27:50 +0000
@@ -523,7 +523,10 @@
     new_upstream_version = packaging_version.split("-")[0]
     files_replaced = set()
     for filename in os.listdir("debian"):
-        for line in fileinput.input(os.path.join('debian', filename), inplace=1):
+        path = os.path.join('debian', filename)
+        if not os.path.isfile(path) or os.path.islink(path):
+            continue
+        for line in fileinput.input(path, inplace=1):
             if settings.REPLACEME_TAG in line:
                 files_replaced.add(filename)
                 line = line.replace(settings.REPLACEME_TAG, new_upstream_version)

=== modified file 'tests/unit/test_packagemanager.py'
--- tests/unit/test_packagemanager.py	2014-06-12 16:49:00 +0000
+++ tests/unit/test_packagemanager.py	2014-07-20 12:27:50 +0000
@@ -954,6 +954,23 @@
             subprocess.Popen(['bzr', 'revno'],
                              stdout=subprocess.PIPE).communicate()[0], "8\n")
 
+    def test_dont_refresh_symbols_files_for_directory(self):
+        '''We don't fail for directories under debian/'''
+        self.get_data_branch('dummypackage')
+        debian_source_path = os.path.join("debian", "source")
+        self.assertTrue(os.path.isdir(debian_source_path))
+        packagemanager.refresh_symbol_files('42.0daily83.09.14-0ubuntu1')
+        self.assertTrue(os.path.isdir(debian_source_path))
+
+    def test_dont_refresh_symbols_files_for_symlink(self):
+        '''We don't touch symlinks under debian/'''
+        self.get_data_branch('basic_symbols')
+        symlink_path = os.path.join("debian", "foo.symbols.link")
+        os.symlink("foo.symbols", symlink_path)
+        packagemanager.refresh_symbol_files('42.0daily83.09.14-0ubuntu1')
+        self.assertTrue(os.path.islink(symlink_path))
+        self.assertEqual("foo.symbols", os.readlink(symlink_path))
+
 
 class PackageManagerOnlineTests(BaseUnitTestCase):
     '''Test that uses online services, but as we just pull from them, we can use them'''