Merge lp:~jelmer/bzr/ghost-diffs into lp:bzr

[Jelmer Vernooij]

[Jelmer Vernooij (jelmer)]

This makes "bzr diff" print the epoch time if it is unable to find the modification time for a commit.

[John A Meinel (jameinel)]

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

Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging lp:~jelmer/bzr/ghost-diffs into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #295611 NoSuchRevision: KnitPackRepository
> https://bugs.launchpad.net/bugs/295611
>
>
> This makes "bzr diff" print the epoch time if it is unable to find the modification time for a commit.
>

Better than failing. I'm not sure that 'epoch' is the best choice, but
we can get there from here.

John
=:->

 merge: approve

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

iEYEARECAAYFAktXq0sACgkQJdeBCYSNAAOYJwCcCGi2z9FSD5BLIqrVhCJmRSr9
81wAoIaZD2J0m2GC05bSkpDiZGTkoCvX
=NNmc
-----END PGP SIGNATURE-----

review: Approve

[Jelmer Vernooij (jelmer)]

On Thu, 2010-01-21 at 01:21 +0000, John A Meinel wrote:
> Jelmer Vernooij wrote:
> > Jelmer Vernooij has proposed merging lp:~jelmer/bzr/ghost-diffs into lp:bzr.
> >
> > Requested reviews:
> > bzr-core (bzr-core)
> > Related bugs:
> > #295611 NoSuchRevision: KnitPackRepository
> > https://bugs.launchpad.net/bugs/295611
> >
> >
> > This makes "bzr diff" print the epoch time if it is unable to find the modification time for a commit.
> Better than failing. I'm not sure that 'epoch' is the best choice, but
> we can get there from here.
The reason I picked the epoch is because it's still a date (requirement
for some patch parsing tools) and the epoch is already used for new
files. But I agree it's not ideal.

Cheers,

Jelmer

=== modified file 'NEWS'
--- NEWS	2010-01-20 16:31:24 +0000
+++ NEWS	2010-01-20 23:32:13 +0000
@@ -64,6 +64,10 @@
 * Better explain the --uncommitted option of merge.
   (Neil Martinsen-Burrell, #505088)
 
+* ``bzr diff`` will now use the epoch when it is unable to determine 
+  the timestamp of a file, if the revision it was introduced in is a
+  ghost. (Jelmer Vernooij, #295611)
+
 * ``bzr export dir`` now requests all file content as a record stream,
   rather than requsting the file content one file-at-a-time. This can make
   exporting over the network significantly faster (54min => 9min in one

=== modified file 'bzrlib/diff.py'
--- bzrlib/diff.py	2009-12-03 07:36:17 +0000
+++ bzrlib/diff.py	2010-01-20 23:32:13 +0000
@@ -453,7 +453,10 @@
 
 def _patch_header_date(tree, file_id, path):
     """Returns a timestamp suitable for use in a patch header."""
-    mtime = tree.get_file_mtime(file_id, path)
+    try:
+        mtime = tree.get_file_mtime(file_id, path)
+    except errors.FileTimestampUnavailable:
+        mtime = 0
     return timestamp.format_patch_date(mtime)
 
 
@@ -747,7 +750,10 @@
             source.close()
         if not allow_write:
             osutils.make_readonly(full_path)
-        mtime = tree.get_file_mtime(file_id)
+        try:
+            mtime = tree.get_file_mtime(file_id)
+        except errors.FileTimestampUnavailable:
+            mtime = 0
         os.utime(full_path, (mtime, mtime))
         return full_path
 

=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	2010-01-12 02:48:41 +0000
+++ bzrlib/errors.py	2010-01-20 23:32:13 +0000
@@ -3114,3 +3114,13 @@
     def __init__(self, source_branch, target_branch):
         self.source_branch = source_branch
         self.target_branch = target_branch
+
+
+class FileTimestampUnavailable(BzrError):
+
+    _fmt = "The filestamp for %(path)s is not available."
+
+    internal_error = True
+
+    def __init__(self, path):
+        self.path = path

=== modified file 'bzrlib/revisiontree.py'
--- bzrlib/revisiontree.py	2009-10-15 02:19:43 +0000
+++ bzrlib/revisiontree.py	2010-01-20 23:32:13 +0000
@@ -103,7 +103,10 @@
 
     def get_file_mtime(self, file_id, path=None):
         ie = self._inventory[file_id]
-        revision = self._repository.get_revision(ie.revision)
+        try:
+            revision = self._repository.get_revision(ie.revision)
+        except errors.NoSuchRevision:
+            raise errors.FileTimestampUnavailable(self.id2path(file_id))
         return revision.timestamp
 
     def is_executable(self, file_id, path=None):

=== modified file 'bzrlib/tests/test_errors.py'
--- bzrlib/tests/test_errors.py	2010-01-12 02:48:41 +0000
+++ bzrlib/tests/test_errors.py	2010-01-20 23:32:13 +0000
@@ -708,3 +708,8 @@
             socket.error(13, 'Permission denied'))
         self.assertContainsRe(str(e),
             r'Cannot bind address "example\.com:22":.*Permission denied')
+
+    def test_file_timestamp_unavailable(self):            
+        e = errors.FileTimestampUnavailable("/path/foo")
+        self.assertEquals("The filestamp for /path/foo is not available.",
+            str(e))

=== modified file 'bzrlib/tests/test_revisiontree.py'
--- bzrlib/tests/test_revisiontree.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/test_revisiontree.py	2010-01-20 23:32:13 +0000
@@ -18,9 +18,11 @@
 """Tests for the RevisionTree class."""
 
 from bzrlib import (
+    errors,
     revision,
     )
 import bzrlib
+from bzrlib.inventory import ROOT_ID
 from bzrlib.tests import TestCaseWithTransport
 
 
@@ -59,3 +61,9 @@
         null_tree = self.t.branch.repository.revision_tree(
             revision.NULL_REVISION)
         self.assertIs(None, null_tree.inventory.root)
+
+    def test_get_file_mtime_ghost(self):
+        file_id = iter(self.rev_tree).next()
+        self.rev_tree.inventory[file_id].revision = 'ghostrev'
+        self.assertRaises(errors.FileTimestampUnavailable, 
+            self.rev_tree.get_file_mtime, file_id)

=== modified file 'bzrlib/workingtree_4.py'
--- bzrlib/workingtree_4.py	2009-12-22 17:05:47 +0000
+++ bzrlib/workingtree_4.py	2010-01-20 23:32:13 +0000
@@ -1755,7 +1755,11 @@
             return None
         parent_index = self._get_parent_index()
         last_changed_revision = entry[1][parent_index][4]
-        return self._repository.get_revision(last_changed_revision).timestamp
+        try:
+            rev = self._repository.get_revision(last_changed_revision)
+        except errors.NoSuchRevision:
+            raise errors.FileTimestampUnavailable(self.id2path(file_id))
+        return rev.timestamp
 
     def get_file_sha1(self, file_id, path=None, stat_value=None):
         entry = self._get_entry(file_id=file_id, path=path)