[RFC PATCH] gitweb: use HEAD as primary sort key in git_get_heads_list()

Prior to this commit, the "heads" section on a gitweb summary page would
list the heads in `-committerdate` order (ie. the most recently-modified
ones at the top).

In my own repos I have started to move from "master" towards "main", but
I keep "master" around and in sync with "main" so as not to break
existing clones. As such, they always point at the same commit.

This means that in the "heads" listing of a gitweb instance, the display
order ends up being determined by how `git for-each-ref` decides to
tie-break "master" and "main"

For example, right now on a sample repo, gitweb shows the heads in this
order, even though "master" and "main" reference the same commit. The
tie-breaking evidently isn't happening lexicographically:

- master
- main
- pu
- next

So, this commit adds another `--sort` parameter to the `git
for-each-ref` invocation in `git_get_heads_list()`, ensuring that the
`HEAD` ref always ends up getting sorted to the top:

- main
- master
- pu
- next

This seems to be a useful change, because I can't see anywhere else in
the gitweb UI where we actually indicate to the user what the "default"
branch is (ie. what they'll checkout if they run `git clone`).
---
 gitweb/gitweb.perl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e09e024a09..e5270b0291 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -3796,7 +3796,8 @@ sub git_get_heads_list {
 	my @headslist;
 
 	open my $fd, '-|', git_cmd(), 'for-each-ref',
-		($limit ? '--count='.($limit+1) : ()), '--sort=-committerdate',
+		($limit ? '--count='.($limit+1) : ()),
+		'--sort=-committerdate', '--sort=-HEAD',
 		'--format=%(objectname) %(refname) %(subject)%00%(committer)',
 		@patterns
 		or return;

-- 
2.29.2

Prior to this commit, the "heads" section on a gitweb summary page would
list the heads in `-committerdate` order (ie. the most recently-modified
ones at the top).

In my own repos I have started to move from "master" towards "main", but
I keep "master" around and in sync with "main" so as not to break
existing clones. As such, they always point at the same commit.

This means that in the "heads" listing of a gitweb instance, the display
order ends up being determined by how `git for-each-ref` decides to
tie-break "master" and "main"

For example, right now on a sample repo, gitweb shows the heads in this
order, even though "master" and "main" reference the same commit. The
tie-breaking evidently isn't happening lexicographically:

- master
- main
- pu
- next

So, this commit adds another `--sort` parameter to the `git
for-each-ref` invocation in `git_get_heads_list()`, ensuring that the
`HEAD` ref always ends up getting sorted to the top:

- main
- master
- pu
- next

This seems to be a useful change, because I can't see anywhere else in
the gitweb UI where we actually indicate to the user what the "default"
branch is (ie. what they'll checkout if they run `git clone`).

Signed-off-by: Greg Hurrell <redacted>
---

Resending because I forgot the Signed-off-by the first time. Sorry for
the noise.

 gitweb/gitweb.perl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e09e024a09..e5270b0291 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -3796,7 +3796,8 @@ sub git_get_heads_list {
 	my @headslist;
 
 	open my $fd, '-|', git_cmd(), 'for-each-ref',
-		($limit ? '--count='.($limit+1) : ()), '--sort=-committerdate',
+		($limit ? '--count='.($limit+1) : ()),
+		'--sort=-committerdate', '--sort=-HEAD',
 		'--format=%(objectname) %(refname) %(subject)%00%(committer)',
 		@patterns
 		or return;

-- 
2.29.2

On Sun, Jun 06, 2021 at 10:57:32AM +0200, Greg Hurrell wrote:

Prior to this commit, the "heads" section on a gitweb summary page would
list the heads in `-committerdate` order (ie. the most recently-modified
ones at the top).

In my own repos I have started to move from "master" towards "main", but
I keep "master" around and in sync with "main" so as not to break
existing clones. As such, they always point at the same commit.

This means that in the "heads" listing of a gitweb instance, the display
order ends up being determined by how `git for-each-ref` decides to
tie-break "master" and "main"

Hmm. I'd have expected it to, because we start the list in lexicographic
order. I suspect the sort we use simply isn't stable (in a simple 3-ref
example I made, "main" did sort before "master" by default). That would
be easy to fix, of course, but there may be value in using the HEAD rule
anyway.

For example, right now on a sample repo, gitweb shows the heads in this
order, even though "master" and "main" reference the same commit. The
tie-breaking evidently isn't happening lexicographically:

- master
- main
- pu
- next

So, this commit adds another `--sort` parameter to the `git
for-each-ref` invocation in `git_get_heads_list()`, ensuring that the
`HEAD` ref always ends up getting sorted to the top:

- main
- master
- pu
- next

In your earlier example, it sounded like you were primarily concerned
with breaking ties. But here it sounds like you're proposing putting the
HEAD first _regardless_ of the committer timestamp.

I don't have a strong feeling either way on that. It may surface an
older branch, but in general I'd expect the HEAD to be reasonably
up-to-date (unless somebody has a weird workflow that does not really
use it at all, and expects people to always clone with "-b" or
whatever. We can probably discount that).

It doesn't help the stability of non-HEAD branches that are in ties.
I.e., I wonder if this should be two separate patches:

  1. break ties by name, like:

       git for-each-ref --sort=refname --sort=-committerdate

  2. emphasize the HEAD branch, even if it isn't the newest:

       git for-each-ref --sort=refname --sort=-committerdate --sort=-HEAD

-Peff

On Tue, Jun 8, 2021, at 10:34 AM, Jeff King wrote:

In your earlier example, it sounded like you were primarily concerned
with breaking ties. But here it sounds like you're proposing putting the
HEAD first _regardless_ of the committer timestamp.

I arrived there organically. My initial goal was merely to find out why
"master" was sorting ahead of "main" even after I switched HEAD, as it
seemed arbitrary and possibly hard-coded.

Adding `--sort=-HEAD` wallpapers over the problem by forcing HEAD to the
top, but it doesn't address the underlying arbitrariness/non-determinism
of how refs with equal committerdate get treated.

So I like your idea of splitting this into two separate changes much
better:

  1. break ties by name, like:

       git for-each-ref --sort=refname --sort=-committerdate

  2. emphasize the HEAD branch, even if it isn't the newest:

       git for-each-ref --sort=refname --sort=-committerdate --sort=-HEAD

I think adding "1" is a clear improvement. "2" is more debatable (I
think it would be of practical use, but as you said, HEAD is very often
going to be the most recently committed thing anyway). In my specific
use case (where "main" is the HEAD but "master" is kept in sync with it
automatically so as not to break existing clones), sorting by refname
_happens_ to mean that "main" will come before "master"; but that is of
course a quirk of my branch naming choices and not something that should
be relied upon.

In any case, splitting this into two pieces sounds good: we have the
option of taking one or both (or none).

Cheers,
Greg

Prior to this commit, the "heads" section on a gitweb summary page would
list the heads in `-committerdate` order (ie. the most recently-modified
ones at the top), tie-breaking equal-dated refs using the implicit
`refname` sort fallback.

This commit adds another `--sort` parameter to the `git for-each-ref`
invocation in `git_get_heads_list()`, ensuring that the `HEAD` ref
always ends up getting sorted to the top, seeing as it is typically the
"primary" line of development in some sense.

This seems to be a useful change, because I can't see anywhere else in
the gitweb UI where we actually indicate to the user what the "default"
branch is (ie. what they'll checkout if they run `git clone`).

Signed-off-by: Greg Hurrell <redacted>
---

On Tue, Jun 8, 2021, at 11:02 AM, Greg Hurrell wrote:

On Tue, Jun 8, 2021, at 10:34 AM, Jeff King wrote:

quoted

  1. break ties by name, like:

       git for-each-ref --sort=refname --sort=-committerdate

  2. emphasize the HEAD branch, even if it isn't the newest:

       git for-each-ref --sort=refname --sort=-committerdate --sort=-HEAD

I was wracking my brains over this one trying to figure out why
it wasn't already doing the right thing based on what I see in
ref-filter.c.  It sure looks like the `--sort=refname` fallback should
be automatic, but I wasn't seeing it happen in my gitweb instance.

Turns out there was a bug that you fixed in 7c5045fc180ed09ed4cb5 which
made it in soon after v2.20.4 fixing a problem. I was seeing different
behavior on gitweb running on Amazon Linux AMI, because that's still
using Git v2.18.5.

So, that means "1" isn't necessary. "2" is the only possibly interesting
bit. I've reworded the commit text accordingly, still labeled as "RFC"
to see if there is any consensus on this being a good idea or not.

Greg

 gitweb/gitweb.perl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e09e024a09..e5270b0291 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl

@@ -3796,7 +3796,8 @@ sub git_get_heads_list {
 	my @headslist;
 
 	open my $fd, '-|', git_cmd(), 'for-each-ref',
-		($limit ? '--count='.($limit+1) : ()), '--sort=-committerdate',
+		($limit ? '--count='.($limit+1) : ()),
+		'--sort=-committerdate', '--sort=-HEAD',
 		'--format=%(objectname) %(refname) %(subject)%00%(committer)',
 		@patterns
 		or return;

-- 
2.29.2

On Tue, Jun 08, 2021 at 11:14:40PM +0200, Greg Hurrell wrote:

Prior to this commit, the "heads" section on a gitweb summary page would
list the heads in `-committerdate` order (ie. the most recently-modified
ones at the top), tie-breaking equal-dated refs using the implicit
`refname` sort fallback.

This commit adds another `--sort` parameter to the `git for-each-ref`
invocation in `git_get_heads_list()`, ensuring that the `HEAD` ref
always ends up getting sorted to the top, seeing as it is typically the
"primary" line of development in some sense.

This seems to be a useful change, because I can't see anywhere else in
the gitweb UI where we actually indicate to the user what the "default"
branch is (ie. what they'll checkout if they run `git clone`).

Your use of "seems" in the final paragraph is a leftover from the
earlier commit message, and I think weakens your message. It's OK to
assert that it really _is_ a useful change, I would say. :)

This patch looks good to me overall. In addition to dropping the RFC
tag, you're more likely to get attention by having a subject line
without "Re:" in it (so people realize it's a patch to look at, and not
just a continuation of the discussion).

On Tue, Jun 8, 2021, at 11:02 AM, Greg Hurrell wrote:

quoted

On Tue, Jun 8, 2021, at 10:34 AM, Jeff King wrote:

quoted

  1. break ties by name, like:

       git for-each-ref --sort=refname --sort=-committerdate

  2. emphasize the HEAD branch, even if it isn't the newest:

       git for-each-ref --sort=refname --sort=-committerdate --sort=-HEAD

I was wracking my brains over this one trying to figure out why
it wasn't already doing the right thing based on what I see in
ref-filter.c.  It sure looks like the `--sort=refname` fallback should
be automatic, but I wasn't seeing it happen in my gitweb instance.

Turns out there was a bug that you fixed in 7c5045fc180ed09ed4cb5 which
made it in soon after v2.20.4 fixing a problem. I was seeing different
behavior on gitweb running on Amazon Linux AMI, because that's still
using Git v2.18.5.

Heh, OK. I almost suggested "gee, wouldn't it be nice if we used the
refname as a fallback tie-breaker by default". You'd think I would
either remember such fixes, or at least bother to look at the code. :)

So, that means "1" isn't necessary. "2" is the only possibly interesting
bit. I've reworded the commit text accordingly, still labeled as "RFC"
to see if there is any consensus on this being a good idea or not.

Yep, I agree on all counts.

In my experience gitweb doesn't tend to get a lot of interest from
reviewers, and I consider it mostly in maintenance mode these days. So
be prepared for silence. In that case, I'd give it a few days and repost
the patch to see if Junio is interested in picking it up.

-Peff