[PATCH] gitweb: prepare for repositories with packed refs.

[PATCH] gitweb: prepare for repositories with packed refs.

[Junio C Hamano]

When a repository is initialized long time ago with symbolic
HEAD, and "git-pack-refs --prune" is run, HEAD will be a
dangling symlink to refs/heads/ somewhere.

Running -e "$dir/HEAD" to guess if $dir is a git repository does
not give us the right answer anymore in such a case.

Also factor out two places that checked if the repository can be
exported with similar code into a call to a new function,
check_export_ok.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * This is to fix a real breakage (iow, not a throw-away patch).

 gitweb/gitweb.perl |   26 ++++++++++++++++++++------
 1 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 55d1b2c..671a4e6 100644
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -180,6 +180,22 @@ sub feature_pickaxe {
 	return ($_[0]);
 }
 
+# checking HEAD file with -e is fragile if the repository was
+# initialized long time ago (i.e. symlink HEAD) and was pack-ref'ed
+# and then pruned.
+sub check_head_link {
+	my ($dir) = @_;
+	my $headfile = "$dir/HEAD";
+	return ((-e $headfile) ||
+		(-l $headfile && readlink($headfile) =~ /^refs\/heads\//));
+}
+
+sub check_export_ok {
+	my ($dir) = @_;
+	return (check_head_link($dir) &&
+		(!$export_ok || -e "$dir/$export_ok"));
+}
+
 # rename detection options for git-diff and git-diff-tree
 # - default is '-M', with the cost proportional to
 #   (number of removed files) * (number of new files).
@@ -212,7 +228,7 @@ our $project = $cgi->param('p');
 if (defined $project) {
 	if (!validate_pathname($project) ||
 	    !(-d "$projectroot/$project") ||
-	    !(-e "$projectroot/$project/HEAD") ||
+	    !check_head_link("$projectroot/$project") ||
 	    ($export_ok && !(-e "$projectroot/$project/$export_ok")) ||
 	    ($strict_export && !project_in_list($project))) {
 		undef $project;
@@ -289,7 +305,7 @@ sub evaluate_path_info {
 	# find which part of PATH_INFO is project
 	$project = $path_info;
 	$project =~ s,/+$,,;
-	while ($project && !-e "$projectroot/$project/HEAD") {
+	while ($project && !check_head_link("$projectroot/$project")) {
 		$project =~ s,/*[^/]*$,,;
 	}
 	# validate project
@@ -816,8 +832,7 @@ sub git_get_projects_list {
 
 				my $subdir = substr($File::Find::name, $pfxlen + 1);
 				# we check related file in $projectroot
-				if (-e "$projectroot/$subdir/HEAD" && (!$export_ok ||
-				    -e "$projectroot/$subdir/$export_ok")) {
+				if (check_export_ok("$projectroot/$subdir")) {
 					push @list, { path => $subdir };
 					$File::Find::prune = 1;
 				}
@@ -838,8 +853,7 @@ sub git_get_projects_list {
 			if (!defined $path) {
 				next;
 			}
-			if (-e "$projectroot/$path/HEAD" && (!$export_ok ||
-			    -e "$projectroot/$path/$export_ok")) {
+			if (check_export_ok("$projectroot/$path")) {
 				my $pr = {
 					path => $path,
 					owner => decode("utf8", $owner, Encode::FB_DEFAULT),
-- 
1.4.2.3.gd1e9e

Re: [PATCH] gitweb: prepare for repositories with packed refs.

[Petr Baudis]

Dear diary, on Tue, Oct 03, 2006 at 11:36:08AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> When a repository is initialized long time ago with symbolic
> HEAD, and "git-pack-refs --prune" is run, HEAD will be a
> dangling symlink to refs/heads/ somewhere.
> 
> Running -e "$dir/HEAD" to guess if $dir is a git repository does
> not give us the right answer anymore in such a case.

I think this is a wrong answer to this problem - I guess Cogito's going
to be confused by HEAD a dangling symlink as well and I'll bet there's
more places where this will give us trouble. Having HEAD a dangling
symlink is just wrong and git-pack-refs --prune is buggy if it causes
that.

  You should fix the problem at that side and make sure it either
changes HEAD to a symref or doesn't pack the ref HEAD points at. Or just
error out and leave the policy decision on the user.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Re: [PATCH] gitweb: prepare for repositories with packed refs.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>> Running -e "$dir/HEAD" to guess if $dir is a git repository does
>> not give us the right answer anymore in such a case.
>
> I think this is a wrong answer to this problem - I guess Cogito's going
> to be confused by HEAD a dangling symlink as well and I'll bet there's
> more places where this will give us trouble. Having HEAD a dangling
> symlink is just wrong and git-pack-refs --prune is buggy if it causes
> that.

I disagree.

When we introduced symref, "cat .git/HEAD" stopped being the way
to read the value of the tip of the current branch, "echo
$commit >.git/HEAD" stopped being the way to update it, and
"rev-parse --verify HEAD" and "update-ref HEAD $commit" were
introduced at the same time as the official alternatives to
support both old and new implementations of .git/HEAD.

The way to find out which branch we are currently on used to be
"readlink .git/HEAD"; that stopped to be true, and we introduced
"symbolic-ref HEAD" as the official alternative to support both
old and new implementation.

The way to see if a random symbolic link whose name happens to
be HEAD is a symref has been to see if it points at a path that
begins with "refs/".

The user has a repository that uses symbolic link HEAD, and is
asking to use pack-refs and is also asking to prune loose refs
that no longer are needed, but definitely is NOT asking to
convert the repository to use symref HEAD.  The repository after
"pack-refs --prune" works perfectly well with symbolic link
HEAD, so doing that unasked-for conversion is unnecessary.

And the thing is, if the repository is actively being used, it
is a non issue -- recent-enough git would create a symref by
default.  The only place where it matters in practice are public
git repositories initialized using git before mid November last
year, and has never switched the "current branch" ever since
using git of newer vintage.

Re: [PATCH] gitweb: prepare for repositories with packed refs.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Having HEAD a dangling
> symlink is just wrong...

While I still disagree with this statement, as a practical
issue, I think it might make sense to have an option to
pack-refs to pack only tags, which are supposed to be mostly
immutable.  Or even introduce --all option and make the default
pack only tags.  Your branches point at commits waiting to be
updated, and if you have remote tracking branches they are meant
to be updated every time you fetch from that remote; otherwise
you would not be using tracking branches but merging straight
into your branch.

Also you are likely to do pack-refs (or whatever repository-wide
git operations) while on an active branch (be it "master" or
some non-master branch similar to my "next" or "pu"), and for
that reason I suspect packing the current branch is almost
always not very useful.  Active branches are by definition
expected to frequently change.

Re: [PATCH] gitweb: prepare for repositories with packed refs.

[Jakub Narebski]

Junio C Hamano wrote:

> Petr Baudis <pasky@suse.cz> writes:
> 
>> Having HEAD a dangling
>> symlink is just wrong...
> 
> While I still disagree with this statement, as a practical
> issue, I think it might make sense to have an option to
> pack-refs to pack only tags, which are supposed to be mostly
> immutable.  Or even introduce --all option and make the default
> pack only tags.

+1. I was actually to asck if there is a way to pack only tags,
or perhaps even only true tags (i.e. using tag objects).

It's usually the number of tags that limits performance by being
I/O bound (for example for gitweb), and tags change rather rarely
so modyfying/adding/deleting packed ref need not to be over-optimized
for speed.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] gitweb: prepare for repositories with packed refs.

[Petr Baudis]

Dear diary, on Sat, Oct 07, 2006 at 11:33:29AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> When we introduced symref, "cat .git/HEAD" stopped being the way
> to read the value of the tip of the current branch, "echo
> $commit >.git/HEAD" stopped being the way to update it, and
> "rev-parse --verify HEAD" and "update-ref HEAD $commit" were
> introduced at the same time as the official alternatives to
> support both old and new implementations of .git/HEAD.
> 
> The way to find out which branch we are currently on used to be
> "readlink .git/HEAD"; that stopped to be true, and we introduced
> "symbolic-ref HEAD" as the official alternative to support both
> old and new implementation.
> 
> The way to see if a random symbolic link whose name happens to
> be HEAD is a symref has been to see if it points at a path that
> begins with "refs/".

Cogito does

	[ ! -s "$_git/HEAD" ] || { _git_head="$(git-symbolic-ref HEAD)"; _git_head="${_git_head#refs/heads/}"; }

in the common initialization code, so that cg-reset works even on
repositories with broken HEAD. It hasn't been an issue in the past since
HEAD as a dangling symlink indeed _has_ been broken HEAD in the past.

That said, at least cg-reset will indeed repair it:

	if ! [ -s "$_git/HEAD" ]; then
		rm -f "$_git/HEAD"
		# XXX: git-symbolic-ref is a weenie and won't do the job at this point.
		echo "ref: refs/heads/$_git_head" >"$_git/HEAD"
	fi

But it's still not nice.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)