Re: [PATCH] cvsimport: new -R option: generate revision-to-commit mapping

[Jeff King <peff@peff.net>]

On Sun, Jan 31, 2010 at 12:43:44PM +0000, Aaron Crane wrote:

> Signed-off-by: Aaron Crane <git@aaroncrane.co.uk>
> ---

Please put a bit of the rationale into the commit message. Even a
sentence or two can help later on when somebody is reading the output of
"git log".

>  Documentation/git-cvsimport.txt |   15 ++++++++++++++-
>  git-cvsimport.perl              |   20 ++++++++++++++++----
>  2 files changed, 30 insertions(+), 5 deletions(-)

A basic test would be nice. You should be able to just use your new "-R"
during the import in t9600, and then check that it generated the correct
mapping.

> +-R <revision-to-commit-file>::
> +	Generate a file containing a mapping from CVS revision numbers to
> +	newly-created Git commit IDs.  The generated file will contain one
> +	line for each (filename, revision) pair found by 'cvsps'; each line
> +	will look like

Is mentioning 'cvsps' right here?  cvsps doesn't know about git commit
id's.

> +open my $revision_map, '>', $opt_R
> +    or die "Can't open -R file $opt_R: $!\n"
> +	if defined $opt_R;

You need to use munge_user_filename here to handle relative paths. See
commit f6fdbb6.

Also, should you perhaps be appending to the file instead of truncating
it? Remember that cvsimport can be used incrementally. I wonder if it
would be better to simply have "-R" without an argument to append the
revision map to a file .git/cvs-revisions or something. And then the
user can easily pull it from there after the import.

-Peff