{"thread":{"id":"5793","subject":"[PATCH] gitweb: prepare for repositories with packed refs.","startedAt":"2006-10-03T09:36:08Z","lastAt":"2006-10-07T12:58:02Z","messageCount":6,"participants":["Junio C Hamano","Petr Baudis","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"28147","messageId":"7vsli5pwqf.fsf@assigned-by-dhcp.cox.net","threadId":"5793","inReplyTo":null,"subject":"[PATCH] gitweb: prepare for repositories with packed refs.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-03T09:36:08Z","receivedAt":"2006-10-03T09:36:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When a repository is initialized long time ago with symbolic\nHEAD, and \"git-pack-refs --prune\" is run, HEAD will be a\ndangling symlink to refs/heads/ somewhere.\n\nRunning -e \"$dir/HEAD\" to guess if $dir is a git repository does\nnot give us the right answer anymore in such a case.\n\nAlso factor out two places that checked if the repository can be\nexported with similar code into a call to a new function,\ncheck_export_ok.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\n * This is to fix a real breakage (iow, not a throw-away patch).\n\n gitweb/gitweb.perl |   26 ++++++++++++++++++++------\n 1 files changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 55d1b2c..671a4e6 100644\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -180,6 +180,22 @@ sub feature_pickaxe {\n \treturn ($_[0]);\n }\n \n+# checking HEAD file with -e is fragile if the repository was\n+# initialized long time ago (i.e. symlink HEAD) and was pack-ref'ed\n+# and then pruned.\n+sub check_head_link {\n+\tmy ($dir) = @_;\n+\tmy $headfile = \"$dir/HEAD\";\n+\treturn ((-e $headfile) ||\n+\t\t(-l $headfile && readlink($headfile) =~ /^refs\\/heads\\//));\n+}\n+\n+sub check_export_ok {\n+\tmy ($dir) = @_;\n+\treturn (check_head_link($dir) &&\n+\t\t(!$export_ok || -e \"$dir/$export_ok\"));\n+}\n+\n # rename detection options for git-diff and git-diff-tree\n # - default is '-M', with the cost proportional to\n #   (number of removed files) * (number of new files).\n@@ -212,7 +228,7 @@ our $project = $cgi->param('p');\n if (defined $project) {\n \tif (!validate_pathname($project) ||\n \t    !(-d \"$projectroot/$project\") ||\n-\t    !(-e \"$projectroot/$project/HEAD\") ||\n+\t    !check_head_link(\"$projectroot/$project\") ||\n \t    ($export_ok && !(-e \"$projectroot/$project/$export_ok\")) ||\n \t    ($strict_export && !project_in_list($project))) {\n \t\tundef $project;\n@@ -289,7 +305,7 @@ sub evaluate_path_info {\n \t# find which part of PATH_INFO is project\n \t$project = $path_info;\n \t$project =~ s,/+$,,;\n-\twhile ($project && !-e \"$projectroot/$project/HEAD\") {\n+\twhile ($project && !check_head_link(\"$projectroot/$project\")) {\n \t\t$project =~ s,/*[^/]*$,,;\n \t}\n \t# validate project\n@@ -816,8 +832,7 @@ sub git_get_projects_list {\n \n \t\t\t\tmy $subdir = substr($File::Find::name, $pfxlen + 1);\n \t\t\t\t# we check related file in $projectroot\n-\t\t\t\tif (-e \"$projectroot/$subdir/HEAD\" && (!$export_ok ||\n-\t\t\t\t    -e \"$projectroot/$subdir/$export_ok\")) {\n+\t\t\t\tif (check_export_ok(\"$projectroot/$subdir\")) {\n \t\t\t\t\tpush @list, { path => $subdir };\n \t\t\t\t\t$File::Find::prune = 1;\n \t\t\t\t}\n@@ -838,8 +853,7 @@ sub git_get_projects_list {\n \t\t\tif (!defined $path) {\n \t\t\t\tnext;\n \t\t\t}\n-\t\t\tif (-e \"$projectroot/$path/HEAD\" && (!$export_ok ||\n-\t\t\t    -e \"$projectroot/$path/$export_ok\")) {\n+\t\t\tif (check_export_ok(\"$projectroot/$path\")) {\n \t\t\t\tmy $pr = {\n \t\t\t\t\tpath => $path,\n \t\t\t\t\towner => decode(\"utf8\", $owner, Encode::FB_DEFAULT),\n-- \n1.4.2.3.gd1e9e\n"},{"id":"28292","messageId":"20061006154059.GQ20017@pasky.or.cz","threadId":"5793","inReplyTo":"7vsli5pwqf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: prepare for repositories with packed refs.","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-06T15:40:59Z","receivedAt":"2006-10-06T15:40:59Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, Oct 03, 2006 at 11:36:08AM CEST, I got a letter\nwhere Junio C Hamano <junkio@cox.net> said that...\n> When a repository is initialized long time ago with symbolic\n> HEAD, and \"git-pack-refs --prune\" is run, HEAD will be a\n> dangling symlink to refs/heads/ somewhere.\n> \n> Running -e \"$dir/HEAD\" to guess if $dir is a git repository does\n> not give us the right answer anymore in such a case.\n\nI think this is a wrong answer to this problem - I guess Cogito's going\nto be confused by HEAD a dangling symlink as well and I'll bet there's\nmore places where this will give us trouble. Having HEAD a dangling\nsymlink is just wrong and git-pack-refs --prune is buggy if it causes\nthat.\n\n  You should fix the problem at that side and make sure it either\nchanges HEAD to a symref or doesn't pack the ref HEAD points at. Or just\nerror out and leave the policy decision on the user.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"},{"id":"28346","messageId":"7vzmc8tqqe.fsf@assigned-by-dhcp.cox.net","threadId":"5793","inReplyTo":"20061006154059.GQ20017@pasky.or.cz","subject":"Re: [PATCH] gitweb: prepare for repositories with packed refs.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-07T09:33:29Z","receivedAt":"2006-10-07T09:33:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n>> Running -e \"$dir/HEAD\" to guess if $dir is a git repository does\n>> not give us the right answer anymore in such a case.\n>\n> I think this is a wrong answer to this problem - I guess Cogito's going\n> to be confused by HEAD a dangling symlink as well and I'll bet there's\n> more places where this will give us trouble. Having HEAD a dangling\n> symlink is just wrong and git-pack-refs --prune is buggy if it causes\n> that.\n\nI disagree.\n\nWhen we introduced symref, \"cat .git/HEAD\" stopped being the way\nto read the value of the tip of the current branch, \"echo\n$commit >.git/HEAD\" stopped being the way to update it, and\n\"rev-parse --verify HEAD\" and \"update-ref HEAD $commit\" were\nintroduced at the same time as the official alternatives to\nsupport both old and new implementations of .git/HEAD.\n\nThe way to find out which branch we are currently on used to be\n\"readlink .git/HEAD\"; that stopped to be true, and we introduced\n\"symbolic-ref HEAD\" as the official alternative to support both\nold and new implementation.\n\nThe way to see if a random symbolic link whose name happens to\nbe HEAD is a symref has been to see if it points at a path that\nbegins with \"refs/\".\n\nThe user has a repository that uses symbolic link HEAD, and is\nasking to use pack-refs and is also asking to prune loose refs\nthat no longer are needed, but definitely is NOT asking to\nconvert the repository to use symref HEAD.  The repository after\n\"pack-refs --prune\" works perfectly well with symbolic link\nHEAD, so doing that unasked-for conversion is unnecessary.\n\nAnd the thing is, if the repository is actively being used, it\nis a non issue -- recent-enough git would create a symref by\ndefault.  The only place where it matters in practice are public\ngit repositories initialized using git before mid November last\nyear, and has never switched the \"current branch\" ever since\nusing git of newer vintage.\n"},{"id":"28355","messageId":"7v1wpks9qf.fsf@assigned-by-dhcp.cox.net","threadId":"5793","inReplyTo":"20061006154059.GQ20017@pasky.or.cz","subject":"Re: [PATCH] gitweb: prepare for repositories with packed refs.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-07T10:26:00Z","receivedAt":"2006-10-07T10:26:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> Having HEAD a dangling\n> symlink is just wrong...\n\nWhile I still disagree with this statement, as a practical\nissue, I think it might make sense to have an option to\npack-refs to pack only tags, which are supposed to be mostly\nimmutable.  Or even introduce --all option and make the default\npack only tags.  Your branches point at commits waiting to be\nupdated, and if you have remote tracking branches they are meant\nto be updated every time you fetch from that remote; otherwise\nyou would not be using tracking branches but merging straight\ninto your branch.\n\nAlso you are likely to do pack-refs (or whatever repository-wide\ngit operations) while on an active branch (be it \"master\" or\nsome non-master branch similar to my \"next\" or \"pu\"), and for\nthat reason I suspect packing the current branch is almost\nalways not very useful.  Active branches are by definition\nexpected to frequently change.\n"},{"id":"28358","messageId":"eg8349$2uq$2@sea.gmane.org","threadId":"5793","inReplyTo":"7v1wpks9qf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: prepare for repositories with packed refs.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-07T11:31:06Z","receivedAt":"2006-10-07T11:31:06Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> Petr Baudis <pasky@suse.cz> writes:\n> \n>> Having HEAD a dangling\n>> symlink is just wrong...\n> \n> While I still disagree with this statement, as a practical\n> issue, I think it might make sense to have an option to\n> pack-refs to pack only tags, which are supposed to be mostly\n> immutable.  Or even introduce --all option and make the default\n> pack only tags.\n\n+1. I was actually to asck if there is a way to pack only tags,\nor perhaps even only true tags (i.e. using tag objects).\n\nIt's usually the number of tags that limits performance by being\nI/O bound (for example for gitweb), and tags change rather rarely\nso modyfying/adding/deleting packed ref need not to be over-optimized\nfor speed.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"28363","messageId":"20061007125802.GA20017@pasky.or.cz","threadId":"5793","inReplyTo":"7vzmc8tqqe.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: prepare for repositories with packed refs.","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-07T12:58:02Z","receivedAt":"2006-10-07T12:58:02Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, Oct 07, 2006 at 11:33:29AM CEST, I got a letter\nwhere Junio C Hamano <junkio@cox.net> said that...\n> When we introduced symref, \"cat .git/HEAD\" stopped being the way\n> to read the value of the tip of the current branch, \"echo\n> $commit >.git/HEAD\" stopped being the way to update it, and\n> \"rev-parse --verify HEAD\" and \"update-ref HEAD $commit\" were\n> introduced at the same time as the official alternatives to\n> support both old and new implementations of .git/HEAD.\n> \n> The way to find out which branch we are currently on used to be\n> \"readlink .git/HEAD\"; that stopped to be true, and we introduced\n> \"symbolic-ref HEAD\" as the official alternative to support both\n> old and new implementation.\n> \n> The way to see if a random symbolic link whose name happens to\n> be HEAD is a symref has been to see if it points at a path that\n> begins with \"refs/\".\n\nCogito does\n\n\t[ ! -s \"$_git/HEAD\" ] || { _git_head=\"$(git-symbolic-ref HEAD)\"; _git_head=\"${_git_head#refs/heads/}\"; }\n\nin the common initialization code, so that cg-reset works even on\nrepositories with broken HEAD. It hasn't been an issue in the past since\nHEAD as a dangling symlink indeed _has_ been broken HEAD in the past.\n\nThat said, at least cg-reset will indeed repair it:\n\n\tif ! [ -s \"$_git/HEAD\" ]; then\n\t\trm -f \"$_git/HEAD\"\n\t\t# XXX: git-symbolic-ref is a weenie and won't do the job at this point.\n\t\techo \"ref: refs/heads/$_git_head\" >\"$_git/HEAD\"\n\tfi\n\nBut it's still not nice.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"}]}