Merge lp:~jameinel/bzr/integration into lp:bzr

Proposed by John A Meinel on 2013-05-23
Status: Merged
Approved by: John A Meinel on 2013-05-23
Approved revision: 6575
Merged at revision: 6575
Proposed branch: lp:~jameinel/bzr/integration
Merge into: lp:bzr
Diff against target: 308 lines (+142/-16)
8 files modified
bzrlib/dirstate.py (+10/-7)
bzrlib/osutils.py (+14/-1)
bzrlib/tests/__init__.py (+8/-2)
bzrlib/tests/test_osutils.py (+44/-0)
bzrlib/tests/test_selftest.py (+40/-4)
doc/en/release-notes/bzr-2.4.txt (+8/-0)
doc/en/release-notes/bzr-2.5.txt (+8/-0)
doc/en/release-notes/bzr-2.6.txt (+10/-2)
To merge this branch: bzr merge lp:~jameinel/bzr/integration
Reviewer Review Type Date Requested Status
Richard Wilbur 2013-05-23 Needs Fixing on 2013-05-23
Review via email: mp+165333@code.launchpad.net

Commit message

Merge bzr/2.5 into trunk.

Description of the change

Merge bzr/2.5 into trunk.

To post a comment you must log in.
John A Meinel (jameinel) wrote :

sent to pqm by email

Richard Wilbur (richard-wilbur) wrote :

I like the changes. It seems that, at least for trunk, we might consider moving the trace.mutter from
bzrlib/osutils.py:66

66 + trace.mutter("ignoring error calling fdatasync: %s" % (e,))
67 + if getattr(e, 'errno', None) not in _fdatasync_ignored:
68 + raise

into an 'else' of the test on line 67, since only in that case are we actually ignoring the error. Something like the following:

