[PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Matthias Urlichs]

Signed-Off-By: Matthias Urlichs <smurf@smurf.noris.de>
---
Please tell me whether that works for you.

Somebody else, preferably its author, can fix git-svn. ;-)

diff --git a/git-svnimport.perl b/git-svnimport.perl
index d3ad5b9..aa5b3b2 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -633,7 +633,7 @@ sub commit {
 
 	my $rev;
 	if($revision > $opt_s and defined $parent) {
-		open(H,"git-rev-parse --verify $parent |");
+		open(H,'-|',"git-rev-parse","--verify",$parent);
 		$rev = <H>;
 		close(H) or do {
 			print STDERR "$revision: cannot find commit '$parent'!\n";
-- 
1.5.2.5

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
"Could a being create the fifty billion galaxies, each with two hundred
 billion stars, then rejoice in the smell of burning goat flesh?"
                         [Ron Patterson]

Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Junio C Hamano]

Matthias Urlichs <smurf@smurf.noris.de> writes:

> Signed-Off-By: Matthias Urlichs <smurf@smurf.noris.de>
> ---
> Please tell me whether that works for you.
>
> Somebody else, preferably its author, can fix git-svn. ;-)
> 
> diff --git a/git-svnimport.perl b/git-svnimport.perl
> index d3ad5b9..aa5b3b2 100755
> --- a/git-svnimport.perl
> +++ b/git-svnimport.perl
> @@ -633,7 +633,7 @@ sub commit {
>  
>  	my $rev;
>  	if($revision > $opt_s and defined $parent) {
> -		open(H,"git-rev-parse --verify $parent |");
> +		open(H,'-|',"git-rev-parse","--verify",$parent);
>  		$rev = <H>;
>  		close(H) or do {
>  			print STDERR "$revision: cannot find commit '$parent'!\n";

I seem to be missing the context, but please describe what
problem this fixes in the commit log message.  I guess some
people use shell metacharacters and/or SP in their branch names
and this is about that problem?

Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Matthias Urlichs]

Some people seem to create SVN branch names with spaces
or other shell metacharacters.

Signed-Off-By: Matthias Urlichs <smurf@smurf.noris.de>
---
Junio C Hamano:
> > -		open(H,"git-rev-parse --verify $parent |");
> > +		open(H,'-|',"git-rev-parse","--verify",$parent);
> 
> I seem to be missing the context, but please describe what
> problem this fixes in the commit log message.  I guess some
> people use shell metacharacters and/or SP in their branch names
> and this is about that problem?

Exactly. Sorry; it seems that the original question hasn't been posted
to the mailing list.

diff --git a/git-svnimport.perl b/git-svnimport.perl
index d3ad5b9..aa5b3b2 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -633,7 +633,7 @@ sub commit {
 
 	my $rev;
 	if($revision > $opt_s and defined $parent) {
-		open(H,"git-rev-parse --verify $parent |");
+		open(H,'-|',"git-rev-parse","--verify",$parent);
 		$rev = <H>;
 		close(H) or do {
 			print STDERR "$revision: cannot find commit '$parent'!\n";



-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
BOFH excuse #11:

magnetic interference from money/credit cards

Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Dan Libby]

Hi, it worked for the small test case.  I am trying it on the large repo now, 
and will let you know how it turns out.  thanks!

On Tuesday 18 September 2007 01:47, Matthias Urlichs wrote:
> Signed-Off-By: Matthias Urlichs <smurf@smurf.noris.de>
> ---
> Please tell me whether that works for you.
>
> Somebody else, preferably its author, can fix git-svn. ;-)
>
> diff --git a/git-svnimport.perl b/git-svnimport.perl
> index d3ad5b9..aa5b3b2 100755
> --- a/git-svnimport.perl
> +++ b/git-svnimport.perl
> @@ -633,7 +633,7 @@ sub commit {
>
>  	my $rev;
>  	if($revision > $opt_s and defined $parent) {
> -		open(H,"git-rev-parse --verify $parent |");
> +		open(H,'-|',"git-rev-parse","--verify",$parent);
>  		$rev = <H>;
>  		close(H) or do {
>  			print STDERR "$revision: cannot find commit '$parent'!\n";
> --
> 1.5.2.5

-- 
Dan Libby

Open Source Consulting
San Jose, Costa Rica
http://osc.co.cr
phone: 011 506 223 7382
Fax: 011 506 223 7359

Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Dan Libby]

Hi Matthias,

So the svnimport (with your patch) chugged along for quite a while, but now 
I've run into a new (related?) problem.  Here's the output:

--
Merge parent branch: 57b2ce794c20e71efa9c7bd0cc71df72e01f5d39
Commit ID 37f501fd2fd0d309b4d3fdce77bac13c84646423
Writing to refs/heads/Verny
DONE: 2385 Verny 37f501fd2fd0d309b4d3fdce77bac13c84646423
Switching from 37f501fd2fd0d309b4d3fdce77bac13c84646423 to 
0e1b0bb88f077b66c6cf537899ab6c0a69d5ec30 (/Cristian new code)
we do not like 'Cristian new code' as a tag name.
Cannot create tag Cristian new code: Bad file descriptor
--

This is a fatal error that stops the import.

regards,

On Tuesday 18 September 2007 03:29, Matthias Urlichs wrote:
> Some people seem to create SVN branch names with spaces
> or other shell metacharacters.
>
> Signed-Off-By: Matthias Urlichs <smurf@smurf.noris.de>
> ---
>
> Junio C Hamano:
> > > -		open(H,"git-rev-parse --verify $parent |");
> > > +		open(H,'-|',"git-rev-parse","--verify",$parent);
> >
> > I seem to be missing the context, but please describe what
> > problem this fixes in the commit log message.  I guess some
> > people use shell metacharacters and/or SP in their branch names
> > and this is about that problem?
>
> Exactly. Sorry; it seems that the original question hasn't been posted
> to the mailing list.
>
> diff --git a/git-svnimport.perl b/git-svnimport.perl
> index d3ad5b9..aa5b3b2 100755
> --- a/git-svnimport.perl
> +++ b/git-svnimport.perl
> @@ -633,7 +633,7 @@ sub commit {
>
>  	my $rev;
>  	if($revision > $opt_s and defined $parent) {
> -		open(H,"git-rev-parse --verify $parent |");
> +		open(H,'-|',"git-rev-parse","--verify",$parent);
>  		$rev = <H>;
>  		close(H) or do {
>  			print STDERR "$revision: cannot find commit '$parent'!\n";

-- 
Dan Libby

Open Source Consulting
San Jose, Costa Rica
http://osc.co.cr
phone: 011 506 223 7382
Fax: 011 506 223 7359

Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Matthias Urlichs]

Hi,

Dan Libby:
> we do not like 'Cristian new code' as a tag name.

Duh? That's a perfectly valid tag name.
I have no idea why git croaked on this one.

Please run 

    strace -f -s300 -eexecve git-svnimport ... 2>&1 | \
		grep check-ref-format | grep -v ENOENT

and mail me the output, replacing the "..." with your normal arguments
of course.

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
Taken as a whole, the universe is absurd.
					-- Walter Savage Landor

Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Junio C Hamano]

Matthias Urlichs <smurf@smurf.noris.de> writes:

>> we do not like 'Cristian new code' as a tag name.
>
> Duh? That's a perfectly valid tag name.

Is it?

$ man git-check-ref-format

Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Matthias Urlichs]

Hi,

Junio C Hamano:
> Matthias Urlichs <smurf@smurf.noris.de> writes:
> 
> >> we do not like 'Cristian new code' as a tag name.
> >
> > Duh? That's a perfectly valid tag name.
> 
> Is it?
> 
> $ man git-check-ref-format

Bah, stupid me. You're right, obviously.

I'll replace them with underscores. :-/

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
Murphy's Law:
If anything can go wrong, it will.

Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse

[Dan Libby]

Hi,

I saw this, so I haven't run the strace command you mentioned.  No need now, 
right?

I'm no expert on these things, but I'd think that it should be replacing (or 
escaping) any characters (not just spaces) that are not allowed by 
git-check-ref-format.

For us, replacing any such characters with _ should work fine.

regards,

On Friday 21 September 2007 04:24, Matthias Urlichs wrote:
> Hi,
>
> Junio C Hamano:
> > Matthias Urlichs <smurf@smurf.noris.de> writes:
> > >> we do not like 'Cristian new code' as a tag name.
> > >
> > > Duh? That's a perfectly valid tag name.
> >
> > Is it?
> >
> > $ man git-check-ref-format
>
> Bah, stupid me. You're right, obviously.
>
> I'll replace them with underscores. :-/

-- 
Dan Libby

Open Source Consulting
San Jose, Costa Rica
http://osc.co.cr
phone: 011 506 223 7382
Fax: 011 506 223 7359