diff mbox series

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

Message ID 20240305013353.1763404-1-sen@hastings.org
State Superseded
Headers show
Series [1/1] support/scripts/pkg-stats: fix/improve git hash sorting | expand

Commit Message

Sen Hastings March 5, 2024, 1:33 a.m. UTC
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.

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(-)

Comments

Yann E. MORIN March 5, 2024, 6:32 a.m. UTC | #1
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
diff mbox series

Patch

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){
+                        pkgStringSortArray.push(listing);
+		} else if ( isNaN(parseInt(listing[1], 10)) ){
                         pkgStringSortArray.push(listing);
                 } else {
                         listing[1] = parseFloat(listing[1]);