Merge lp:~mwhudson/launchpad-cscvs/more-revision-properties into lp:launchpad-cscvs

[Michael Hudson-Doyle]

[Michael Hudson-Doyle (mwhudson)]

Hi Paul,

Hope you don't mind me slinging this cscvs branch at you. It addresses bug #164065 where Jelmer asks for us to record more information about where an import comes from. I'm also going to ask Jelmer to look at this merge proposal...

Cheers,
mwh

[Jelmer Vernooij (jelmer)]

Hi Michael,

The code changes look good, and the information provided would be sufficient for the in-progress "bzr rebase --by-path" to use. I'm not familiar with the Launchpad coding style, so I'll leave it up to rockstar to comment on that.

review: Approve

[Paul Hummer (rockstar)]

So, I feel the technical merits of this branch have really been addressed by Jelmer's comments, and that I have nothing to add. I do, however, wonder how this is going to affect existing branches. It looks like it's not, until we make the change, but it also looks like it's going to require a rebasing of these existing import branches to get these extra rev props. Is this true? Are we okay with this. If we're okay with breaking old branches, why aren't we just going directly to bzr-svn.

I'm approving this branch, but also making a note that I'd like to chat with you about this when you're around.

review: Approve

[Jelmer Vernooij (jelmer)]

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

Paul Hummer wrote:
> Review: Approve So, I feel the technical merits of this branch have
> really been addressed by Jelmer's comments, and that I have nothing
> to add. I do, however, wonder how this is going to affect existing
> branches. It looks like it's not, until we make the change, but it
> also looks like it's going to require a rebasing of these existing
> import branches to get these extra rev props. Is this true? Are
> we okay with this. If we're okay with breaking old branches, why
> aren't we just going directly to bzr-svn.
>
> I'm approving this branch, but also making a note that I'd like to
> chat with you about this when you're around
We plan to add magic in bzr-rebase that allows it to recognize
revisions that were imported by bzr-svn and cscvs independently are
both created from the same original svn revision. These new properties
should make it easier in the future for people to rebase their
existing branches that are were derived from cscvs imports onto a
bzr-svn branch by running a simple command ("bzr svn-upgrade") rather
than having to replay every single commit manually.

Re-annotating old revisions with this information would make rebase's
job a bit easier, but shouldn't strictly be necessary and it would
indeed kind of defeat the purpose of doing all of this in the first
place :-)

Cheers,

Jelmer
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iJwEAQECAAYFAko32dYACgkQDLQl4QYPZuVjGQP+PwMrSHyPRGm5Omk5dPvqumuO
C4q7MCskIeizYCBDlBqwwfEjt4+hi9t9mmDxADx0P4sGrapLvmQK1c4a5FDlMG2d
kr2cFBbAYBynldNEdBN5e/kDn/x+c4XGCFje6Cr0ruV1KiXO43V30Eeg+b24tqA+
5BX6pODB78obdjz5NiY=
=q8yG
-----END PGP SIGNATURE-----

[lp:~mwhudson/launchpad-cscvs/more-revision-properties]

433. By Michael Hudson-Doyle on 2009-06-16

fix a test

=== modified file 'modules/CVS/changeset.py'
--- modules/CVS/changeset.py	2008-09-15 19:03:13 +0000
+++ modules/CVS/changeset.py	2009-06-16 03:28:06 +0000
@@ -173,6 +173,10 @@
         """the 'name' of this changeset"""
         return ".".join([self.branchName, str(self.index)])
 
+    def get_extra_revprops(self):
+        """See `IRevision`."""
+        return {}
+
     def get_log(self):
         return self.log
 

=== modified file 'modules/SCM/__init__.py'
--- modules/SCM/__init__.py	2007-05-07 18:00:34 +0000
+++ modules/SCM/__init__.py	2009-06-16 08:39:52 +0000
@@ -124,7 +124,10 @@
         """my date in seconds since the epoch"""
 
     def get_fullname():
-        """the fully qualified identifier to refere to this revision"""
+        """the fully qualified identifier to refer to this revision"""
+
+    def get_extra_revprops():
+        """any extra data to store as revision properties"""
 
     def get_log():
         """my unparsed, raw log"""

=== modified file 'modules/cscvs/bzr.py'
--- modules/cscvs/bzr.py	2008-09-16 16:47:31 +0000
+++ modules/cscvs/bzr.py	2009-06-16 03:28:06 +0000
@@ -163,6 +163,7 @@
         revprops = {
             'cscvs-id': revision.get_fullname(),
             'converted-by': 'launchpad.net'}
+        revprops.update(revision.get_extra_revprops())
         cvs_default_filler = revision.getCvsDefaultFiller()
         if cvs_default_filler is not None:
             revprops['cscvs-defaultfiller'] = cvs_default_filler

=== modified file 'modules/cscvs/tests/test_bzr.py'
--- modules/cscvs/tests/test_bzr.py	2008-09-16 16:47:31 +0000
+++ modules/cscvs/tests/test_bzr.py	2009-06-16 06:04:43 +0000
@@ -192,6 +192,15 @@
             sorted([self.dir_relpath, self.file_relpath]))
 
 
