Re: [Buildroot] [PATCH 1/1] support/scripts/pkg-stats: fix/improve git hash sorting

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Sen, All,

On 2024-03-04 17:33 -0800, Sen Hastings spake thusly:
> sortGrid()'s handling of git hashes has been inconsistent, they can
> be detected as strings or numbers depending on what type of character
> they start with. This patch fixes the behaviour by using a regex to
> capture everything that looks like a git hash and treat it as a
> string. This means when you sort by current version ascending
> all the version strings with git hashes should show up first, sorted
> 0-9,a-f.

There are non-git-hash versions that get interspersed in the list,
though:

  - package/sunxi-tools/sunxi-tools.mk  1.4.2-168-ged3039cdbeeb28fc0011c3585d8f7dfb91038292
  - package/glibc/glibc.mk              2.38-44-gd37c2b20a4787463d192b32041c3406c2bd91de0
  - package/localedef/localedef.mk      2.38-44-gd37c2b20a4787463d192b32041c3406c2bd91de0
  - package/ctorrent/ctorrent.mk        dnh3.3.2
  - boot/edk2/edk2.mk                   edk2-stable202308

But honestly, that's really not a problem (I'm not even sure that
sorting by version is even meaningful for this table).

> A demo is available here:
> https://sen-h.codeberg.page/pkg-stats-demos/@pages/fix-improve-git-hash-sorting.html
> 
> Signed-off-by: Sen Hastings <sen@hastings.org>
> ---
>  support/scripts/pkg-stats | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> index 3295eb7a48..4dc1857a9e 100755
> --- a/support/scripts/pkg-stats
> +++ b/support/scripts/pkg-stats
> @@ -741,6 +741,7 @@ addedCSSRules.forEach(rule => styleSheet.insertRule(rule));
>  function sortGrid(sortLabel){
>  	let i = 0;
>  	let pkgSortArray = [], sortedPkgArray = [], pkgStringSortArray = [], pkgNumSortArray = [];
> +	const git_hash_regex = /[a-f,0-9]/gi;
>  	const columnValues = Array.from(document.getElementsByClassName(sortLabel));
>  	const columnName = document.getElementById(sortLabel);
>  	let lastStyle = document.getElementById("sort-css");
> @@ -765,7 +766,9 @@ function sortGrid(sortLabel){
>                  pkgSortArray.push(sortArr);
>          });
>          pkgSortArray.forEach((listing) => {
> -                if ( isNaN(parseInt(listing[1], 10)) ){
> +                if ( listing[1].length >= 39 && listing[1].match(git_hash_regex).length >= 39){

Why at least 39? Git hashes are exactly 40 char long, no more, no less.
So if you need to check for a little bit less, or for more, then this
should be explained.

For 39, this can indeed be explained: we do have a 39-long hash, to avoid
collisions with a previous release of Buildroot (in package
package/rockchip-mali/rockchip-mali.mk). But for more than 40, I don't
immediately see...

Could you please respin with an extended commit log, that explains the
39-long hash, and explains the above-40?

Regards,
Yann E. MORIN.

> +                        pkgStringSortArray.push(listing);
> +		} else if ( isNaN(parseInt(listing[1], 10)) ){
>                          pkgStringSortArray.push(listing);
>                  } else {
>                          listing[1] = parseFloat(listing[1]);
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot