Merge lp:~lifeless/bzr-builddeb/trunk into lp:bzr-builddeb

[Robert Collins]

Candidate (missing tests) fix for bug lp:588060.

This fixes the unversioned executability issue for me, but I haven't
written a unit test yet. Merging this as-is may be a step forward, though
a test is obviously preferrable, I'm not sure when I'll have tuits to get
around to that.

[lp:~lifeless/bzr-builddeb/trunk]

463. By Robert Collins on 2010-07-06

Happy path tests R us. bzrtools_import file id resolution fix tested.

[Robert Collins (lifeless)]

This now has a happy path test. \o/

[James Westby (james-w)]

Merged with a tweak to the tests to use a lower level API for setup.

Thanks,

James

=== modified file 'bzrtools_import.py'
--- bzrtools_import.py	2010-05-28 00:36:04 +0000
+++ bzrtools_import.py	2010-07-06 05:35:40 +0000
@@ -197,12 +197,20 @@
     import_archive(tree, dir_file, file_ids_from=file_ids_from)
 
 def import_archive(tree, archive_file, file_ids_from=None):
+    if file_ids_from is None:
+        file_ids_from = []
+    for other_tree in file_ids_from:
+        other_tree.lock_read()
+    try:
+        return _import_archive(tree, archive_file, file_ids_from)
+    finally:
+        for other_tree in file_ids_from:
+            other_tree.unlock()
+
+
+def _import_archive(tree, archive_file, file_ids_from):
     prefix = common_directory(names_of_files(archive_file))
     tt = TreeTransform(tree)
-
-    if file_ids_from is None:
-        file_ids_from = []
-
     removed = set()
     for path, entry in tree.inventory.iter_entries():
         if entry.parent_id is None:
@@ -231,6 +239,26 @@
         add_implied_parents(implied_parents, relative_path)
         trans_id = tt.trans_id_tree_path(relative_path)
         added.add(relative_path.rstrip('/'))
+        # To handle renames, we need to not use the preserved file id, rather
+        # we need to lookup the file id in a from_tree, if there is one. If
+        # there isn't, we should use the one in the current tree, and failing
+        # that we will allocate one. In this importer we want the file_ids_from
+        # tree to authoritative about id2path, which is why we consult it
+        # first.
+        existing_file_id = tt.tree_file_id(trans_id)
+        for other_tree in file_ids_from:
+            found_file_id = other_tree.path2id(relative_path)
+            if found_file_id:
+                if found_file_id != existing_file_id:
+                    # Found a specific file id in one of the source trees
+                    tt.version_file(found_file_id, trans_id)
+                break
+        if not found_file_id and not existing_file_id:
+            # No file_id in any of the source trees and no file id in the base
+            # tree.
+            name = basename(member.name.rstrip('/'))
+            file_id = generate_ids.gen_file_id(name)
+            tt.version_file(file_id, trans_id)
         path = tree.abspath(relative_path)
         if member.name in seen:
             if tt.final_kind(trans_id) == 'file':
@@ -248,23 +276,6 @@
             tt.create_symlink(member.linkname, trans_id)
         else:
             raise UnknownType(relative_path)
-        if tt.tree_file_id(trans_id) is None:
-            found = False
-            for other_tree in file_ids_from:
-                other_tree.lock_read()
-                try:
-                    if other_tree.has_filename(relative_path):
-                        file_id = other_tree.path2id(relative_path)
-                        if file_id is not None:
-                            tt.version_file(file_id, trans_id)
-                            found = True
-                            break
-                finally:
-                    other_tree.unlock()
-            if not found:
-                name = basename(member.name.rstrip('/'))
-                file_id = generate_ids.gen_file_id(name)
-                tt.version_file(file_id, trans_id)
 
     for relative_path in implied_parents.difference(added):
         if relative_path == "":

=== modified file 'debian/changelog'
--- debian/changelog	2010-05-31 17:33:49 +0000
+++ debian/changelog	2010-07-06 05:35:40 +0000
@@ -1,3 +1,10 @@
+bzr-builddeb (2.5lifeless1) UNRELEASED; urgency=low
+
+  * (No tests yet) treat the upstream branch as authoritative for file ids -
+    causes renamed to be honoured. (LP: #588060)
+
+ -- Robert Collins <[e-mail]>  Sun, 04 Jul 2010 20:29:17 +1000
+
 bzr-builddeb (2.5) UNRELEASED; urgency=low
 
   [ Colin Watson ]

=== modified file 'dh_make.py'
--- dh_make.py	2010-05-05 07:20:17 +0000
+++ dh_make.py	2010-07-06 05:35:40 +0000
@@ -108,8 +108,17 @@
             tree.add("debian/source/format")
     command = ["dh_make", "--addmissing", "--packagename",
                 "%s_%s" % (package_name, version)]
+    if getattr(sys.stdin, 'fileno', None) is None:
+        # running in a test or something
+        stdin = subprocess.PIPE
+        input = "s\n\n"
+    else:
+        stdin = sys.stdin
+        input = None
     proc = subprocess.Popen(command, cwd=tree.basedir,
-            preexec_fn=util.subprocess_setup, stdin=sys.stdin)
+            preexec_fn=util.subprocess_setup, stdin=stdin)
+    if input:
+        proc.stdin.write(input)
     retcode = proc.wait()
     if retcode != 0:
         raise bzr_errors.BzrCommandError("dh_make failed.")