+class TestChangeset(CVS.TemporaryChangeset):
+    """A changeset that allows specification of extra revision properties."""
+
+    _extra_revprops = {}
+
+    def get_extra_revprops(self):
+        return self._extra_revprops
+
+
 class BzrCommitTestCase(BzrWorkingTreeTestCase):
 
     def setUp(self):
@@ -205,10 +214,9 @@
         self.message = 'message'
         self.default_filler = '1.1.1.1 1.1 file_name'
 
-    def makeChangeset(self, is_filler):
+    def makeChangeset(self, is_filler, extra_revprops):
         """Create test changeset, either normal or filler."""
-        changeset = CVS.TemporaryChangeset(
-            self.branch, self.creator, self.logger)
+        changeset = TestChangeset(self.branch, self.creator, self.logger)
         changeset.log = self.message
         changeset.index = self.branch_index
         factory = TestUtil.RevisionFactory()
@@ -221,18 +229,20 @@
         if is_filler:
             revision.fromRevision = '1.1'
         changeset.append(revision)
+        if extra_revprops is not None:
+            changeset._extra_revprops = extra_revprops
         return changeset
 
-    def commit(self, strict, is_filler):
+    def commit(self, strict, is_filler, extra_revprops=None):
         """Commit with a test changeset, either normal or filler."""
-        changeset = self.makeChangeset(is_filler)
+        changeset = self.makeChangeset(is_filler, extra_revprops)
         self.tree.commitWithAutomaticLog(
             changeset, logger=self.logger, strict=strict, summarize=False)
 
 
 class TestBzrWorkingTreeCommit(BzrCommitTestCase):
 
-    def assertRevisionEqualsExpected(self, is_filler):
+    def assertRevisionEqualsExpected(self, is_filler, extra_revprops=None):
         """Check that the last revision matches expectations."""
         revision = self.getLastRevision()
         self.assertEqual(revision.committer, self.creator)
@@ -245,6 +255,8 @@
             'converted-by': 'launchpad.net'}
         if is_filler:
             expected_properties['cscvs-defaultfiller'] = self.default_filler
+        if extra_revprops is not None:
+            expected_properties.update(extra_revprops)
         self.assertEqual(revision.properties, expected_properties)
 
     def getLastRevision(self):
@@ -312,6 +324,13 @@
         self.commit(strict=False, is_filler=False)
         self.assertNotEqual(self.getLastRevision(), None)
 
+    def testCommitWithExtraProperties(self):
+        # commitWithAutomaticLog stores any extra revision properties defined
+        # by the revision.
+        self.commit(strict=True, is_filler=False, extra_revprops={'foo': 'bar'})
+        self.assertRevisionEqualsExpected(
+            is_filler=False, extra_revprops={'foo': 'bar'})
+
 
 class TestBzrBranch(BzrCommitTestCase):
 

=== modified file 'modules/svn_oo/Revision.py'
--- modules/svn_oo/Revision.py	2009-04-30 07:00:14 +0000
+++ modules/svn_oo/Revision.py	2009-06-16 08:37:12 +0000
@@ -14,6 +14,7 @@
 from svn.core import (svn_opt_revision_t,
                       svn_opt_revision_head,
                       svn_opt_revision_number)
+from svn.wc import (svn_wc_adm_open, svn_wc_entry)
 import pysvn
 
 import SCM
@@ -96,6 +97,14 @@
         """The current revision identifier."""
         return str(self.rev.number)
 
+    def get_extra_revprops(self):
+        """See `IRevision`."""
+        adm = svn_wc_adm_open(None, self.path, False, 0)
+        uuid = svn_wc_entry(self.path, adm, True).uuid
+        return {'cscvs-svn-branch-path': self.repoPrefix(),
+                'cscvs-svn-repository-uuid': uuid,
+                'cscvs-svn-revision-number': str(self.rev.number)}
+
     def get_log(self):
         """My raw log, as an utf8 encoded str."""
         return self.log['message'].encode('utf8')

=== modified file 'modules/svn_oo/tests/test_revision.py'
--- modules/svn_oo/tests/test_revision.py	2009-01-25 16:23:16 +0000
+++ modules/svn_oo/tests/test_revision.py	2009-06-16 08:24:20 +0000
@@ -12,6 +12,7 @@
 import unittest
 
 import pysvn
+import svn.repos, svn.fs
 from zope.interface.verify import verifyClass
 
 import SCM
@@ -128,6 +129,17 @@
         # svn_oo.Revision.get_fullname return something useful
         self.assertEqual(self.rev1().get_fullname(), "1")
 
+    def testExtraRevprops(self):
+        # svn_oo.Revision.get_extra_revprops returns information about where
+        # the revision came from.
+        repo = svn.repos.open(repo_cache.oneFileRepo().repo.path)
+        uuid = svn.fs.get_uuid(svn.repos.fs(repo))
+        self.assertEqual(
+            {'cscvs-svn-branch-path': 'modulefile1',
+             'cscvs-svn-repository-uuid': uuid,
+             'cscvs-svn-revision-number': '1'},
+            self.rev1().get_extra_revprops())
+
     def testGetDate(self):
         # svn_oo.Revision.get_timestamp does something useful
         csetdir=",,csetget_fullname"