{"thread":{"id":"29634","subject":"[PATCHv2 0/3] gitweb: Dealing with being on unborn branch","startedAt":"2012-02-15T15:36:40Z","lastAt":"2012-02-17T14:28:18Z","messageCount":9,"participants":["Jakub Narebski","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"184764","messageId":"1329320203-20272-1-git-send-email-jnareb@gmail.com","threadId":"29634","inReplyTo":null,"subject":"[PATCHv2 0/3] gitweb: Dealing with being on unborn branch","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-15T15:36:40Z","receivedAt":"2012-02-15T15:36:40Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This is resend (as series) of patches that were sent first time\nindividually in \"Fwd: Git-web error\" thread sent to git mailing list\nby rajesh boyapati:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/189228\n\nUnfortunately the thread is heavily fragmented in GMane interface,\nbecause Rajesh responses were not sent to git mailing list.\n\nThe problem that original poster seen was caused by the fact that:\n\nrb> For my git projects on gerrit, our main branch name is \"base\".\nrb> We don't have any code on \"master\" branch. [...]\nrb> [...] the HEAD file is pointing to \"ref: refs/heads/master\".\nrb> So, I think that's the reason for errors.\n\nSome of errors might appear in the way Rajesh sees them because gitweb\nin this case is deployed from Gerrit (Git-based review board, in Java);\nat least I couldn't reproduce some of errors in exactly the same way\nas presented.\n\nNote that there is no problem if repository is totally empty, without\nany valid branch.  The problem is when HEAD doesn't point to the valid\ncommit object, but there are other commits and branches.\n\nAnyway beside not using 'master' branch, but not renaming it /\nrepointing HEAD, such situation might happen when you have just\ncreated an orphan branch, but haven't made any commits on it yet.\n\n\nTable of contents:\n~~~~~~~~~~~~~~~~~~\n* [PATCHv2 1/3] gitweb: Deal with HEAD pointing to unborn branch\n  in \"heads\" view\n\nThis fixes a real issue, and is a true fix (for current way of\ngenerating \"heads\" view).  Note that there is \"heads\" excerpt in\n\"summary\" view.\n\n* [PATCHv2/RFC 2/3] gitweb: Harden parse_commit and parse_commits\n\nJust add an early exit in the case of invalid parameters.  Though if\ncaller is passing undefined value for commit identifier, it would\nprobably not deal correctly with parse_commit() returning undef,\nthough it should deal correctly with parse_commits() returning empty\nlist.\n\nMore of band-aid than a real fix.\n\n* [RFC/PATCHv2 3/3] gitweb: Silence stderr in parse_commit*()\n  subroutines\n\nThis adds an overhead of forking a shell, and possibility of code\ninjection if our quote_command() is incorrect... and actually does not\nfix issue for the original poster, and is not a problem for ordinary\n(non-Gerrit) gitweb.\n\n\nShortlog:\n~~~~~~~~~\nJakub Narebski (3):\n  gitweb: Deal with HEAD pointing to unborn branch in \"heads\" view\n  gitweb: Harden parse_commit and parse_commits\n  gitweb: Silence stderr in parse_commit*() subroutines\n\n\nDiffstat:\n~~~~~~~~~\n gitweb/gitweb.perl                     |   21 ++++++++++++++-------\n t/t9500-gitweb-standalone-no-errors.sh |    9 +++++++++\n 2 files changed, 23 insertions(+), 7 deletions(-)\n\n-- \n1.7.9\n"},{"id":"184765","messageId":"1329320203-20272-2-git-send-email-jnareb@gmail.com","threadId":"29634","inReplyTo":"1329320203-20272-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv2 1/3] gitweb: Deal with HEAD pointing to unborn branch in \"heads\" view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-15T15:36:41Z","receivedAt":"2012-02-15T15:36:41Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Gitweb has problems if HEAD points to an unborn branch, with no\ncommits on it yet, but there are other branches present (so it is not\nnewly initialized repository).\n\nThis can happen if non-bare repository (with default 'master' branch)\nis updated not via committing but by other means like push to it, or\nGerrit.  It can happen also just after running \"git checkout --orphan\n<new branch>\" but before creating any new commit on this branch.\n\nThis commit adds test and fixes the issue of being on unborn branch\n(of HEAD not pointing to a commit) in \"heads\" view, and also in\n\"summary\" view -- which includes \"heads\" excerpt as subview.\n\nWhile at it rename local variable $head to $head_at, as it points to\ncurrent commit rather than current branch name (HEAD contents).\n\nIncludes simple test for 'summary' view and being on unborn branch.\n\nReported-by: rajesh boyapati <boyapatisrajesh@gmail.com>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis patch first appeared on git mailing list in \"Fwd: Git-web error\"\nas\n\n  [PATCH] gitweb: Deal with HEAD pointing to unborn branch in \"heads\" view\n  Message-Id: <201202032233.05324.jnareb@gmail.com>\n  http://article.gmane.org/gmane.comp.version-control.git/189805\n\nThe $head -> $head_at change is not really necessary, but while we are\nchanging that area of code I felt it would be good to make it better.\n\n\nWhat we should do in the future is rework git_heads_body to work with\nsymbolic references rather than mark branches that point to the same\ncommit as HEAD does.\n\nThat would also help in the \"detached HEAD\" case...\n\n gitweb/gitweb.perl                     |    4 ++--\n t/t9500-gitweb-standalone-no-errors.sh |    9 +++++++++\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 87a95e2..0fdca5b 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5633,7 +5633,7 @@ sub git_tags_body {\n \n sub git_heads_body {\n \t# uses global variable $project\n-\tmy ($headlist, $head, $from, $to, $extra) = @_;\n+\tmy ($headlist, $head_at, $from, $to, $extra) = @_;\n \t$from = 0 unless defined $from;\n \t$to = $#{$headlist} if (!defined $to || $#{$headlist} < $to);\n \n@@ -5642,7 +5642,7 @@ sub git_heads_body {\n \tfor (my $i = $from; $i <= $to; $i++) {\n \t\tmy $entry = $headlist->[$i];\n \t\tmy %ref = %$entry;\n-\t\tmy $curr = $ref{'id'} eq $head;\n+\t\tmy $curr = defined $head_at && $ref{'id'} eq $head_at;\n \t\tif ($alternate) {\n \t\t\tprint \"<tr class=\\\"dark\\\">\\n\";\n \t\t} else {\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 0f771c6..81246a6 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -739,4 +739,13 @@ test_expect_success \\\n \t'echo \"\\$projects_list_group_categories = 1;\" >>gitweb_config.perl &&\n \t gitweb_run'\n \n+# ----------------------------------------------------------------------\n+# unborn branches\n+\n+test_expect_success \\\n+\t'unborn HEAD: \"summary\" page (with \"heads\" subview)' \\\n+\t'git checkout orphan_branch || git checkout --orphan orphan_branch &&\n+\t test_when_finished \"git checkout master\" &&\n+\t gitweb_run \"p=.git;a=summary\"'\n+\n test_done\n-- \n1.7.9\n"},{"id":"184767","messageId":"1329320203-20272-3-git-send-email-jnareb@gmail.com","threadId":"29634","inReplyTo":"1329320203-20272-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv2/RFC 2/3] gitweb: Harden parse_commit and parse_commits","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-15T15:36:42Z","receivedAt":"2012-02-15T15:36:42Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Gitweb has problems and gives errors when repository it shows is on\nunborn branch (HEAD doesn't point to a valid commit), but there exist\nother branches.\n\nOne of errors that shows in gitweb logs is undefined $commit_id in\nparse_commits() subroutine.  Therefore we harden both parse_commit()\nand parse_commits() against undefined $commit_id, and against no\noutput from git-rev-list because HEAD doesn't point to a commit.\n\nReported-by: rajesh boyapati <boyapatisrajesh@gmail.com>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis patch first appeared on git mailing list in \"Fwd: Git-web error\"\nthread as\n\n  [PATCH] gitweb: Harden parse_commit and parse_commits\n  Message-Id: <201202081604.17187.jnareb@gmail.com>\n  http://article.gmane.org/gmane.comp.version-control.git/190237\n\nMore prevention of generating warnings, rather than real fix.\n\n gitweb/gitweb.perl |    7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0fdca5b..2eaf585 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3334,6 +3334,8 @@ sub parse_commit {\n \tmy ($commit_id) = @_;\n \tmy %co;\n \n+\treturn unless defined $commit_id;\n+\n \tlocal $/ = \"\\0\";\n \n \topen my $fd, \"-|\", git_cmd(), \"rev-list\",\n@@ -3343,7 +3345,9 @@ sub parse_commit {\n \t\t$commit_id,\n \t\t\"--\",\n \t\tor die_error(500, \"Open git-rev-list failed\");\n-\t%co = parse_commit_text(<$fd>, 1);\n+\tmy $commit_text = <$fd>;\n+\t%co = parse_commit_text($commit_text, 1)\n+\t\tif defined $commit_text;\n \tclose $fd;\n \n \treturn %co;\n@@ -3353,6 +3357,7 @@ sub parse_commits {\n \tmy ($commit_id, $maxcount, $skip, $filename, @args) = @_;\n \tmy @cos;\n \n+\treturn unless defined $commit_id;\n \t$maxcount ||= 1;\n \t$skip ||= 0;\n \n-- \n1.7.9\n"},{"id":"184766","messageId":"1329320203-20272-4-git-send-email-jnareb@gmail.com","threadId":"29634","inReplyTo":"1329320203-20272-1-git-send-email-jnareb@gmail.com","subject":"[RFC/PATCHv2 3/3] gitweb: Silence stderr in parse_commit*() subroutines","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-15T15:36:43Z","receivedAt":"2012-02-15T15:36:43Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"git-rev-list command in parse_commit() and parse_commits() can fail\nbecause $commit_id doesn't point to a valid commit; for example if we\nare on unborn branch HEAD doesn't point to a valid commit.\n\nIn this case git-rev-list prints\n\n  fatal: bad revision 'HEAD'\n\non its standard error.  This commit silences this warning, at the cost\nof using shell to redirect it to /dev/null, and relying on\nquote_command() to protect against code injection.\n\nNote however that such error message from git (from external command)\nusually (but not always) does not appear in web server logs.\n\nReported-by: rajesh boyapati <boyapatisrajesh@gmail.com>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis patch first appeared on git mailing list in \"Fwd: Git-web error\"\nthread as\n\n  [PATCH] gitweb: Silence stderr in parse_commit*() subroutines\n  Message-Id: <201202111402.31684.jnareb@gmail.com>\n  http://article.gmane.org/gmane.comp.version-control.git/190511\n\nMore proof of concept rather than real fix for real issue;\nit doesn't even fix the issue (for some reason) for original\nposter.\n\n gitweb/gitweb.perl |   10 ++++++----\n 1 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 2eaf585..224ace4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3338,12 +3338,13 @@ sub parse_commit {\n \n \tlocal $/ = \"\\0\";\n \n-\topen my $fd, \"-|\", git_cmd(), \"rev-list\",\n+\topen my $fd, \"-|\", quote_command(\n+\t\tgit_cmd(), \"rev-list\",\n \t\t\"--parents\",\n \t\t\"--header\",\n \t\t\"--max-count=1\",\n \t\t$commit_id,\n-\t\t\"--\",\n+\t\t\"--\") . ' 2>/dev/null',\n \t\tor die_error(500, \"Open git-rev-list failed\");\n \tmy $commit_text = <$fd>;\n \t%co = parse_commit_text($commit_text, 1)\n@@ -3363,7 +3364,8 @@ sub parse_commits {\n \n \tlocal $/ = \"\\0\";\n \n-\topen my $fd, \"-|\", git_cmd(), \"rev-list\",\n+\topen my $fd, \"-|\", quote_command(\n+\t\tgit_cmd(), \"rev-list\",\n \t\t\"--header\",\n \t\t@args,\n \t\t(\"--max-count=\" . $maxcount),\n@@ -3371,7 +3373,7 @@ sub parse_commits {\n \t\t@extra_options,\n \t\t$commit_id,\n \t\t\"--\",\n-\t\t($filename ? ($filename) : ())\n+\t\t($filename ? ($filename) : ())) . ' 2>/dev/null'\n \t\tor die_error(500, \"Open git-rev-list failed\");\n \twhile (my $line = <$fd>) {\n \t\tmy %co = parse_commit_text($line);\n-- \n1.7.9\n"},{"id":"184836","messageId":"7vr4xuy12f.fsf@alter.siamese.dyndns.org","threadId":"29634","inReplyTo":"1329320203-20272-2-git-send-email-jnareb@gmail.com","subject":"Re: [PATCHv2 1/3] gitweb: Deal with HEAD pointing to unborn branch in \"heads\" view","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-16T20:29:12Z","receivedAt":"2012-02-16T20:29:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Gitweb has problems if HEAD points to an unborn branch, with no\n> commits on it yet, but there are other branches present (so it is not\n> newly initialized repository).\n\nIt would be more readable if you rephrase the vague \"has problems\" with a\nconcrete description of what the problem is.\n\nAlso, drop the \" (so it is ...)\"  part, which does not add much useful\ninformation.  Your next paragraph describes how a repository can come to\nthis state anyway.\n\n> This can happen if non-bare repository (with default 'master' branch)\n> is updated not via committing but by other means like push to it, or\n> Gerrit.  It can happen also just after running \"git checkout --orphan\n> <new branch>\" but before creating any new commit on this branch.\n>\n> This commit adds test and fixes the issue of being on unborn branch\n> (of HEAD not pointing to a commit) in \"heads\" view, and also in\n> \"summary\" view -- which includes \"heads\" excerpt as subview.\n\nThe reader has not seen anything more than \"has problems\" at this point,\nso \"fixes the issue of ...\" is not very helpful.  You could have just said\n\"adds tests and fixes it\", if you said that the unspecified \"problems\"\napear in \"heads\" and \"summary\" view at the beginning of the log message.\n\nThanks.\n"},{"id":"184847","messageId":"201202162341.09712.jnareb@gmail.com","threadId":"29634","inReplyTo":"7vr4xuy12f.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 1/3] gitweb: Deal with HEAD pointing to unborn branch in \"heads\" view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-16T22:41:09Z","receivedAt":"2012-02-16T22:41:09Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 16 Feb 2012, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > Gitweb has problems if HEAD points to an unborn branch, with no\n> > commits on it yet, but there are other branches present (so it is not\n> > newly initialized repository).\n> \n> It would be more readable if you rephrase the vague \"has problems\" with a\n> concrete description of what the problem is.\n\nSorry about this.\n\nThe problem is that gitweb would generate the following warning, writing\nit in web server logs:\n\n  Use of uninitialized value in string eq\n\n> Also, drop the \" (so it is ...)\"  part, which does not add much useful\n> information.  Your next paragraph describes how a repository can come to\n> this state anyway.\n\nO.K.\n\nAnyway the description that repository might be in such a strange state\nmight be more important that the patch in itself...\n\n> > This can happen if non-bare repository (with default 'master' branch)\n> > is updated not via committing but by other means like push to it, or\n> > Gerrit.  It can happen also just after running \"git checkout --orphan\n> > <new branch>\" but before creating any new commit on this branch.\n> >\n> > This commit adds test and fixes the issue of being on unborn branch\n> > (of HEAD not pointing to a commit) in \"heads\" view, and also in\n> > \"summary\" view -- which includes \"heads\" excerpt as subview.\n> \n> The reader has not seen anything more than \"has problems\" at this point,\n> so \"fixes the issue of ...\" is not very helpful.  You could have just said\n> \"adds tests and fixes it\", if you said that the unspecified \"problems\"\n> apear in \"heads\" and \"summary\" view at the beginning of the log message.\n\nO.K.\n\n\nShould I re-roll this patch with improved commit message?\n-- \nJakub Narebski\nPoland\n"},{"id":"184851","messageId":"7vsjiawe74.fsf@alter.siamese.dyndns.org","threadId":"29634","inReplyTo":"201202162341.09712.jnareb@gmail.com","subject":"Re: [PATCHv2 1/3] gitweb: Deal with HEAD pointing to unborn branch in \"heads\" view","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-16T23:28:31Z","receivedAt":"2012-02-16T23:28:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> On Thu, 16 Feb 2012, Junio C Hamano wrote:\n>> Jakub Narebski <jnareb@gmail.com> writes:\n>> \n>> > Gitweb has problems if HEAD points to an unborn branch, with no\n>> > commits on it yet, but there are other branches present (so it is not\n>> > newly initialized repository).\n>> \n>> It would be more readable if you rephrase the vague \"has problems\" with a\n>> concrete description of what the problem is.\n>\n> Sorry about this.\n>\n> The problem is that gitweb would generate the following warning, writing\n> it in web server logs:\n>\n>   Use of uninitialized value in string eq\n\nWhen known and easily describable in a short paragraph, let's write both\nthe cause and the symptom together.\n\n> Should I re-roll this patch with improved commit message?\n\nI was following Peff's 4 step review process, and I was in step #1 (read\nthe log message---can I understand what this is about without looking at\nthe patch?), so I haven't reached the diff part of your message ;-)\n\nYou added \"defined $head_at &&\" to protect against the undef comparison\nfor all uses of $head and I do not see any $head that was left unconverted\nby mistake, so the patch looks OK to me (at times I wish gitweb were\nwritten in a language more strict than Perl so that a compiler can catch\nsuch mistakes).\n\nBut after trying to write a reroll myself, I have to wonder what would\nhappen if you have two branches pointing at the same commit as the one at\nHEAD.  Why isn't the use of current_head class controlled by comparison\nbetween the name of the ref and the output from \"symbolic-ref HEAD\"?\n\n-- >8 --\nFrom: Jakub Narebski <jnareb@gmail.com>\nDate: Wed, 15 Feb 2012 16:36:41 +0100\nSubject: [PATCH] gitweb: Fix \"heads\" view when there is no current branch\n\nIn a repository whose HEAD points to an unborn branch with no commits,\n\"heads\" view and \"summary\" view (which shows what is shown in \"heads\"\nview) compared the object names of commits at the tip of branches with the\noutput from \"git rev-parse HEAD\", which caused comparison of a string with\nundef and resulted in a warning in the server log.\n\nThis can happen if non-bare repository (with default 'master' branch)\nis updated not via committing but by other means like push to it, or\nGerrit.  It can happen also just after running \"git checkout --orphan\n<new branch>\" but before creating any new commit on this branch.\n\nRewrite the comparison so that it also works when $head points at nothing;\nin such a case, no branch can be \"the current branch\", add a test for it.\n\nWhile at it rename local variable $head to $head_at, as it points to\ncurrent commit rather than current branch name (HEAD contents).\n\nReported-by: Rajesh Boyapati\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n gitweb/gitweb.perl                     |    4 ++--\n t/t9500-gitweb-standalone-no-errors.sh |    9 +++++++++\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3fc7380..af154c3 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5633,7 +5633,7 @@ sub git_tags_body {\n \n sub git_heads_body {\n \t# uses global variable $project\n-\tmy ($headlist, $head, $from, $to, $extra) = @_;\n+\tmy ($headlist, $head_at, $from, $to, $extra) = @_;\n \t$from = 0 unless defined $from;\n \t$to = $#{$headlist} if (!defined $to || $#{$headlist} < $to);\n \n@@ -5642,7 +5642,7 @@ sub git_heads_body {\n \tfor (my $i = $from; $i <= $to; $i++) {\n \t\tmy $entry = $headlist->[$i];\n \t\tmy %ref = %$entry;\n-\t\tmy $curr = $ref{'id'} eq $head;\n+\t\tmy $curr = defined $head_at && $ref{'id'} eq $head_at;\n \t\tif ($alternate) {\n \t\t\tprint \"<tr class=\\\"dark\\\">\\n\";\n \t\t} else {\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 0f771c6..81246a6 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -739,4 +739,13 @@ test_expect_success \\\n \t'echo \"\\$projects_list_group_categories = 1;\" >>gitweb_config.perl &&\n \t gitweb_run'\n \n+# ----------------------------------------------------------------------\n+# unborn branches\n+\n+test_expect_success \\\n+\t'unborn HEAD: \"summary\" page (with \"heads\" subview)' \\\n+\t'git checkout orphan_branch || git checkout --orphan orphan_branch &&\n+\t test_when_finished \"git checkout master\" &&\n+\t gitweb_run \"p=.git;a=summary\"'\n+\n test_done\n-- \n1.7.9.1.236.gedc23\n"},{"id":"184882","messageId":"201202171441.35618.jnareb@gmail.com","threadId":"29634","inReplyTo":"7vsjiawe74.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 1/3] gitweb: Deal with HEAD pointing to unborn branch in \"heads\" view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-17T13:41:34Z","receivedAt":"2012-02-17T13:41:34Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 17 Feb 2012, Junio C Hamano wrote:\n \n> But after trying to write a reroll myself, I have to wonder what would\n> happen if you have two branches pointing at the same commit as the one at\n> HEAD.  Why isn't the use of current_head class controlled by comparison\n> between the name of the ref and the output from \"symbolic-ref HEAD\"?\n\nIf there is more than one branch that points to HEAD commit, they all\nwill be highlighted.\n\nUsing \"git symbolic-ref HEAD\", or just reading '.git/HEAD' file or symlink\nis on my todo list.  This will make gitweb highlight current branch\ncorrectly even if there is more than one branch that point to the same\nHEAD commit, and make it possible to support \"detached HEAD\" (which I think\nis not supported at all now).\n\nAnyway the test is here to stay... :-)\n \n> -- >8 --\n> From: Jakub Narebski <jnareb@gmail.com>\n> Date: Wed, 15 Feb 2012 16:36:41 +0100\n> Subject: [PATCH] gitweb: Fix \"heads\" view when there is no current branch\n> \n> In a repository whose HEAD points to an unborn branch with no commits,\n> \"heads\" view and \"summary\" view (which shows what is shown in \"heads\"\n> view) compared the object names of commits at the tip of branches with the\n> output from \"git rev-parse HEAD\", which caused comparison of a string with\n> undef and resulted in a warning in the server log.\n> \n> This can happen if non-bare repository (with default 'master' branch)\n> is updated not via committing but by other means like push to it, or\n> Gerrit.  It can happen also just after running \"git checkout --orphan\n> <new branch>\" but before creating any new commit on this branch.\n> \n> Rewrite the comparison so that it also works when $head points at nothing;\n> in such a case, no branch can be \"the current branch\", add a test for it.\n> \n> While at it rename local variable $head to $head_at, as it points to\n> current commit rather than current branch name (HEAD contents).\n> \n> Reported-by: Rajesh Boyapati\n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n\nThanks!\n\n-- \nJakub Narebski\nPoland\n"},{"id":"184885","messageId":"7vipj5v8jh.fsf@alter.siamese.dyndns.org","threadId":"29634","inReplyTo":"201202171441.35618.jnareb@gmail.com","subject":"Re: [PATCHv2 1/3] gitweb: Deal with HEAD pointing to unborn branch in \"heads\" view","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-17T14:28:18Z","receivedAt":"2012-02-17T14:28:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> On Fri, 17 Feb 2012, Junio C Hamano wrote:\n>  \n>> But after trying to write a reroll myself, I have to wonder what would\n>> happen if you have two branches pointing at the same commit as the one at\n>> HEAD.  Why isn't the use of current_head class controlled by comparison\n>> between the name of the ref and the output from \"symbolic-ref HEAD\"?\n>\n> If there is more than one branch that points to HEAD commit, they all\n> will be highlighted.\n>\n> Using \"git symbolic-ref HEAD\", or just reading '.git/HEAD' file or symlink\n> is on my todo list.  This will make gitweb highlight current branch\n> correctly even if there is more than one branch that point to the same\n> HEAD commit, and make it possible to support \"detached HEAD\" (which I think\n> is not supported at all now).\n\nYou should be more honest and admit that showing unrelated branches that\nhappen to point at the same commit as the current HEAD does (this includes\nthe case where HEAD is detached) as if they are *ALL* current branch is\n*NEITER* working *CORRECTLY* nor *SUPPORT*ing \"detached HEAD\" at all.  It\nmay not be giving a runtime error, but instead it is showing AN INCORRECT\nRESULT.\n\nI'd grant you that this is not a new problem this patch introduces, and it\nmay not even be a bug you introduced long time ago.  The patch gives the\nsame INCORRECT RESULT as it intended to do before the patch, and removes\none runtime error, so it is not worsening the situation, but that does not\nchange the fact that the code after the patch is still *WRONG*.\n"}]}