{"thread":{"id":"4639","subject":"CVS import broken?","startedAt":"2006-06-23T16:14:48Z","lastAt":"2006-06-24T19:05:57Z","messageCount":9,"participants":["Johannes Schindelin","Martin Langhoff","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"22364","messageId":"Pine.LNX.4.63.0606231811200.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4639","inReplyTo":null,"subject":"CVS import broken?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-23T16:14:48Z","receivedAt":"2006-06-23T16:14:48Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nI track quite a few CVS repos with 'git-cvsimport -k -i', but recently it \nstopped working (of course, I was not reimporting, but incrementally \ntracking them). I think it is the introduction of one-index-per-branch \npolicy:\n\n> fatal: index file mmap failed (Invalid argument)\n> unable to write to git-update-index: at /.../git-cvsimport line 606, line 277425.\n\nIt seems that git-cvsimport makes a temporary file of size 0, which cannot \nget mmap()ed, because it has size 0.\n\nAny help appreciated,\nDscho\n"},{"id":"22418","messageId":"11511257501323-git-send-email-martin@catalyst.net.nz","threadId":"4639","inReplyTo":"Pine.LNX.4.63.0606231811200.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"[PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles","fromName":"Martin Langhoff","fromEmail":"martin@catalyst.net.nz","sentAt":"2006-06-24T05:09:10Z","receivedAt":"2006-06-24T05:09:10Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"On 6/24/06, Johannes Schindelin wrote:\n> It seems that git-cvsimport makes a temporary file of size 0, which cannot\n> get mmap()ed, because it has size 0.\n\nThis switch to tmpnam() avoids creating the tmpfile in the first place and\nstreamlines the code. This handling of tmpfiles is slightly safer, but there\nis an inherent race condition.\n\n---\nNOTE: (a) I cannot reproduce the problem and (b) this is only lightly tested,\nif trivial.\n\nHowever, this switch to tmpnam() avoids creating the tmpfile in the first place\nand streamlines the code. This usage of tempfiles is open to a race condition\nif someone could guess the name returned by tmpnam, but even this is safer than\nwhat we did before, which was creating a file, closing the fh and then\nclobbering it from git-read-tree.\n\nAnd if someone can guess the name that tmpnam() returns their magic is strong\nenough that they'll go for more interesting targets.\nSigned-off-by: Martin Langhoff \n---\n git-cvsimport.perl | 20 +++++---------------\n 1 files changed, 5 insertions(+), 15 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex f3daa6c..d961b7b 100644\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -17,7 +17,7 @@ use strict;\n use warnings;\n use Getopt::Std;\n use File::Spec;\n-use File::Temp qw(tempfile);\n+use File::Temp qw(tempfile tmpnam);\n use File::Path qw(mkpath);\n use File::Basename qw(basename dirname);\n use Time::Local;\n@@ -467,12 +467,8 @@ my $orig_git_index;\n $orig_git_index = $ENV{GIT_INDEX_FILE} if exists $ENV{GIT_INDEX_FILE};\n \n my %index; # holds filenames of one index per branch\n-{ # init with an index for origin\n- my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',\n-\t\t\t DIR => File::Spec->tmpdir());\n- close ($fh);\n- $index{$opt_o} = $fn;\n-}\n+$index{$opt_o} = tmpnam();\n+\n $ENV{GIT_INDEX_FILE} = $index{$opt_o};\n unless(-d $git_dir) {\n \tsystem(\"git-init-db\");\n@@ -502,10 +498,7 @@ unless(-d $git_dir) {\n \n \t# populate index\n \tunless ($index{$last_branch}) {\n-\t my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',\n-\t\t\t\t DIR => File::Spec->tmpdir());\n-\t close ($fh);\n-\t $index{$last_branch} = $fn;\n+\t $index{$last_branch} = tmpnam();\n \t}\n \t$ENV{GIT_INDEX_FILE} = $index{$last_branch};\n \tsystem('git-read-tree', $last_branch);\n@@ -818,10 +811,7 @@ while() {\n \t\tif(($ancestor || $branch) ne $last_branch) {\n \t\t\tprint \"Switching from $last_branch to $branch\\n\" if $opt_v;\n \t\t\tunless ($index{$branch}) {\n-\t\t\t my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',\n-\t\t\t\t\t\t DIR => File::Spec->tmpdir());\n-\t\t\t close ($fh);\n-\t\t\t $index{$branch} = $fn;\n+\t\t\t $index{$branch} = tmpnam();\n \t\t\t $ENV{GIT_INDEX_FILE} = $index{$branch};\n \t\t\t system(\"git-read-tree\", $branch);\n \t\t\t die \"read-tree failed: $?\\n\" if $?;\n-- \n1.4.0.gcda2\n"},{"id":"22422","messageId":"7vk67711kf.fsf@assigned-by-dhcp.cox.net","threadId":"4639","inReplyTo":"11511257501323-git-send-email-martin@catalyst.net.nz","subject":"Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-24T06:59:28Z","receivedAt":"2006-06-24T06:59:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> On 6/24/06, Johannes Schindelin wrote:\n>> It seems that git-cvsimport makes a temporary file of size 0, which cannot\n>> get mmap()ed, because it has size 0.\n>\n> This switch to tmpnam() avoids creating the tmpfile in the first place and\n> streamlines the code. This handling of tmpfiles is slightly safer, but there\n> is an inherent race condition.\n>\n> ---\n> NOTE: (a) I cannot reproduce the problem and (b) this is only lightly tested,\n> if trivial.\n\nThanks both.\n\nI'd take this to \"next\" after I hear from somebody, most likely\nJohannes, who had trouble with the code earlier that the problem\nis fixed.\n"},{"id":"22430","messageId":"Pine.LNX.4.63.0606241145280.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4639","inReplyTo":"11511257501323-git-send-email-martin@catalyst.net.nz","subject":"Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-24T09:50:09Z","receivedAt":"2006-06-24T09:50:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 24 Jun 2006, Martin Langhoff wrote:\n\n> On 6/24/06, Johannes Schindelin wrote:\n> > It seems that git-cvsimport makes a temporary file of size 0, which cannot\n> > get mmap()ed, because it has size 0.\n> \n> This switch to tmpnam() avoids creating the tmpfile in the first place and\n> streamlines the code. This handling of tmpfiles is slightly safer, but there\n> is an inherent race condition.\n\nThank you. This fixes the error.\n\nHOWEVER, it does not fix the main problem: when I try to git-cvsimport, \nthere is no index for that branch yet, since I used to git-cvsimport with \nthe old cvsimport.\n\nNow, when cvsimport sees there is no index, it evidently assumes that the \ncurrent state is an empty tree, which is *not* true.\n\nThe effect is: the first commit removes all files from the tree which were \nnot touched by the cvs commit. Bad.\n\n> This usage of tempfiles is open to a race condition\n\nI would not care too strongly about that. Eventually, I really would like \nthis file to reside in $GIT_DIR, not /tmp, but whatever. That is not my \nbiggest concern right now. That I cannot update since June 18th, however, \nis.\n\nCiao,\nDscho\n"},{"id":"22432","messageId":"7vslluyika.fsf@assigned-by-dhcp.cox.net","threadId":"4639","inReplyTo":"Pine.LNX.4.63.0606241145280.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-24T10:05:57Z","receivedAt":"2006-06-24T10:05:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> I would not care too strongly about that. Eventually, I really would like \n> this file to reside in $GIT_DIR, not /tmp, but whatever. That is not my \n> biggest concern right now. That I cannot update since June 18th, however, \n> is.\n\nWould reverting 8f732649 in the meantime be an option for you?\n"},{"id":"22433","messageId":"46a038f90606240308g8ec9ceelb18ea505a8708b91@mail.gmail.com","threadId":"4639","inReplyTo":"Pine.LNX.4.63.0606241145280.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2006-06-24T10:08:44Z","receivedAt":"2006-06-24T10:08:44Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 6/24/06, Johannes Schindelin wrote:\n> Thank you. This fixes the error.\n\nYour welcome!\n\n> HOWEVER, it does not fix the main problem: when I try to git-cvsimport,\n> there is no index for that branch yet, since I used to git-cvsimport with\n> the old cvsimport.\n>\n> Now, when cvsimport sees there is no index, it evidently assumes that the\n> current state is an empty tree, which is *not* true.\n>\n> The effect is: the first commit removes all files from the tree which were\n> not touched by the cvs commit. Bad.\n\nI don't quite understand. No it shouldn't be the case -- it should\ncreate the index using git-read-tree based on the tip of the branch.\nRight after the call to tmpnam() the code looks like\n\n $index{$branch} = tmpnam();\n $ENV{GIT_INDEX_FILE} = $index{$branch};\n system(\"git-read-tree\", $branch);\n die \"read-tree failed: $?\\n\" if $?;\n\n> > This usage of tempfiles is open to a race condition\n>\n> I would not care too strongly about that. Eventually, I really would like\n> this file to reside in $GIT_DIR, not /tmp, but whatever. That is not my\n> biggest concern right now. That I cannot update since June 18th, however,\n> is.\n\nIt's worrying me too. Running some tests now...\n\n\n\n\nmartin\n"},{"id":"22436","messageId":"46a038f90606240416n563288f5q99a5ac81723776c3@mail.gmail.com","threadId":"4639","inReplyTo":"7vslluyika.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2006-06-24T11:16:04Z","receivedAt":"2006-06-24T11:16:04Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"Johannes, Junio,\n\nI've managed to repro the problem -- which was totally reproduceable,\nI was just testing the wrong version of the script. The problem was\nquite obvious: when running an incremental, the first head would not\nget the index created properly. Even worse, when forking a new branch,\nthe index would be empty too.\n\nFixed both cases and posted separately. Thanks for the sharp eyes, and\nsorry about the bug!\n\n\nmartin\n"},{"id":"22439","messageId":"7vr71ex05i.fsf@assigned-by-dhcp.cox.net","threadId":"4639","inReplyTo":"46a038f90606240416n563288f5q99a5ac81723776c3@mail.gmail.com","subject":"Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-24T11:28:57Z","receivedAt":"2006-06-24T11:28:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Martin Langhoff\" writes:\n\n> Johannes, Junio,\n>\n> I've managed to repro the problem -- which was totally reproduceable,\n> I was just testing the wrong version of the script. The problem was\n> quite obvious: when running an incremental, the first head would not\n> get the index created properly. Even worse, when forking a new branch,\n> the index would be empty too.\n>\n> Fixed both cases and posted separately.\n\nThanks. Will be in \"next\".\n"},{"id":"22470","messageId":"Pine.LNX.4.63.0606242105160.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4639","inReplyTo":"7vslluyika.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-24T19:05:57Z","receivedAt":"2006-06-24T19:05:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 24 Jun 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > I would not care too strongly about that. Eventually, I really would like \n> > this file to reside in $GIT_DIR, not /tmp, but whatever. That is not my \n> > biggest concern right now. That I cannot update since June 18th, however, \n> > is.\n> \n> Would reverting 8f732649 in the meantime be an option for you?\n\nI think it is worth fixing instead of working around.\n\nCiao,\nDscho\n"}]}