{"thread":{"id":"35411","subject":"[PATCH v2] gitweb: Add an option for adding more branch refs","startedAt":"2013-11-27T15:30:21Z","lastAt":"2013-11-28T11:49:17Z","messageCount":5,"participants":["Krzesimir Nowak","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"231191","messageId":"1385566221-7977-1-git-send-email-krzesimir@endocode.com","threadId":"35411","inReplyTo":null,"subject":"[PATCH v2] gitweb: Add an option for adding more branch refs","fromName":"Krzesimir Nowak","fromEmail":"krzesimir@endocode.com","sentAt":"2013-11-27T15:30:21Z","receivedAt":"2013-11-27T15:30:21Z","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 Documentation/gitweb.conf.txt | 14 ++++++++\n gitweb/gitweb.perl            | 75 +++++++++++++++++++++++++++++++++----------\n 2 files changed, 72 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex e2113d9..74de87a 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -549,6 +549,20 @@ This variable matters only when using persistent web environments that\n serve multiple requests using single gitweb instance, like mod_perl,\n FastCGI or Plackup.\n \n+@additional_branch_refs::\n+\tList of additional directories under \"refs\" which are going to be used\n+\tas branch refs. You might want to set this variable if you have a gerrit\n+\tsetup where all branches under refs/heads/ are official,\n+\tpush-after-review ones and branches under refs/sandbox/, refs/wip and\n+\trefs/other are user ones where permissions are much wider, for example\n++\n+--------------------------------------------------------------------------------\n+our @additional_branch_refs = ('sandbox', 'wip', 'other');\n+--------------------------------------------------------------------------------\n++\n+It is an error to specify 'heads' in the list - it is implicitly added. It is an\n+error to specify a ref that does not pass \"git check-ref-format\" scrutiny. It is\n+an error to specify a ref twice in the list.\n \n Other variables\n ~~~~~~~~~~~~~~~\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 68c77f6..499281b 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -122,6 +122,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,6 +630,10 @@ 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@@ -680,6 +688,19 @@ sub read_config_file {\n \treturn;\n }\n \n+# performs sanity checks on parts of configuration.\n+sub config_sanity_check {\n+\t# check additional refs validity\n+\tmy %unique_branch_refs = ();\n+\tfor my $ref (@additional_branch_refs) {\n+\t\tdie_error(500, 'Invalid ref in @additional_branch_refs') unless (validate_ref($ref));\n+\t\tdie_error(500, '\"heads\" specified in @additional_branch_refs') if ($ref eq 'heads');\n+\t\tdie_error(500, \"Ref '$ref' repeated in \\@additional_branch_refs\") if (exists $unique_branch_refs{$ref});\n+\t\t$unique_branch_refs{$ref} = 1;\n+\t}\n+\t%unique_branch_refs = undef;\n+}\n+\n our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM, $GITWEB_CONFIG_COMMON);\n sub evaluate_gitweb_config {\n \tour $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n@@ -698,8 +719,11 @@ sub evaluate_gitweb_config {\n \n \t# Use first config file that exists.  This means use the per-instance\n \t# GITWEB_CONFIG if exists, otherwise use GITWEB_SYSTEM_CONFIG.\n-\tread_config_file($GITWEB_CONFIG) and return;\n-\tread_config_file($GITWEB_CONFIG_SYSTEM);\n+\tif (!read_config_file($GITWEB_CONFIG)) {\n+\t\tread_config_file($GITWEB_CONFIG_SYSTEM);\n+\t}\n+\n+\tconfig_sanity_check();\n }\n \n # Get loadavg of system, to compare against $maxload.\n@@ -1452,6 +1476,16 @@ sub validate_pathname {\n \treturn $input;\n }\n \n+sub validate_ref {\n+\tmy $input = shift || return undef;\n+\n+\t# restrictions on ref name according to git-check-ref-format\n+\tif ($input =~ m!(/\\.|\\.\\.|[\\000-\\040\\177 ~^:?*\\[]|/$)!) {\n+\t\treturn undef;\n+\t}\n+\treturn $input;\n+}\n+\n sub validate_refname {\n \tmy $input = shift || return undef;\n \n@@ -1462,10 +1496,9 @@ sub validate_refname {\n \t# it must be correct pathname\n \t$input = validate_pathname($input)\n \t\tor return undef;\n-\t# restrictions on ref name according to git-check-ref-format\n-\tif ($input =~ m!(/\\.|\\.\\.|[\\000-\\040\\177 ~^:?*\\[]|/$)!) {\n-\t\treturn undef;\n-\t}\n+\t# check git-check-ref-format restrictions\n+\t$input = validate_ref($input)\n+\t\tor return undef;\n \treturn $input;\n }\n \n@@ -2515,6 +2548,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 +2556,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/\\Q$ref\\E/(.*)$!) ||\n+\t\t    (defined $hash      && $hash      =~ m!^refs/\\Q$ref\\E/(.*)$!)) {\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 +2579,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@@ -3193,7 +3232,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@@ -3644,7 +3683,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 +3701,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 '|', map { quotemeta } 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@@ -7179,7 +7219,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 '|', map { quotemeta } 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":"231212","messageId":"CAPig+cT=P3ZC9j-WwQjr7gu6RrNgLrXsPkiVmdy5OGYfu8mPqA@mail.gmail.com","threadId":"35411","inReplyTo":"1385566221-7977-1-git-send-email-krzesimir@endocode.com","subject":"Re: [PATCH v2] gitweb: Add an option for adding more branch refs","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-11-27T20:34:49Z","receivedAt":"2013-11-27T20:34:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Nov 27, 2013 at 10:30 AM, Krzesimir Nowak\n<krzesimir@endocode.com> wrote:\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> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 68c77f6..499281b 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -680,6 +688,19 @@ sub read_config_file {\n>         return;\n>  }\n>\n> +# performs sanity checks on parts of configuration.\n> +sub config_sanity_check {\n> +       # check additional refs validity\n> +       my %unique_branch_refs = ();\n> +       for my $ref (@additional_branch_refs) {\n> +               die_error(500, 'Invalid ref in @additional_branch_refs') unless (validate_ref($ref));\n\nMentioning $ref in the error message would help the user resolve the\nproblem more quickly.\n\n> +               die_error(500, '\"heads\" specified in @additional_branch_refs') if ($ref eq 'heads');\n\nRephrasing this as\n\n    \"heads\" disallowed in @additional_branch_refs\n\nwould better explain the problem to a user who has only made a cursory\nread of the documentation.\n\n> +               die_error(500, \"Ref '$ref' repeated in \\@additional_branch_refs\") if (exists $unique_branch_refs{$ref});\n> +               $unique_branch_refs{$ref} = 1;\n> +       }\n> +       %unique_branch_refs = undef;\n> +}\n> +\n>  our ($GITWEB_CONFIG, $GITWEB_CONFIG_SYSTEM, $GITWEB_CONFIG_COMMON);\n>  sub evaluate_gitweb_config {\n>         our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n"},{"id":"231213","messageId":"CAPig+cR3hoWmDPJyWP6HbpuDvRwgdNj5VwX8A20DCfRBK5OuSQ@mail.gmail.com","threadId":"35411","inReplyTo":"CAPig+cT=P3ZC9j-WwQjr7gu6RrNgLrXsPkiVmdy5OGYfu8mPqA@mail.gmail.com","subject":"Re: [PATCH v2] gitweb: Add an option for adding more branch refs","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-11-27T20:38:11Z","receivedAt":"2013-11-27T20:38:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Nov 27, 2013 at 3:34 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Wed, Nov 27, 2013 at 10:30 AM, Krzesimir Nowak\n> <krzesimir@endocode.com> wrote:\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>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 68c77f6..499281b 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -680,6 +688,19 @@ sub read_config_file {\n>>         return;\n>>  }\n>>\n>> +# performs sanity checks on parts of configuration.\n>> +sub config_sanity_check {\n>> +       # check additional refs validity\n>> +       my %unique_branch_refs = ();\n>> +       for my $ref (@additional_branch_refs) {\n>> +               die_error(500, 'Invalid ref in @additional_branch_refs') unless (validate_ref($ref));\n>\n> Mentioning $ref in the error message would help the user resolve the\n> problem more quickly.\n>\n>> +               die_error(500, '\"heads\" specified in @additional_branch_refs') if ($ref eq 'heads');\n>\n> Rephrasing this as\n>\n>     \"heads\" disallowed in @additional_branch_refs\n>\n> would better explain the problem to a user who has only made a cursory\n> read of the documentation.\n\nThe program could easily filter out the redundant 'heads', so does\nthis really deserve a diagnostic?\n"},{"id":"231214","messageId":"xmqqob55sgww.fsf@gitster.dls.corp.google.com","threadId":"35411","inReplyTo":"CAPig+cR3hoWmDPJyWP6HbpuDvRwgdNj5VwX8A20DCfRBK5OuSQ@mail.gmail.com","subject":"Re: [PATCH v2] gitweb: Add an option for adding more branch refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-27T20:55:59Z","receivedAt":"2013-11-27T20:55:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Wed, Nov 27, 2013 at 3:34 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>> On Wed, Nov 27, 2013 at 10:30 AM, Krzesimir Nowak\n>> <krzesimir@endocode.com> wrote:\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).\n\n\"branches\" are by definition what are in refs/heads/ hierarchy, so \n\n  Allow @additional_branch_refs configuration variable to tell\n  gitweb to show refs from additional hierarchies in addition to\n  branches in the list-of-branches view.\n\nwould be more appropriate and sufficient.\n\n>> Mentioning $ref in the error message would help the user resolve the\n>> problem more quickly.\n>>\n>>> +               die_error(500, '\"heads\" specified in @additional_branch_refs') if ($ref eq 'heads');\n>>\n>> Rephrasing this as\n>>\n>>     \"heads\" disallowed in @additional_branch_refs\n>>\n>> would better explain the problem to a user who has only made a cursory\n>> read of the documentation.\n>\n> The program could easily filter out the redundant 'heads', so does\n> this really deserve a diagnostic?\n\nTrue.\n\nI was primarily worried about metacharacters in the specified\nstrings getting in the way of regexp matches the new code allows on\nthem, but that has been resolved with the use of \\Q..\\E; if that\nautomatic deduping is done, I do not immediately see any remaining\nissues in the latest round of the patch.\n\nThanks.\n"},{"id":"231242","messageId":"1385639357.2075.3.camel@localhost.localdomain","threadId":"35411","inReplyTo":"xmqqob55sgww.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] gitweb: Add an option for adding more branch refs","fromName":"Krzesimir Nowak","fromEmail":"krzesimir@endocode.com","sentAt":"2013-11-28T11:49:17Z","receivedAt":"2013-11-28T11:49:17Z","isPatch":true,"sender":{"key":"krzesimir@endocode.com","avatar":"https://avatars.githubusercontent.com/u/96081?v=4"},"body":"On Wed, 2013-11-27 at 12:55 -0800, Junio C Hamano wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n> \n> > On Wed, Nov 27, 2013 at 3:34 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> >> On Wed, Nov 27, 2013 at 10:30 AM, Krzesimir Nowak\n> >> <krzesimir@endocode.com> wrote:\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).\n> \n> \"branches\" are by definition what are in refs/heads/ hierarchy, so \n> \n>   Allow @additional_branch_refs configuration variable to tell\n>   gitweb to show refs from additional hierarchies in addition to\n>   branches in the list-of-branches view.\n> \n> would be more appropriate and sufficient.\n\nThanks.\n\n> \n> >> Mentioning $ref in the error message would help the user resolve the\n> >> problem more quickly.\n> >>\n> >>> +               die_error(500, '\"heads\" specified in @additional_branch_refs') if ($ref eq 'heads');\n> >>\n> >> Rephrasing this as\n> >>\n> >>     \"heads\" disallowed in @additional_branch_refs\n> >>\n> >> would better explain the problem to a user who has only made a cursory\n> >> read of the documentation.\n> >\n> > The program could easily filter out the redundant 'heads', so does\n> > this really deserve a diagnostic?\n> \n> True.\n\nOk, I'm deduping both heads and other refs as well. Now we send 500 only\nif the ref is simply invalid.\n\n> \n> I was primarily worried about metacharacters in the specified\n> strings getting in the way of regexp matches the new code allows on\n> them, but that has been resolved with the use of \\Q..\\E; if that\n> automatic deduping is done, I do not immediately see any remaining\n> issues in the latest round of the patch.\n> \n\nNew patch in \"[PATCH v3] gitweb: Add an option for adding more branch\nrefs\". Thanks for reviews!\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"}]}