threads / patch / 9911

patchgit-svnimport: Use separate arguments in the pipe for git-rev-parse

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

## tl;dr

9 messages between Sep 18, 2007 and Sep 21, 2007. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

Matthias Urlichs· Sep 18, 2007, 07:47 UTC · lore
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. ;-)
Show changes to git-svnimport.perl +1 −1
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]
Junio C Hamano· Sep 18, 2007, 08:54 UTC · re: Matthias Urlichs · lore

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

Matthias Urlichs <smurf@smurf.noris.de> writes:
Show 19 quoted lines
> 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?

Matthias Urlichs· Sep 18, 2007, 09:29 UTC · re: Junio C Hamano · lore

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

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:
Show 7 quoted lines
> > -		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.

Show changes to git-svnimport.perl +1 −1
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
Dan Libby· Sep 20, 2007, 19:40 UTC · re: Matthias Urlichs · lore

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

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:
Show 31 quoted lines
> 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
Matthias Urlichs· Sep 21, 2007, 06:11 UTC · re: Dan Libby · lore

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

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
Junio C Hamano· Sep 21, 2007, 06:59 UTC · re: Matthias Urlichs · lore

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

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
Matthias Urlichs· Sep 21, 2007, 10:24 UTC · re: Junio C Hamano · lore

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

Hi,
Junio C Hamano:
Show 9 quoted lines
> 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.
Dan Libby· Sep 21, 2007, 20:21 UTC · re: Matthias Urlichs · lore

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

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:
Show 15 quoted lines
> 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
Dan Libby· Sep 20, 2007, 19:07 UTC · re: Matthias Urlichs · lore

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

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:
Show 21 quoted lines
> 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

← back to recent threads