=== modified file 'tests/blackbox/test_merge_upstream.py'
--- tests/blackbox/test_merge_upstream.py	2008-03-05 17:00:51 +0000
+++ tests/blackbox/test_merge_upstream.py	2010-07-06 05:35:40 +0000
@@ -18,12 +18,141 @@
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 #
 
-from bzrlib.plugins.builddeb.tests import BuilddebTestCase
+import os.path
+
+import bzrlib.export
+
+from bzrlib.plugins.builddeb.tests import (
+    BuilddebTestCase,
+    SourcePackageBuilder,
+    )
+
+
+class Fixture(object):
+    """A test fixture."""
+
+    def __init__(self):
+        pass
+
+    def setUp(self, test_case):
+        test_case.addCleanup(self.tearDown)
+
+    def tearDown(self):
+        pass
+
+
+class Upstream(Fixture):
+    """An upstream.
+    
+    :ivar tree: The tree of the upstream.
+    """
+
+    def setUp(self, test_case):
+        Fixture.setUp(self, test_case)
+        treename = test_case.getUniqueString()
+        tree = test_case.make_branch_and_tree(treename)
+        filename = test_case.getUniqueString()
+        test_case.build_tree(["%s/%s" % (treename, filename)])
+        tree.add([filename])
+        tree.commit(test_case.getUniqueString())
+        self.tree = tree
+
+
+class ExportedTarball(Fixture):
+    """An exported tarball 'release'."""
+
+    def __init__(self, upstream, version):
+        self.upstream = upstream
+        self.version = version
+
+    def setUp(self, test_case):
+        filename = "project-%s.tar.gz" % self.version
+        tree = self.upstream.tree.branch.repository.revision_tree(
+            self.upstream.tree.branch.last_revision())
+        bzrlib.export.export(tree, filename)
+        self.tarball = filename
+
+
+class DHMadePackage(Fixture):
+    """A package made via dh-make."""
+
+    def __init__(self, tar, upstream):
+        self.tar = tar
+        self.upstream = upstream
+
+    def setUp(self, test_case):
+        branchpath = test_case.getUniqueString()
+        tree = self.upstream.tree.bzrdir.sprout(branchpath).open_workingtree()
+        # UGH: spews, because the subprocess output isn't captured. Be nicer to
+        # use closer to the metal functions here, but I'm overwhelmed by the
+        # API today, and there is a grue.
+        test_case.run_bzr(['dh-make', 'package', str(self.tar.version),
+            os.path.abspath(self.tar.tarball)], working_dir=branchpath)
+        tree.smart_add([tree.basedir])
+        tree.commit('debianised.') # Honestly.
+        self.tree = tree
+
+
+class FileMovedReplacedUpstream(Fixture):
+    """An upstream that has been changed by moving and replacing a file."""
+
+    def __init__(self, upstream):
+        self.upstream = upstream
+
+    def setUp(self, test_case):
+        branchpath = test_case.getUniqueString()
+        tree = self.upstream.tree.bzrdir.sprout(branchpath).open_workingtree()
+        self.tree = tree
+        tree.lock_write()
+        try:
+            newpath = test_case.getUniqueString()
+            for child in tree.inventory.root.children.values():
+                if child.kind == 'file':
+                    oldpath = child.name
+            tree.rename_one(oldpath, newpath)
+            test_case.build_tree(["%s/%s" % (os.path.basename(tree.basedir),
+                oldpath)])
+            tree.add([oldpath])
+            tree.commit('yo, renaming and replacing')
+        finally:
+            tree.unlock()
+
 
 
 class TestMergeUpstream(BuilddebTestCase):
 
-  def test_merge_upstream_available(self):
-    self.run_bzr('merge-upstream --help')
-
-# vim: ts=2 sts=2 sw=2
+    def test_merge_upstream_available(self):
+        self.run_bzr('merge-upstream --help')
+
+    def make_upstream(self):
+        result = Upstream()
+        result.setUp(self)
+        return result
+
+    def release_upstream(self, upstream):
+        tar = ExportedTarball(upstream, version=self.getUniqueInteger())
+        tar.setUp(self)
+        return tar
+
+    def import_upstream(self, tar, upstream):
+        packaging = DHMadePackage(tar, upstream)
+        packaging.setUp(self)
+        return packaging
+        
+    def file_moved_replaced_upstream(self, upstream):
+        result = FileMovedReplacedUpstream(upstream)
+        result.setUp(self)
+        return result
+
+    def test_smoke_renamed_file(self):
+        # When a file is renamed by upstream, it should still import ok.
+        upstream = self.make_upstream()
+        rel1 = self.release_upstream(upstream)
+        package = self.import_upstream(rel1, upstream)
+        changed_upstream = self.file_moved_replaced_upstream(upstream)
+        rel2 = self.release_upstream(changed_upstream)
+        self.run_bzr(['merge-upstream', '--version', str(rel2.version),
+            os.path.abspath(rel2.tarball), changed_upstream.tree.basedir],
+            working_dir=package.tree.basedir)
+
+# vim: ts=4 sts=4 sw=4