threads / patch / 4649

patchcvsimport: setup indexes correctly for ancestors and incremental imports

Subject: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports

## tl;dr

7 messages between Jun 24, 2006 and Jun 25, 2006. Diffs are folded; open one to read it.

replies: 6people: 5as markdown or json

Martin Langhoff· Jun 24, 2006, 11:13 UTC · lore

Two bugs had slipped in the "keep one index per branch during import" patch. Both incremental imports and new branches would see an empty tree for their initial commit. Now we cover all the relevant cases, checking whether we actually need to setup the index before preparing the actual commit, and doing it.

Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
 git-cvsimport.perl |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)
Show changes to git-cvsimport.perl +17 −2
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
old mode 100644
new mode 100755
index d961b7b..1c1fd02
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -813,11 +813,26 @@ while(<CVS>) {
 			unless ($index{$branch}) {
 			    $index{$branch} = tmpnam();
 			    $ENV{GIT_INDEX_FILE} = $index{$branch};
-			    system("git-read-tree", $branch);
+			}
+			if ($ancestor) {
+			    system("git-read-tree", $ancestor);
 			    die "read-tree failed: $?\n" if $?;
 			} else {
+			    unless ($index{$branch}) {
+				$index{$branch} = tmpnam();
+				$ENV{GIT_INDEX_FILE} = $index{$branch};
+				system("git-read-tree", $branch);
+				die "read-tree failed: $?\n" if $?;
+			    }
+			}    
+		} else {
+			# just in case
+			unless ($index{$branch}) {
+			    $index{$branch} = tmpnam();
 			    $ENV{GIT_INDEX_FILE} = $index{$branch};
-		        }
+			    system("git-read-tree", $branch);
+			    die "read-tree failed: $?\n" if $?;
+			}
 		}
 		$last_branch = $branch if $branch ne $last_branch;
 		$state = 9;

-- 
1.4.1.rc1.g59c8
Sergey Vlasov· Jun 24, 2006, 13:06 UTC · re: Martin Langhoff · lore

Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports

On Sat, 24 Jun 2006 23:13:08 +1200 Martin Langhoff wrote:
Show 29 quoted lines
> Two bugs had slipped in the "keep one index per branch during import"
> patch. Both incremental imports and new branches would see an
> empty tree for their initial commit. Now we cover all the relevant
> cases, checking whether we actually need to setup the index before
> preparing the actual commit, and doing it.
> 
> Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
> 
> ---
>  git-cvsimport.perl |   19 +++++++++++++++++--
>  1 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> old mode 100644
> new mode 100755
> index d961b7b..1c1fd02
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -813,11 +813,26 @@ while(<CVS>) {
>  			unless ($index{$branch}) {
>  			    $index{$branch} = tmpnam();
>  			    $ENV{GIT_INDEX_FILE} = $index{$branch};
> -			    system("git-read-tree", $branch);
> +			}
> +			if ($ancestor) {
> +			    system("git-read-tree", $ancestor);
>  			    die "read-tree failed: $?\n" if $?;
>  			} else {
> +			    unless ($index{$branch}) {

This seems to be dead code - even if $index{$branch} was not set, it will be set inside "unless ($index{$branch})" above. Or there is another bug here?

Show 18 quoted lines
> +				$index{$branch} = tmpnam();
> +				$ENV{GIT_INDEX_FILE} = $index{$branch};
> +				system("git-read-tree", $branch);
> +				die "read-tree failed: $?\n" if $?;
> +			    }
> +			}    
> +		} else {
> +			# just in case
> +			unless ($index{$branch}) {
> +			    $index{$branch} = tmpnam();
>  			    $ENV{GIT_INDEX_FILE} = $index{$branch};
> -		        }
> +			    system("git-read-tree", $branch);
> +			    die "read-tree failed: $?\n" if $?;
> +			}
>  		}
>  		$last_branch = $branch if $branch ne $last_branch;
>  		$state = 9;
Johannes Schindelin· Jun 24, 2006, 19:42 UTC · re: Martin Langhoff · lore

Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports

Hi,
almost happy.
On Sat, 24 Jun 2006, Martin Langhoff wrote:
Show 16 quoted lines
>  			unless ($index{$branch}) {
>  			    $index{$branch} = tmpnam();
>  			    $ENV{GIT_INDEX_FILE} = $index{$branch};
> -			    system("git-read-tree", $branch);
> +			}
> +			if ($ancestor) {
> +			    system("git-read-tree", $ancestor);
>  			    die "read-tree failed: $?\n" if $?;
>  			} else {
> +			    unless ($index{$branch}) {
> +				$index{$branch} = tmpnam();
> +				$ENV{GIT_INDEX_FILE} = $index{$branch};
> +				system("git-read-tree", $branch);
> +				die "read-tree failed: $?\n" if $?;
> +			    }
> +			}    
This line ^ seems to have white space at the end.

