[bug#35684] import: github: Sort releases before picking the latest one.

[bug#35684] import: github: Sort releases before picking the latest one.

[Arun Isaac]

[-- Attachment #1.1: Type: text/plain, Size: 138 bytes --]


Prior to this, for some packages (e.g., osc in (gnu packages
build-tools)), `guix refresh' could not find the latest version
correctly.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-import-github-Sort-releases-before-picking-the-lates.patch --]
[-- Type: text/x-patch, Size: 3681 bytes --]

From 33f42ea8a33c6e502062708336a5ab8be871c92b Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Sat, 11 May 2019 16:40:38 +0530
Subject: [PATCH] import: github: Sort releases before picking the latest one.

* guix/import/github.scm (latest-released-version): Sort releases before
picking the first one as the latest.
---
 guix/import/github.scm | 55 ++++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/guix/import/github.scm b/guix/import/github.scm
index 4d12339204..a8af318bc8 100644
--- a/guix/import/github.scm
+++ b/guix/import/github.scm
@@ -183,32 +183,35 @@ API when using a GitHub token")
 API. This may be fixed by using an access token and setting the environment
 variable GUIX_GITHUB_TOKEN, for instance one procured from
 https://github.com/settings/tokens"))
-        (any
-         (lambda (release)
-           (let ((tag (or (hash-ref release "tag_name") ;a "release"
-                          (hash-ref release "name")))   ;a tag
-                 (name-length (string-length package-name)))
-             (cond
-              ;; some tags include the name of the package e.g. "fdupes-1.51"
-              ;; so remove these
-              ((and (< name-length (string-length tag))
-                    (string=? (string-append package-name "-")
-                              (substring tag 0 (+ name-length 1))))
-               (substring tag (+ name-length 1)))
-              ;; some tags start with a "v" e.g. "v0.25.0"
-              ;; where some are just the version number
-              ((string-prefix? "v" tag)
-               (substring tag 1))
-              ;; Finally, reject tags that don't start with a digit:
-              ;; they may not represent a release.
-              ((and (not (string-null? tag))
-                    (char-set-contains? char-set:digit
-                                        (string-ref tag 0)))
-               tag)
-              (else #f))))
-         (match (remove pre-release? json)
-           (() json) ; keep everything
-           (releases releases))))))
+        (first
+         (sort
+          (filter-map
+           (lambda (release)
+             (let ((tag (or (hash-ref release "tag_name") ;a "release"
+                            (hash-ref release "name")))   ;a tag
+                   (name-length (string-length package-name)))
+               (cond
+                ;; some tags include the name of the package e.g. "fdupes-1.51"
+                ;; so remove these
+                ((and (< name-length (string-length tag))
+                      (string=? (string-append package-name "-")
+                                (substring tag 0 (+ name-length 1))))
+                 (substring tag (+ name-length 1)))
+                ;; some tags start with a "v" e.g. "v0.25.0"
+                ;; where some are just the version number
+                ((string-prefix? "v" tag)
+                 (substring tag 1))
+                ;; Finally, reject tags that don't start with a digit:
+                ;; they may not represent a release.
+                ((and (not (string-null? tag))
+                      (char-set-contains? char-set:digit
+                                          (string-ref tag 0)))
+                 tag)
+                (else #f))))
+           (match (remove pre-release? json)
+             (() json) ; keep everything
+             (releases releases)))
+          version>?)))))
 
 (define (latest-release pkg)
   "Return an <upstream-source> for the latest release of PKG."
-- 
2.21.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#35684] import: github: Sort releases before picking the latest one.

[Ludovic Courtès]

Arun Isaac <arunisaac@systemreboot.net> skribis:

> Prior to this, for some packages (e.g., osc in (gnu packages
> build-tools)), `guix refresh' could not find the latest version
> correctly.

Good catch.

> From 33f42ea8a33c6e502062708336a5ab8be871c92b Mon Sep 17 00:00:00 2001
> From: Arun Isaac <arunisaac@systemreboot.net>
> Date: Sat, 11 May 2019 16:40:38 +0530
> Subject: [PATCH] import: github: Sort releases before picking the latest one.
>
> * guix/import/github.scm (latest-released-version): Sort releases before
> picking the first one as the latest.

[...]

> +        (first
> +         (sort
> +          (filter-map
> +           (lambda (release)
> +             (let ((tag (or (hash-ref release "tag_name") ;a "release"
> +                            (hash-ref release "name")))   ;a tag
> +                   (name-length (string-length package-name)))
> +               (cond
> +                ;; some tags include the name of the package e.g. "fdupes-1.51"
> +                ;; so remove these
> +                ((and (< name-length (string-length tag))
> +                      (string=? (string-append package-name "-")
> +                                (substring tag 0 (+ name-length 1))))
> +                 (substring tag (+ name-length 1)))
> +                ;; some tags start with a "v" e.g. "v0.25.0"
> +                ;; where some are just the version number
> +                ((string-prefix? "v" tag)
> +                 (substring tag 1))
> +                ;; Finally, reject tags that don't start with a digit:
> +                ;; they may not represent a release.
> +                ((and (not (string-null? tag))
> +                      (char-set-contains? char-set:digit
> +                                          (string-ref tag 0)))
> +                 tag)
> +                (else #f))))
> +           (match (remove pre-release? json)
> +             (() json) ; keep everything
> +             (releases releases)))
> +          version>?)))))

LGTM, but…

… while you’re at it, would you mind cleaning this up a bit?  :-)
Namely, I think this big ‘lambda’ could be given a name and moved out of
the way to make ‘latest-released-version’ easier to read.  Also, it
would probably be reasonable to avoid ‘first’ and instead write:

  (match (sort …)
    ((first . _) first)
    (()
     (leave (G_ "no releases found etc.~%"))))

WDYT?  :-)

If you’d rather leave that for later, you can also just go ahead and
commit your patch.

Thanks,
Ludo’.

[bug#35684] import: github: Sort releases before picking the latest one.

[Arun Isaac]

[-- Attachment #1.1: Type: text/plain, Size: 507 bytes --]


> Namely, I think this big ‘lambda’ could be given a name and moved out of
> the way to make ‘latest-released-version’ easier to read.  Also, it
> would probably be reasonable to avoid ‘first’ and instead write:
>
>   (match (sort …)
>     ((first . _) first)
>     (()
>      (leave (G_ "no releases found etc.~%"))))
>
> WDYT?  :-)
>
> If you’d rather leave that for later, you can also just go ahead and
> commit your patch.

No problem! :-) Here is the updated patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-import-github-Sort-releases-before-picking-the-lates.patch --]
[-- Type: text/x-patch, Size: 3899 bytes --]

From d3f28de8fedc41732a07edf2ea91222208ccc73f Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Sat, 11 May 2019 16:40:38 +0530
Subject: [PATCH] import: github: Sort releases before picking the latest one.

* guix/import/github.scm (latest-released-version): Sort releases before
picking the first one as the latest.
---
 guix/import/github.scm | 56 ++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/guix/import/github.scm b/guix/import/github.scm
index 4d12339204..cdac70420a 100644
--- a/guix/import/github.scm
+++ b/guix/import/github.scm
@@ -174,6 +174,29 @@ the package e.g. 'bedtools2'.  Return #f if there is no releases"
   (define (pre-release? x)
     (hash-ref x "prerelease"))
 
+  (define (release->version release)
+    (let ((tag (or (hash-ref release "tag_name") ;a "release"
+                   (hash-ref release "name")))   ;a tag
+          (name-length (string-length package-name)))
+      (cond
+       ;; some tags include the name of the package e.g. "fdupes-1.51"
+       ;; so remove these
+       ((and (< name-length (string-length tag))
+             (string=? (string-append package-name "-")
+                       (substring tag 0 (+ name-length 1))))
+        (substring tag (+ name-length 1)))
+       ;; some tags start with a "v" e.g. "v0.25.0"
+       ;; where some are just the version number
+       ((string-prefix? "v" tag)
+        (substring tag 1))
+       ;; Finally, reject tags that don't start with a digit:
+       ;; they may not represent a release.
+       ((and (not (string-null? tag))
+             (char-set-contains? char-set:digit
+                                 (string-ref tag 0)))
+        tag)
+       (else #f))))
+
   (let* ((json (fetch-releases-or-tags url)))
     (if (eq? json #f)
         (if (%github-token)
@@ -183,32 +206,13 @@ API when using a GitHub token")
 API. This may be fixed by using an access token and setting the environment
 variable GUIX_GITHUB_TOKEN, for instance one procured from
 https://github.com/settings/tokens"))
-        (any
-         (lambda (release)
-           (let ((tag (or (hash-ref release "tag_name") ;a "release"
-                          (hash-ref release "name")))   ;a tag
-                 (name-length (string-length package-name)))
-             (cond
-              ;; some tags include the name of the package e.g. "fdupes-1.51"
-              ;; so remove these
-              ((and (< name-length (string-length tag))
-                    (string=? (string-append package-name "-")
-                              (substring tag 0 (+ name-length 1))))
-               (substring tag (+ name-length 1)))
-              ;; some tags start with a "v" e.g. "v0.25.0"
-              ;; where some are just the version number
-              ((string-prefix? "v" tag)
-               (substring tag 1))
-              ;; Finally, reject tags that don't start with a digit:
-              ;; they may not represent a release.
-              ((and (not (string-null? tag))
-                    (char-set-contains? char-set:digit
-                                        (string-ref tag 0)))
-               tag)
-              (else #f))))
-         (match (remove pre-release? json)
-           (() json) ; keep everything
-           (releases releases))))))
+        (match (sort (filter-map release->version
+                                 (match (remove pre-release? json)
+                                   (() json) ; keep everything
+                                   (releases releases)))
+                     version>?)
+          ((latest-release . _) latest-release)
+          (() #f)))))
 
 (define (latest-release pkg)
   "Return an <upstream-source> for the latest release of PKG."
-- 
2.21.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#35684] import: github: Sort releases before picking the latest one.

[Ludovic Courtès]

Arun Isaac <arunisaac@systemreboot.net> skribis:

> From d3f28de8fedc41732a07edf2ea91222208ccc73f Mon Sep 17 00:00:00 2001
> From: Arun Isaac <arunisaac@systemreboot.net>
> Date: Sat, 11 May 2019 16:40:38 +0530
> Subject: [PATCH] import: github: Sort releases before picking the latest one.
>
> * guix/import/github.scm (latest-released-version): Sort releases before
> picking the first one as the latest.

Very nice, LGTM!  If you don’t mind, you can make it two patches for
clarity (one that defines ‘release->version’, and one that adds the call
to ‘sort’); otherwise just amend the commit log above to mention
‘release->version’.

Thank you!

Ludo’.

[bug#35684] import: github: Sort releases before picking the latest one.

[Arun Isaac]

[-- Attachment #1.1: Type: text/plain, Size: 185 bytes --]


> If you don’t mind, you can make it two patches for clarity (one that
> defines ‘release->version’, and one that adds the call to ‘sort’)

Sure, pleae find attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-import-github-Improve-readability.patch --]
[-- Type: text/x-patch, Size: 3703 bytes --]

From c96956bc6e944f9691773640e402c15215078943 Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Tue, 14 May 2019 15:44:46 +0530
Subject: [PATCH 1/2] import: github: Improve readability.

* guix/import/github.scm (latest-released-version)[release->version]: Separate
out release->version as a new function.
---
 guix/import/github.scm | 53 +++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/guix/import/github.scm b/guix/import/github.scm
index 4d12339204..5f4d9c7267 100644
--- a/guix/import/github.scm
+++ b/guix/import/github.scm
@@ -174,6 +174,29 @@ the package e.g. 'bedtools2'.  Return #f if there is no releases"
   (define (pre-release? x)
     (hash-ref x "prerelease"))
 
+  (define (release->version release)
+    (let ((tag (or (hash-ref release "tag_name") ;a "release"
+                   (hash-ref release "name")))   ;a tag
+          (name-length (string-length package-name)))
+      (cond
+       ;; some tags include the name of the package e.g. "fdupes-1.51"
+       ;; so remove these
+       ((and (< name-length (string-length tag))
+             (string=? (string-append package-name "-")
+                       (substring tag 0 (+ name-length 1))))
+        (substring tag (+ name-length 1)))
+       ;; some tags start with a "v" e.g. "v0.25.0"
+       ;; where some are just the version number
+       ((string-prefix? "v" tag)
+        (substring tag 1))
+       ;; Finally, reject tags that don't start with a digit:
+       ;; they may not represent a release.
+       ((and (not (string-null? tag))
+             (char-set-contains? char-set:digit
+                                 (string-ref tag 0)))
+        tag)
+       (else #f))))
+
   (let* ((json (fetch-releases-or-tags url)))
     (if (eq? json #f)
         (if (%github-token)
@@ -183,32 +206,10 @@ API when using a GitHub token")
 API. This may be fixed by using an access token and setting the environment
 variable GUIX_GITHUB_TOKEN, for instance one procured from
 https://github.com/settings/tokens"))
-        (any
-         (lambda (release)
-           (let ((tag (or (hash-ref release "tag_name") ;a "release"
-                          (hash-ref release "name")))   ;a tag
-                 (name-length (string-length package-name)))
-             (cond
-              ;; some tags include the name of the package e.g. "fdupes-1.51"
-              ;; so remove these
-              ((and (< name-length (string-length tag))
-                    (string=? (string-append package-name "-")
-                              (substring tag 0 (+ name-length 1))))
-               (substring tag (+ name-length 1)))
-              ;; some tags start with a "v" e.g. "v0.25.0"
-              ;; where some are just the version number
-              ((string-prefix? "v" tag)
-               (substring tag 1))
-              ;; Finally, reject tags that don't start with a digit:
-              ;; they may not represent a release.
-              ((and (not (string-null? tag))
-                    (char-set-contains? char-set:digit
-                                        (string-ref tag 0)))
-               tag)
-              (else #f))))
-         (match (remove pre-release? json)
-           (() json) ; keep everything
-           (releases releases))))))
+        (any release->version
+             (match (remove pre-release? json)
+               (() json) ; keep everything
+               (releases releases))))))
 
 (define (latest-release pkg)
   "Return an <upstream-source> for the latest release of PKG."
-- 
2.21.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0002-import-github-Sort-releases-before-picking-the-lates.patch --]
[-- Type: text/x-patch, Size: 1484 bytes --]

From 8192712f8108d0abcd527ff6b16af073c453b780 Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Tue, 14 May 2019 15:46:19 +0530
Subject: [PATCH 2/2] import: github: Sort releases before picking the latest
 one.

* guix/import/github.scm (latest-released-version): Sort releases before
picking the first one as the latest.
---
 guix/import/github.scm | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/guix/import/github.scm b/guix/import/github.scm
index 5f4d9c7267..cdac70420a 100644
--- a/guix/import/github.scm
+++ b/guix/import/github.scm
@@ -206,10 +206,13 @@ API when using a GitHub token")
 API. This may be fixed by using an access token and setting the environment
 variable GUIX_GITHUB_TOKEN, for instance one procured from
 https://github.com/settings/tokens"))
-        (any release->version
-             (match (remove pre-release? json)
-               (() json) ; keep everything
-               (releases releases))))))
+        (match (sort (filter-map release->version
+                                 (match (remove pre-release? json)
+                                   (() json) ; keep everything
+                                   (releases releases)))
+                     version>?)
+          ((latest-release . _) latest-release)
+          (() #f)))))
 
 (define (latest-release pkg)
   "Return an <upstream-source> for the latest release of PKG."
-- 
2.21.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#35684] import: github: Sort releases before picking the latest one.

[Ludovic Courtès]

Arun Isaac <arunisaac@systemreboot.net> skribis:

>> If you don’t mind, you can make it two patches for clarity (one that
>> defines ‘release->version’, and one that adds the call to ‘sort’)
>
> Sure, pleae find attached.

Thank you, it still LGTM.  :-)

Ludo’.

bug#35684: import: github: Sort releases before picking the latest one.

[Arun Isaac]

[-- Attachment #1: Type: text/plain, Size: 27 bytes --]


Pushed to master, thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]