[bug#27596] [PATCH] guix: lint: Add checker for new upstream versions.

[bug#27596] [PATCH] guix: lint: Add checker for new upstream versions.

[Efraim Flashner]

* guix/scripts/lint.scm (check-for-updates): New procedure.
(%checkers): Add it.
---
 guix/scripts/lint.scm | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 04ab85299..665db158c 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -35,6 +35,7 @@
   #:use-module (guix utils)
   #:use-module (guix memoization)
   #:use-module (guix scripts)
+  #:use-module (guix scripts refresh)
   #:use-module (guix gnu-maintenance)
   #:use-module (guix monads)
   #:use-module (guix cve)
@@ -72,6 +73,7 @@
             check-mirror-url
             check-license
             check-vulnerabilities
+            check-for-updates
             check-formatting
             run-checkers
 
@@ -821,6 +823,10 @@ from ~s: ~a (~s)~%")
                                  (string-join (map vulnerability-id unpatched)
                                               ", ")))))))))
 
+(define (check-for-updates package)
+  "Check if there is an update available for PACKAGE."
+  (guix-refresh (package-name package)))
+
 \f
 ;;;
 ;;; Source code formatting.
@@ -972,6 +978,10 @@ or a list thereof")
  (CVE) database")
      (check       check-vulnerabilities))
    (lint-checker
+     (name        'refresh)
+     (description "Check the package for new upstream releases")
+     (check       check-for-updates))
+   (lint-checker
      (name        'formatting)
      (description "Look for formatting issues in the source")
      (check       check-formatting))))
-- 
2.13.2

[bug#27596] [PATCH] guix: lint: Add checker for new upstream versions.

[Ludovic Courtès]

Efraim Flashner <efraim@flashner.co.il> skribis:

> * guix/scripts/lint.scm (check-for-updates): New procedure.
> (%checkers): Add it.

Good idea.

> +(define (check-for-updates package)
> +  "Check if there is an update available for PACKAGE."
> +  (guix-refresh (package-name package)))

I think we should use the (guix upstream) API directly because
‘guix-refresh’ can call ‘exit’ and it formats messages in a way that is
not consistent with the rest of ‘guix lint’.

To do that I think we’ll first have to move ‘%updaters’ to (guix
upstream), probably turning it into a procedure.

WDYT?

Thanks,
Ludo’.

[bug#27596] [PATCH] guix: lint: Add checker for new upstream versions.

[Efraim Flashner]

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

On Thu, Jul 06, 2017 at 04:35:38PM +0200, Ludovic Courtès wrote:
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > * guix/scripts/lint.scm (check-for-updates): New procedure.
> > (%checkers): Add it.
> 
> Good idea.
> 
> > +(define (check-for-updates package)
> > +  "Check if there is an update available for PACKAGE."
> > +  (guix-refresh (package-name package)))
> 
> I think we should use the (guix upstream) API directly because
> ‘guix-refresh’ can call ‘exit’ and it formats messages in a way that is
> not consistent with the rest of ‘guix lint’.
> 
> To do that I think we’ll first have to move ‘%updaters’ to (guix
> upstream), probably turning it into a procedure.
> 
> WDYT?
> 
> Thanks,
> Ludo’.
> 

This actually wasn't too bad. I had to move %updaters and another
function to guix/upstream, and then I copied the code for checking the
upstream version vs the current version and tried to shorten it a bit.

I tested it with 'guix lint -c refresh perl{,-svg}'

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #1.2: 0001-guix-lint-Add-checker-for-new-upstream-versions.patch --]
[-- Type: text/plain, Size: 5469 bytes --]

From 136841864c883338381764ef09a6f4ec967949e7 Mon Sep 17 00:00:00 2001
From: Efraim Flashner <efraim@flashner.co.il>
Date: Thu, 6 Jul 2017 09:13:31 +0300
Subject: [PATCH] guix: lint: Add checker for new upstream versions.

* guix/scripts/lint.scm (check-for-updates): New procedure.
(%checkers): Add it.
* guix/scripts/refresh.scm (importer-modules, %updaters): Move
from here ...
* guix/upstream.scm: ... to here.
---
 guix/scripts/lint.scm    | 16 ++++++++++++++++
 guix/scripts/refresh.scm | 20 +-------------------
 guix/upstream.scm        | 18 ++++++++++++++++++
 3 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 04ab85299..6cfdf25e3 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -32,6 +32,7 @@
   #:use-module (guix licenses)
   #:use-module (guix records)
   #:use-module (guix ui)
+  #:use-module (guix upstream)
   #:use-module (guix utils)
   #:use-module (guix memoization)
   #:use-module (guix scripts)
@@ -72,6 +73,7 @@
             check-mirror-url
             check-license
             check-vulnerabilities
+            check-for-updates
             check-formatting
             run-checkers
 
@@ -821,6 +823,16 @@ from ~s: ~a (~s)~%")
                                  (string-join (map vulnerability-id unpatched)
                                               ", ")))))))))
 
