Merge lp:~agrimm/bzr-fastimport/baseline-commit into lp:bzr-fastimport

[Andy Grimm]

Adds a --baseline option to fast-export to produce an export with a truncated history (a "baseline" plus all commits in the range given by the revisionspec).

[Jelmer Vernooij (jelmer)]

Hi Andy,

Thanks again for working on this.

Generally, we try to put commands that modify fastexport streams into "bzr fast-import-filter", rather than "bzr fast-export". In this case, I guess it also skips

+ help="Export an 'abolute' baseline commit prior to"
This should probably say "absolute".

+ assert(len(revobj.parent_ids))
+ previd = revobj.parent_ids[0]
It seems a bit odd that the code uses the parent of the revision that was specified, rather than that revision itself.

Can you add some tests for this new behaviour?

Cheers,

Jelmer

[Andy Grimm (agrimm)]

> Hi Andy,
>
> Thanks again for working on this.
>
> Generally, we try to put commands that modify fastexport streams into "bzr
> fast-import-filter", rather than "bzr fast-export". In this case, I guess it
> also skips

If you think that having it in fast-import-filter is useful, I can do that. Will that change the logic about how the baseline is produced? Also, was your last sentence cut off here? I didn't understand it as written (skips what?)

> + help="Export an 'abolute' baseline commit prior to"
> This should probably say "absolute".

Indeed.

> + assert(len(revobj.parent_ids))
> + previd = revobj.parent_ids[0]
> It seems a bit odd that the code uses the parent of the revision that was
> specified, rather than that revision itself.

I wrestled with that a little. If the baseline argument were to take a single revision as an argument, I would agree that that is what should be used. Instead, I required the user to specify a revisionspec, which indicates the commits which they'd like to preserve. If I did not use the parent revision here, then they'd lose the first diff they wanted to be able to see. That might be confusing, though, and I'm not set on this behavior.

Do you think that it would be better to have the baseline option take a revisionspec directly, rather than requiring -r along with this option? (If I continue to require -r, I guess I need to writing some code to enforce that requirement.)

> Can you add some tests for this new behaviour?

Sure, I haven't looked at the test suite yet, but I'll give it a try.

> Cheers,
>
> Jelmer

[Jelmer Vernooij (jelmer)]

Am 22/11/11 16:08, schrieb Andy Grimm:
>> Hi Andy,
>>
>> Thanks again for working on this.
>>
>> Generally, we try to put commands that modify fastexport streams into "bzr
>> fast-import-filter", rather than "bzr fast-export". In this case, I guess it
>> also skips
> If you think that having it in fast-import-filter is useful, I can do that. Will that change the logic about how the baseline is produced? Also, was your last sentence cut off here? I didn't understand it as written (skips what?)
Sorry, I meant to say that using a baseline can skip processing of a lot
of revisions. If you have a 20k revision history, and specify a baseline
of r19000 then there are a lot of revisions you don't have to export. If
this functionality is added to fast-import-filter, then you would still
export all 20k revisions but then throw 19k of them away.

In other words, I think it's fine to do this in fast-export. But I
wanted to emphasize that generally we add history-modifying commands to
fast-import-filter.
>
>> + help="Export an 'abolute' baseline commit prior to"
>> This should probably say "absolute".
> Indeed.
>
>> + assert(len(revobj.parent_ids))
>> + previd = revobj.parent_ids[0]
>> It seems a bit odd that the code uses the parent of the revision that was
>> specified, rather than that revision itself.
> I wrestled with that a little. If the baseline argument were to take a single revision as an argument, I would agree that that is what should be used. Instead, I required the user to specify a revisionspec, which indicates the commits which they'd like to preserve. If I did not use the parent revision here, then they'd lose the first diff they wanted to be able to see. That might be confusing, though, and I'm not set on this behavior.
>
> Do you think that it would be better to have the baseline option take a revisionspec directly, rather than requiring -r along with this option? (If I continue to require -r, I guess I need to writing some code to enforce that requirement.)
-r specifies a revision - not a delta between revisions, so I do think
--baseline should only include the revision that was specified and not
its left hand side parent. For deltas (changes) we usually use the -c
option.
>
>> Can you add some tests for this new behaviour?
> Sure, I haven't looked at the test suite yet, but I'll give it a try.
Cool. You know where to find me on IRC if there's anything I can help with.

Cheers,

Jelmer

[Andy Grimm (agrimm)]

