Merge lp:~jelmer/bzr/847435-diff-time into lp:bzr

[Jelmer Vernooij]

Raise NoSuchId from Tree.get_file_mtime if the file id was not found.

Test the Tree.get_file_mtime API to raise NoSuchId if an unknown file id is specified.

the working tree 4 basis tree implementations would return None rather than raising NoSuchId, which was inconsistent with the other tree implementations.

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 9/21/2011 2:49 AM, Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging
> lp:~jelmer/bzr/847435-diff-time into lp:bzr.
>
> Requested reviews: bzr-core (bzr-core) Related bugs: Bug #847435 in
> Bazaar: "bzr crashed with TypeError in format_patch_date():
> unsupported operand type(s) for +: 'NoneType' and 'int'"
> https://bugs.launchpad.net/bzr/+bug/847435
>
> For more details, see:
> https://code.launchpad.net/~jelmer/bzr/847435-diff-time/+merge/76313
>
> Test the Tree.get_file_mtime API to raise NoSuchId if an unknown
> file id is specified.
>
> the working tree 4 basis tree implementations would return None
> rather than raising NoSuchId, which was inconsistent with the other
> tree implementations.

 merge: approve

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk55wpUACgkQJdeBCYSNAAPP8ACeI63+qGSFUjZuN7ao6KyBJOCe
xUsAnioPJOLFmkelOa/b0sHlVeE9+psD
=ucm3
-----END PGP SIGNATURE-----

review: Approve

[Jelmer Vernooij (jelmer)]

sent to pqm by email

=== modified file 'bzrlib/tests/per_tree/test_get_file_mtime.py'
--- bzrlib/tests/per_tree/test_get_file_mtime.py	2011-05-13 12:51:05 +0000
+++ bzrlib/tests/per_tree/test_get_file_mtime.py	2011-09-21 00:52:23 +0000
@@ -18,6 +18,8 @@
 
 import time
 
+from bzrlib import errors
+
 from bzrlib.tests.per_tree import TestCaseWithTree
 
 
@@ -42,3 +44,10 @@
                         'now: %f, mtime_file_id: %f' % (now, mtime_file_id ))
         mtime_path = tree.get_file_mtime(file_id='one-id', path='one')
         self.assertEqual(mtime_file_id, mtime_path)
+
+    def test_nonexistant(self):
+        tree = self.get_basic_tree()
+        tree.lock_read()
+        self.addCleanup(tree.unlock)
+        self.assertRaises(errors.NoSuchId,
+            tree.get_file_mtime, file_id='unexistant')

=== modified file 'bzrlib/tests/per_workingtree/test_get_file_mtime.py'
--- bzrlib/tests/per_workingtree/test_get_file_mtime.py	2009-07-10 07:14:02 +0000
+++ bzrlib/tests/per_workingtree/test_get_file_mtime.py	2011-09-21 00:52:23 +0000
@@ -18,6 +18,7 @@
 
 import os
 
+from bzrlib import errors
 from bzrlib.tests.per_workingtree import TestCaseWithWorkingTree
 
 
@@ -98,3 +99,15 @@
             self.assertAlmostEqual(st.st_mtime, mtime)
         finally:
             tree.unlock()
+
+    def test_missing(self):
+        tree = self.make_basic_tree()
+
+        os.remove('tree/one')
+        tree.lock_read()
+        try:
+            self.assertRaises(errors.FileTimestampUnavailable,
+                tree.get_file_mtime, file_id='one-id')
+        finally:
+            tree.unlock()
+

=== modified file 'bzrlib/workingtree.py'
--- bzrlib/workingtree.py	2011-09-15 11:31:47 +0000
+++ bzrlib/workingtree.py	2011-09-21 00:52:23 +0000
@@ -2134,7 +2134,12 @@
         """See Tree.get_file_mtime."""
         if not path:
             path = self.inventory.id2path(file_id)
-        return os.lstat(self.abspath(path)).st_mtime
+        try:
+            return os.lstat(self.abspath(path)).st_mtime
+        except OSError, e:
+            if e.errno == errno.ENOENT:
+                raise errors.FileTimestampUnavailable(path)
+            raise
 
     def _is_executable_from_path_and_stat_from_basis(self, path, stat_result):
         file_id = self.path2id(path)

=== modified file 'bzrlib/workingtree_4.py'
--- bzrlib/workingtree_4.py	2011-08-12 14:47:42 +0000
+++ bzrlib/workingtree_4.py	2011-09-21 00:52:23 +0000
@@ -1849,7 +1849,7 @@
         # Make sure the file exists
         entry = self._get_entry(file_id, path=path)
         if entry == (None, None): # do we raise?
-            return None
+            raise errors.NoSuchId(self, file_id)
         parent_index = self._get_parent_index()
         last_changed_revision = entry[1][parent_index][4]
         try:

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-09-19 15:22:23 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-09-21 00:52:23 +0000
@@ -35,6 +35,9 @@
 * Redirects between http and https no longer discard path information
   in some cases. (Jelmer Vernooij, #853765)
 
+* ``WorkingTree.get_file_mtime`` now raises NoSuchId if a file id is
+  specified that is unknown. (Jelmer Vernooij, #847435)
+
 Documentation
 *************