+(define (check-for-updates package)
+  "Check if there is an update available for PACKAGE."
+  (match (package-latest-release package (force %updaters))
+    ((? upstream-source? source)
+     (when (version>? (upstream-source-version source)
+                      (package-version package))
+       (emit-warning package
+                     (format #f (G_ "can be upgraded to ~a~%")
+                             (upstream-source-version source)))))))
+
 \f
 ;;;
 ;;; Source code formatting.
@@ -972,6 +984,10 @@ or a list thereof")
  (CVE) database")
      (check       check-vulnerabilities))
    (lint-checker
+     (name        'refresh)
+     (description "Check the package for new upstream releases")
+     (check       check-for-updates))
+   (lint-checker
      (name        'formatting)
      (description "Look for formatting issues in the source")
      (check       check-formatting))))
diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index 5add64d8e..d638d744a 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -30,7 +30,6 @@
   #:use-module (guix packages)
   #:use-module (guix profiles)
   #:use-module (guix upstream)
-  #:use-module (guix discovery)
   #:use-module (guix graph)
   #:use-module (guix scripts graph)
   #:use-module (guix monads)
@@ -46,8 +45,7 @@
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-37)
   #:use-module (ice-9 binary-ports)
-  #:export (guix-refresh
-            %updaters))
+  #:export (guix-refresh))
 
 \f
 ;;;
