Merge lp:~jelmer/bzr/export-use-tree-timestamp into lp:bzr

[Jelmer Vernooij]

This adds an extra argument ``use_tree_timestamp`` to the exporters, to use the commit time of the commit in which a file revision was introduced as the mtime when exporting. This is useful as a means of getting deterministic exports (i.e. the generated tarball from a particular revision is always the same).

use_tree_timestamps is not the default for performance reasons, but the provided infrastructure is useful for bzr-builddeb.

[lp:~jelmer/bzr/export-use-tree-timestamp]

5078. By Jelmer Vernooij on 2010-03-07

``bzr export`` now takes an optional argument ``--use-tree-timestamp``
to set file mtimes to the last timestamp of the last revision in which
they were changed rather than the current time.

[Robert Collins (lifeless)]

 review: needsfixing

perhaps per_file_timestamps - the source is the tree, but its the
lookup-per-rev that will hurt.

Crucially the docstring change should be a little more verbose about
this, so that folk reading it can predict the behaviour/tradeoff.

Should this be exposed in 'bzr export's command line?

Cheers,
Rob

review: Needs Fixing

[lp:~jelmer/bzr/export-use-tree-timestamp]

5079. By Jelmer Vernooij on 2010-03-08

Review comments from Rob.

[John A Meinel (jameinel)]

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

Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging lp:~jelmer/bzr/export-use-tree-timestamp into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> This adds an extra argument ``use_tree_timestamp`` to the exporters, to use the commit time of the commit in which a file revision was introduced as the mtime when exporting. This is useful as a means of getting deterministic exports (i.e. the generated tarball from a particular revision is always the same).
>
> use_tree_timestamps is not the default for performance reasons, but the provided infrastructure is useful for bzr-builddeb.
>

Note that using 'tree.get_file_mtime()' for each file is a bit of a
shame. Specifically, the api doesn't really do any caching (and it isn't
very easy to do so).

If we wanted it to perform well, doing 1 pass over the inventory to find
revisions we will care about, doing another request to read all of those
revisions in 1 pass, should actually be pretty decent. (log can display
all revisions in generally a rather decent amount of time, this should
be no slower than that.)

John
=:->

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

iEYEARECAAYFAkuVKG0ACgkQJdeBCYSNAAMtNQCgqkS8W/byYFqC3XpeK9QeQCjV
fVcAmgNaJOolG9yNfUFmqnOwEwuYYLDQ
=b4zc
-----END PGP SIGNATURE-----

[Jelmer Vernooij (jelmer)]

> Jelmer Vernooij wrote:
> > Jelmer Vernooij has proposed merging lp:~jelmer/bzr/export-use-tree-
> timestamp into lp:bzr.
> >
> > Requested reviews:
> > bzr-core (bzr-core)
> >
> >
> > This adds an extra argument ``use_tree_timestamp`` to the exporters, to use
> the commit time of the commit in which a file revision was introduced as the
> mtime when exporting. This is useful as a means of getting deterministic
> exports (i.e. the generated tarball from a particular revision is always the
> same).
> >
> > use_tree_timestamps is not the default for performance reasons, but the
> provided infrastructure is useful for bzr-builddeb.
> >
>
> Note that using 'tree.get_file_mtime()' for each file is a bit of a
> shame. Specifically, the api doesn't really do any caching (and it isn't
> very easy to do so).
>
> If we wanted it to perform well, doing 1 pass over the inventory to find
> revisions we will care about, doing another request to read all of those
> revisions in 1 pass, should actually be pretty decent. (log can display
> all revisions in generally a rather decent amount of time, this should
> be no slower than that.)
Yeah,I realize that. That seemed like significantly more work though so it's something I'd rather do in a separate patch.

This at least adds the functionality, but doesn't have any performance consequences for the default behaviour. The overhead at the moment doesn't seem too bad. On a Samba tree with a warm cache:

./bzr export x.tar.gz ~/src/samba/trunk 15.58s user 0.28s system 96% cpu 16.374 total
./bzr export --per-file-timestamps x.tar.gz ~/src/samba/trunk 19.06s user 0.43s system 86% cpu 22.629 total
./bzr ls --versioned --recursive ~/src/samba/trunk | wc -l
7921

Cheers,

Jelmer

