git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH] shortlog: use a stable sort
Date: Thu, 14 Jul 2022 15:43:49 +0000	[thread overview]
Message-ID: <pull.1290.git.1657813429221.gitgitgadget@gmail.com> (raw)

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When sorting the output of `git shortlog` by count, a list of authors in
alphabetical order is then sorted by contribution count. Obviously, the
idea is to maintain the alphabetical order for items with identical
contribution count.

At the moment, this job is performed by `qsort()`. As that function is
not guaranteed to implement a stable sort algorithm, this can lead to
inconsistent and/or surprising behavior: items with identical
contribution count could lose their alphabetical sub-order.

The `qsort()` in MS Visual C's runtime does _not_ implement a stable
sort algorithm, and under certain circumstances this even causes a test
failure in t4201.21 "shortlog can match multiple groups", where two
authors both are listed with 2 contributions, and are listed in inverse
alphabetical order.

Let's instead use the stable sort provided by `git_stable_qsort()` to
avoid this inconsistency.

This is a companion to 2049b8dc65 (diffcore_rename(): use a stable sort,
2019-09-30).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    shortlog: use a stable sort
    
    This is another fall-out from validating Git for Windows v2.37.1 via the
    reinstated Azure Pipeline.
    
    The most valid question a reviewer might have about this patch is: "But
    why does our current CI not fail in the vs-test job?". The answer is
    surprisingly trivial: Because our CMake-based definition universally
    defines INTERNAL_QSORT
    [https://github.com/git/git/blob/v2.37.1/contrib/buildsystems/CMakeLists.txt#L227],
    which should actually not even be necessary, while the Azure Pipeline I
    used still calls make vcxproj to generate the Visual Studio build
    definition and does not define that flag.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1290%2Fdscho%2Fshortlog-requires-stable-sort-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1290/dscho/shortlog-requires-stable-sort-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1290

 builtin/shortlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 35825f075e3..086dfee45aa 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -443,7 +443,7 @@ void shortlog_output(struct shortlog *log)
 	struct strbuf sb = STRBUF_INIT;
 
 	if (log->sort_by_number)
-		QSORT(log->list.items, log->list.nr,
+		STABLE_QSORT(log->list.items, log->list.nr,
 		      log->summary ? compare_by_counter : compare_by_list);
 	for (i = 0; i < log->list.nr; i++) {
 		const struct string_list_item *item = &log->list.items[i];

base-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca
-- 
gitgitgadget

             reply	other threads:[~2022-07-14 15:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-14 15:43 Johannes Schindelin via GitGitGadget [this message]
2022-07-14 16:28 ` [PATCH] shortlog: use a stable sort Junio C Hamano
2022-07-14 18:25 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1290.git.1657813429221.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).