Also, in my case, this is still broken. $index{$branch} is set, $ancestor not, but $ENV{GIT_INDEX_FILE} ne $index{$branch}.

You can reproduce this by importing into branch origin while branch master (which is not equal to origin) is checked out.

Plus, the initial setting of GIT_INDEX_FILE forgot to perform a git-read-tree.

How about this on top of your patch (which fixes things with my setup):

--- [PATCH] cvsimport: always set $ENV{GIT_INDEX_FILE} to $index{$branch}

Also, make sure that the initial git-read-tree is performed.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 git-cvsimport |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

--- git-cvsimport.perl 2006-06-24 20:30:33.000000000 +0200 +++ git-cvsimport 2006-06-24 21:35:42.000000000 +0200

Show changes to diff +8 −7
@@ -470,6 +470,9 @@
 $index{$opt_o} = tmpnam();
 
 $ENV{GIT_INDEX_FILE} = $index{$opt_o};
+system("git-read-tree", $opt_o);
+die "read-tree failed: $?\n" if $?;
+
 unless(-d $git_dir) {
 	system("git-init-db");
 	die "Cannot init the GIT db at $git_tree: $?\n" if $?;
@@ -813,17 +816,15 @@
 			unless ($index{$branch}) {
 			    $index{$branch} = tmpnam();
 			    $ENV{GIT_INDEX_FILE} = $index{$branch};
+			    system("git-read-tree", $branch);
+			    die "read-tree failed: $?\n" if $?;
 			}
+			# just in case
+			$ENV{GIT_INDEX_FILE} = $index{$branch};
 			if ($ancestor) {
+			    print "have ancestor $ancestor" if $opt_v;
 			    system("git-read-tree", $ancestor);
 			    die "read-tree failed: $?\n" if $?;
-			} else {
-			    unless ($index{$branch}) {
-				$index{$branch} = tmpnam();
-				$ENV{GIT_INDEX_FILE} = $index{$branch};
-				system("git-read-tree", $branch);
-				die "read-tree failed: $?\n" if $?;
-			    }
 			}
 		} else {
 			# just in case
Junio C Hamano· Jun 25, 2006, 03:10 UTC · re: Johannes Schindelin · lore

Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 10 quoted lines
> How about this on top of your patch (which fixes things with my setup):
>
> ---
> [PATCH] cvsimport: always set $ENV{GIT_INDEX_FILE} to $index{$branch}
>
> Also, make sure that the initial git-read-tree is performed.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>
> ---

Please please please do not use --- between the cover letter and commit message body if you choose to do the cover letter first.

Superficial look tells me this would be OK, so it will go to "next". Please fix things up as needed and let's have this in upcoming 1.4.1 whenever that happens.

Petr Baudis· Jun 25, 2006, 08:53 UTC · re: Junio C Hamano · lore

Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports

Dear diary, on Sun, Jun 25, 2006 at 05:10:36AM CEST, I got a letter where Junio C Hamano <junkio@cox.net> said that...

> Please please please do not use --- between the cover letter and
> commit message body if you choose to do the cover letter first.

Oops, I suspect that I'm a huge offender in this regard in that case. Can I format my patch mails so that they look like natural replies _and_ you can still apply them easily?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.
Junio C Hamano· Jun 25, 2006, 09:27 UTC · re: Petr Baudis · lore

Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports

Petr Baudis <pasky@suse.cz> writes:
Show 8 quoted lines
> Dear diary, on Sun, Jun 25, 2006 at 05:10:36AM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
>> Please please please do not use --- between the cover letter and
>> commit message body if you choose to do the cover letter first.
>
> Oops, I suspect that I'm a huge offender in this regard in that case.
> Can I format my patch mails so that they look like natural replies _and_
> you can still apply them easily?
It's not a big deal.

I do it with scissors mark "-- >8 --" because I can use "git-am" while still in GNUS with '|' (gnus-summary-pipe-output) on the message, and immediately after that I can amend the commit with "git-commit --amend" to remove everything up to the scissors mark.

If you have --- then I have to save the message in a separate file with 'C-o' (gnus-summary-save-article-mail), open the file and remove up to that first --- in the editor, and then run git-am on the file.

Only when the cover letter is short, I'd prefer Linus style, which places cover letter material after the --- and the diffstat, although that makes the message like top-posting.

Johannes Schindelin· Jun 25, 2006, 09:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports

Hi,
On Sat, 24 Jun 2006, Junio C Hamano wrote:
> Please please please do not use --- between the cover letter and commit 
> message body if you choose to do the cover letter first.
Sorry. Will not happen again.

Ciao, Dscho

← back to recent threads