[Martin Pool (mbp)]

I see you renamed the option per Robert's suggestion, but you need to also update the news!

I'm fine for you to make this faster in a later patch.

review: Needs Fixing

[lp:~jelmer/bzr/export-use-tree-timestamp]

5080. By Jelmer Vernooij on 2010-03-11

Fix NEWS.

[Jelmer Vernooij (jelmer)]

> I see you renamed the option per Robert's suggestion, but you need to also
> update the news!
>
> I'm fine for you to make this faster in a later patch.
I've fixed NEWS.

[lp:~jelmer/bzr/export-use-tree-timestamp]

5081. By Jelmer Vernooij on 2010-03-11

Merge bzr.dev.

=== modified file 'NEWS'
--- NEWS	2010-03-11 11:07:32 +0000
+++ NEWS	2010-03-11 18:02:25 +0000
@@ -37,6 +37,10 @@
 New Features
 ************
 
+* ``bzr export`` now takes an optional argument ``--per-file-timestamps``
+  to set file mtimes to the last timestamp of the last revision in which
+  they were changed rather than the current time. (Jelmer Vernooij)
+
 * If the Apport crash-reporting tool is available, bzr crashes are now
   stored into the ``/var/crash`` apport spool directory, and the user is
   invited to report them to the developers from there, either
@@ -147,6 +151,10 @@
   to be done separately.
   (Martin Pool, #507710)
 
+* Exporters now support a ``per_file_timestamps`` argument to write out the 
+  timestamp of the commit in which a file revision was introduced.
+  (Jelmer Vernooij)
+
 * New method ``BzrDir.list_branches()`` that returns a sequence of branches 
   present in a control directory. (Jelmer Vernooij)
 

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2010-03-03 08:25:15 +0000
+++ bzrlib/builtins.py	2010-03-11 18:02:25 +0000
@@ -2806,9 +2806,12 @@
         Option('root',
                type=str,
                help="Name of the root directory inside the exported file."),
+        Option('per-file-timestamps',
+               help='Set modification time of files to that of the last '
+                    'revision in which it was changed.'),
         ]
     def run(self, dest, branch_or_subdir=None, revision=None, format=None,
-        root=None, filters=False):
+        root=None, filters=False, per_file_timestamps=False):
         from bzrlib.export import export
 
         if branch_or_subdir is None:
@@ -2821,7 +2824,8 @@
 
         rev_tree = _get_one_revision_tree('export', revision, branch=b, tree=tree)
         try:
-            export(rev_tree, dest, format, root, subdir, filtered=filters)
+            export(rev_tree, dest, format, root, subdir, filtered=filters,
+                   per_file_timestamps=per_file_timestamps)
         except errors.NoSuchExportFormat, e:
             raise errors.BzrCommandError('Unsupported export format: %s' % e.format)
 

=== modified file 'bzrlib/export/__init__.py'
--- bzrlib/export/__init__.py	2010-01-29 12:04:38 +0000
+++ bzrlib/export/__init__.py	2010-03-11 18:02:25 +0000
@@ -19,7 +19,6 @@
 Such as non-controlled directories, tarfiles, zipfiles, etc.
 """
 
-from bzrlib.trace import mutter
 import os
 import bzrlib.errors as errors
 
@@ -55,14 +54,16 @@
 
     When requesting a specific type of export, load the respective path.
     """
-    def _loader(tree, dest, root, subdir, filtered):
+    def _loader(tree, dest, root, subdir, filtered, per_file_timestamps):
         mod = __import__(module, globals(), locals(), [funcname])
         func = getattr(mod, funcname)
-        return func(tree, dest, root, subdir, filtered=filtered)
+        return func(tree, dest, root, subdir, filtered=filtered,
+                    per_file_timestamps=per_file_timestamps)
     register_exporter(scheme, extensions, _loader)
 
 
-def export(tree, dest, format=None, root=None, subdir=None, filtered=False):
+def export(tree, dest, format=None, root=None, subdir=None, filtered=False,
+           per_file_timestamps=False):
     """Export the given Tree to the specific destination.
 
     :param tree: A Tree (such as RevisionTree) to export
@@ -81,6 +82,9 @@
         a directory to start exporting from.
     :param filtered: If True, content filtering is applied to the
                      files exported.
+    :param per_file_timestamps: Whether to use the timestamp stored in the 
+        tree rather than now(). This will do a revision lookup 
+        for every file so will be significantly slower.
     """
     global _exporters, _exporter_extensions
 