66 + if getattr(e, 'errno', None) not in _fdatasync_ignored:
67 + raise
68 + else:
69 + trace.mutter("ignoring error calling fdatasync: %s" % (e,))

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/dirstate.py'
2--- bzrlib/dirstate.py 2012-01-24 01:35:56 +0000
3+++ bzrlib/dirstate.py 2013-05-23 10:07:40 +0000
4@@ -2566,13 +2566,6 @@
5 self.update_minimal(('', '', new_id), 'd',
6 path_utf8='', packed_stat=entry[1][0][4])
7 self._mark_modified()
8- # XXX: This was added by Ian, we need to make sure there
9- # are tests for it, because it isn't in bzr.dev TRUNK
10- # It looks like the only place it is called is in setting the root
11- # id of the tree. So probably we never had an _id_index when we
12- # don't even have a root yet.
13- if self._id_index is not None:
14- self._add_to_id_index(self._id_index, entry[0])
15
16 def set_parent_trees(self, trees, ghosts):
17 """Set the parent trees for the dirstate.
18@@ -3285,10 +3278,20 @@
19 if self._id_index is not None:
20 for file_id, entry_keys in self._id_index.iteritems():
21 for entry_key in entry_keys:
22+ # Check that the entry in the map is pointing to the same
23+ # file_id
24 if entry_key[2] != file_id:
25 raise AssertionError(
26 'file_id %r did not match entry key %s'
27 % (file_id, entry_key))
28+ # And that from this entry key, we can look up the original
29+ # record
30+ block_index, present = self._find_block_index_from_key(entry_key)
31+ if not present:
32+ raise AssertionError('missing block for entry key: %r', entry_key)
33+ entry_index, present = self._find_entry_index(entry_key, self._dirblocks[block_index][1])
34+ if not present:
35+ raise AssertionError('missing entry for key: %r', entry_key)
36 if len(entry_keys) != len(set(entry_keys)):
37 raise AssertionError(
38 'id_index contained non-unique data for %s'
39
40=== modified file 'bzrlib/osutils.py'
41--- bzrlib/osutils.py 2012-09-19 07:58:27 +0000
42+++ bzrlib/osutils.py 2013-05-23 10:07:40 +0000
43@@ -2554,6 +2554,10 @@
44 else:
45 is_local_pid_dead = _posix_is_local_pid_dead
46
47+_maybe_ignored = ['EAGAIN', 'EINTR', 'ENOTSUP', 'EOPNOTSUPP', 'EACCES']
48+_fdatasync_ignored = [getattr(errno, name) for name in _maybe_ignored
49+ if getattr(errno, name, None) is not None]
50+
51
52 def fdatasync(fileno):
53 """Flush file contents to disk if possible.
54@@ -2563,7 +2567,16 @@
55 """
56 fn = getattr(os, 'fdatasync', getattr(os, 'fsync', None))
57 if fn is not None:
58- fn(fileno)
59+ try:
60+ fn(fileno)
61+ except IOError, e:
62+ # See bug #1075108, on some platforms fdatasync exists, but can
63+ # raise ENOTSUP. However, we are calling fdatasync to be helpful
64+ # and reduce the chance of corruption-on-powerloss situations. It
65+ # is not a mandatory call, so it is ok to suppress failures.
66+ trace.mutter("ignoring error calling fdatasync: %s" % (e,))
67+ if getattr(e, 'errno', None) not in _fdatasync_ignored:
68+ raise
69
70
71 def ensure_empty_directory_exists(path, exception_class):
72
73=== modified file 'bzrlib/tests/__init__.py'
74--- bzrlib/tests/__init__.py 2012-08-03 14:23:05 +0000
75+++ bzrlib/tests/__init__.py 2013-05-23 10:07:40 +0000
76@@ -1780,9 +1780,15 @@
77
78 :returns: The actual attr value.
79 """
80- value = getattr(obj, attr_name)
81 # The actual value is captured by the call below
82- self.addCleanup(setattr, obj, attr_name, value)
83+ value = getattr(obj, attr_name, _unitialized_attr)
84+ if value is _unitialized_attr:
85+ # When the test completes, the attribute should not exist, but if
86+ # we aren't setting a value, we don't need to do anything.
87+ if new is not _unitialized_attr:
88+ self.addCleanup(delattr, obj, attr_name)
89+ else:
90+ self.addCleanup(setattr, obj, attr_name, value)
91 if new is not _unitialized_attr:
92 setattr(obj, attr_name, new)
93 return value
94
95=== modified file 'bzrlib/tests/test_osutils.py'
96--- bzrlib/tests/test_osutils.py 2012-09-19 07:58:27 +0000
97+++ bzrlib/tests/test_osutils.py 2013-05-23 10:07:40 +0000
98@@ -23,6 +23,7 @@
99 import select
100 import socket
101 import sys
102+import tempfile
103 import time
104
105 from bzrlib import (
106@@ -436,6 +437,49 @@
107 self.assertTrue(-eighteen_hours < offset < eighteen_hours)
108
109
110+class TestFdatasync(tests.TestCaseInTempDir):
111+
112+ def do_fdatasync(self):
113+ f = tempfile.NamedTemporaryFile()
114+ osutils.fdatasync(f.fileno())
115+ f.close()
116+
117+ @staticmethod
118+ def raise_eopnotsupp(*args, **kwargs):
119+ raise IOError(errno.EOPNOTSUPP, os.strerror(errno.EOPNOTSUPP))
120+
121+ @staticmethod
122+ def raise_enotsup(*args, **kwargs):
123+ raise IOError(errno.ENOTSUP, os.strerror(errno.ENOTSUP))
124+
125+ def test_fdatasync_handles_system_function(self):
126+ self.overrideAttr(os, "fdatasync")
127+ self.do_fdatasync()
128+
129+ def test_fdatasync_handles_no_fdatasync_no_fsync(self):
130+ self.overrideAttr(os, "fdatasync")
131+ self.overrideAttr(os, "fsync")
132+ self.do_fdatasync()
133+
134+ def test_fdatasync_handles_no_EOPNOTSUPP(self):
135+ self.overrideAttr(errno, "EOPNOTSUPP")
136+ self.do_fdatasync()
137+
138+ def test_fdatasync_catches_ENOTSUP(self):
139+ enotsup = getattr(errno, "ENOTSUP", None)
140+ if enotsup is None:
141+ raise tests.TestNotApplicable("No ENOTSUP on this platform")
142+ self.overrideAttr(os, "fdatasync", self.raise_enotsup)
143+ self.do_fdatasync()
144+
145+ def test_fdatasync_catches_EOPNOTSUPP(self):
146+ enotsup = getattr(errno, "EOPNOTSUPP", None)
147+ if enotsup is None:
148+ raise tests.TestNotApplicable("No EOPNOTSUPP on this platform")
149+ self.overrideAttr(os, "fdatasync", self.raise_eopnotsupp)
150+ self.do_fdatasync()
151+
152+
153 class TestLinks(tests.TestCaseInTempDir):
154
155 def test_dereference_path(self):
156
157=== modified file 'bzrlib/tests/test_selftest.py'
158--- bzrlib/tests/test_selftest.py 2012-09-19 07:58:27 +0000
159+++ bzrlib/tests/test_selftest.py 2013-05-23 10:07:40 +0000
160@@ -1654,6 +1654,12 @@
161 self.assertRaises(AssertionError,
162 self.assertListRaises, _TestException, success_generator)
163
164+ def _run_successful_test(self, test):
165+ result = testtools.TestResult()
166+ test.run(result)
167+ self.assertTrue(result.wasSuccessful())
168+ return result
169+
170 def test_overrideAttr_without_value(self):
171 self.test_attr = 'original' # Define a test attribute
172 obj = self # Make 'obj' visible to the embedded test
173@@ -1669,8 +1675,7 @@
174 obj.test_attr = 'modified'
175 self.assertEqual('modified', obj.test_attr)
176
177- test = Test('test_value')
178- test.run(unittest.TestResult())
179+ self._run_successful_test(Test('test_value'))
180 self.assertEqual('original', obj.test_attr)
181
182 def test_overrideAttr_with_value(self):
183@@ -1686,10 +1691,41 @@
184 self.assertEqual('original', self.orig)
185 self.assertEqual('modified', obj.test_attr)
186
187- test = Test('test_value')
188- test.run(unittest.TestResult())
189+ self._run_successful_test(Test('test_value'))
190 self.assertEqual('original', obj.test_attr)
191
192+ def test_overrideAttr_with_no_existing_value_and_value(self):
193+ # Do not define the test_attribute
194+ obj = self # Make 'obj' visible to the embedded test
195+ class Test(tests.TestCase):
196+
197+ def setUp(self):
198+ tests.TestCase.setUp(self)
199+ self.orig = self.overrideAttr(obj, 'test_attr', new='modified')
200+
201+ def test_value(self):
202+ self.assertEqual(tests._unitialized_attr, self.orig)
203+ self.assertEqual('modified', obj.test_attr)
204+
205+ self._run_successful_test(Test('test_value'))
206+ self.assertRaises(AttributeError, getattr, obj, 'test_attr')
207+
208+ def test_overrideAttr_with_no_existing_value_and_no_value(self):
209+ # Do not define the test_attribute
210+ obj = self # Make 'obj' visible to the embedded test
211+ class Test(tests.TestCase):
212+
213+ def setUp(self):
214+ tests.TestCase.setUp(self)
215+ self.orig = self.overrideAttr(obj, 'test_attr')
216+
217+ def test_value(self):
218+ self.assertEqual(tests._unitialized_attr, self.orig)
219+ self.assertRaises(AttributeError, getattr, obj, 'test_attr')
220+
221+ self._run_successful_test(Test('test_value'))
222+ self.assertRaises(AttributeError, getattr, obj, 'test_attr')
223+
224 def test_recordCalls(self):
225 from bzrlib.tests import test_selftest
226 calls = self.recordCalls(
227
228=== modified file 'doc/en/release-notes/bzr-2.4.txt'
229--- doc/en/release-notes/bzr-2.4.txt 2012-09-05 20:22:17 +0000
230+++ doc/en/release-notes/bzr-2.4.txt 2013-05-23 10:07:40 +0000
231@@ -35,6 +35,10 @@
232 * Cope with Unix filesystems, such as smbfs, where chmod gives 'permission
233 denied'. (Martin Pool, #606537)
234
235+* Fix a traceback when trying to checkout a tree that also has an entry
236+ with file-id `TREE_ROOT` somewhere other than at the root directory.
237+ (John Arbash Meinel, #830947)
238+
239 * When the ``limbo`` or ``pending-deletion`` directories exist, typically
240 because of an interrupted tree update, but are empty, bzr no longer
241 errors out, because there is nothing for the user to clean up. Also,
242@@ -55,6 +59,10 @@
243 * Prevent a traceback being printed to stderr when logging has problems and
244 accept utf-8 byte string without breaking. (Martin Packman, #714449)
245
246+* Some filesystems give ``EOPNOTSUPP`` when trying to call ``fdatasync``.
247+ This shouldn't be treated as a fatal error.
248+ (John Arbash Meinel, #1075108)
249+
250 * Use ``encoding_type='exact'`` for ``bzr testament`` so that on Windows
251 the sha hash of the long testament matches the sha hash in the short
252 form. (John Arbash Meinel, #1010339)
253
254=== modified file 'doc/en/release-notes/bzr-2.5.txt'
255--- doc/en/release-notes/bzr-2.5.txt 2012-09-19 07:58:27 +0000
256+++ doc/en/release-notes/bzr-2.5.txt 2013-05-23 10:07:40 +0000
257@@ -35,6 +35,10 @@
258 * ``bzr config`` properly handles aliases and references in the
259 ``--directory`` parameter (Vincent Ladeuil, Wouter van Heyst, #947049)
260
261+* Fix a traceback when trying to checkout a tree that also has an entry
262+ with file-id `TREE_ROOT` somewhere other than at the root directory.
263+ (John Arbash Meinel, #830947)
264+
265 * Lightweight checkouts of remote repositories had a bug with how they
266 extracted texts from the repository. (Just an ordering constraint on how
267 they consumed the stream.) (John Arbash Meinel, #1046284)
268@@ -48,6 +52,10 @@
269
270 * Revert use of --no-tty when gpg signing commits. (Jelmer Vernooij, #1014570)
271
272+* Some filesystems give ``EOPNOTSUPP`` when trying to call ``fdatasync``.
273+ This shouldn't be treated as a fatal error.
274+ (John Arbash Meinel, #1075108)
275+
276 * Some small bug fixes wrt lightweight checkouts and remote repositories.
277 A test permutation was added that runs all working tree tests against a
278 lightweight checkout. (John Arbash Meinel, #1046697)
279
280=== modified file 'doc/en/release-notes/bzr-2.6.txt'
281--- doc/en/release-notes/bzr-2.6.txt 2012-12-04 16:36:55 +0000
282+++ doc/en/release-notes/bzr-2.6.txt 2013-05-23 10:07:40 +0000
283@@ -60,6 +60,13 @@
284 ``BZR_PROGRESS_BAR``. This can be set in ``bazaar.conf`` or specified from
285 the command line with ``-Oprogress_bar=text``. (Vincent Ladeuil, #388275)
286
287+* ``Authentication.Config`` now always returns unicode user names and passwords.
288+ (Vincent Ladeuil, #514301)
289+
290+* Fix a traceback when trying to checkout a tree that also has an entry
291+ with file-id `TREE_ROOT` somewhere other than at the root directory.
292+ (John Arbash Meinel, #830947)
293+
294 * Fixed a bug where the entire contents of ``/etc/mailname`` is read in.
295 We only want to read in the first line so that comments could be added
296 and would be ignored.
297@@ -69,8 +76,9 @@
298 causes breakage with docutils 0.9.1.
299 (Vincent Ladeuil, Jelmer Vernooij, #1066307)
300
301-* ``Authentication.Config`` now always returns unicode user names and passwords.
302- (Vincent Ladeuil, #514301)
303+* Some filesystems give ``EOPNOTSUPP`` when trying to call ``fdatasync``.
304+ This shouldn't be treated as a fatal error.
305+ (John Arbash Meinel, #1075108)
306
307 * Warn when ``--show-base`` is used for ``pull`` in a treeless branch
308 instead of failing. It's useless but harmless. (Vincent Ladeuil, #1022160)