Merge lp:~al-maisan/bzr-builddeb/odd-behaviour-385667 into lp:~bzr-builddeb-hackers/bzr-builddeb/trunk-old

[Muharem Hrnjadovic]

[Muharem Hrnjadovic (al-maisan)]

Hello there!

This branch fixes

  - the glitch described in
    https://bugs.launchpad.net/bzr-builddeb/+bug/385667/comments/1 and
  - the problem with creating debian changelog files in empty branches

The other problems described in bug #385667 either do not occur any more
(negative upstream version numbers) or are not specific to merge-upstream
(deleted .bzrignore files).

Please take a look and let me know what you think. Thanks!

[lp:~al-maisan/bzr-builddeb/odd-behaviour-385667]

385. By Muharem Hrnjadovic on 2009-09-19

We should not be removing .bzrignore, and particularly not in a merge-upstream operation with a tarball and a branch.

[James Westby (james-w)]

> Hello there!
>
> This branch fixes
>
> - the glitch described in
> https://bugs.launchpad.net/bzr-builddeb/+bug/385667/comments/1 and
> - the problem with creating debian changelog files in empty branches

Hi,

Thanks for this.

28 + else:
29 + # changelog not in place yet.
30 + if not os.path.exists(debian_dir):
31 + os.makedirs(debian_dir)
32 + dch_args.extend(["--create", "--package", package])

is “package” guaranteed to not be None in this case?

166 + cmd.run(
167 + directory='%s/empty-branch'%workdir,
168 + location='%s/%s' % (workdir, self.upstream_tarball_name),
169 + package='test', version=self.upstream_version)

We shouldn't be calling cmd.run in the tests.

170 + # The merge-upstream will add a changelog but leave the tree in an
171 + # uncommitted state.
172 + self.assertRaises(AddChangelogError, find_changelog, tree, False)

why are you using this as the test? It doesn't seem to show
your intent with this part of the test, it feels as though you are
testing a side-effect.

Thinking again, should we be creating a changelog? There won't be
the rest of the packaging files, so you can't work with it, but using
something like dh_make will fail. Should we in fact run dh_make?

>
> The other problems described in bug #385667 either do not occur any more
> (negative upstream version numbers)

I don't know why this would have changed?

Thanks,

James

[James Westby (james-w)]

review: Needs Resubmitting

[Jelmer Vernooij (jelmer)]

(an alternative fix for this has landed, so marking this as rejected)

Unmerged revisions

385. By Muharem Hrnjadovic on 2009-09-19

We should not be removing .bzrignore, and particularly not in a merge-upstream operation with a tarball and a branch.

384. By Muharem Hrnjadovic on 2009-09-19

Cosmetic fixes.

383. By Muharem Hrnjadovic on 2009-09-19

Make sure upstream revisions are correct.

382. By Muharem Hrnjadovic on 2009-09-18

Moved blackbox test to appropriate file.

381. By Muharem Hrnjadovic on 2009-09-18

A changelog file is created now when merging into an empty branch.

380. By Muharem Hrnjadovic on 2009-09-18

Fixed glitch described in https://bugs.launchpad.net/bzr-builddeb/+bug/385667/comments/1

=== modified file 'cmds.py'
--- cmds.py	2009-09-16 12:47:18 +0000
+++ cmds.py	2009-09-19 05:48:51 +0000
@@ -483,14 +483,30 @@
     def _update_changelog(self, tree, version, distribution_name, changelog,
             package):
         from bzrlib.plugins.builddeb.merge_upstream import package_version
+
         if "~bzr" in str(version) or "+bzr" in str(version):
             entry_description = "New upstream snapshot."
         else:
             entry_description = "New upstream release."