@@ -99,7 +103,8 @@
         raise errors.NoSuchExportFormat(format)
     tree.lock_read()
     try:
-        return _exporters[format](tree, dest, root, subdir, filtered=filtered)
+        return _exporters[format](tree, dest, root, subdir, filtered=filtered,
+                                  per_file_timestamps=per_file_timestamps)
     finally:
         tree.unlock()
 

=== modified file 'bzrlib/export/dir_exporter.py'
--- bzrlib/export/dir_exporter.py	2010-02-23 07:43:11 +0000
+++ bzrlib/export/dir_exporter.py	2010-03-11 18:02:25 +0000
@@ -18,7 +18,6 @@
 
 import errno
 import os
-import StringIO
 import time
 
 from bzrlib import errors, osutils
@@ -30,7 +29,8 @@
 from bzrlib.trace import mutter
 
 
-def dir_exporter(tree, dest, root, subdir, filtered=False):
+def dir_exporter(tree, dest, root, subdir, filtered=False,
+                 per_file_timestamps=False):
     """Export this tree to a new directory.
 
     `dest` should either not exist or should be empty. If it does not exist it
@@ -92,4 +92,8 @@
             out.writelines(chunks)
         finally:
             out.close()
-        os.utime(fullpath, (now, now))
+        if per_file_timestamps:
+            mtime = tree.get_file_mtime(tree.path2id(relpath), relpath)
+        else:
+            mtime = now
+        os.utime(fullpath, (mtime, mtime))

=== modified file 'bzrlib/export/tar_exporter.py'
--- bzrlib/export/tar_exporter.py	2009-03-23 14:59:43 +0000
+++ bzrlib/export/tar_exporter.py	2010-03-11 18:02:25 +0000
@@ -17,13 +17,12 @@
 """Export a Tree to a non-versioned directory.
 """
 
-import os
 import StringIO
 import sys
 import tarfile
 import time
 
-from bzrlib import errors, export, osutils
+from bzrlib import export, osutils
 from bzrlib.export import _export_iter_entries
 from bzrlib.filters import (
     ContentFilterContext,
@@ -32,7 +31,8 @@
 from bzrlib.trace import mutter
 
 
-def tar_exporter(tree, dest, root, subdir, compression=None, filtered=False):
+def tar_exporter(tree, dest, root, subdir, compression=None, filtered=False,
+                 per_file_timestamps=False):
     """Export this tree to a new tar file.
 
     `dest` will be created holding the contents of this tree; if it
@@ -52,7 +52,10 @@
     for dp, ie in _export_iter_entries(tree, subdir):
         filename = osutils.pathjoin(root, dp).encode('utf8')
         item = tarfile.TarInfo(filename)
-        item.mtime = now
+        if per_file_timestamps:
+            item.mtime = tree.get_file_mtime(ie.file_id, dp)
+        else:
+            item.mtime = now
         if ie.kind == "file":
             item.type = tarfile.REGTYPE
             if tree.is_executable(ie.file_id):
@@ -89,9 +92,13 @@
     ball.close()
 
 
-def tgz_exporter(tree, dest, root, subdir, filtered=False):
-    tar_exporter(tree, dest, root, subdir, compression='gz', filtered=filtered)
-
-
-def tbz_exporter(tree, dest, root, subdir, filtered=False):
-    tar_exporter(tree, dest, root, subdir, compression='bz2', filtered=filtered)
+def tgz_exporter(tree, dest, root, subdir, filtered=False,
+                 per_file_timestamps=False):
+    tar_exporter(tree, dest, root, subdir, compression='gz',
+                 filtered=filtered, per_file_timestamps=per_file_timestamps)
+
+
+def tbz_exporter(tree, dest, root, subdir, filtered=False,
+                 per_file_timestamps=False):
+    tar_exporter(tree, dest, root, subdir, compression='bz2',
+                 filtered=filtered, per_file_timestamps=per_file_timestamps)

=== modified file 'bzrlib/export/zip_exporter.py'
--- bzrlib/export/zip_exporter.py	2010-02-17 17:11:16 +0000
+++ bzrlib/export/zip_exporter.py	2010-03-11 18:02:25 +0000
@@ -42,7 +42,8 @@
 _DIR_ATTR = stat.S_IFDIR | ZIP_DIRECTORY_BIT
 
 
-def zip_exporter(tree, dest, root, subdir, filtered=False):
+def zip_exporter(tree, dest, root, subdir, filtered=False,
+                 per_file_timestamps=False):
     """ Export this tree to a new zip file.
 
     `dest` will be created holding the contents of this tree; if it
@@ -62,11 +63,15 @@
 
             # zipfile.ZipFile switches all paths to forward
             # slashes anyway, so just stick with that.
+            if per_file_timestamps:
+                mtime = tree.get_file_mtime(ie.file_id, dp)
+            else:
+                mtime = now
             filename = osutils.pathjoin(root, dp).encode('utf8')
             if ie.kind == "file":
                 zinfo = zipfile.ZipInfo(
                             filename=filename,
-                            date_time=now)
+                            date_time=mtime)
                 zinfo.compress_type = compression
                 zinfo.external_attr = _FILE_ATTR
                 if filtered:
@@ -84,14 +89,14 @@
                 # not just empty files.
                 zinfo = zipfile.ZipInfo(
                             filename=filename + '/',
-                            date_time=now)
+                            date_time=mtime)
                 zinfo.compress_type = compression
                 zinfo.external_attr = _DIR_ATTR
                 zipf.writestr(zinfo,'')
             elif ie.kind == "symlink":
                 zinfo = zipfile.ZipInfo(
                             filename=(filename + '.lnk'),
-                            date_time=now)
+                            date_time=mtime)
                 zinfo.compress_type = compression
                 zinfo.external_attr = _FILE_ATTR
                 zipf.writestr(zinfo, ie.symlink_target)

=== modified file 'bzrlib/tests/blackbox/test_export.py'
--- bzrlib/tests/blackbox/test_export.py	2010-02-17 17:11:16 +0000
+++ bzrlib/tests/blackbox/test_export.py	2010-03-11 18:02:25 +0000
@@ -293,3 +293,13 @@
         tree.commit('more setup')
         out, err = self.run_bzr('export exported branch/subdir')
         self.assertEqual(['foo.txt'], os.listdir('exported'))
+
+    def test_dir_export_per_file_timestamps(self):
+        tree = self.example_branch()
+        self.build_tree_contents([('branch/har', 'foo')])
+        tree.add('har')
+        tree.commit('setup', timestamp=42)
+        self.run_bzr('export --per-file-timestamps t branch')
+        har_st = os.stat('t/har')
+        self.assertEquals(42, har_st.st_mtime)
+

=== modified file 'bzrlib/tests/test_export.py'
--- bzrlib/tests/test_export.py	2010-02-04 16:06:36 +0000
+++ bzrlib/tests/test_export.py	2010-03-11 18:02:25 +0000
@@ -20,7 +20,6 @@
 from bzrlib import (
     errors,
     export,
-    osutils,
     tests,
     )
 
@@ -99,3 +98,25 @@
         st_b = t.stat('b')
         # All files must be given the same mtime.
         self.assertEqual(st_a.st_mtime, st_b.st_mtime)
+
+    def test_dir_export_files_per_file_timestamps(self):
+        builder = self.make_branch_builder('source')
+        builder.start_series()
+        builder.build_snapshot(None, None, [
+            ('add', ('', 'root-id', 'directory', '')),
+            ('add', ('a', 'a-id', 'file', 'content\n'))],
+            timestamp=3423)
+        builder.build_snapshot(None, None, [
+            ('add', ('b', 'b-id', 'file', 'content\n'))],
+            timestamp=42)
+        builder.finish_series()
+        b = builder.get_branch()
+        b.lock_read()
+        self.addCleanup(b.unlock)
+        tree = b.basis_tree()
+        export.export(tree, 'target', format='dir', per_file_timestamps=True)
+        t = self.get_transport('target')
+        st_a = t.stat('a')
+        st_b = t.stat('b')
+        self.assertEqual(42.0, st_b.st_mtime)
+        self.assertEqual(3423.0, st_a.st_mtime)