support bzr per-file comments

[vuvova]

Bazaar supports per-file comments in revisions - using bzr-gtk one can write a global revisions comment and comments for individual files too (this was apparently modeled after a similar feature in BitKeeper). When exporting a bzr repository to git these per-file comments are lost.

This patch makes them to be not lost but appear as a part of the commit comment instead. Empty file comments and file comments identical to the commit comment are skipped (they originate from tools and policies that require writing per-file comments and users simply copy-paste revision comment over, these comments add no value as a part of the commit comment). Corrupted file comments are ignored too (there is one old - 2008 - revision corrupted like that in the MariaDB/MySQL tree).

[felipec]

Looks fine to me. I added comments into the patch. The only thing is that it's missing a test.

[vuvova]

I would prefer to have a warning — I mean, as a user. I wrote this patch to convert MariaDB repositories to git, and I wanted to know that our repository is corrupted and what revision is at fault. Then I've looked at this revision with various bzr tools and decided that it's fine to ignore its per-file comments. I wouldn't want the tool to make this decision silently for me without saying anything. But after all, it's a minor detail, I don't expect these broken revisions to be seen often. We've only got one in our very active and almost 15-year-old repository (converted from BitKeeper to bzr).

I can rewrite the logic for appending new lines, is it still needed? (your first comments suggests that, the second says that only the test is missing)

Test... It's a bit complicated. As far as I know, bzr cannot create per-file comments from the command line. And bzr gtk gui is not scriptable. I see two options

• create a small repository and check it in as binary files (or a tarball). Then the test will only test the export, it won't run bzr to create the repository.
• or write a python script that uses bzrlib to create a revision with per-file comments. The test will run bzr to create a repository and add some files, then run the script to commit a revision with per-file comments. What would you prefer?

[felipec]

It's not needed to rewrite the logic, but would be nice. I'll rewrite the logic if you don't want to,

However, I'll need a way to verify it works, you mention adding the comments with bzr gtk gui, I'll give that a try.

If it's not possible to easily script the test, then it might not make sense to have one.

As for the warning, I would be OK with adding a warning if there was something that could be done about such corrupt comments, but what can the user do after they notice such a corrupt comment? Can they fix it?

[vuvova]

okay. I've rewritten the logic as you suggested. And also added a test case, by committing binary bzr repository. This way I also also able to test a corrupted per file comments.

Anyway, the test case is in a separate commit, so that you could easily ignore it if you won't like it.

[vuvova]

Felipe, did you have a chance to look at the updated patch yet?

[felipec]

Yes, I took a look. First of all, I couldn't run the tests unless I set TEST_DIRECTORY. Then, I see a failure because the variable file_info doesn't exist, after fixing that. I get the following diff:

--- /home/felipec/dev/git-remote-bzr/test/per-file/expected 2014-05-19 22:59:45.393419834 +0000
+++ actual  2014-05-21 11:16:36.762234454 +0000
@@ -1,10 +1,6 @@
 test for a corrupted per-file comment
 ===
-global commit message copy-pasted for file2
-readme:
-  committed
+global commit message copy-pasted for file2 readme:   committed
 ===
-this is a global commit message
-readme:
-  this is commit message for 'readme'
+this is a global commit message readme:   this is commit message for 'readme'
 ===
not ok 15 - per-file comments

[felipec]

This fixes the problem:

--- a/git-remote-bzr
+++ b/git-remote-bzr
@@ -314,7 +314,7 @@ def export_branch(repo, name):
             author = "%s %u %s" % (fixup_user(author), time, gittz(tz))
         else:
             author = committer
-        msg = rev.message.encode('utf-8') + '\n'
+        msg = rev.message.encode('utf-8')

         if rev.properties.has_key('file-info'):
             from bzrlib import bencode
@@ -341,7 +341,7 @@ def export_branch(repo, name):
               for l in fmsg.split('\n'):
                   file_comments.append('  ' + l)

-            msg += '\n'.join(file_info) + '\n' 
+            msg += '\n\n' + '\n'.join(file_comments) + '\n'

         if len(parents) == 0:
             parent = bzrlib.revision.NULL_REVISION

[felipec]

All right, I've pushed my proposed patch squashing all the changes:

https://github.com/felipec/git-remote-bzr/commit/536fa514d28c1b1c3727522e09dc9b6f65bfe227

[vuvova]

great, thanks a lot!

[felipec]

Er, it's still not pushed to master. I pushed to a separate branch so you could test and verify it's correct.

[vuvova]

Right, I've noticed only after closing the issue. Sorry. I'll re-export the whole MariaDB tree (that takes about two hours) and let you know how it worked.

[vuvova]

Yes, seems to work fine!

[felipec]

All right. I've pushed it to master.

Thanks!