{"thread":{"id":"11051","subject":"[PATCH 1/3] Add basic cvsimport tests","startedAt":"2007-11-28T18:55:04Z","lastAt":"2007-11-30T18:41:23Z","messageCount":16,"participants":["Jeff King","Johannes Schindelin","Junio C Hamano","David Kastrup","Andreas Ericsson","Emanuele Giaquinta"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"61269","messageId":"20071128185504.GA11236@coredump.intra.peff.net","threadId":"11051","inReplyTo":"20071128165746.GC20308@coredump.intra.peff.net","subject":"Re: git-cvsimport bug","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T18:55:04Z","receivedAt":"2007-11-28T18:55:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 11:57:46AM -0500, Jeff King wrote:\n\n> Some of git-cvsimport is quite old, and it accesses the ref files\n> directly. It should be fairly easy to fix; I will post a patch in a few\n> minutes.\n\nThe patch series is:\n\n  1/3: Add basic cvsimport tests\n\n       We had no tests before, so this at least gives a sanity check. I\n       added a t9600 series, though perhaps the cvs-related tests\n       (cvsserver and exportcommit) should collapse to a single t9[0-9]*\n       series.\n\n  2/3: cvsimport: use show-ref to support packed refs\n\n       This fix is hopefully obvious, and the included test fails\n       without it (and this should probably fix Emanuele's problem).\n\n  3/3: cvsimport: miscellaneous packed-ref fixes\n\n       This fixes all of the packed-ref problem spots I could find.\n       However, I have no tests that show the problems or that verify\n       that the fixes are sane. So apply with caution.\n\n       Probably cvsimport would benefit greatly from a conversion to\n       Git.pm, but that is likely to involve a lot of rewriting, and I\n       have neither the time nor the inclination for that right now.\n\n-Peff\n"},{"id":"61268","messageId":"20071128185546.GA11320@coredump.intra.peff.net","threadId":"11051","inReplyTo":"20071128185504.GA11236@coredump.intra.peff.net","subject":"[PATCH 1/3] Add basic cvsimport tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T18:55:46Z","receivedAt":"2007-11-28T18:55:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We weren't even testing basic things before, so let's at\nleast try importing and updating a trivial repository, which\nwill catch total breakage.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t9600-cvsimport.sh |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 99 insertions(+), 0 deletions(-)\n create mode 100755 t/t9600-cvsimport.sh\n\ndiff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh\nnew file mode 100755\nindex 0000000..1ee06bb\n--- /dev/null\n+++ b/t/t9600-cvsimport.sh\n@@ -0,0 +1,99 @@\n+#!/bin/sh\n+\n+test_description='git-cvsimport basic tests'\n+. ./test-lib.sh\n+\n+if ! ( type cvs && type cvsps ) >/dev/null 2>&1\n+then\n+\ttest_expect_success 'skipping cvsimport tests, cvs/cvsps not found' ''\n+\ttest_done\n+\texit\n+fi\n+\n+CVSROOT=$(pwd)/cvsroot\n+export CVSROOT\n+# for clean cvsps cache\n+HOME=$(pwd)\n+export HOME\n+\n+test_expect_success 'setup cvsroot' 'cvs init'\n+\n+test_expect_success 'setup a cvs module' '\n+\n+\tmkdir $CVSROOT/module &&\n+\tcvs co -d module-cvs module &&\n+\tcd module-cvs &&\n+\tcat <<EOF >o_fortuna &&\n+O Fortuna\n+velut luna\n+statu variabilis,\n+\n+semper crescis\n+aut decrescis;\n+vita detestabilis\n+\n+nunc obdurat\n+et tunc curat\n+ludo mentis aciem,\n+\n+egestatem,\n+potestatem\n+dissolvit ut glaciem.\n+EOF\n+\tcvs add o_fortuna &&\n+\tcat <<EOF >message &&\n+add \"O Fortuna\" lyrics\n+\n+These public domain lyrics make an excellent sample text.\n+EOF\n+\tcvs commit -F message &&\n+\tcd ..\n+'\n+\n+test_expect_success 'import a trivial module' '\n+\n+\tgit cvsimport -a -z 0 -C module-git module &&\n+\tgit diff module-cvs/o_fortuna module-git/o_fortuna\n+\n+'\n+\n+test_expect_success 'update cvs module' '\n+\n+\tcd module-cvs &&\n+\tcat <<EOF >o_fortuna &&\n+O Fortune,\n+like the moon\n+you are changeable,\n+\n+ever waxing\n+and waning;\n+hateful life\n+\n+first oppresses\n+and then soothes\n+as fancy takes it;\n+\n+poverty\n+and power\n+it melts them like ice.\n+EOF\n+\tcat <<EOF >message &&\n+translate to English\n+\n+My Latin is terrible.\n+EOF\n+\tcvs commit -F message &&\n+\tcd ..\n+'\n+\n+test_expect_success 'update git module' '\n+\n+\tcd module-git &&\n+\tgit cvsimport -a -z 0 module &&\n+\tgit merge origin &&\n+\tcd .. &&\n+\tgit diff module-cvs/o_fortuna module-git/o_fortuna\n+\n+'\n+\n+test_done\n-- \n1.5.3.6.2039.g0495\n"},{"id":"61270","messageId":"20071128185611.GB11320@coredump.intra.peff.net","threadId":"11051","inReplyTo":"20071128185504.GA11236@coredump.intra.peff.net","subject":"[PATCH 2/3] cvsimport: use show-ref to support packed refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T18:56:11Z","receivedAt":"2007-11-28T18:56:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Previously, if refs were packed, git-cvsimport would assume\nthat particular refs did not exist. This could lead to, for\nexample, overwriting previous 'origin' commits that were\npacked.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-cvsimport.perl   |   24 +++++++++---------------\n t/t9600-cvsimport.sh |    2 ++\n 2 files changed, 11 insertions(+), 15 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex efa6a0c..b852f2f 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -527,18 +527,12 @@ sub is_sha1 {\n \treturn $s =~ /^[a-f0-9]{40}$/;\n }\n \n-sub get_headref ($$) {\n-    my $name    = shift;\n-    my $git_dir = shift;\n-\n-    my $f = \"$git_dir/$remote/$name\";\n-    if (open(my $fh, $f)) {\n-\t    chomp(my $r = <$fh>);\n-\t    is_sha1($r) or die \"Cannot get head id for $name ($r): $!\";\n-\t    return $r;\n-    }\n-    die \"unable to open $f: $!\" unless $! == POSIX::ENOENT;\n-    return undef;\n+sub get_headref ($) {\n+\tmy $name = shift;\n+\tmy $r = `git show-ref -s '$name'`;\n+\treturn undef unless $? == 0;\n+\tchomp $r;\n+\treturn $r;\n }\n \n -d $git_tree\n@@ -698,7 +692,7 @@ my (@old,@new,@skipped,%ignorebranch);\n $ignorebranch{'#CVSPS_NO_BRANCH'} = 1;\n \n sub commit {\n-\tif ($branch eq $opt_o && !$index{branch} && !get_headref($branch, $git_dir)) {\n+\tif ($branch eq $opt_o && !$index{branch} && !get_headref($branch)) {\n \t    # looks like an initial commit\n \t    # use the index primed by git-init\n \t    $ENV{GIT_INDEX_FILE} = \"$git_dir/index\";\n@@ -722,7 +716,7 @@ sub commit {\n \tupdate_index(@old, @new);\n \t@old = @new = ();\n \tmy $tree = write_tree();\n-\tmy $parent = get_headref($last_branch, $git_dir);\n+\tmy $parent = get_headref($last_branch);\n \tprint \"Parent ID \" . ($parent ? $parent : \"(empty)\") . \"\\n\" if $opt_v;\n \n \tmy @commit_args;\n@@ -733,7 +727,7 @@ sub commit {\n \tforeach my $rx (@mergerx) {\n \t\tnext unless $logmsg =~ $rx && $1;\n \t\tmy $mparent = $1 eq 'HEAD' ? $opt_o : $1;\n-\t\tif (my $sha1 = get_headref($mparent, $git_dir)) {\n+\t\tif (my $sha1 = get_headref($mparent)) {\n \t\t\tpush @commit_args, '-p', $mparent;\n \t\t\tprint \"Merge parent branch: $mparent\\n\" if $opt_v;\n \t\t}\ndiff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh\nindex 1ee06bb..3338d44 100755\n--- a/t/t9600-cvsimport.sh\n+++ b/t/t9600-cvsimport.sh\n@@ -57,6 +57,8 @@ test_expect_success 'import a trivial module' '\n \n '\n \n+test_expect_success 'pack refs' 'cd module-git && git gc && cd ..'\n+\n test_expect_success 'update cvs module' '\n \n \tcd module-cvs &&\n-- \n1.5.3.6.2039.g0495\n"},{"id":"61271","messageId":"20071128185628.GC11320@coredump.intra.peff.net","threadId":"11051","inReplyTo":"20071128185504.GA11236@coredump.intra.peff.net","subject":"[PATCH 3/3] cvsimport: miscellaneous packed-ref fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T18:56:28Z","receivedAt":"2007-11-28T18:56:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These were found with a grep for '$git_dir'; they all\nreplace a direct access of \"$git_dir/refs/...\" with a call\nto git-show-ref or git-update-ref.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-cvsimport.perl |   20 +++++++++-----------\n 1 files changed, 9 insertions(+), 11 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex b852f2f..1b5f187 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -864,29 +864,27 @@ while (<CVS>) {\n \t\t\t\tprint STDERR \"Branch $branch erroneously stems from itself -- changed ancestor to $opt_o\\n\";\n \t\t\t\t$ancestor = $opt_o;\n \t\t\t}\n-\t\t\tif (-f \"$git_dir/$remote/$branch\") {\n+\t\t\tif (defined get_headref(\"$remote/$branch\")) {\n \t\t\t\tprint STDERR \"Branch $branch already exists!\\n\";\n \t\t\t\t$state=11;\n \t\t\t\tnext;\n \t\t\t}\n-\t\t\tunless (open(H,\"$git_dir/$remote/$ancestor\")) {\n+\t\t\tmy $id = get_headref(\"$remote/$ancestor\");\n+\t\t\tif (!$id) {\n \t\t\t\tprint STDERR \"Branch $ancestor does not exist!\\n\";\n \t\t\t\t$ignorebranch{$branch} = 1;\n \t\t\t\t$state=11;\n \t\t\t\tnext;\n \t\t\t}\n-\t\t\tchomp(my $id = <H>);\n-\t\t\tclose(H);\n-\t\t\tunless (open(H,\"> $git_dir/$remote/$branch\")) {\n-\t\t\t\tprint STDERR \"Could not create branch $branch: $!\\n\";\n+\n+\t\t\tsystem(qw(git update-ref -m cvsimport),\n+\t\t\t\t\"$remote/$branch\", $id);\n+\t\t\tif($? != 0) {\n+\t\t\t\tprint STDERR \"Could not create branch $branch\\n\";\n \t\t\t\t$ignorebranch{$branch} = 1;\n \t\t\t\t$state=11;\n \t\t\t\tnext;\n \t\t\t}\n-\t\t\tprint H \"$id\\n\"\n-\t\t\t\tor die \"Could not write branch $branch: $!\";\n-\t\t\tclose(H)\n-\t\t\t\tor die \"Could not write branch $branch: $!\";\n \t\t}\n \t\t$last_branch = $branch if $branch ne $last_branch;\n \t\t$state = 9;\n@@ -998,7 +996,7 @@ if ($orig_branch) {\n \t$orig_branch = \"master\";\n \tprint \"DONE; creating $orig_branch branch\\n\" if $opt_v;\n \tsystem(\"git-update-ref\", \"refs/heads/master\", \"$remote/$opt_o\")\n-\t\tunless -f \"$git_dir/refs/heads/master\";\n+\t\tdefined get_headref('refs/heads/master');\n \tsystem(\"git-symbolic-ref\", \"$remote/HEAD\", \"$remote/$opt_o\")\n \t\tif ($opt_r && $opt_o ne 'HEAD');\n \tsystem('git-update-ref', 'HEAD', \"$orig_branch\");\n-- \n1.5.3.6.2039.g0495\n"},{"id":"61275","messageId":"Pine.LNX.4.64.0711281916140.27959@racer.site","threadId":"11051","inReplyTo":"20071128185611.GB11320@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-28T19:16:36Z","receivedAt":"2007-11-28T19:16:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 28 Nov 2007, Jeff King wrote:\n\n> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> index efa6a0c..b852f2f 100755\n> --- a/git-cvsimport.perl\n> +++ b/git-cvsimport.perl\n> @@ -527,18 +527,12 @@ sub is_sha1 {\n>  \treturn $s =~ /^[a-f0-9]{40}$/;\n>  }\n>  \n> -sub get_headref ($$) {\n> -    my $name    = shift;\n> -    my $git_dir = shift;\n> -\n> -    my $f = \"$git_dir/$remote/$name\";\n> -    if (open(my $fh, $f)) {\n> -\t    chomp(my $r = <$fh>);\n> -\t    is_sha1($r) or die \"Cannot get head id for $name ($r): $!\";\n> -\t    return $r;\n> -    }\n> -    die \"unable to open $f: $!\" unless $! == POSIX::ENOENT;\n> -    return undef;\n> +sub get_headref ($) {\n> +\tmy $name = shift;\n> +\tmy $r = `git show-ref -s '$name'`;\n> +\treturn undef unless $? == 0;\n> +\tchomp $r;\n> +\treturn $r;\n>  }\n\nWhere has $remote gone?\n\nCiao,\nDscho\n"},{"id":"61278","messageId":"20071128194423.GB11396@coredump.intra.peff.net","threadId":"11051","inReplyTo":"Pine.LNX.4.64.0711281916140.27959@racer.site","subject":"Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T19:44:23Z","receivedAt":"2007-11-28T19:44:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 07:16:36PM +0000, Johannes Schindelin wrote:\n\n> > -sub get_headref ($$) {\n> > -    my $name    = shift;\n> > -    my $git_dir = shift;\n> > -\n> > -    my $f = \"$git_dir/$remote/$name\";\n> > -    if (open(my $fh, $f)) {\n> > -\t    chomp(my $r = <$fh>);\n> > -\t    is_sha1($r) or die \"Cannot get head id for $name ($r): $!\";\n> > -\t    return $r;\n> > -    }\n> > -    die \"unable to open $f: $!\" unless $! == POSIX::ENOENT;\n> > -    return undef;\n> > +sub get_headref ($) {\n> > +\tmy $name = shift;\n> > +\tmy $r = `git show-ref -s '$name'`;\n> > +\treturn undef unless $? == 0;\n> > +\tchomp $r;\n> > +\treturn $r;\n> >  }\n> \n> Where has $remote gone?\n\nGah, thank you. Obviously I deleted it without looking when I removed\nthe now-unnecessary $git_dir. However, let me put a curse on whoever is\nresponsible for randomly using \"$remote\" as a global in this function,\nwhen all of the other parameters (including the\nobvious-candidate-for-a-global $git_dir) are passed in.\n\nSince get_headref is useful in contexts where \"$remote\" is not always\nprepended (see patch 3/3), I think the best solution is:\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 1b5f187..bbf9799 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -692,7 +692,8 @@ my (@old,@new,@skipped,%ignorebranch);\n $ignorebranch{'#CVSPS_NO_BRANCH'} = 1;\n \n sub commit {\n-\tif ($branch eq $opt_o && !$index{branch} && !get_headref($branch)) {\n+\tif ($branch eq $opt_o && !$index{branch} &&\n+\t\t!get_headref(\"$remote/$branch\")) {\n \t    # looks like an initial commit\n \t    # use the index primed by git-init\n \t    $ENV{GIT_INDEX_FILE} = \"$git_dir/index\";\n@@ -716,7 +717,7 @@ sub commit {\n \tupdate_index(@old, @new);\n \t@old = @new = ();\n \tmy $tree = write_tree();\n-\tmy $parent = get_headref($last_branch);\n+\tmy $parent = get_headref(\"$remote/$last_branch\");\n \tprint \"Parent ID \" . ($parent ? $parent : \"(empty)\") . \"\\n\" if $opt_v;\n \n \tmy @commit_args;\n@@ -727,7 +728,7 @@ sub commit {\n \tforeach my $rx (@mergerx) {\n \t\tnext unless $logmsg =~ $rx && $1;\n \t\tmy $mparent = $1 eq 'HEAD' ? $opt_o : $1;\n-\t\tif (my $sha1 = get_headref($mparent)) {\n+\t\tif (my $sha1 = get_headref(\"$remote/$mparent\")) {\n \t\t\tpush @commit_args, '-p', $mparent;\n \t\t\tprint \"Merge parent branch: $mparent\\n\" if $opt_v;\n \t\t}\n"},{"id":"61285","messageId":"Pine.LNX.4.64.0711282037100.27959@racer.site","threadId":"11051","inReplyTo":"20071128194423.GB11396@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-28T20:37:48Z","receivedAt":"2007-11-28T20:37:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 28 Nov 2007, Jeff King wrote:\n\n> Since get_headref is useful in contexts where \"$remote\" is not always\n> prepended (see patch 3/3), I think the best solution is:\n>\n> [PATCH prefixing the argument to get_headref() with \"$remote/\"]\n\nYes, I think so, too.\n\nThanks,\nDscho\n"},{"id":"61302","messageId":"7v8x4i3tn4.fsf@gitster.siamese.dyndns.org","threadId":"11051","inReplyTo":"20071128185546.GA11320@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] Add basic cvsimport tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T22:14:07Z","receivedAt":"2007-11-28T22:14:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> +\tcat <<EOF >o_fortuna &&\n> +O Fortuna\n> + ...\n\nCopyrights?\n\n> +add \"O Fortuna\" lyrics\n> +\n> +These public domain lyrics make an excellent sample text.\n> +EOF\n\nAh, really?  Ok... \n"},{"id":"61303","messageId":"8563zm811r.fsf@lola.goethe.zz","threadId":"11051","inReplyTo":"7v8x4i3tn4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] Add basic cvsimport tests","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-11-28T22:20:32Z","receivedAt":"2007-11-28T22:20:32Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> +\tcat <<EOF >o_fortuna &&\n>> +O Fortuna\n>> + ...\n>\n> Copyrights?\n\nBenediktbeuern, thirteenth century.\n\n>> +add \"O Fortuna\" lyrics\n>> +\n>> +These public domain lyrics make an excellent sample text.\n>> +EOF\n\nAs long as nobody adds Orff's musical arrangements...\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"61304","messageId":"7v4pf63t7y.fsf@gitster.siamese.dyndns.org","threadId":"11051","inReplyTo":"20071128194423.GB11396@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T22:23:13Z","receivedAt":"2007-11-28T22:23:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Since get_headref is useful in contexts where \"$remote\" is not always\n> prepended (see patch 3/3), I think the best solution is:\n\n...\n\n> +\tif ($branch eq $opt_o && !$index{branch} &&\n> +\t\t!get_headref(\"$remote/$branch\")) {\n> +\tmy $parent = get_headref(\"$remote/$last_branch\");\n> +\t\tif (my $sha1 = get_headref(\"$remote/$mparent\")) {\n\nSo the definition of get_headref() is to always take everything under\nbut not including \"refs/\"?  IOW, the tip of the master branch is asked\nwith get_headref(\"heads/master\")?\n\nI think show-ref can easily be confused by its tail matching behaviour,\nand is a bad command to use in a context like this.  To be safe, I think\nget_headref() should be:\n\n\tsub get_headref {\n        \tmy ($it) = (@_);\n                my $r = `git-rev-parse --verify \"refs/$it\"`;\n\t\treturn undef unless $? == 0;\n\t\tchomp $r;\n\t\treturn $r;\n\t}\n"},{"id":"61308","messageId":"20071128224313.GB13298@coredump.intra.peff.net","threadId":"11051","inReplyTo":"7v4pf63t7y.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T22:43:13Z","receivedAt":"2007-11-28T22:43:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 02:23:13PM -0800, Junio C Hamano wrote:\n\n> > +\tif ($branch eq $opt_o && !$index{branch} &&\n> > +\t\t!get_headref(\"$remote/$branch\")) {\n> > +\tmy $parent = get_headref(\"$remote/$last_branch\");\n> > +\t\tif (my $sha1 = get_headref(\"$remote/$mparent\")) {\n> \n> So the definition of get_headref() is to always take everything under\n> but not including \"refs/\"?  IOW, the tip of the master branch is asked\n> with get_headref(\"heads/master\")?\n\nNo, the $remote variable is \"refs/remotes/origin\" or \"refs/heads\". See\nthe old version of get_headref, which actually looked up\n\"$git_dir/$remote/$branch\".\n\n> I think show-ref can easily be confused by its tail matching behaviour,\n> and is a bad command to use in a context like this.  To be safe, I think\n> get_headref() should be:\n> \n> \tsub get_headref {\n>         \tmy ($it) = (@_);\n>                 my $r = `git-rev-parse --verify \"refs/$it\"`;\n> \t\treturn undef unless $? == 0;\n> \t\tchomp $r;\n> \t\treturn $r;\n> \t}\n\nI have no comment on which is the best command to use (you would know\nmuch better than I), but adding \"refs/\" is wrong.\n\n-Peff\n"},{"id":"61313","messageId":"20071128225229.GA13836@coredump.intra.peff.net","threadId":"11051","inReplyTo":"20071128224313.GB13298@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] cvsimport: use show-ref to support packed refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T22:52:29Z","receivedAt":"2007-11-28T22:52:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 05:43:13PM -0500, Jeff King wrote:\n\n> > \tsub get_headref {\n> >         \tmy ($it) = (@_);\n> >                 my $r = `git-rev-parse --verify \"refs/$it\"`;\n> > \t\treturn undef unless $? == 0;\n> > \t\tchomp $r;\n> > \t\treturn $r;\n> > \t}\n> \n> I have no comment on which is the best command to use (you would know\n> much better than I), but adding \"refs/\" is wrong.\n\nBTW, one bad thing about the new get_headref compared to the original is\nthat it does not distinguish very well between \"ref does not exist\" and\n\"an error occurred.\" Is there a lookup command that makes such a\ndistinction possible?\n\n-Peff\n"},{"id":"61342","messageId":"7vsl2pzxdf.fsf@gitster.siamese.dyndns.org","threadId":"11051","inReplyTo":"20071128185628.GC11320@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] cvsimport: miscellaneous packed-ref fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-29T00:52:28Z","receivedAt":"2007-11-29T00:52:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> @@ -998,7 +996,7 @@ if ($orig_branch) {\n>  \t$orig_branch = \"master\";\n>  \tprint \"DONE; creating $orig_branch branch\\n\" if $opt_v;\n>  \tsystem(\"git-update-ref\", \"refs/heads/master\", \"$remote/$opt_o\")\n> -\t\tunless -f \"$git_dir/refs/heads/master\";\n> +\t\tdefined get_headref('refs/heads/master');\n\nWhere did the unless go ;-)?\n\nThanks, queued.\n"},{"id":"61382","messageId":"474E8274.7030801@op5.se","threadId":"11051","inReplyTo":"7v8x4i3tn4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] Add basic cvsimport tests","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-11-29T09:12:20Z","receivedAt":"2007-11-29T09:12:20Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n>> +\tcat <<EOF >o_fortuna &&\n>> +O Fortuna\n>> + ...\n> \n> Copyrights?\n> \n>> +add \"O Fortuna\" lyrics\n>> +\n>> +These public domain lyrics make an excellent sample text.\n>> +EOF\n> \n> Ah, really?  Ok... \n\n\nThe poet, whoever he/she was (no-one knows today), has been dead\nnearly 800 years, so any copyrights that might once have been in\nplace have expired.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"61397","messageId":"20071129141938.GB32670@coredump.intra.peff.net","threadId":"11051","inReplyTo":"7vsl2pzxdf.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/3] cvsimport: miscellaneous packed-ref fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-29T14:19:38Z","receivedAt":"2007-11-29T14:19:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 04:52:28PM -0800, Junio C Hamano wrote:\n\n> > @@ -998,7 +996,7 @@ if ($orig_branch) {\n> >  \t$orig_branch = \"master\";\n> >  \tprint \"DONE; creating $orig_branch branch\\n\" if $opt_v;\n> >  \tsystem(\"git-update-ref\", \"refs/heads/master\", \"$remote/$opt_o\")\n> > -\t\tunless -f \"$git_dir/refs/heads/master\";\n> > +\t\tdefined get_headref('refs/heads/master');\n> \n> Where did the unless go ;-)?\n\nBah, I even ran this through the test suite, but obviously forgot a\n'make' in the middle.\n\n> Thanks, queued.\n\nThanks. Sorry for all the mistakes; I think I made more work in catching\nmy errors than went into the original patch series. ;)\n\n-Peff\n"},{"id":"61519","messageId":"20071130184123.GA75770@orion.lan","threadId":"11051","inReplyTo":"20071128185504.GA11236@coredump.intra.peff.net","subject":"Re: git-cvsimport bug","fromName":"Emanuele Giaquinta","fromEmail":"e.giaquinta@glauco.it","sentAt":"2007-11-30T18:41:23Z","receivedAt":"2007-11-30T18:41:23Z","isPatch":false,"sender":{"key":"e.giaquinta@glauco.it","avatar":null},"body":"On Wed, Nov 28, 2007 at 01:55:04PM -0500, Jeff King wrote:\n\n>   2/3: cvsimport: use show-ref to support packed refs\n> \n>        This fix is hopefully obvious, and the included test fails\n>        without it (and this should probably fix Emanuele's problem).\n\nIt does, thanks!\n\nEmanuele\n"}]}