Merge lp:~joe.julian/bzr/bdist_rpm into lp:bzr

Proposed by Joe Julian on 2010-01-10
Status: Merged
Merged at revision: not available
Proposed branch: lp:~joe.julian/bzr/bdist_rpm
Merge into: lp:bzr
Diff against target: 187 lines (+78/-20)
7 files modified
MANIFEST.in (+3/-0)
NEWS (+9/-0)
README_BDIST_RPM (+8/-0)
bzrlib/builtins.py (+19/-15)
bzrlib/tests/blackbox/test_annotate.py (+18/-4)
bzrlib/tests/test_trace.py (+15/-0)
bzrlib/trace.py (+6/-1)
To merge this branch: bzr merge lp:~joe.julian/bzr/bdist_rpm
Reviewer Review Type Date Requested Status
John A Meinel 2010-01-10 Approve on 2010-01-13
Review via email: mp+17085@code.launchpad.net
To post a comment you must log in.
Joe Julian (joe.julian) wrote :

This patch creates a MANIFEST.in file so the include files and tools directory are included in the rpmbuild source. There is also a README_BDIST_RPM explaining about the bug in disttools that prevents a successful build on some distributions and how to fix that bug.

This README can be removed once disttools is repaired and released.

John A Meinel (jameinel) wrote :

The diff doesn't look quite right, as it includes a bunch of things other than just MANIFEST.in and README_BDIST_RPM. And the other things look like changes that Andrew and myself had done...

Anyway, I'm sure the MANIFEST.in needs at least a *.txt, since we use bzrlib/help_topics/en/* as part of the runtime system (eg, bzr help authentication).

Using:
  bzr ls -R bzrlib/ | sed 's/.*\(\..*\)/\1/' | grep -v '/' | sort -u

We have these extensions:
.c
.crt
.csr
.h
.key
.patch
.pxd
.py
.pyc
.pyd
.pyx
.txt

Assuming we don't care about the test suite, the .crt and .csr aren't needed (we also have a few files without extensions used by the test suite). Same for .patch .key. Certainly .pyc isn't needed, and we wouldn't want to bundle .pyd (or .so). So I think the line should be:
recursive-include bzrlib *.py *.pyx *.pxd *.txt *.c *.h

review: Needs Fixing
John A Meinel (jameinel) wrote :

I went ahead and cleaned it up a bit, added a NEWS entry, and submitted it from:
lp:~jameinel/bzr/bdist_rpm