-        proc = subprocess.Popen(["/usr/bin/dch", "-v",
-                str(package_version(version, distribution_name)),
-                "-D", "UNRELEASED", "--release-heuristic", "changelog",
-                entry_description], cwd=tree.basedir)
+
+        # Handle the case where we merge into a branch without a
+        # "debian/changelog" file.
+        debian_dir = "%s/debian" % tree.basedir
+        dch_args = [
+            "/usr/bin/dch", "-D", "UNRELEASED",
+            "-v", str(package_version(version, distribution_name)),
+            ]
+        if os.path.exists('%s/changelog' % debian_dir):
+            # changelog exists.
+            dch_args.extend(["--release-heuristic", "changelog"])
+        else:
+            # changelog not in place yet.
+            if not os.path.exists(debian_dir):
+                os.makedirs(debian_dir)
+            dch_args.extend(["--create", "--package", package])
+
+        dch_args.append(entry_description)
+        proc = subprocess.Popen(dch_args, cwd=tree.basedir)
         proc.wait()
         if proc.returncode != 0:
             raise BzrCommandError('Adding a new changelog stanza after the '

=== modified file 'import_dsc.py'
--- import_dsc.py	2009-09-16 14:52:14 +0000
+++ import_dsc.py	2009-09-18 11:23:37 +0000
@@ -1617,8 +1617,8 @@
                 "Should use self.upstream_branch if set"
         tempdir = tempfile.mkdtemp(dir=os.path.join(self.tree.basedir, '..'))
         try:
-            previous_upstream_revision = get_snapshot_revision(previous_version.upstream_version)
             if previous_version is not None:
+                previous_upstream_revision = get_snapshot_revision(previous_version.upstream_version)
                 if self.has_upstream_version_in_packaging_branch(
                         previous_version.upstream_version):
                     upstream_tip = self.revid_of_upstream_version_from_branch(

=== 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	2009-09-19 05:43:24 +0000
@@ -18,12 +18,43 @@
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 #
 
+import os
+import tarfile
+
 from bzrlib.plugins.builddeb.tests import BuilddebTestCase
 
 
 class TestMergeUpstream(BuilddebTestCase):
 
+  upstream_dir = property(lambda self:
+      self.package_name + '-' + self.upstream_version)
+  upstream_tarball_name = property(lambda self:
+      self.package_name + '_' + self.upstream_version + '.orig.tar.gz')
+
+  def make_unpacked_upstream_source(self):
+    os.mkdir(self.upstream_dir)
+    files = ['README']
+    self.build_tree([os.path.join(self.upstream_dir, filename)
+                     for filename in files])
+
+  def make_upstream_tarball(self):
+    self.make_unpacked_upstream_source()
+    tar = tarfile.open(self.upstream_tarball_name, 'w:gz')
+    try:
+      tar.add(self.upstream_dir)
+    finally:
+      tar.close()
+
   def test_merge_upstream_available(self):
     self.run_bzr('merge-upstream --help')
 
+  def test_merge_upstream_into_empty_branch(self):
+    self.make_branch_and_tree('.')
+    self.make_upstream_tarball()
+    cmd = (
+      'merge-upstream %s --package %s --version %s' %
+      (self.upstream_tarball_name, self.package_name, self.upstream_version))
+    self.run_bzr(cmd)
+
+
 # vim: ts=2 sts=2 sw=2

=== modified file 'tests/test_merge_upstream.py'
--- tests/test_merge_upstream.py	2009-03-10 01:57:05 +0000
+++ tests/test_merge_upstream.py	2009-09-19 05:48:51 +0000
@@ -18,11 +18,18 @@
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 #
 
+import os
+import tarfile
+
 from debian_bundle.changelog import Version
 
 from bzrlib.revision import Revision
 from bzrlib.tests import TestCase, TestCaseWithTransport
 
+from bzrlib.plugins.builddeb.cmds import cmd_merge_upstream
+from bzrlib.plugins.builddeb.errors import AddChangelogError
+from bzrlib.plugins.builddeb.util import find_changelog
+
 from bzrlib.plugins.builddeb.merge_upstream import (
         package_version,
         _upstream_branch_version,
@@ -80,10 +87,31 @@
         upstream_version_add_revision(self, "1.3~svn800", "somesvnrev"))
 
 
-class TestUpstreamBranchVersion(TestCase):
+class TestUpstreamBranchVersion(TestCaseWithTransport):
   """Test that the upstream version of a branch can be determined correctly.
   """
 
+  package_name = 'test'
+  upstream_version = '0.1'
+  upstream_dir = property(lambda self:
+      self.package_name + '-' + self.upstream_version)
+  upstream_tarball_name = property(lambda self:
+      self.package_name + '_' + self.upstream_version + '.orig.tar.gz')
+
+  def make_unpacked_upstream_source(self):
+    os.mkdir(self.upstream_dir)
+    files = ['README']
+    self.build_tree([os.path.join(self.upstream_dir, filename)
+                     for filename in files])
+
+  def make_upstream_tarball(self):
+    self.make_unpacked_upstream_source()
+    tar = tarfile.open(self.upstream_tarball_name, 'w:gz')
+    try:
+      tar.add(self.upstream_dir)
+    finally:
+      tar.close()
+
   def get_suffix(self, version_string, revid):
     revno = self.revhistory.index(revid)+1
     if "bzr" in version_string:
@@ -122,6 +150,32 @@
         _upstream_branch_version(self.revhistory, 
                                 {"somerevid": ["1.3"]}, "bla", "1.2+bzr1", self.get_suffix))
 
+  def test_merge_upstream_in_empty_branch(self):
+    tree = self.make_branch_and_tree('empty-branch')
+    self.make_upstream_tarball()
+    cmd = cmd_merge_upstream()
+    workdir = '%s/work' % self.test_base_dir
+    cmd.run(
+        directory='%s/empty-branch'%workdir,
+        location='%s/%s' % (workdir, self.upstream_tarball_name),
+        package='test', version=self.upstream_version)
+    # The merge-upstream will add a changelog but leave the tree in an
+    # uncommitted state.
+    self.assertRaises(AddChangelogError, find_changelog, tree, False)
+
+    # Commit the tree and try again.
+    tree.lock_write()
+    try:
+        tree.add('debian')
+        tree.add('debian/changelog')
+        tree.commit('merge-upstream into empty branch')
+    finally:
+        tree.unlock()
+    changelog, _ignore = find_changelog(tree, False)
+    # Make sure we're getting the correct upstream version.
+    self.assertEquals(
+        changelog.version.upstream_version, self.upstream_version)
+
 
 class TestUpstreamTagToVersion(TestCase):