{"thread":{"id":"58159","subject":"[PATCH] shortlog: use a stable sort","startedAt":"2022-07-14T15:44:04Z","lastAt":"2022-07-14T18:25:15Z","messageCount":3,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"459047","messageId":"pull.1290.git.1657813429221.gitgitgadget@gmail.com","threadId":"58159","inReplyTo":null,"subject":"[PATCH] shortlog: use a stable sort","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-14T15:43:49Z","receivedAt":"2022-07-14T15:44:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen sorting the output of `git shortlog` by count, a list of authors in\nalphabetical order is then sorted by contribution count. Obviously, the\nidea is to maintain the alphabetical order for items with identical\ncontribution count.\n\nAt the moment, this job is performed by `qsort()`. As that function is\nnot guaranteed to implement a stable sort algorithm, this can lead to\ninconsistent and/or surprising behavior: items with identical\ncontribution count could lose their alphabetical sub-order.\n\nThe `qsort()` in MS Visual C's runtime does _not_ implement a stable\nsort algorithm, and under certain circumstances this even causes a test\nfailure in t4201.21 \"shortlog can match multiple groups\", where two\nauthors both are listed with 2 contributions, and are listed in inverse\nalphabetical order.\n\nLet's instead use the stable sort provided by `git_stable_qsort()` to\navoid this inconsistency.\n\nThis is a companion to 2049b8dc65 (diffcore_rename(): use a stable sort,\n2019-09-30).\n\nSigned-off-by: Johannes Schindelin \n---\n shortlog: use a stable sort\n \n This is another fall-out from validating Git for Windows v2.37.1 via the\n reinstated Azure Pipeline.\n \n The most valid question a reviewer might have about this patch is: \"But\n why does our current CI not fail in the vs-test job?\". The answer is\n surprisingly trivial: Because our CMake-based definition universally\n defines INTERNAL_QSORT\n [https://github.com/git/git/blob/v2.37.1/contrib/buildsystems/CMakeLists.txt#L227],\n which should actually not even be necessary, while the Azure Pipeline I\n used still calls make vcxproj to generate the Visual Studio build\n definition and does not define that flag.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1290%2Fdscho%2Fshortlog-requires-stable-sort-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1290/dscho/shortlog-requires-stable-sort-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1290\n\n builtin/shortlog.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 35825f075e3..086dfee45aa 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -443,7 +443,7 @@ void shortlog_output(struct shortlog *log)\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tif (log->sort_by_number)\n-\t\tQSORT(log->list.items, log->list.nr,\n+\t\tSTABLE_QSORT(log->list.items, log->list.nr,\n \t\t log->summary ? compare_by_counter : compare_by_list);\n \tfor (i = 0; i < log->list.nr; i++) {\n \t\tconst struct string_list_item *item = &log->list.items[i];\n\nbase-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca\n-- \ngitgitgadget\n"},{"id":"459051","messageId":"xmqqtu7j7j6s.fsf@gitster.g","threadId":"58159","inReplyTo":"pull.1290.git.1657813429221.gitgitgadget@gmail.com","subject":"Re: [PATCH] shortlog: use a stable sort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-14T16:28:11Z","receivedAt":"2022-07-14T16:28:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> When sorting the output of `git shortlog` by count, a list of authors in\n> alphabetical order is then sorted by contribution count. Obviously, the\n> idea is to maintain the alphabetical order for items with identical\n> contribution count.\n> ...\n> \tif (log->sort_by_number)\n> -\t\tQSORT(log->list.items, log->list.nr,\n> +\t\tSTABLE_QSORT(log->list.items, log->list.nr,\n\nMakes perfect sense. Will queue.\n\nThanks.\n"},{"id":"459061","messageId":"xmqqr12n5z7i.fsf@gitster.g","threadId":"58159","inReplyTo":"pull.1290.git.1657813429221.gitgitgadget@gmail.com","subject":"Re: [PATCH] shortlog: use a stable sort","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-14T18:25:05Z","receivedAt":"2022-07-14T18:25:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> why does our current CI not fail in the vs-test job?\". The answer is\n> surprisingly trivial: Because our CMake-based definition universally\n> defines INTERNAL_QSORT\n> [https://github.com/git/git/blob/v2.37.1/contrib/buildsystems/CMakeLists.txt#L227],\n> which should actually not even be necessary, while the Azure Pipeline I\n> used still calls make vcxproj to generate the Visual Studio build\n> definition and does not define that flag.\n\nDiversity in test environments is very good.\n\n"}]}