@@ -162,22 +160,6 @@ specified with `--select'.\n"))
 ;;; Updates.
 ;;;
 
-(define (importer-modules)
-  "Return the list of importer modules."
-  (cons (resolve-interface '(guix gnu-maintenance))
-        (all-modules (map (lambda (entry)
-                            `(,entry . "guix/import"))
-                          %load-path))))
-
-(define %updaters
-  ;; The list of publically-known updaters.
-  (delay (fold-module-public-variables (lambda (obj result)
-                                         (if (upstream-updater? obj)
-                                             (cons obj result)
-                                             result))
-                                       '()
-                                       (importer-modules))))
-
 (define (lookup-updater-by-name name)
   "Return the updater called NAME."
   (or (find (lambda (updater)
diff --git a/guix/upstream.scm b/guix/upstream.scm
index 5083e6b80..70b7e295f 100644
--- a/guix/upstream.scm
+++ b/guix/upstream.scm
@@ -20,6 +20,7 @@
 (define-module (guix upstream)
   #:use-module (guix records)
   #:use-module (guix utils)
+  #:use-module (guix discovery)
   #:use-module ((guix download)
                 #:select (download-to-store))
   #:use-module (guix gnupg)
@@ -54,6 +55,7 @@
             upstream-updater-predicate
             upstream-updater-latest
 
+            %updaters
             lookup-updater
 
             download-tarball
@@ -127,6 +129,22 @@ correspond to the same version."
   (pred        upstream-updater-predicate)
   (latest      upstream-updater-latest))
 
+(define (importer-modules)
+  "Return the list of importer modules."
+  (cons (resolve-interface '(guix gnu-maintenance))
+        (all-modules (map (lambda (entry)
+                            `(,entry . "guix/import"))
+                          %load-path))))
+
+(define %updaters
+  ;; The list of publically-known updaters.
+  (delay (fold-module-public-variables (lambda (obj result)
+                                         (if (upstream-updater? obj)
+                                             (cons obj result)
+                                             result))
+                                       '()
+                                       (importer-modules))))
+
 (define (lookup-updater package updaters)
   "Return an updater among UPDATERS that matches PACKAGE, or #f if none of
 them matches."
-- 
2.13.3


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

[bug#27596] [PATCH] guix: lint: Add checker for new upstream versions.

[Ludovic Courtès]

Hi Efraim,

Sorry for the delay.

Efraim Flashner <efraim@flashner.co.il> skribis:

> From 136841864c883338381764ef09a6f4ec967949e7 Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim@flashner.co.il>
> Date: Thu, 6 Jul 2017 09:13:31 +0300
> Subject: [PATCH] guix: lint: Add checker for new upstream versions.
>
> * guix/scripts/lint.scm (check-for-updates): New procedure.
> (%checkers): Add it.
> * guix/scripts/refresh.scm (importer-modules, %updaters): Move
> from here ...
> * guix/upstream.scm: ... to here.

[...]

> +(define (check-for-updates package)
> +  "Check if there is an update available for PACKAGE."
> +  (match (package-latest-release package (force %updaters))
> +    ((? upstream-source? source)
> +     (when (version>? (upstream-source-version source)
> +                      (package-version package))
> +       (emit-warning package
> +                     (format #f (G_ "can be upgraded to ~a~%")
> +                             (upstream-source-version source)))))))

I think you can (1) use ‘package-latest-release*’ which ensures that the
returned version is newer, and (2) add a case for #f since
‘package-latest-release*’ can return #f.

Apart from that it LGTM, thank you!

Ludo’.

[bug#27596] [PATCH] guix: lint: Add checker for new upstream versions.

[Ludovic Courtès]

Ping!  :-)

ludo@gnu.org (Ludovic Courtès) skribis:

> Hi Efraim,
>
> Sorry for the delay.
>
> Efraim Flashner <efraim@flashner.co.il> skribis:
>
>> From 136841864c883338381764ef09a6f4ec967949e7 Mon Sep 17 00:00:00 2001
>> From: Efraim Flashner <efraim@flashner.co.il>
>> Date: Thu, 6 Jul 2017 09:13:31 +0300
>> Subject: [PATCH] guix: lint: Add checker for new upstream versions.
>>
>> * guix/scripts/lint.scm (check-for-updates): New procedure.
>> (%checkers): Add it.
>> * guix/scripts/refresh.scm (importer-modules, %updaters): Move
>> from here ...
>> * guix/upstream.scm: ... to here.
>
> [...]
>
>> +(define (check-for-updates package)
>> +  "Check if there is an update available for PACKAGE."
>> +  (match (package-latest-release package (force %updaters))
>> +    ((? upstream-source? source)
>> +     (when (version>? (upstream-source-version source)
>> +                      (package-version package))
>> +       (emit-warning package
>> +                     (format #f (G_ "can be upgraded to ~a~%")
>> +                             (upstream-source-version source)))))))
>
> I think you can (1) use ‘package-latest-release*’ which ensures that the
> returned version is newer, and (2) add a case for #f since
> ‘package-latest-release*’ can return #f.
>
> Apart from that it LGTM, thank you!
>
> Ludo’.

[bug#27596] [PATCH] guix: lint: Add checker for new upstream versions.

[Efraim Flashner]

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

On Sat, Oct 07, 2017 at 10:50:21PM +0200, Ludovic Courtès wrote:
> Ping!  :-)
> 
> ludo@gnu.org (Ludovic Courtès) skribis:
> 
> > Hi Efraim,
> >
> > Sorry for the delay.
> >
> > Efraim Flashner <efraim@flashner.co.il> skribis:
> >
> >> From 136841864c883338381764ef09a6f4ec967949e7 Mon Sep 17 00:00:00 2001
> >> From: Efraim Flashner <efraim@flashner.co.il>
> >> Date: Thu, 6 Jul 2017 09:13:31 +0300
> >> Subject: [PATCH] guix: lint: Add checker for new upstream versions.
> >>
> >> * guix/scripts/lint.scm (check-for-updates): New procedure.
> >> (%checkers): Add it.
> >> * guix/scripts/refresh.scm (importer-modules, %updaters): Move
> >> from here ...
> >> * guix/upstream.scm: ... to here.
> >
> > [...]
> >
> >> +(define (check-for-updates package)
> >> +  "Check if there is an update available for PACKAGE."
> >> +  (match (package-latest-release package (force %updaters))
> >> +    ((? upstream-source? source)
> >> +     (when (version>? (upstream-source-version source)
> >> +                      (package-version package))
> >> +       (emit-warning package
> >> +                     (format #f (G_ "can be upgraded to ~a~%")
> >> +                             (upstream-source-version source)))))))
> >
> > I think you can (1) use ‘package-latest-release*’ which ensures that the
> > returned version is newer, and (2) add a case for #f since
> > ‘package-latest-release*’ can return #f.
> >
> > Apart from that it LGTM, thank you!
> >
> > Ludo’.

I've been working on it with package-latest-release*, and its nice not
having to reimplement the logic, but I can't get it to return the newest
version of the upstream-package. I got it to return a useful message
with:

(define (check-for-updates package)
  (if (package-latest-release* package (force %updaters))
       (emit-warning package
                     (format #f (G_ "can be upgraded to ~a~%")
                             (upstream-source-version (package-latest-release package (force %updaters)))))
       #t))

but I've already checked if there's an upstream release, I shouldn't
need to check a second time. package-latest-release* returns true/false,
so if I want to print the version to upgrade to I need to check it a
second time. package-latest-release returns an <upstream-source> which
is more easily queried for the latest upstream version.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

[bug#27596] [PATCH] guix: lint: Add checker for new upstream versions.

[Ludovic Courtès]

Hello!

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Sat, Oct 07, 2017 at 10:50:21PM +0200, Ludovic Courtès wrote:

[...]

>> >> +(define (check-for-updates package)
>> >> +  "Check if there is an update available for PACKAGE."
>> >> +  (match (package-latest-release package (force %updaters))
>> >> +    ((? upstream-source? source)
>> >> +     (when (version>? (upstream-source-version source)
>> >> +                      (package-version package))
>> >> +       (emit-warning package
>> >> +                     (format #f (G_ "can be upgraded to ~a~%")
>> >> +                             (upstream-source-version source)))))))
>> >
>> > I think you can (1) use ‘package-latest-release*’ which ensures that the
>> > returned version is newer, and (2) add a case for #f since
>> > ‘package-latest-release*’ can return #f.
>> >
>> > Apart from that it LGTM, thank you!
>> >
>> > Ludo’.
>
> I've been working on it with package-latest-release*, and its nice not
> having to reimplement the logic, but I can't get it to return the newest
> version of the upstream-package. I got it to return a useful message
> with:
>
> (define (check-for-updates package)
>   (if (package-latest-release* package (force %updaters))
>        (emit-warning package
>                      (format #f (G_ "can be upgraded to ~a~%")
>                              (upstream-source-version (package-latest-release package (force %updaters)))))
>        #t))
>
> but I've already checked if there's an upstream release, I shouldn't
> need to check a second time. package-latest-release* returns true/false,


AFAICS ‘package-latest-release*’ returns #f or an <upstream-source>:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (package-latest-release* binutils (force %updaters))
$4 = #<<upstream-source> package: "binutils" version: "2.29.1" urls: ("mirror://gnu/binutils/binutils-2.29.1.tar.gz" "mirror://gnu/binutils/binutils-2.29.1.tar.lz" "mirror://gnu/binutils/binutils-2.29.1.tar.bz2" "mirror://gnu/binutils/binutils-2.29.1.tar.xz") signature-urls: ("mirror://gnu/binutils/binutils-2.29.1.tar.gz.sig" "mirror://gnu/binutils/binutils-2.29.1.tar.lz.sig" "mirror://gnu/binutils/binutils-2.29.1.tar.bz2.sig" "mirror://gnu/binutils/binutils-2.29.1.tar.xz.sig")>
--8<---------------cut here---------------end--------------->8---

So you can write:

  (match (package-latest-release* package (force %updaters))
    ((? upstream-source? source)
     …)
    (#f   ;cannot determine latest release
     #f))

HTH!

Ludo’.

bug#27596: [PATCH] guix: lint: Add checker for new upstream versions.

[Efraim Flashner]

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

On Mon, Oct 09, 2017 at 09:28:04AM +0200, Ludovic Courtès wrote:
> Hello!
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > On Sat, Oct 07, 2017 at 10:50:21PM +0200, Ludovic Courtès wrote:
> 
> [...]
> 
> >> >> +(define (check-for-updates package)
> >> >> +  "Check if there is an update available for PACKAGE."
> >> >> +  (match (package-latest-release package (force %updaters))
> >> >> +    ((? upstream-source? source)
> >> >> +     (when (version>? (upstream-source-version source)
> >> >> +                      (package-version package))
> >> >> +       (emit-warning package
> >> >> +                     (format #f (G_ "can be upgraded to ~a~%")
> >> >> +                             (upstream-source-version source)))))))
> >> >
> >> > I think you can (1) use ‘package-latest-release*’ which ensures that the
> >> > returned version is newer, and (2) add a case for #f since
> >> > ‘package-latest-release*’ can return #f.
> >> >
> >> > Apart from that it LGTM, thank you!
> >> >
> >> > Ludo’.
> >
> > I've been working on it with package-latest-release*, and its nice not
> > having to reimplement the logic, but I can't get it to return the newest
> > version of the upstream-package. I got it to return a useful message
> > with:
> >
> > (define (check-for-updates package)
> >   (if (package-latest-release* package (force %updaters))
> >        (emit-warning package
> >                      (format #f (G_ "can be upgraded to ~a~%")
> >                              (upstream-source-version (package-latest-release package (force %updaters)))))
> >        #t))
> >
> > but I've already checked if there's an upstream release, I shouldn't
> > need to check a second time. package-latest-release* returns true/false,
> 
> 
> AFAICS ‘package-latest-release*’ returns #f or an <upstream-source>:
> 
> --8<---------------cut here---------------start------------->8---
> scheme@(guile-user)> (package-latest-release* binutils (force %updaters))
> $4 = #<<upstream-source> package: "binutils" version: "2.29.1" urls: ("mirror://gnu/binutils/binutils-2.29.1.tar.gz" "mirror://gnu/binutils/binutils-2.29.1.tar.lz" "mirror://gnu/binutils/binutils-2.29.1.tar.bz2" "mirror://gnu/binutils/binutils-2.29.1.tar.xz") signature-urls: ("mirror://gnu/binutils/binutils-2.29.1.tar.gz.sig" "mirror://gnu/binutils/binutils-2.29.1.tar.lz.sig" "mirror://gnu/binutils/binutils-2.29.1.tar.bz2.sig" "mirror://gnu/binutils/binutils-2.29.1.tar.xz.sig")>
> --8<---------------cut here---------------end--------------->8---
> 
> So you can write:
> 
>   (match (package-latest-release* package (force %updaters))
>     ((? upstream-source? source)
>      …)
>     (#f   ;cannot determine latest release
>      #f))
> 
> HTH!
> 
> Ludo’.

I looked at the final diff between my earlier patch and my final
version, it pretty much added up to the lines you wrote above ;) I
should get myself a rubber duck, should help my debugging and reading
skills.


-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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