> >> + assert(len(revobj.parent_ids))
> >> + previd = revobj.parent_ids[0]
> >> It seems a bit odd that the code uses the parent of the revision that was
> >> specified, rather than that revision itself.
> > I wrestled with that a little. If the baseline argument were to take a
> single revision as an argument, I would agree that that is what should be
> used. Instead, I required the user to specify a revisionspec, which indicates
> the commits which they'd like to preserve. If I did not use the parent
> revision here, then they'd lose the first diff they wanted to be able to see.
> That might be confusing, though, and I'm not set on this behavior.
> >
> > Do you think that it would be better to have the baseline option take a
> revisionspec directly, rather than requiring -r along with this option? (If I
> continue to require -r, I guess I need to writing some code to enforce that
> requirement.)
> -r specifies a revision - not a delta between revisions, so I do think
> --baseline should only include the revision that was specified and not
> its left hand side parent. For deltas (changes) we usually use the -c
> option.

Oh, I see your point here. when someone gives the command "bzr diff -r 4..6" for example, 4 is the baseline, and the diff shown is the delta between 4 and 6, so it makes sense to make the baseline the state at the first revision given. Thanks, I'll make that change.

[Andy Grimm (agrimm)]

> > >> + assert(len(revobj.parent_ids))
> > >> + previd = revobj.parent_ids[0]
> > >> It seems a bit odd that the code uses the parent of the revision that was
> > >> specified, rather than that revision itself.
> > > I wrestled with that a little. If the baseline argument were to take a
> > single revision as an argument, I would agree that that is what should be
> > used. Instead, I required the user to specify a revisionspec, which
> indicates
> > the commits which they'd like to preserve. If I did not use the parent
> > revision here, then they'd lose the first diff they wanted to be able to
> see.
> > That might be confusing, though, and I'm not set on this behavior.
> > >
> > > Do you think that it would be better to have the baseline option take a
> > revisionspec directly, rather than requiring -r along with this option? (If
> I
> > continue to require -r, I guess I need to writing some code to enforce that
> > requirement.)
> > -r specifies a revision - not a delta between revisions, so I do think
> > --baseline should only include the revision that was specified and not
> > its left hand side parent. For deltas (changes) we usually use the -c
> > option.
>
> Oh, I see your point here. when someone gives the command "bzr diff -r 4..6"
> for example, 4 is the baseline, and the diff shown is the delta between 4 and
> 6, so it makes sense to make the baseline the state at the first revision
> given. Thanks, I'll make that change.

Upon writing the test, I realized that my original code was using parent there because the starting revision was already being excluded, so getting the parent of the first revision in "interesting" was actually just putting back the starting revision. I've changed how this works now and added a test, which should clear up any confusion about how this behaves.

[Jelmer Vernooij (jelmer)]

review: Approve

[Jelmer Vernooij (jelmer)]

Merged, thanks !

[lp:~agrimm/bzr-fastimport/baseline-commit]

340. By Jelmer Vernooij on 2011-11-23

Merge support for --baseline option to 'bzr export'.

=== modified file 'cmds.py'
--- cmds.py	2011-10-25 23:21:02 +0000
+++ cmds.py	2011-11-22 20:40:47 +0000
@@ -606,6 +606,16 @@
      unsupported in git with an underscore instead, specify
      --rewrite-tag-names.
 
+    :History truncation:
+
+     When code has been significantly refactored over time (e.g., to separate 
+     proprietary code from open source code), it is sometimes convenient to
+     simply truncate the revision history at a certain point.  The --baseline 
+     option, to be used in conjunction with -r, emits a baseline commit
+     containing the state of the entire source tree at the first requested 
+     revision.  This allows a user to produce a tree identical to the original 
+     without munging multiple exports.
+
     :Examples:
 
      To produce data destined for import into Bazaar::
@@ -656,12 +666,16 @@
                         help="Replace characters invalid in git with '_'"
                              " (plain mode only).",
                         ),
+                    Option('baseline',
+                        help="Export an 'absolute' baseline commit prior to"
+                             "the first relative commit",
+                        ),
                      ]
     encoding_type = 'exact'
     def run(self, source, destination=None, verbose=False,
         git_branch="master", checkpoint=10000, marks=None,
         import_marks=None, export_marks=None, revision=None,
-        plain=True, rewrite_tag_names=False):
+        plain=True, rewrite_tag_names=False, baseline=False):
         load_fastimport()
         from bzrlib.branch import Branch
         from bzrlib.plugins.fastimport import exporter
@@ -676,7 +690,7 @@
             outf=outf, git_branch=git_branch, checkpoint=checkpoint,
             import_marks_file=import_marks, export_marks_file=export_marks,
             revision=revision, verbose=verbose, plain_format=plain,
-            rewrite_tags=rewrite_tag_names)
+            rewrite_tags=rewrite_tag_names, baseline=baseline)
         return exporter.run()
 
 
 
