git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUG?] git-cvsimport: path to cvspsfile
@ 2009-09-23 18:27 Kacper Kornet
  2009-09-23 19:14 ` Jeff King
  0 siblings, 1 reply; 4+ messages in thread
From: Kacper Kornet @ 2009-09-23 18:27 UTC (permalink / raw)
  To: git

Hi,

When I use:

git cvs-import -C <dir> -P <cvspsfile>

it looks for <cvpsfile> relative to <dir>, not the working directory.
Is it a bug or a feature?

Best wishes,
-- 
  Kacper

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG?] git-cvsimport: path to cvspsfile
  2009-09-23 18:27 [BUG?] git-cvsimport: path to cvspsfile Kacper Kornet
@ 2009-09-23 19:14 ` Jeff King
  2009-09-24  5:59   ` Johannes Sixt
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2009-09-23 19:14 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt

On Wed, Sep 23, 2009 at 08:27:56PM +0200, Kacper Kornet wrote:

> When I use:
> 
> git cvs-import -C <dir> -P <cvspsfile>
> 
> it looks for <cvpsfile> relative to <dir>, not the working directory.
> Is it a bug or a feature?

Bug. The script does a chdir() and then looks at the cvspsfile later. I
think "-A" would have the same problem. Here is a totally untested patch
to address the issue. Johannes, will this is_absolute_path actually work
on Windows? I think The Right Way would be to use
File::Spec::file_name_is_absolute, but I haven't checked whether that is
part of core perl and if so, which version it appeared in.

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 1ad20ac..08a30ec 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -579,10 +579,26 @@ sub get_headref ($) {
 	return $r;
 }
 
+sub is_absolute_path {
+	local $_ = shift;
+	return m{^/};
+}
+
+my $user_filename_prepend = '';
+sub munge_user_filename {
+	my $name = shift;
+	return is_absolute_path($name) ?
+		$name :
+		$user_filename_prepend . $name;
+}
+
 -d $git_tree
 	or mkdir($git_tree,0777)
 	or die "Could not create $git_tree: $!";
-chdir($git_tree);
+if ($git_tree ne '.') {
+	$user_filename_prepend = getwd() . '/';
+	chdir($git_tree);
+}
 
 my $last_branch = "";
 my $orig_branch = "";
@@ -644,7 +660,7 @@ unless (-d $git_dir) {
 -f "$git_dir/cvs-authors" and
   read_author_info("$git_dir/cvs-authors");
 if ($opt_A) {
-	read_author_info($opt_A);
+	read_author_info(munge_user_filename($opt_A));
 	write_author_info("$git_dir/cvs-authors");
 }
 
@@ -679,7 +695,7 @@ unless ($opt_P) {
 	$? == 0 or die "git-cvsimport: fatal: cvsps reported error\n";
 	close $cvspsfh;
 } else {
-	$cvspsfile = $opt_P;
+	$cvspsfile = munge_user_filename($opt_P);
 }
 
 open(CVS, "<$cvspsfile") or die $!;

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [BUG?] git-cvsimport: path to cvspsfile
  2009-09-23 19:14 ` Jeff King
@ 2009-09-24  5:59   ` Johannes Sixt
  2009-09-24  6:11     ` Jeff King
  0 siblings, 1 reply; 4+ messages in thread
From: Johannes Sixt @ 2009-09-24  5:59 UTC (permalink / raw)
  To: Jeff King; +Cc: git

Jeff King schrieb:
> Bug. The script does a chdir() and then looks at the cvspsfile later. I
> think "-A" would have the same problem. Here is a totally untested patch
> to address the issue. Johannes, will this is_absolute_path actually work
> on Windows? I think The Right Way would be to use
> File::Spec::file_name_is_absolute, but I haven't checked whether that is
> part of core perl and if so, which version it appeared in.

We have File::Spec::file_name_is_absolute in the msysgit installation. I
suggest you use it. It sounds like a very basic feature, and I'd be
surprised if it were not part of core perl.

-- Hannes

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG?] git-cvsimport: path to cvspsfile
  2009-09-24  5:59   ` Johannes Sixt
@ 2009-09-24  6:11     ` Jeff King
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2009-09-24  6:11 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git

On Thu, Sep 24, 2009 at 07:59:43AM +0200, Johannes Sixt wrote:

> Jeff King schrieb:
> > Bug. The script does a chdir() and then looks at the cvspsfile later. I
> > think "-A" would have the same problem. Here is a totally untested patch
> > to address the issue. Johannes, will this is_absolute_path actually work
> > on Windows? I think The Right Way would be to use
> > File::Spec::file_name_is_absolute, but I haven't checked whether that is
> > part of core perl and if so, which version it appeared in.
> 
> We have File::Spec::file_name_is_absolute in the msysgit installation. I
> suggest you use it. It sounds like a very basic feature, and I'd be
> surprised if it were not part of core perl.

Looks like File::Spec at least goes back to perl 5.004, and we are
already using it. I'll assume file_name_is_absolute has been there a
while, then. Thanks.

-Peff

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-09-24  6:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-23 18:27 [BUG?] git-cvsimport: path to cvspsfile Kacper Kornet
2009-09-23 19:14 ` Jeff King
2009-09-24  5:59   ` Johannes Sixt
2009-09-24  6:11     ` Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).