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

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

[Martin Langhoff]

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}) {
+				$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

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

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

On Sat, 24 Jun 2006 23:13:08 +1200 Martin Langhoff wrote:

> 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?

> +				$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;

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

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

[Johannes Schindelin]

Hi,

almost happy.

On Sat, 24 Jun 2006, Martin Langhoff wrote:

>  			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
@@ -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

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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.

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

[Petr Baudis]

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.

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

[Johannes Schindelin]

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

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

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> 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.