# [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports 7 messages from 2006-06-24 to 2006-06-25. Participants: Martin Langhoff, Sergey Vlasov, Johannes Schindelin, Junio C Hamano, Petr Baudis. Thread: https://gitlist.dev/t/4649 ## Martin Langhoff, 2006-06-24 11:13 Subject: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports Message-ID: <11511475882820-git-send-email-martin@catalyst.net.nz> URL: https://gitlist.dev/e/11511475882820-git-send-email-martin%40catalyst.net.nz ``` 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 --- 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() { 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, 2006-06-24 13:06 Subject: Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports Message-ID: <20060624170638.6f127420.vsu@altlinux.ru> URL: https://gitlist.dev/e/20060624170638.6f127420.vsu%40altlinux.ru In-Reply-To: <11511475882820-git-send-email-martin@catalyst.net.nz> ``` 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 > > --- > 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() { > 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; ``` ## Johannes Schindelin, 2006-06-24 19:42 Subject: Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0606242111250.29667%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <11511475882820-git-send-email-martin@catalyst.net.nz> ``` 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 --- 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 ``` ## Junio C Hamano, 2006-06-25 03:10 Subject: Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports Message-ID: <7v64iqq6ab.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v64iqq6ab.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin 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 > > --- 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, 2006-06-25 08:53 Subject: Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports Message-ID: <20060625085359.GC21864@pasky.or.cz> URL: https://gitlist.dev/e/20060625085359.GC21864%40pasky.or.cz In-Reply-To: <7v64iqq6ab.fsf@assigned-by-dhcp.cox.net> ``` Dear diary, on Sun, Jun 25, 2006 at 05:10:36AM CEST, I got a letter where Junio C Hamano 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. ``` ## Johannes Schindelin, 2006-06-25 09:26 Subject: Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0606251125330.29667%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7v64iqq6ab.fsf@assigned-by-dhcp.cox.net> ``` 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 ``` ## Junio C Hamano, 2006-06-25 09:27 Subject: Re: [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports Message-ID: <7v4py9y48t.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v4py9y48t.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060625085359.GC21864@pasky.or.cz> ``` Petr Baudis writes: > Dear diary, on Sun, Jun 25, 2006 at 05:10:36AM CEST, I got a letter > where Junio C Hamano 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. ```