bzr-builddeb

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

  1. odd-behaviour-385667
  2. Merge into trunk-old
Proposed by Muharem Hrnjadovic
Status: Rejected
Rejected by: Jelmer Vernooij
Proposed branch: lp://staging/~al-maisan/bzr-builddeb/odd-behaviour-385667
Merge into: lp://staging/~bzr-builddeb-hackers/bzr-builddeb/trunk-old
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~al-maisan/bzr-builddeb/odd-behaviour-385667
Reviewer Review Type Date Requested Status
James Westby Needs Resubmitting
Review via email: mp+12111@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

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://staging/~al-maisan/bzr-builddeb/odd-behaviour-385667 updated
385. By Muharem Hrnjadovic

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

Revision history for this message
James Westby (james-w) wrote :

> 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

Revision history for this message
James Westby (james-w) :
review: Needs Resubmitting
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

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

Unmerged revisions

385. By Muharem Hrnjadovic

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

384. By Muharem Hrnjadovic

Cosmetic fixes.

383. By Muharem Hrnjadovic

Make sure upstream revisions are correct.

382. By Muharem Hrnjadovic

Moved blackbox test to appropriate file.

381. By Muharem Hrnjadovic

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

380. By Muharem Hrnjadovic

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmds.py'
2--- cmds.py 2009-09-16 12:47:18 +0000
3+++ cmds.py 2009-09-19 05:48:51 +0000
4@@ -483,14 +483,30 @@
5 def _update_changelog(self, tree, version, distribution_name, changelog,
6 package):
7 from bzrlib.plugins.builddeb.merge_upstream import package_version
8+
9 if "~bzr" in str(version) or "+bzr" in str(version):
10 entry_description = "New upstream snapshot."
11 else:
12 entry_description = "New upstream release."
13- proc = subprocess.Popen(["/usr/bin/dch", "-v",
14- str(package_version(version, distribution_name)),
15- "-D", "UNRELEASED", "--release-heuristic", "changelog",
16- entry_description], cwd=tree.basedir)
17+
18+ # Handle the case where we merge into a branch without a
19+ # "debian/changelog" file.
20+ debian_dir = "%s/debian" % tree.basedir
21+ dch_args = [
22+ "/usr/bin/dch", "-D", "UNRELEASED",
23+ "-v", str(package_version(version, distribution_name)),
24+ ]
25+ if os.path.exists('%s/changelog' % debian_dir):
26+ # changelog exists.
27+ dch_args.extend(["--release-heuristic", "changelog"])
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])
33+
34+ dch_args.append(entry_description)
35+ proc = subprocess.Popen(dch_args, cwd=tree.basedir)
36 proc.wait()
37 if proc.returncode != 0:
38 raise BzrCommandError('Adding a new changelog stanza after the '
39
40=== modified file 'import_dsc.py'
41--- import_dsc.py 2009-09-16 14:52:14 +0000
42+++ import_dsc.py 2009-09-18 11:23:37 +0000
43@@ -1617,8 +1617,8 @@
44 "Should use self.upstream_branch if set"
45 tempdir = tempfile.mkdtemp(dir=os.path.join(self.tree.basedir, '..'))
46 try:
47- previous_upstream_revision = get_snapshot_revision(previous_version.upstream_version)
48 if previous_version is not None:
49+ previous_upstream_revision = get_snapshot_revision(previous_version.upstream_version)
50 if self.has_upstream_version_in_packaging_branch(
51 previous_version.upstream_version):
52 upstream_tip = self.revid_of_upstream_version_from_branch(
53
54=== modified file 'tests/blackbox/test_merge_upstream.py'
55--- tests/blackbox/test_merge_upstream.py 2008-03-05 17:00:51 +0000
56+++ tests/blackbox/test_merge_upstream.py 2009-09-19 05:43:24 +0000
57@@ -18,12 +18,43 @@
58 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
59 #
60
61+import os
62+import tarfile
63+
64 from bzrlib.plugins.builddeb.tests import BuilddebTestCase
65
66
67 class TestMergeUpstream(BuilddebTestCase):
68
69+ upstream_dir = property(lambda self:
70+ self.package_name + '-' + self.upstream_version)
71+ upstream_tarball_name = property(lambda self:
72+ self.package_name + '_' + self.upstream_version + '.orig.tar.gz')
73+
74+ def make_unpacked_upstream_source(self):
75+ os.mkdir(self.upstream_dir)
76+ files = ['README']
77+ self.build_tree([os.path.join(self.upstream_dir, filename)
78+ for filename in files])
79+
80+ def make_upstream_tarball(self):
81+ self.make_unpacked_upstream_source()
82+ tar = tarfile.open(self.upstream_tarball_name, 'w:gz')
83+ try:
84+ tar.add(self.upstream_dir)
85+ finally:
86+ tar.close()
87+
88 def test_merge_upstream_available(self):
89 self.run_bzr('merge-upstream --help')
90
91+ def test_merge_upstream_into_empty_branch(self):
92+ self.make_branch_and_tree('.')
93+ self.make_upstream_tarball()
94+ cmd = (
95+ 'merge-upstream %s --package %s --version %s' %
96+ (self.upstream_tarball_name, self.package_name, self.upstream_version))
97+ self.run_bzr(cmd)
98+
99+
100 # vim: ts=2 sts=2 sw=2
101
102=== modified file 'tests/test_merge_upstream.py'
103--- tests/test_merge_upstream.py 2009-03-10 01:57:05 +0000
104+++ tests/test_merge_upstream.py 2009-09-19 05:48:51 +0000
105@@ -18,11 +18,18 @@
106 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
107 #
108
109+import os
110+import tarfile
111+
112 from debian_bundle.changelog import Version
113
114 from bzrlib.revision import Revision
115 from bzrlib.tests import TestCase, TestCaseWithTransport
116
117+from bzrlib.plugins.builddeb.cmds import cmd_merge_upstream
118+from bzrlib.plugins.builddeb.errors import AddChangelogError
119+from bzrlib.plugins.builddeb.util import find_changelog
120+
121 from bzrlib.plugins.builddeb.merge_upstream import (
122 package_version,
123 _upstream_branch_version,
124@@ -80,10 +87,31 @@
125 upstream_version_add_revision(self, "1.3~svn800", "somesvnrev"))
126
127
128-class TestUpstreamBranchVersion(TestCase):
129+class TestUpstreamBranchVersion(TestCaseWithTransport):
130 """Test that the upstream version of a branch can be determined correctly.
131 """
132
133+ package_name = 'test'
134+ upstream_version = '0.1'
135+ upstream_dir = property(lambda self:
136+ self.package_name + '-' + self.upstream_version)
137+ upstream_tarball_name = property(lambda self:
138+ self.package_name + '_' + self.upstream_version + '.orig.tar.gz')
139+
140+ def make_unpacked_upstream_source(self):
141+ os.mkdir(self.upstream_dir)
142+ files = ['README']
143+ self.build_tree([os.path.join(self.upstream_dir, filename)
144+ for filename in files])
145+
146+ def make_upstream_tarball(self):
147+ self.make_unpacked_upstream_source()
148+ tar = tarfile.open(self.upstream_tarball_name, 'w:gz')
149+ try:
150+ tar.add(self.upstream_dir)
151+ finally:
152+ tar.close()
153+
154 def get_suffix(self, version_string, revid):
155 revno = self.revhistory.index(revid)+1
156 if "bzr" in version_string:
157@@ -122,6 +150,32 @@
158 _upstream_branch_version(self.revhistory,
159 {"somerevid": ["1.3"]}, "bla", "1.2+bzr1", self.get_suffix))
160
161+ def test_merge_upstream_in_empty_branch(self):
162+ tree = self.make_branch_and_tree('empty-branch')
163+ self.make_upstream_tarball()
164+ cmd = cmd_merge_upstream()
165+ workdir = '%s/work' % self.test_base_dir
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)
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)
173+
174+ # Commit the tree and try again.
175+ tree.lock_write()
176+ try:
177+ tree.add('debian')
178+ tree.add('debian/changelog')
179+ tree.commit('merge-upstream into empty branch')
180+ finally:
181+ tree.unlock()
182+ changelog, _ignore = find_changelog(tree, False)
183+ # Make sure we're getting the correct upstream version.
184+ self.assertEquals(
185+ changelog.version.upstream_version, self.upstream_version)
186+
187
188 class TestUpstreamTagToVersion(TestCase):
189

Subscribers

People subscribed via source and target branches