{"thread":{"id":"35407","subject":"[PATCH] gitweb: Add an option for adding more branch refs","startedAt":"2013-11-26T10:37:23Z","lastAt":"2013-11-27T16:15:27Z","messageCount":5,"participants":["Krzesimir Nowak","Junio C Hamano","Jakub Narębski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"231129","messageId":"1385462243-7898-1-git-send-email-krzesimir@endocode.com","threadId":"35407","inReplyTo":null,"subject":"[PATCH] gitweb: Add an option for adding more branch refs","fromName":"Krzesimir Nowak","fromEmail":"krzesimir@endocode.com","sentAt":"2013-11-26T10:37:23Z","receivedAt":"2013-11-26T10:37:23Z","isPatch":true,"sender":{"key":"krzesimir@endocode.com","avatar":"https://avatars.githubusercontent.com/u/96081?v=4"},"body":"Overriding an @additional_branch_refs configuration variable with\nvalue ('wip') will make gitweb to show branches that appear in\nrefs/heads and refs/wip (refs/heads is hardcoded). Might be useful for\ngerrit setups where user branches are not stored under refs/heads/.\n\nSigned-off-by: Krzesimir Nowak <krzesimir@endocode.com>\n---\n gitweb/gitweb.perl | 99 ++++++++++++++++++++++++++++++++++++++++--------------\n 1 file changed, 74 insertions(+), 25 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 68c77f6..9bfd38b 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -17,6 +17,7 @@ use Encode;\n use Fcntl ':mode';\n use File::Find qw();\n use File::Basename qw(basename);\n+use List::Util qw(min);\n use Time::HiRes qw(gettimeofday tv_interval);\n binmode STDOUT, ':utf8';\n \n@@ -122,6 +123,10 @@ our $logo_label = \"git homepage\";\n # source of projects list\n our $projects_list = \"++GITWEB_LIST++\";\n \n+# list of additional \"directories\" under \"refs/\" we want to display as\n+# branches\n+our @additional_branch_refs = ();\n+\n # the width (in characters) of the projects list \"Description\" column\n our $projects_list_description_width = 25;\n \n@@ -626,14 +631,29 @@ sub feature_avatar {\n \treturn @val ? @val : @_;\n }\n \n+sub get_branch_refs {\n+    return ('heads', @additional_branch_refs);\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+\tif (-e $headfile) {\n+\t\treturn 1;\n+\t}\n+\tif (-l $headfile) {\n+\t\tmy $rl = readlink($headfile);\n+\n+\t\tfor my $ref (get_branch_refs()) {\n+\t\t\treturn 1 if $rl =~ /^refs\\/$ref\\//;\n+\t\t}\n+\t}\n+\n+\treturn 0;\n }\n \n sub check_export_ok {\n@@ -2515,6 +2535,7 @@ sub format_snapshot_links {\n sub get_feed_info {\n \tmy $format = shift || 'Atom';\n \tmy %res = (action => lc($format));\n+\tmy $matched_ref = 0;\n \n \t# feed links are possible only for project views\n \treturn unless (defined $project);\n@@ -2522,12 +2543,17 @@ sub get_feed_info {\n \t# or don't have specific feed yet (so they should use generic)\n \treturn if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);\n \n-\tmy $branch;\n-\t# branches refs uses 'refs/heads/' prefix (fullname) to differentiate\n-\t# from tag links; this also makes possible to detect branch links\n-\tif ((defined $hash_base && $hash_base =~ m!^refs/heads/(.*)$!) ||\n-\t    (defined $hash      && $hash      =~ m!^refs/heads/(.*)$!)) {\n-\t\t$branch = $1;\n+\tmy $branch = undef;\n+\t# branches refs uses 'refs/' + $get_branch_refs()[x] + '/' prefix\n+\t# (fullname) to differentiate from tag links; this also makes\n+\t# possible to detect branch links\n+\tfor my $ref (get_branch_refs()) {\n+\t\tif ((defined $hash_base && $hash_base =~ m!^refs/$ref/(.*)$!) ||\n+\t\t    (defined $hash      && $hash      =~ m!^refs/$ref/(.*)$!)) {\n+\t\t\t$branch = $1;\n+\t\t\t$matched_ref = $ref;\n+\t\t\tlast;\n+\t\t}\n \t}\n \t# find log type for feed description (title)\n \tmy $type = 'log';\n@@ -2540,7 +2566,7 @@ sub get_feed_info {\n \t}\n \n \t$res{-title} = $type;\n-\t$res{'hash'} = (defined $branch ? \"refs/heads/$branch\" : undef);\n+\t$res{'hash'} = (defined $branch ? \"refs/$matched_ref/$branch\" : undef);\n \t$res{'file_name'} = $file_name;\n \n \treturn %res;\n@@ -3184,24 +3210,43 @@ sub git_get_project_owner {\n \treturn $owner;\n }\n \n-sub git_get_last_activity {\n-\tmy ($path) = @_;\n-\tmy $fd;\n+sub git_get_last_activity_age {\n+\tmy ($refs) = @_;\n+\tmy $fd = -1;\n \n-\t$git_dir = \"$projectroot/$path\";\n \topen($fd, \"-|\", git_cmd(), 'for-each-ref',\n \t     '--format=%(committer)',\n \t     '--sort=-committerdate',\n \t     '--count=1',\n-\t     'refs/heads') or return;\n+\t     $refs) or return undef;\n+\n \tmy $most_recent = <$fd>;\n-\tclose $fd or return;\n+\tclose $fd or return undef;\n \tif (defined $most_recent &&\n \t    $most_recent =~ / (\\d+) [-+][01]\\d\\d\\d$/) {\n \t\tmy $timestamp = $1;\n-\t\tmy $age = time - $timestamp;\n-\t\treturn ($age, age_string($age));\n+\t\treturn time - $timestamp;\n \t}\n+\n+\treturn undef;\n+}\n+\n+sub git_get_last_activity {\n+\tmy ($path) = @_;\n+\tmy @ages = ();\n+\n+\t$git_dir = \"$projectroot/$path\";\n+\tfor my $ref (get_branch_refs()) {\n+\t\tmy $age = git_get_last_activity_age('refs/' . $_);\n+\n+\t\tpush @ages, $age if defined $age;\n+\t}\n+\tif (@ages) {\n+\t\tmy $min_age = min(@ages);\n+\n+\t\treturn ($min_age, age_string($min_age));\n+\t}\n+\n \treturn (undef, undef);\n }\n \n@@ -3223,7 +3268,7 @@ sub git_get_remotes_list {\n \t\tnext if $wanted and not $remote eq $wanted;\n \t\tmy ($url, $key) = ($1, $2);\n \n-\t\t$remotes{$remote} ||= { 'heads' => () };\n+\t\t$remotes{$remote} ||= { map { $_ => () } get_branch_refs() };\n \t\t$remotes{$remote}{$key} = $url;\n \t}\n \tclose $fd or return;\n@@ -3237,9 +3282,11 @@ sub fill_remote_heads {\n \tmy @heads = map { \"remotes/$_\" } keys %$remotes;\n \tmy @remoteheads = git_get_heads_list(undef, @heads);\n \tforeach my $remote (keys %$remotes) {\n-\t\t$remotes->{$remote}{'heads'} = [ grep {\n-\t\t\t$_->{'name'} =~ s!^$remote/!!\n-\t\t\t} @remoteheads ];\n+\t\tforeach my $ref (get_branch_refs()) {\n+\t\t\t$remotes->{$remote}{$ref} = [ grep {\n+\t\t\t\t$_->{'name'} =~ s!^$remote/!!\n+\t\t\t\t} @remoteheads ];\n+\t\t}\n \t}\n }\n \n@@ -3644,7 +3691,7 @@ sub parse_from_to_diffinfo {\n \n sub git_get_heads_list {\n \tmy ($limit, @classes) = @_;\n-\t@classes = ('heads') unless @classes;\n+\t@classes = get_branch_refs() unless @classes;\n \tmy @patterns = map { \"refs/$_\" } @classes;\n \tmy @headslist;\n \n@@ -3662,7 +3709,8 @@ sub git_get_heads_list {\n \t\tmy ($committer, $epoch, $tz) =\n \t\t\t($committerinfo =~ /^(.*) ([0-9]+) (.*)$/);\n \t\t$ref_item{'fullname'}  = $name;\n-\t\t$name =~ s!^refs/(?:head|remote)s/!!;\n+\t\tmy $strip_refs = join '|', get_branch_refs();\n+\t\t$name =~ s!^refs/(?:$strip_refs|remotes)/!!;\n \n \t\t$ref_item{'name'}  = $name;\n \t\t$ref_item{'id'}    = $hash;\n@@ -4286,7 +4334,7 @@ sub git_print_page_nav {\n # available if the feature is enabled\n sub format_ref_views {\n \tmy ($current) = @_;\n-\tmy @ref_views = qw{tags heads};\n+\tmy @ref_views = ('tags', get_branch_refs());\n \tpush @ref_views, 'remotes' if gitweb_check_feature('remote_heads');\n \treturn join \" | \", map {\n \t\t$_ eq $current ? $_ :\n@@ -7179,7 +7227,8 @@ sub snapshot_name {\n \t\t$ver = $1;\n \t} else {\n \t\t# branches and other need shortened SHA-1 hash\n-\t\tif ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {\n+\t\tmy $strip_refs = join '|', get_branch_refs();\n+\t\tif ($hash =~ m!^refs/(?:$strip_refs|remotes)/(.*)$!) {\n \t\t\t$ver = $1;\n \t\t}\n \t\t$ver .= '-' . git_get_short_hash($project, $hash);\n-- \n1.8.3.1\n"},{"id":"231147","messageId":"xmqqr4a2vnqd.fsf@gitster.dls.corp.google.com","threadId":"35407","inReplyTo":"1385462243-7898-1-git-send-email-krzesimir@endocode.com","subject":"Re: [PATCH] gitweb: Add an option for adding more branch refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-26T21:48:10Z","receivedAt":"2013-11-26T21:48:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Krzesimir Nowak <krzesimir@endocode.com> writes:\n\n> Overriding an @additional_branch_refs configuration variable with\n> value ('wip') will make gitweb to show branches that appear in\n> refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for\n> gerrit setups where user branches are not stored under refs/heads/.\n>\n> Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>\n> ---\n>  gitweb/gitweb.perl | 99 ++++++++++++++++++++++++++++++++++++++++--------------\n>  1 file changed, 74 insertions(+), 25 deletions(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 68c77f6..9bfd38b 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -17,6 +17,7 @@ use Encode;\n>  use Fcntl ':mode';\n>  use File::Find qw();\n>  use File::Basename qw(basename);\n> +use List::Util qw(min);\n>  use Time::HiRes qw(gettimeofday tv_interval);\n>  binmode STDOUT, ':utf8';\n>  \n> @@ -122,6 +123,10 @@ our $logo_label = \"git homepage\";\n>  # source of projects list\n>  our $projects_list = \"++GITWEB_LIST++\";\n>  \n> +# list of additional \"directories\" under \"refs/\" we want to display as\n> +# branches\n> +our @additional_branch_refs = ();\n> +\n>  # the width (in characters) of the projects list \"Description\" column\n>  our $projects_list_description_width = 25;\n>  \n> @@ -626,14 +631,29 @@ sub feature_avatar {\n>  \treturn @val ? @val : @_;\n>  }\n>  \n> +sub get_branch_refs {\n> +    return ('heads', @additional_branch_refs);\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> +\tif (-e $headfile) {\n> +\t\treturn 1;\n> +\t}\n> +\tif (-l $headfile) {\n> +\t\tmy $rl = readlink($headfile);\n> +\n> +\t\tfor my $ref (get_branch_refs()) {\n> +\t\t\treturn 1 if $rl =~ /^refs\\/$ref\\//;\n> +\t\t}\n> +\t}\n> +\treturn 0;\n\nThe change to this function is wrong. A non-detached HEAD that\npoints at anything other than refs/heads/ should be considered a\nrepository corruption and should not be encouraged.\n\n> @@ -2515,6 +2535,7 @@ sub format_snapshot_links {\n>  sub get_feed_info {\n>  \tmy $format = shift || 'Atom';\n>  \tmy %res = (action => lc($format));\n> +\tmy $matched_ref = 0;\n>  \n>  \t# feed links are possible only for project views\n>  \treturn unless (defined $project);\n> @@ -2522,12 +2543,17 @@ sub get_feed_info {\n>  \t# or don't have specific feed yet (so they should use generic)\n>  \treturn if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);\n>  \n> -\tmy $branch;\n> -\t# branches refs uses 'refs/heads/' prefix (fullname) to differentiate\n> -\t# from tag links; this also makes possible to detect branch links\n> -\tif ((defined $hash_base && $hash_base =~ m!^refs/heads/(.*)$!) ||\n> -\t    (defined $hash      && $hash      =~ m!^refs/heads/(.*)$!)) {\n> -\t\t$branch = $1;\n> +\tmy $branch = undef;\n> +\t# branches refs uses 'refs/' + $get_branch_refs()[x] + '/' prefix\n> +\t# (fullname) to differentiate from tag links; this also makes\n> +\t# possible to detect branch links\n> +\tfor my $ref (get_branch_refs()) {\n> +\t\tif ((defined $hash_base && $hash_base =~ m!^refs/$ref/(.*)$!) ||\n> +\t\t    (defined $hash      && $hash      =~ m!^refs/$ref/(.*)$!)) {\n> +\t\t\t$branch = $1;\n> +\t\t\t$matched_ref = $ref;\n> +\t\t\tlast;\n> +\t\t}\n>  \t}\n>  \t# find log type for feed description (title)\n>  \tmy $type = 'log';\n> @@ -2540,7 +2566,7 @@ sub get_feed_info {\n>  \t}\n>  \n>  \t$res{-title} = $type;\n> -\t$res{'hash'} = (defined $branch ? \"refs/heads/$branch\" : undef);\n> +\t$res{'hash'} = (defined $branch ? \"refs/$matched_ref/$branch\" : undef);\n>  \t$res{'file_name'} = $file_name;\n\nOK.\n\n> @@ -3184,24 +3210,43 @@ sub git_get_project_owner {\n>  \treturn $owner;\n>  }\n>  \n> -sub git_get_last_activity {\n> -\tmy ($path) = @_;\n> -\tmy $fd;\n> +sub git_get_last_activity_age {\n> +\tmy ($refs) = @_;\n> +\tmy $fd = -1;\n>  \n> -\t$git_dir = \"$projectroot/$path\";\n>  \topen($fd, \"-|\", git_cmd(), 'for-each-ref',\n>  \t     '--format=%(committer)',\n>  \t     '--sort=-committerdate',\n>  \t     '--count=1',\n> -\t     'refs/heads') or return;\n> +\t     $refs) or return undef;\n> +\n>  \tmy $most_recent = <$fd>;\n> -\tclose $fd or return;\n> +\tclose $fd or return undef;\n>  \tif (defined $most_recent &&\n>  \t    $most_recent =~ / (\\d+) [-+][01]\\d\\d\\d$/) {\n>  \t\tmy $timestamp = $1;\n> -\t\tmy $age = time - $timestamp;\n> -\t\treturn ($age, age_string($age));\n> +\t\treturn time - $timestamp;\n>  \t}\n> +\n> +\treturn undef;\n> +}\n> +\n> +sub git_get_last_activity {\n> +\tmy ($path) = @_;\n> +\tmy @ages = ();\n> +\n> +\t$git_dir = \"$projectroot/$path\";\n> +\tfor my $ref (get_branch_refs()) {\n> +\t\tmy $age = git_get_last_activity_age('refs/' . $_);\n> +\n> +\t\tpush @ages, $age if defined $age;\n> +\t}\n> +\tif (@ages) {\n> +\t\tmy $min_age = min(@ages);\n> +\n> +\t\treturn ($min_age, age_string($min_age));\n> +\t}\n> +\n>  \treturn (undef, undef);\n>  }\n\nThe original runs\n\n\tfor-each-ref --count=1 refs/heads\n\nand picks the latest one, so shouldn't it be the matter of running\n\n\tfor-each-ref --count=1 refs/heads refs/extra\n\ni.e. something like\n\n@@ -3193,7 +3193,7 @@ sub git_get_last_activity {\n \t     '--format=%(committer)',\n \t     '--sort=-committerdate',\n \t     '--count=1',\n-\t     'refs/heads') or return;\n+\t     map { \"refs/$_\" } get_branch_refs()) or return;\n \tmy $most_recent = <$fd>;\n \tclose $fd or return;\n \tif (defined $most_recent &&\n\nto grab the latest among the refs you care about?\n\n> @@ -3223,7 +3268,7 @@ sub git_get_remotes_list {\n>  \t\tnext if $wanted and not $remote eq $wanted;\n>  \t\tmy ($url, $key) = ($1, $2);\n>  \n> -\t\t$remotes{$remote} ||= { 'heads' => () };\n> +\t\t$remotes{$remote} ||= { map { $_ => () } get_branch_refs() };\n>  \t\t$remotes{$remote}{$key} = $url;\n>  \t}\n>  \tclose $fd or return;\n> @@ -3237,9 +3282,11 @@ sub fill_remote_heads {\n>  \tmy @heads = map { \"remotes/$_\" } keys %$remotes;\n>  \tmy @remoteheads = git_get_heads_list(undef, @heads);\n>  \tforeach my $remote (keys %$remotes) {\n> -\t\t$remotes->{$remote}{'heads'} = [ grep {\n> -\t\t\t$_->{'name'} =~ s!^$remote/!!\n> -\t\t\t} @remoteheads ];\n> +\t\tforeach my $ref (get_branch_refs()) {\n> +\t\t\t$remotes->{$remote}{$ref} = [ grep {\n> +\t\t\t\t$_->{'name'} =~ s!^$remote/!!\n> +\t\t\t\t} @remoteheads ];\n> +\t\t}\n>  \t}\n>  }\n\nHmmm, why?  Aren't these additional ones about the local\n\"branch-like\" refs?  What makes us think that these names are also\nshared with the names from remote hierarchies?\n\n> @@ -3644,7 +3691,7 @@ sub parse_from_to_diffinfo {\n>  \n>  sub git_get_heads_list {\n>  \tmy ($limit, @classes) = @_;\n> -\t@classes = ('heads') unless @classes;\n> +\t@classes = get_branch_refs() unless @classes;\n>  \tmy @patterns = map { \"refs/$_\" } @classes;\n>  \tmy @headslist;\n>  \n> @@ -3662,7 +3709,8 @@ sub git_get_heads_list {\n>  \t\tmy ($committer, $epoch, $tz) =\n>  \t\t\t($committerinfo =~ /^(.*) ([0-9]+) (.*)$/);\n>  \t\t$ref_item{'fullname'}  = $name;\n> -\t\t$name =~ s!^refs/(?:head|remote)s/!!;\n> +\t\tmy $strip_refs = join '|', get_branch_refs();\n> +\t\t$name =~ s!^refs/(?:$strip_refs|remotes)/!!;\n>  \n>  \t\t$ref_item{'name'}  = $name;\n>  \t\t$ref_item{'id'}    = $hash;\n> @@ -4286,7 +4334,7 @@ sub git_print_page_nav {\n>  # available if the feature is enabled\n>  sub format_ref_views {\n>  \tmy ($current) = @_;\n> -\tmy @ref_views = qw{tags heads};\n> +\tmy @ref_views = ('tags', get_branch_refs());\n>  \tpush @ref_views, 'remotes' if gitweb_check_feature('remote_heads');\n>  \treturn join \" | \", map {\n>  \t\t$_ eq $current ? $_ :\n> @@ -7179,7 +7227,8 @@ sub snapshot_name {\n>  \t\t$ver = $1;\n>  \t} else {\n>  \t\t# branches and other need shortened SHA-1 hash\n> -\t\tif ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {\n> +\t\tmy $strip_refs = join '|', get_branch_refs();\n> +\t\tif ($hash =~ m!^refs/(?:$strip_refs|remotes)/(.*)$!) {\n>  \t\t\t$ver = $1;\n>  \t\t}\n>  \t\t$ver .= '-' . git_get_short_hash($project, $hash);\n\nAs the elements of @additional_branch_refs are expected by code\nadded by this patch to never have any special metacharacters in\nthem, it probably is a good idea to sanity check it to catch\nmisconfigurations and typos before doing anything else.\n\nOther than that, looks good to me.\n\nThanks.\n"},{"id":"231192","messageId":"1385566482.2131.22.camel@localhost.localdomain","threadId":"35407","inReplyTo":"xmqqr4a2vnqd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gitweb: Add an option for adding more branch refs","fromName":"Krzesimir Nowak","fromEmail":"krzesimir@endocode.com","sentAt":"2013-11-27T15:34:42Z","receivedAt":"2013-11-27T15:34:42Z","isPatch":true,"sender":{"key":"krzesimir@endocode.com","avatar":"https://avatars.githubusercontent.com/u/96081?v=4"},"body":"On Tue, 2013-11-26 at 13:48 -0800, Junio C Hamano wrote:\n> Krzesimir Nowak <krzesimir@endocode.com> writes:\n> \n> > Overriding an @additional_branch_refs configuration variable with\n> > value ('wip') will make gitweb to show branches that appear in\n> > refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for\n> > gerrit setups where user branches are not stored under refs/heads/.\n> >\n> > Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>\n> > ---\n> >  gitweb/gitweb.perl | 99 ++++++++++++++++++++++++++++++++++++++++--------------\n> >  1 file changed, 74 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index 68c77f6..9bfd38b 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -17,6 +17,7 @@ use Encode;\n> >  use Fcntl ':mode';\n> >  use File::Find qw();\n> >  use File::Basename qw(basename);\n> > +use List::Util qw(min);\n> >  use Time::HiRes qw(gettimeofday tv_interval);\n> >  binmode STDOUT, ':utf8';\n> >  \n> > @@ -122,6 +123,10 @@ our $logo_label = \"git homepage\";\n> >  # source of projects list\n> >  our $projects_list = \"++GITWEB_LIST++\";\n> >  \n> > +# list of additional \"directories\" under \"refs/\" we want to display as\n> > +# branches\n> > +our @additional_branch_refs = ();\n> > +\n> >  # the width (in characters) of the projects list \"Description\" column\n> >  our $projects_list_description_width = 25;\n> >  \n> > @@ -626,14 +631,29 @@ sub feature_avatar {\n> >  \treturn @val ? @val : @_;\n> >  }\n> >  \n> > +sub get_branch_refs {\n> > +    return ('heads', @additional_branch_refs);\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> > +\tif (-e $headfile) {\n> > +\t\treturn 1;\n> > +\t}\n> > +\tif (-l $headfile) {\n> > +\t\tmy $rl = readlink($headfile);\n> > +\n> > +\t\tfor my $ref (get_branch_refs()) {\n> > +\t\t\treturn 1 if $rl =~ /^refs\\/$ref\\//;\n> > +\t\t}\n> > +\t}\n> > +\treturn 0;\n> \n> The change to this function is wrong. A non-detached HEAD that\n> points at anything other than refs/heads/ should be considered a\n> repository corruption and should not be encouraged.\n\nOk, I'll get rid of it.\n\n> \n> > @@ -2515,6 +2535,7 @@ sub format_snapshot_links {\n> >  sub get_feed_info {\n> >  \tmy $format = shift || 'Atom';\n> >  \tmy %res = (action => lc($format));\n> > +\tmy $matched_ref = 0;\n> >  \n> >  \t# feed links are possible only for project views\n> >  \treturn unless (defined $project);\n> > @@ -2522,12 +2543,17 @@ sub get_feed_info {\n> >  \t# or don't have specific feed yet (so they should use generic)\n> >  \treturn if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);\n> >  \n> > -\tmy $branch;\n> > -\t# branches refs uses 'refs/heads/' prefix (fullname) to differentiate\n> > -\t# from tag links; this also makes possible to detect branch links\n> > -\tif ((defined $hash_base && $hash_base =~ m!^refs/heads/(.*)$!) ||\n> > -\t    (defined $hash      && $hash      =~ m!^refs/heads/(.*)$!)) {\n> > -\t\t$branch = $1;\n> > +\tmy $branch = undef;\n> > +\t# branches refs uses 'refs/' + $get_branch_refs()[x] + '/' prefix\n> > +\t# (fullname) to differentiate from tag links; this also makes\n> > +\t# possible to detect branch links\n> > +\tfor my $ref (get_branch_refs()) {\n> > +\t\tif ((defined $hash_base && $hash_base =~ m!^refs/$ref/(.*)$!) ||\n> > +\t\t    (defined $hash      && $hash      =~ m!^refs/$ref/(.*)$!)) {\n> > +\t\t\t$branch = $1;\n> > +\t\t\t$matched_ref = $ref;\n> > +\t\t\tlast;\n> > +\t\t}\n> >  \t}\n> >  \t# find log type for feed description (title)\n> >  \tmy $type = 'log';\n> > @@ -2540,7 +2566,7 @@ sub get_feed_info {\n> >  \t}\n> >  \n> >  \t$res{-title} = $type;\n> > -\t$res{'hash'} = (defined $branch ? \"refs/heads/$branch\" : undef);\n> > +\t$res{'hash'} = (defined $branch ? \"refs/$matched_ref/$branch\" : undef);\n> >  \t$res{'file_name'} = $file_name;\n> \n> OK.\n> \n> > @@ -3184,24 +3210,43 @@ sub git_get_project_owner {\n> >  \treturn $owner;\n> >  }\n> >  \n> > -sub git_get_last_activity {\n> > -\tmy ($path) = @_;\n> > -\tmy $fd;\n> > +sub git_get_last_activity_age {\n> > +\tmy ($refs) = @_;\n> > +\tmy $fd = -1;\n> >  \n> > -\t$git_dir = \"$projectroot/$path\";\n> >  \topen($fd, \"-|\", git_cmd(), 'for-each-ref',\n> >  \t     '--format=%(committer)',\n> >  \t     '--sort=-committerdate',\n> >  \t     '--count=1',\n> > -\t     'refs/heads') or return;\n> > +\t     $refs) or return undef;\n> > +\n> >  \tmy $most_recent = <$fd>;\n> > -\tclose $fd or return;\n> > +\tclose $fd or return undef;\n> >  \tif (defined $most_recent &&\n> >  \t    $most_recent =~ / (\\d+) [-+][01]\\d\\d\\d$/) {\n> >  \t\tmy $timestamp = $1;\n> > -\t\tmy $age = time - $timestamp;\n> > -\t\treturn ($age, age_string($age));\n> > +\t\treturn time - $timestamp;\n> >  \t}\n> > +\n> > +\treturn undef;\n> > +}\n> > +\n> > +sub git_get_last_activity {\n> > +\tmy ($path) = @_;\n> > +\tmy @ages = ();\n> > +\n> > +\t$git_dir = \"$projectroot/$path\";\n> > +\tfor my $ref (get_branch_refs()) {\n> > +\t\tmy $age = git_get_last_activity_age('refs/' . $_);\n> > +\n> > +\t\tpush @ages, $age if defined $age;\n> > +\t}\n> > +\tif (@ages) {\n> > +\t\tmy $min_age = min(@ages);\n> > +\n> > +\t\treturn ($min_age, age_string($min_age));\n> > +\t}\n> > +\n> >  \treturn (undef, undef);\n> >  }\n> \n> The original runs\n> \n> \tfor-each-ref --count=1 refs/heads\n> \n> and picks the latest one, so shouldn't it be the matter of running\n> \n> \tfor-each-ref --count=1 refs/heads refs/extra\n> \n> i.e. something like\n> \n> @@ -3193,7 +3193,7 @@ sub git_get_last_activity {\n>  \t     '--format=%(committer)',\n>  \t     '--sort=-committerdate',\n>  \t     '--count=1',\n> -\t     'refs/heads') or return;\n> +\t     map { \"refs/$_\" } get_branch_refs()) or return;\n>  \tmy $most_recent = <$fd>;\n>  \tclose $fd or return;\n>  \tif (defined $most_recent &&\n> \n> to grab the latest among the refs you care about?\n\nThat's simpler, yes.\n\n> \n> > @@ -3223,7 +3268,7 @@ sub git_get_remotes_list {\n> >  \t\tnext if $wanted and not $remote eq $wanted;\n> >  \t\tmy ($url, $key) = ($1, $2);\n> >  \n> > -\t\t$remotes{$remote} ||= { 'heads' => () };\n> > +\t\t$remotes{$remote} ||= { map { $_ => () } get_branch_refs() };\n> >  \t\t$remotes{$remote}{$key} = $url;\n> >  \t}\n> >  \tclose $fd or return;\n> > @@ -3237,9 +3282,11 @@ sub fill_remote_heads {\n> >  \tmy @heads = map { \"remotes/$_\" } keys %$remotes;\n> >  \tmy @remoteheads = git_get_heads_list(undef, @heads);\n> >  \tforeach my $remote (keys %$remotes) {\n> > -\t\t$remotes->{$remote}{'heads'} = [ grep {\n> > -\t\t\t$_->{'name'} =~ s!^$remote/!!\n> > -\t\t\t} @remoteheads ];\n> > +\t\tforeach my $ref (get_branch_refs()) {\n> > +\t\t\t$remotes->{$remote}{$ref} = [ grep {\n> > +\t\t\t\t$_->{'name'} =~ s!^$remote/!!\n> > +\t\t\t\t} @remoteheads ];\n> > +\t\t}\n> >  \t}\n> >  }\n> \n> Hmmm, why?  Aren't these additional ones about the local\n> \"branch-like\" refs?  What makes us think that these names are also\n> shared with the names from remote hierarchies?\n\nMight be another place where I had no clue if the change is right.\nSorry.\n\n> \n> > @@ -3644,7 +3691,7 @@ sub parse_from_to_diffinfo {\n> >  \n> >  sub git_get_heads_list {\n> >  \tmy ($limit, @classes) = @_;\n> > -\t@classes = ('heads') unless @classes;\n> > +\t@classes = get_branch_refs() unless @classes;\n> >  \tmy @patterns = map { \"refs/$_\" } @classes;\n> >  \tmy @headslist;\n> >  \n> > @@ -3662,7 +3709,8 @@ sub git_get_heads_list {\n> >  \t\tmy ($committer, $epoch, $tz) =\n> >  \t\t\t($committerinfo =~ /^(.*) ([0-9]+) (.*)$/);\n> >  \t\t$ref_item{'fullname'}  = $name;\n> > -\t\t$name =~ s!^refs/(?:head|remote)s/!!;\n> > +\t\tmy $strip_refs = join '|', get_branch_refs();\n> > +\t\t$name =~ s!^refs/(?:$strip_refs|remotes)/!!;\n> >  \n> >  \t\t$ref_item{'name'}  = $name;\n> >  \t\t$ref_item{'id'}    = $hash;\n> > @@ -4286,7 +4334,7 @@ sub git_print_page_nav {\n> >  # available if the feature is enabled\n> >  sub format_ref_views {\n> >  \tmy ($current) = @_;\n> > -\tmy @ref_views = qw{tags heads};\n> > +\tmy @ref_views = ('tags', get_branch_refs());\n\nI'll get rid of that change, too. I just verified that it is wrong too.\n\n> >  \tpush @ref_views, 'remotes' if gitweb_check_feature('remote_heads');\n> >  \treturn join \" | \", map {\n> >  \t\t$_ eq $current ? $_ :\n> > @@ -7179,7 +7227,8 @@ sub snapshot_name {\n> >  \t\t$ver = $1;\n> >  \t} else {\n> >  \t\t# branches and other need shortened SHA-1 hash\n> > -\t\tif ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {\n> > +\t\tmy $strip_refs = join '|', get_branch_refs();\n> > +\t\tif ($hash =~ m!^refs/(?:$strip_refs|remotes)/(.*)$!) {\n> >  \t\t\t$ver = $1;\n> >  \t\t}\n> >  \t\t$ver .= '-' . git_get_short_hash($project, $hash);\n> \n> As the elements of @additional_branch_refs are expected by code\n> added by this patch to never have any special metacharacters in\n> them, it probably is a good idea to sanity check it to catch\n> misconfigurations and typos before doing anything else.\n> \n\nI assume that in case of misconfiguration we are returning 500.\n\nI added a sanity checking for duplicate refs, for explicitly specified\n'heads' and refs that fail to pass the validate_ref (which I splitted\nfrom validate_refname).\n\nAs for metacharacters - valid ref can contain a '|' which is a\nmetacharacter in regex, so I wrapped some uses of\n@additional_branch_refs members in regexes into \\Q and \\E or used a\nquotemeta on them. That escapes metacharacters.\n\nAlso, I added some docs to gitweb.conf.txt.\n\nPatch in \"[PATCH v2] gitweb: Add an option for adding more branch refs\"\n\n> Other than that, looks good to me.\n> \n> Thanks.\n\n-- \nKrzesimir Nowak\nSoftware Developer\nEndocode AG\n\nkrzesimir@endocode.com\n\n------\nEndocode AG, Johannisstraße 20, 10117 Berlin\ninfo@endocode.com | www.endocode.com\n\nVorstandsvorsitzender: Mirko Boehm\nVorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker\nAufsichtsratsvorsitzende: Jennifer Beecher\n\nRegistergericht: Amtsgericht Charlottenburg - HRB 150748 B\n"},{"id":"231196","messageId":"52961618.1070603@gmail.com","threadId":"35407","inReplyTo":"1385462243-7898-1-git-send-email-krzesimir@endocode.com","subject":"Re: [PATCH] gitweb: Add an option for adding more branch refs","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2013-11-27T15:56:08Z","receivedAt":"2013-11-27T15:56:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Krzesimir Nowak wrote:\n\n> Overriding an @additional_branch_refs configuration variable with\n> value ('wip') will make gitweb to show branches that appear in\n> refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for\n> gerrit setups where user branches are not stored under refs/heads/.\n>\n\nThe description of this change starts with technical details,\ninstead of starting with intent of this change.\n\nPerhaps (this is only a proposal)\n\n   Introduce @additional_branch_refs configuration variable, holding\n   names of references to be considered branches; by default empty.\n   For example setting it to ('wip') will make gitweb ...\n\n\nBTW. I have thought at first that is something similar to 'remote_heads'\nfeature, which among others adds 'remotes' section to 'summary' view\ndisplaying refs/remotes/* refs... but no, gitweb still doesn't treat \nrefs/remotes as branches, even with this feature set.\n\nNb. why new configuration variable, and not new %feature?\n\n> Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>\n> ---\n>  gitweb/gitweb.perl | 99 ++++++++++++++++++++++++++++++++++++++++--------------\n>  1 file changed, 74 insertions(+), 25 deletions(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 68c77f6..9bfd38b 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -17,6 +17,7 @@ use Encode;\n>  use Fcntl ':mode';\n>  use File::Find qw();\n>  use File::Basename qw(basename);\n> +use List::Util qw(min);\n>  use Time::HiRes qw(gettimeofday tv_interval);\n>  binmode STDOUT, ':utf8';\n>\n[...]\n > @@ -3184,24 +3210,43 @@ sub git_get_project_owner {\n >   \treturn $owner;\n >   }\n >\n > -sub git_get_last_activity {\n > -\tmy ($path) = @_;\n > -\tmy $fd;\n > +sub git_get_last_activity_age {\n > +\tmy ($refs) = @_;\n > +\tmy $fd = -1;\n >\n > -\t$git_dir = \"$projectroot/$path\";\n >   \topen($fd, \"-|\", git_cmd(), 'for-each-ref',\n >   \t     '--format=%(committer)',\n >   \t     '--sort=-committerdate',\n >   \t     '--count=1',\n > -\t     'refs/heads') or return;\n > +\t     $refs) or return undef;\n\ngit-for-each-ref accepts more than one pattern. Why not simply\n\n    \topen($fd, \"-|\", git_cmd(), 'for-each-ref',\n    \t     '--format=%(committer)',\n    \t     '--sort=-committerdate',\n    \t     '--count=1',\n   -\t     'refs/heads') or return;\n   +\t     get_branch_refs()) or return;\n\nThen we won't need List::Util::min.\n\n[...]\n > +sub git_get_last_activity {\n > +\tmy ($path) = @_;\n > +\tmy @ages = ();\n > +\n > +\t$git_dir = \"$projectroot/$path\";\n > +\tfor my $ref (get_branch_refs()) {\n > +\t\tmy $age = git_get_last_activity_age('refs/' . $_);\n > +\n > +\t\tpush @ages, $age if defined $age;\n > +\t}\n > +\tif (@ages) {\n > +\t\tmy $min_age = min(@ages);\n > +\n > +\t\treturn ($min_age, age_string($min_age));\n > +\t}\n > +\n >   \treturn (undef, undef);\n >   }\n >\n[...]\n-- \nJakub Narębski\n"},{"id":"231197","messageId":"1385568927.2131.30.camel@localhost.localdomain","threadId":"35407","inReplyTo":"52961618.1070603@gmail.com","subject":"Re: [PATCH] gitweb: Add an option for adding more branch refs","fromName":"Krzesimir Nowak","fromEmail":"krzesimir@endocode.com","sentAt":"2013-11-27T16:15:27Z","receivedAt":"2013-11-27T16:15:27Z","isPatch":true,"sender":{"key":"krzesimir@endocode.com","avatar":"https://avatars.githubusercontent.com/u/96081?v=4"},"body":"On Wed, 2013-11-27 at 16:56 +0100, Jakub Narębski wrote:\n> Krzesimir Nowak wrote:\n> \n> > Overriding an @additional_branch_refs configuration variable with\n> > value ('wip') will make gitweb to show branches that appear in\n> > refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for\n> > gerrit setups where user branches are not stored under refs/heads/.\n> >\n> \n> The description of this change starts with technical details,\n> instead of starting with intent of this change.\n> \n> Perhaps (this is only a proposal)\n> \n>    Introduce @additional_branch_refs configuration variable, holding\n>    names of references to be considered branches; by default empty.\n>    For example setting it to ('wip') will make gitweb ...\n> \n\nI have already posted second version of the patch. But I didn't change\nthe commit message though. But thanks for proposal - it sounds better.\nI'll try to make it better next time I post a patch.\n\n> \n> BTW. I have thought at first that is something similar to 'remote_heads'\n> feature, which among others adds 'remotes' section to 'summary' view\n> displaying refs/remotes/* refs... but no, gitweb still doesn't treat \n> refs/remotes as branches, even with this feature set.\n> \n> Nb. why new configuration variable, and not new %feature?\n\nI dunno. Hard to tell where it fits. Junio told me about using \"normal\ngitweb configuration mechanism\", so that's the first thing that got my\nattention.\n\nhttp://www.mail-archive.com/git@vger.kernel.org/msg39859.html\n\n> \n> > Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>\n> > ---\n> >  gitweb/gitweb.perl | 99 ++++++++++++++++++++++++++++++++++++++++--------------\n> >  1 file changed, 74 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index 68c77f6..9bfd38b 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -17,6 +17,7 @@ use Encode;\n> >  use Fcntl ':mode';\n> >  use File::Find qw();\n> >  use File::Basename qw(basename);\n> > +use List::Util qw(min);\n> >  use Time::HiRes qw(gettimeofday tv_interval);\n> >  binmode STDOUT, ':utf8';\n> >\n> [...]\n>  > @@ -3184,24 +3210,43 @@ sub git_get_project_owner {\n>  >   \treturn $owner;\n>  >   }\n>  >\n>  > -sub git_get_last_activity {\n>  > -\tmy ($path) = @_;\n>  > -\tmy $fd;\n>  > +sub git_get_last_activity_age {\n>  > +\tmy ($refs) = @_;\n>  > +\tmy $fd = -1;\n>  >\n>  > -\t$git_dir = \"$projectroot/$path\";\n>  >   \topen($fd, \"-|\", git_cmd(), 'for-each-ref',\n>  >   \t     '--format=%(committer)',\n>  >   \t     '--sort=-committerdate',\n>  >   \t     '--count=1',\n>  > -\t     'refs/heads') or return;\n>  > +\t     $refs) or return undef;\n> \n> git-for-each-ref accepts more than one pattern. Why not simply\n> \n>     \topen($fd, \"-|\", git_cmd(), 'for-each-ref',\n>     \t     '--format=%(committer)',\n>     \t     '--sort=-committerdate',\n>     \t     '--count=1',\n>    -\t     'refs/heads') or return;\n>    +\t     get_branch_refs()) or return;\n> \n> Then we won't need List::Util::min.\n\nYes, Junio pointed that out to me - fixed in second version of the\npatch.\n\n> \n> [...]\n>  > +sub git_get_last_activity {\n>  > +\tmy ($path) = @_;\n>  > +\tmy @ages = ();\n>  > +\n>  > +\t$git_dir = \"$projectroot/$path\";\n>  > +\tfor my $ref (get_branch_refs()) {\n>  > +\t\tmy $age = git_get_last_activity_age('refs/' . $_);\n>  > +\n>  > +\t\tpush @ages, $age if defined $age;\n>  > +\t}\n>  > +\tif (@ages) {\n>  > +\t\tmy $min_age = min(@ages);\n>  > +\n>  > +\t\treturn ($min_age, age_string($min_age));\n>  > +\t}\n>  > +\n>  >   \treturn (undef, undef);\n>  >   }\n>  >\n> [...]\n\n-- \nKrzesimir Nowak\nSoftware Developer\nEndocode AG\n\nkrzesimir@endocode.com\n\n------\nEndocode AG, Johannisstraße 20, 10117 Berlin\ninfo@endocode.com | www.endocode.com\n\nVorstandsvorsitzender: Mirko Boehm\nVorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker\nAufsichtsratsvorsitzende: Jennifer Beecher\n\nRegistergericht: Amtsgericht Charlottenburg - HRB 150748 B\n"}]}