=== modified file 'exporter.py'
--- exporter.py	2011-10-25 23:21:02 +0000
+++ exporter.py	2011-11-22 20:40:47 +0000
@@ -148,7 +148,8 @@
 
     def __init__(self, source, outf, git_branch=None, checkpoint=-1,
         import_marks_file=None, export_marks_file=None, revision=None,
-        verbose=False, plain_format=False, rewrite_tags=False):
+        verbose=False, plain_format=False, rewrite_tags=False,
+        baseline=False):
         """Export branch data in fast import format.
 
         :param plain_format: if True, 'classic' fast-import format is
@@ -170,6 +171,7 @@
         self.excluded_revisions = set()
         self.plain_format = plain_format
         self.rewrite_tags = rewrite_tags
+        self.baseline = baseline
         self._multi_author_api_available = hasattr(bzrlib.revision.Revision,
             'get_apparent_authors')
         self.properties_to_exclude = ['authors', 'author']
@@ -213,6 +215,10 @@
             self.note("Calculating the revisions to exclude ...")
             self.excluded_revisions = set([rev_id for rev_id, _, _, _ in
                 self.branch.iter_merge_sorted_revisions(start_rev_id)])
+            if self.baseline:
+                # needed so the first relative commit knows its parent
+                self.excluded_revisions.remove(start_rev_id)
+                view_revisions.insert(0, start_rev_id)
         return list(view_revisions)
 
     def run(self):
@@ -225,6 +231,8 @@
                 self._commit_total)
             if not self.plain_format:
                 self.emit_features()
+            if self.baseline:
+                self.emit_baseline(interesting.pop(0), self.git_branch)
             for revid in interesting:
                 self.emit_commit(revid, self.git_branch)
             if self.branch.supports_tags():
@@ -302,6 +310,16 @@
         for feature in sorted(commands.FEATURE_NAMES):
             self.print_cmd(commands.FeatureCommand(feature))
 
+    def emit_baseline(self, revid, git_branch):
+        # Emit a full source tree of the first commit's parent
+        git_ref = 'refs/heads/%s' % (git_branch,)
+        revobj = self.branch.repository.get_revision(revid)
+        mark = 1
+        self.revid_to_mark[revid] = mark
+        file_cmds = self._get_filecommands(bzrlib.revision.NULL_REVISION, revid)
+        self.print_cmd(self._get_commit_command(git_ref, mark, revobj,
+            file_cmds))
+
     def emit_commit(self, revid, git_branch):
         if revid in self.revid_to_mark or revid in self.excluded_revisions:
             return
 
=== modified file 'tests/test_commands.py'
--- tests/test_commands.py	2011-10-17 10:25:55 +0000
+++ tests/test_commands.py	2011-11-22 20:40:47 +0000
@@ -16,6 +16,7 @@
 """Test the command implementations."""
 
 import os
+import re
 import tempfile
 import gzip
 
@@ -58,6 +59,40 @@
         self.assertIsNot("bla", stream.read())
 
 
+fast_export_baseline_data = """commit refs/heads/master
+mark :1
+committer
+data 15
+add c, remove b
+M 644 inline a
+data 13
+test 1
+test 3
+M 644 inline c
+data 6
+test 4
+commit refs/heads/master
+mark :2
+committer
+data 14
+modify a again
+from :1
+M 644 inline a
+data 20
+test 1
+test 3
+test 5
+commit refs/heads/master
+mark :3
+committer
+data 5
+add d
+from :2
+M 644 inline d
+data 6
+test 6
+"""
+
 class TestFastExport(ExternalBase):
 
     def test_empty(self):
@@ -99,6 +134,45 @@
         # "bad Tag" should be exported as bad_Tag
         self.assertNotEqual(-1, data.find("reset refs/tags/bad_Tag"))
 
+    def test_baseline_option(self):
+        tree = self.make_branch_and_tree("bl")
+
+        # Revision 1
+        file('bl/a', 'w').write('test 1')
+        tree.add('a')
+        tree.commit(message='add a')
+
+        # Revision 2
+        file('bl/b', 'w').write('test 2')
+        file('bl/a', 'a').write('\ntest 3')
+        tree.add('b')
+        tree.commit(message='add b, modify a')
+
+        # Revision 3
+        file('bl/c', 'w').write('test 4')
+        tree.add('c')
+        tree.remove('b')
+        tree.commit(message='add c, remove b')
+
+        # Revision 4
+        file('bl/a', 'a').write('\ntest 5')
+        tree.commit(message='modify a again')
+
+        # Revision 5
+        file('bl/d', 'w').write('test 6')
+        tree.add('d')
+        tree.commit(message='add d')
+
+        # This exports the baseline state at Revision 3,
+        # followed by the deltas for 4 and 5
+        data = self.run_bzr("fast-export --baseline -r 3.. bl")[0]
+        data = re.sub('committer.*', 'committer', data)
+        self.assertEquals(fast_export_baseline_data, data)
+
+        # Also confirm that --baseline with no args is identical to full export
+        data1 = self.run_bzr("fast-export --baseline bl")[0]
+        data2 = self.run_bzr("fast-export bl")[0]
+        self.assertEquals(data1, data2)
 
 simple_fast_import_stream = """commit refs/heads/master
 mark :1