Re: reflogs generated by git-cvsimport
On Fri, Jul 31, 2009 at 08:42:26PM +0300, Kalle Olavi Niemitalo wrote:
Show 12 quoted lines
> If git-cvsimport.perl kept the commit IDs in Perl variables only,
> and then updated the refs once at the end, I'd get the reflogs I
> prefer. However, if the script were interrupted in the middle,
> it would then leave the refs unchanged, and the next cvsimport
> run would have to download the same commits again. I suppose
> that could be fixed with some $SIG{'INT'} handler. But then how
> about the git repack -a -d that git-cvsimport.perl runs every
> 1024 commits: could that lose some commits that have been
> imported from CVS but not yet saved in any ref?
>
> Would it be better to create temporary refs/cvsimport/* and then
> finally update the real refs based on those?The "rebase" command does something similar by performing its changes on a detached HEAD, and then writing the finished result to the real ref. This gives you a detailed log in the HEAD reflog, but branch@{1} shows the rebase as a single step.
However, I'm not sure that such a strategy would work for cvsimport, which (IIRC) operates on branches that are not the HEAD. So I think using refs/cvsimport/* instead of a detached HEAD makes sense.
Trying to do it all internally to the script seems needlessly complex and error prone (and you would lose the detailed reflog, which you might sometimes want for debugging or whatever).
Show 8 quoted lines
> There's also another problem with the reflogs. The current
> version of git-cvsimport sets GIT_COMMITTER_DATE and related
> variables for git-commit-tree, and then leaves them set for
> git-update-ref. So git-update-ref saves the author and date of
> the CVS commit into the reflog. It would be better to save the
> name of the person who is running git cvsimport, and the local
> time. That one I've already fixed in my local version, but I'm
> not really happy with how the code turned out.
Yeah, it probably should not munge the reflog with the CVS committer information. I suspect it would be as easy as the following (totally untested, not even syntax checked) patch:
---
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index e439202..a94c48d 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -742,14 +742,16 @@ sub commit {
}
my $commit_date = strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date));
- $ENV{GIT_AUTHOR_NAME} = $author_name;
- $ENV{GIT_AUTHOR_EMAIL} = $author_email;
- $ENV{GIT_AUTHOR_DATE} = $commit_date;
- $ENV{GIT_COMMITTER_NAME} = $author_name;
- $ENV{GIT_COMMITTER_EMAIL} = $author_email;
- $ENV{GIT_COMMITTER_DATE} = $commit_date;
- my $pid = open2(my $commit_read, my $commit_write,
- 'git-commit-tree', $tree, @commit_args);
+ my $pid = do {
+ local $ENV{GIT_AUTHOR_NAME} = $author_name;
+ local $ENV{GIT_AUTHOR_EMAIL} = $author_email;
+ local $ENV{GIT_AUTHOR_DATE} = $commit_date;
+ local $ENV{GIT_COMMITTER_NAME} = $author_name;
+ local $ENV{GIT_COMMITTER_EMAIL} = $author_email;
+ local $ENV{GIT_COMMITTER_DATE} = $commit_date;
+ open2(my $commit_read, my $commit_write,
+ 'git-commit-tree', $tree, @commit_args);
+ }
# compatibility with git2cvs
substr($logmsg,32767) = "" if length($logmsg) > 32767;
