git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 08/11] gitweb: display HEAD in heads list when detached

From
Jakub Narebski <jnareb@gmail.com>
Date
Nov 15, 2008, 22:31 UTC
Message-ID
<200811152331.17294.jnareb@gmail.com>
In-Reply-To
<1226616555-24503-9-git-send-email-giuseppe.bilotta@gmail.com>
On Thu, 13 Nov 2008, Giuseppe Bilotta wrote:

First, I think that _at least_ the first two patches dealing with detached should be squashed.

Second, not this way!

But I think that support for detached HEAD (I am not sure if it should have to be explicitly turned on using some %feature, or reusing some existing feature like 'remote_heads') is a very good idea. Especially for git-instaweb.

Show 14 quoted lines
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>  gitweb/gitweb.perl |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 09728cb..a168f6f 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2672,6 +2672,27 @@ sub git_get_heads_list {
>  	my @refs = map { "refs/$_" } @class;
>  	my @headslist;
>  
> +	if (grep { $_ eq 'heads' } @class) {

First, IMHO layering violation. Resolving detached HEAD should not be in my opinion left to git_get_heads_list, which is wrapped around git-for-each-ref, which for some reason (contrary for example to "git ls-remote ." or "git show-ref -h") doesn't show HEAD even if it is detached. Probably misfeature / a bug in git-for-each-ref. I guess that we should resolve detached HEAD in caller.

But I am not sure about this decision. Maybe instead of showing detached HEAD (if it is detached) for 'heads', we should show it if there is 'HEAD' in @class (well, @refs would have to be corrected, too)?

> +		my @x = (git_cmd(), 'branch');
> +		my @ret = split("\n", qx(@x));
                                      ^^^^^^^- bit strange
Especially compared to almost everywhere else using open ... "-|"
> +		if (grep { /^\* \(no branch\)$/ } @ret) { ;
                                                          ^
                            WTF? -------------------------|

Second, if we go the route of manually resolving detached HEAD, instead of adding [-h|--head] (from git-show-ref) option equivalent to git-for-each-ref, which would work only for detached HEAD (fixing kind of a bug), this is *not* the way to do it.

A. It should be done using encapsulation, adding is_HEAD_detached() or git_is_head_detached() subroutine (squashing the next patch), or simply using !defined($current_branch), where $current_branch would be set using git_get_symbolic_ref('HEAD') or something.

B. Using porcelain, especially end-user porcelain such as git-branch, which can change its output format (because they are porcelain). Use equivalent plumbing, be if git-symbolic-ref ("git symbolic-ref -q HEAD" to be more exact) to get current branch name[1], or just simply do that in Perl: check if it is symlink, or if it starts with "ref: " if it is regular file (IIRC HEAD, even detached HEAD, cannot get packed into .git/packed-refs and deleted... at least I think so).

[1] This means that we have better way of detecting (and showing) which branch is current one than comparing sha1 with resolved HEAD. ($head_hash).

> +			my %ref_item;
> +			@x = (git_cmd(), 'log', '-1', '--pretty=format:%H%n%ct%n%s');
> +			my ($hash, $epoch, $title) = split("\n", qx(@x), 3);

Errr... if we don't fix git-for-each-ref, and go that route, why not simply use parse_commit subroutine, and extract relevant info from there, instead of handcrafting git-log (why not git-show?) call?

You get more info than needed, but I think the cost of getting it is almost the same, and you can reuse existing code.

And if we go --pretty=format:<...> or --pretty=tformat:<...> route for git-log, git-rev-list or git-show, wouldn't it be possible to generate the same output format as git-for-each-ref below?

Show 7 quoted lines
> +
> +			$ref_item{'class'} = 'head';
> +			$ref_item{'name'} = 'HEAD';
> +			$ref_item{'id'} = $hash;
> +			$ref_item{'title'} = $title || '(no commit message)';
> +			if ($ref_item{'epoch'} = $epoch) {
> +				$ref_item{'age'} = age_string(time - $ref_item{'epoch'});
Hmmm...
Show 14 quoted lines
> +			} else {
> +				$ref_item{'age'} = "unknown";
> +			}
> +			push @headslist, \%ref_item;
> +		}
> +	}
> +
>  	open my $fd, '-|', git_cmd(), 'for-each-ref',
>  		($limit ? '--count='.($limit+1) : ()), '--sort=-committerdate',
>  		'--format=%(objectname) %(refname) %(subject)%00%(committer)',
> -- 
> 1.5.6.5
> 
> 
-- 
Jakub Narebski
Poland
Previous: Jakub NarebskiNext: Jakub Narebski
Message 20 of 46 in “gitweb: display remote heads”
  1. 00/11 gitweb: display remote headsGiuseppe Bilotta, Nov 13, 2008
  2. 01/11 gitweb: introduce remote_heads featureGiuseppe Bilotta, Nov 13, 2008
  3. 02/11 gitweb: git_get_heads_list accepts an optional list of refs.Giuseppe Bilotta, Nov 13, 2008
  4. 03/11 gitweb: separate heads and remotes list in summary viewGiuseppe Bilotta, Nov 13, 2008
  5. 04/11 gitweb: optional custom name for refs in git_heads_bodyGiuseppe Bilotta, Nov 13, 2008
  6. 05/11 gitweb: git_split_heads_body function.Giuseppe Bilotta, Nov 13, 2008
  7. 06/11 gitweb: use CSS to style split head lists.Giuseppe Bilotta, Nov 13, 2008
  8. 07/11 gitweb: add 'remotes' actionGiuseppe Bilotta, Nov 13, 2008
  9. 08/11 gitweb: display HEAD in heads list when detachedGiuseppe Bilotta, Nov 13, 2008
  10. 09/11 gitweb: git_is_head_detached() functionGiuseppe Bilotta, Nov 13, 2008
  11. 10/11 gitweb: add HEAD to list of shortlog refs if detachedGiuseppe Bilotta, Nov 13, 2008
  12. 11/11 gitweb: CSS style and refs mark for detached HEADGiuseppe Bilotta, Nov 13, 2008
  13. Jakub NarebskiNov 16, 2008
  14. Jakub NarebskiNov 15, 2008
  15. Junio C HamanoNov 14, 2008
  16. Giuseppe BilottaNov 14, 2008
  17. Junio C HamanoNov 14, 2008
  18. Nanako ShiraishiNov 14, 2008
  19. Jakub NarebskiNov 15, 2008
  20. Jakub NarebskiNov 15, 2008
  21. Jakub NarebskiNov 15, 2008
  22. Giuseppe BilottaNov 15, 2008
  23. Jakub NarebskiNov 16, 2008
  24. Giuseppe BilottaNov 16, 2008
  25. Jakub NarebskiNov 15, 2008
  26. Jakub NarebskiNov 14, 2008
  27. Giuseppe BilottaNov 15, 2008
  28. Jakub NarebskiNov 16, 2008
  29. Giuseppe BilottaNov 16, 2008
  30. Junio C HamanoNov 15, 2008
  31. Giuseppe BilottaNov 15, 2008
  32. Jakub NarebskiNov 16, 2008
  33. Giuseppe BilottaNov 16, 2008
  34. Jakub NarebskiNov 16, 2008
  35. Giuseppe BilottaNov 16, 2008
  36. Jakub NarebskiNov 14, 2008
  37. Giuseppe BilottaNov 15, 2008
  38. Jakub NarebskiNov 14, 2008
  39. Giuseppe BilottaNov 14, 2008
  40. Jakub NarebskiNov 14, 2008
  41. Giuseppe BilottaNov 14, 2008
  42. Jakub NarebskiNov 14, 2008
  43. Giuseppe BilottaNov 14, 2008
  44. Jakub NarebskiNov 14, 2008
  45. Sverre RabbelierNov 14, 2008
  46. Giuseppe BilottaNov 14, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.