I also submitted this to the 2.0 branch since it seems reasonable and linked it to bug #175839.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'MANIFEST.in'
2--- MANIFEST.in 1970-01-01 00:00:00 +0000
3+++ MANIFEST.in 2010-01-10 02:33:27 +0000
4@@ -0,0 +1,3 @@
5+include bzr README setup.py
6+recursive-include bzrlib *.py *.pyx *.pxd *.c *.h
7+recursive-include tools *.py
8
9=== modified file 'NEWS'
10--- NEWS 2010-01-08 09:27:39 +0000
11+++ NEWS 2010-01-10 02:33:27 +0000
12@@ -177,6 +177,9 @@
13 Bug Fixes
14 *********
15
16+* ``bzr annotate`` on another branch with ``-r branch:...`` no longer
17+ fails with an ``ObjectNotLocked`` error. (Andrew Bennetts, #496590)
18+
19 * ``bzr export dir`` now requests all file content as a record stream,
20 rather than requsting the file content one file-at-a-time. This can make
21 exporting over the network significantly faster (54min => 9min in one
22@@ -195,6 +198,12 @@
23 * Give a clearer message if the lockdir disappears after being apparently
24 successfully taken. (Martin Pool, #498378)
25
26+* If we fail to open ``~/.bzr.log`` write a clear message to stderr rather
27+ than using ``warning()``. The log file is opened before logging is set
28+ up, and it leads to very confusing: 'no handlers for "bzr"' messages for
29+ users, rather than something nicer.
30+ (John Arbash Meinel, Barry Warsaw, #503886)
31+
32 * The 2a format wasn't properly restarting autopacks when something
33 changed underneath it (like another autopack). Now concurrent
34 autopackers will properly succeed. (John Arbash Meinel, #495000)
35
36=== added file 'README_BDIST_RPM'
37--- README_BDIST_RPM 1970-01-01 00:00:00 +0000
38+++ README_BDIST_RPM 2010-01-10 02:33:27 +0000
39@@ -0,0 +1,8 @@
40+There is a bug in disttools for distributions who's rpmbuild compresses
41+the man pages. This causes an error building the final packages as it's
42+expecting bzr.1 and not finding it, but finding bzr.1.gz that's unpackaged.
43+
44+This bug is known to affect Fedora, RHEL, and Centos distributions.
45+
46+There is a preliminary patch at http://bugs.python.org/issue644744 that
47+fixes this issue with disttools.
48
49=== modified file 'bzrlib/builtins.py'
50--- bzrlib/builtins.py 2009-12-23 05:42:33 +0000
51+++ bzrlib/builtins.py 2010-01-10 02:33:27 +0000
52@@ -4537,21 +4537,25 @@
53 branch.lock_read()
54 try:
55 tree = _get_one_revision_tree('annotate', revision, branch=branch)
56- if wt is not None:
57- file_id = wt.path2id(relpath)
58- else:
59- file_id = tree.path2id(relpath)
60- if file_id is None:
61- raise errors.NotVersionedError(filename)
62- file_version = tree.inventory[file_id].revision
63- if wt is not None and revision is None:
64- # If there is a tree and we're not annotating historical
65- # versions, annotate the working tree's content.
66- annotate_file_tree(wt, file_id, self.outf, long, all,
67- show_ids=show_ids)
68- else:
69- annotate_file(branch, file_version, file_id, long, all, self.outf,
70- show_ids=show_ids)
71+ tree.lock_read()
72+ try:
73+ if wt is not None:
74+ file_id = wt.path2id(relpath)
75+ else:
76+ file_id = tree.path2id(relpath)
77+ if file_id is None:
78+ raise errors.NotVersionedError(filename)
79+ file_version = tree.inventory[file_id].revision
80+ if wt is not None and revision is None:
81+ # If there is a tree and we're not annotating historical
82+ # versions, annotate the working tree's content.
83+ annotate_file_tree(wt, file_id, self.outf, long, all,
84+ show_ids=show_ids)
85+ else:
86+ annotate_file(branch, file_version, file_id, long, all,
87+ self.outf, show_ids=show_ids)
88+ finally:
89+ tree.unlock()
90 finally:
91 if wt is not None:
92 wt.unlock()
93
94=== modified file 'bzrlib/tests/blackbox/test_annotate.py'
95--- bzrlib/tests/blackbox/test_annotate.py 2009-03-23 14:59:43 +0000
96+++ bzrlib/tests/blackbox/test_annotate.py 2010-01-10 02:33:27 +0000
97@@ -29,6 +29,7 @@
98 from bzrlib.branch import Branch
99 from bzrlib.config import extract_email_address
100 from bzrlib.tests import TestCaseWithTransport
101+from bzrlib.urlutils import joinpath
102
103
104 class TestAnnotate(TestCaseWithTransport):
105@@ -153,14 +154,27 @@
106 class TestSimpleAnnotate(TestCaseWithTransport):
107 """Annotate tests with no complex setup."""
108
109- def _setup_edited_file(self):
110+ def _setup_edited_file(self, relpath='.'):
111 """Create a tree with a locally edited file."""
112- tree = self.make_branch_and_tree('.')
113- self.build_tree_contents([('file', 'foo\ngam\n')])
114+ tree = self.make_branch_and_tree(relpath)
115+ file_relpath = joinpath(relpath, 'file')
116+ self.build_tree_contents([(file_relpath, 'foo\ngam\n')])
117 tree.add('file')
118 tree.commit('add file', committer="test@host", rev_id="rev1")
119- self.build_tree_contents([('file', 'foo\nbar\ngam\n')])
120+ self.build_tree_contents([(file_relpath, 'foo\nbar\ngam\n')])
121 tree.branch.get_config().set_user_option('email', 'current@host2')
122+ return tree
123+
124+ def test_annotate_cmd_revspec_branch(self):
125+ tree = self._setup_edited_file('trunk')
126+ tree.branch.create_checkout(self.get_url('work'), lightweight=True)
127+ os.chdir('work')
128+ out, err = self.run_bzr('annotate file -r branch:../trunk')
129+ self.assertEqual('', err)
130+ self.assertEqual(
131+ '1 test@ho | foo\n'
132+ ' | gam\n',
133+ out)
134
135 def test_annotate_edited_file(self):
136 tree = self._setup_edited_file()
137
138=== modified file 'bzrlib/tests/test_trace.py'
139--- bzrlib/tests/test_trace.py 2009-12-16 22:29:31 +0000
140+++ bzrlib/tests/test_trace.py 2010-01-10 02:33:27 +0000
141@@ -27,6 +27,7 @@
142
143 from bzrlib import (
144 errors,
145+ trace,
146 )
147 from bzrlib.tests import TestCaseInTempDir, TestCase
148 from bzrlib.trace import (
149@@ -248,6 +249,20 @@
150 tmp1.close()
151 tmp2.close()
152
153+ def test__open_bzr_log_uses_stderr_for_failures(self):
154+ # If _open_bzr_log cannot open the file, then we should write the
155+ # warning to stderr. Since this is normally happening before logging is
156+ # set up.
157+ self.addCleanup(setattr, sys, 'stderr', sys.stderr)
158+ self.addCleanup(setattr, trace, '_bzr_log_filename',
159+ trace._bzr_log_filename)
160+ sys.stderr = StringIO()
161+ # Set the log file to something that cannot exist
162+ os.environ['BZR_LOG'] = os.getcwd() + '/no-dir/bzr.log'
163+ logf = trace._open_bzr_log()
164+ self.assertIs(None, logf)
165+ self.assertContainsRe(sys.stderr.getvalue(),
166+ 'failed to open trace file: .*/no-dir/bzr.log')
167
168 class TestVerbosityLevel(TestCase):
169
170
171=== modified file 'bzrlib/trace.py'
172--- bzrlib/trace.py 2009-12-01 05:35:52 +0000
173+++ bzrlib/trace.py 2010-01-10 02:33:27 +0000
174@@ -250,7 +250,12 @@
175 bzr_log_file.write("bug reports to https://bugs.launchpad.net/bzr/+filebug\n\n")
176 return bzr_log_file
177 except IOError, e:
178- warning("failed to open trace file: %s" % (e))
179+ # If we are failing to open the log, then most likely logging has not
180+ # been set up yet. So we just write to stderr rather than using
181+ # 'warning()'. If we using warning(), users get the unhelpful 'no
182+ # handlers registered for "bzr"' when something goes wrong on the
183+ # server. (bug #503886)
184+ sys.stderr.write("failed to open trace file: %s\n" % (e,))
185 # TODO: What should happen if we fail to open the trace file? Maybe the
186 # objects should be pointed at /dev/null or the equivalent? Currently
187 # returns None which will cause failures later.