Suggest input changes when updating packages

  • Done
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Ricardo Wurmus
  • zimoun
Owner
unassigned
Submitted by
Ricardo Wurmus
Severity
normal
R
R
Ricardo Wurmus wrote on 11 Jan 2019 10:11
(address . guix-patches@gnu.org)
8736pzpnhg.fsf@elephly.net
Hi Guix,

Here are two patches: the first changes (guix upstream) and (guix
scripts refresh) to support reporting of input changes when updating a
package. The second lets the CRAN and Bioconductor importers use these
features.

--
Ricardo
R
R
Ricardo Wurmus wrote on 11 Jan 2019 10:42
[PATCH 2/2] import: cran: Suggest input changes.
(address . 34040@debbugs.gnu.org)(name . Ricardo Wurmus)(address . rekado@elephly.net)
20190111094208.28327-2-rekado@elephly.net
* guix/import/cran.scm (latest-cran-release, latest-bioconductor-release):
Return input-changes.
---
guix/import/cran.scm | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)

Toggle diff (68 lines)
diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index 15163bd16..b287be694 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2015, 2016, 2017, 2018 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2015, 2016, 2017, 2018, 2019 Ricardo Wurmus <rekado@elephly.net>
;;; Copyright © 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;;
@@ -390,11 +390,11 @@ s-expression corresponding to that package, or #f on failure."
(_ #f)))
(_ #f)))))
-(define (latest-cran-release package)
- "Return an <upstream-source> for the latest release of PACKAGE."
+(define (latest-cran-release pkg)
+ "Return an <upstream-source> for the latest release of the package PKG."
(define upstream-name
- (package->upstream-name package))
+ (package->upstream-name pkg))
(define meta
(fetch-description 'cran upstream-name))
@@ -403,15 +403,18 @@ s-expression corresponding to that package, or #f on failure."
(let ((version (assoc-ref meta "Version")))
;; CRAN does not provide signatures.
(upstream-source
- (package (package-name package))
+ (package (package-name pkg))
(version version)
- (urls (cran-uri upstream-name version))))))
+ (urls (cran-uri upstream-name version))
+ (input-changes
+ (changed-inputs pkg
+ (description->package 'cran meta)))))))
-(define (latest-bioconductor-release package)
- "Return an <upstream-source> for the latest release of PACKAGE."
+(define (latest-bioconductor-release pkg)
+ "Return an <upstream-source> for the latest release of the package PKG."
(define upstream-name
- (package->upstream-name package))
+ (package->upstream-name pkg))
(define version
(latest-bioconductor-package-version upstream-name))
@@ -419,9 +422,13 @@ s-expression corresponding to that package, or #f on failure."
(and version
;; Bioconductor does not provide signatures.
(upstream-source
- (package (package-name package))
+ (package (package-name pkg))
(version version)
- (urls (bioconductor-uri upstream-name version)))))
+ (urls (bioconductor-uri upstream-name version))
+ (input-changes
+ (changed-inputs
+ pkg
+ (cran->guix-package upstream-name 'bioconductor))))))
(define (cran-package? package)
"Return true if PACKAGE is an R package from CRAN."
--
2.20.1
R
R
Ricardo Wurmus wrote on 11 Jan 2019 10:42
[PATCH 1/2] refresh: Suggest input changes when updating.
(address . 34040@debbugs.gnu.org)(name . Ricardo Wurmus)(address . rekado@elephly.net)
20190111094208.28327-1-rekado@elephly.net
* guix/upstream.scm (<upstream-source>)[input-changes]: New field.
(<upstream-input-change>): New record.
(upstream-input-change?, upstream-input-change-name,
upstream-input-change-type, upstream-input-change-action, changed-inputs): New
procedures.
(package-update): Pass along input changes.
* guix/script/refresh.scm (update-package): Process input changes.
---
guix/scripts/refresh.scm | 23 +++++++++-
guix/upstream.scm | 90 ++++++++++++++++++++++++++++++++++++----
2 files changed, 104 insertions(+), 9 deletions(-)

Toggle diff (188 lines)
diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index 003c915da..15cf385fb 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -6,6 +6,7 @@
;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;; Copyright © 2018 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -224,7 +225,7 @@ KEY-DOWNLOAD specifies a download policy for missing OpenPGP keys; allowed
values: 'interactive' (default), 'always', and 'never'. When WARN? is true,
warn about packages that have no matching updater."
(if (lookup-updater package updaters)
- (let-values (((version tarball)
+ (let-values (((version tarball changes)
(package-update store package updaters
#:key-download key-download))
((loc)
@@ -238,6 +239,26 @@ warn about packages that have no matching updater."
(location->string loc)
(package-name package)
(package-version package) version)
+ (for-each
+ (lambda (change)
+ (format (current-error-port)
+ (match (list (upstream-input-change-action change)
+ (upstream-input-change-type change))
+ (('add 'regular)
+ (G_ "~a: consider adding this input: ~a~%"))
+ (('add 'native)
+ (G_ "~a: consider adding this native input: ~a~%"))
+ (('add 'propagated)
+ (G_ "~a: consider adding this propagated input: ~a~%"))
+ (('remove 'regular)
+ (G_ "~a: consider removing this input: ~a~%"))
+ (('remove 'native)
+ (G_ "~a: consider removing this native input: ~a~%"))
+ (('remove 'propagated)
+ (G_ "~a: consider removing this propagated input: ~a~%")))
+ (package-name package)
+ (upstream-input-change-name change)))
+ (changes))
(let ((hash (call-with-input-file tarball
port-sha256)))
(update-package-source package version hash)))
diff --git a/guix/upstream.scm b/guix/upstream.scm
index 9e1056f7a..880cb9094 100644
--- a/guix/upstream.scm
+++ b/guix/upstream.scm
@@ -1,6 +1,7 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2015 Alex Kost <alezost@gmail.com>
+;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -45,6 +46,7 @@
upstream-source-urls
upstream-source-signature-urls
upstream-source-archive-types
+ upstream-source-input-changes
url-prefix-predicate
coalesce-sources
@@ -56,6 +58,12 @@
upstream-updater-predicate
upstream-updater-latest
+ upstream-input-change?
+ upstream-input-change-name
+ upstream-input-change-type
+ upstream-input-change-action
+ changed-inputs
+
%updaters
lookup-updater
@@ -82,7 +90,73 @@
(version upstream-source-version) ;string
(urls upstream-source-urls) ;list of strings
(signature-urls upstream-source-signature-urls ;#f | list of strings
- (default #f)))
+ (default #f))
+ (input-changes upstream-source-input-changes
+ (default '()) (thunked)))
+
+;; Representation of an upstream input change.
+(define-record-type* <upstream-input-change>
+ upstream-input-change make-upstream-input-change
+ upstream-input-change?
+ (name upstream-input-change-name) ;string
+ (type upstream-input-change-type) ;symbol: regular | native | propagated
+ (action upstream-input-change-action)) ;symbol: add | remove
+
+(define (changed-inputs package package-sexp)
+ "Return a list of input changes for PACKAGE based on the newly imported
+S-expression PACKAGE-SEXP."
+ (match package-sexp
+ ((and expr ('package fields ...))
+ (let* ((input->name (match-lambda ((name pkg . out) name)))
+ (new-regular
+ (match expr
+ ((path *** ('inputs
+ ('quasiquote ((label ('unquote sym)) ...)))) label)
+ (_ '())))
+ (new-native
+ (match expr
+ ((path *** ('native-inputs
+ ('quasiquote ((label ('unquote sym)) ...)))) label)
+ (_ '())))
+ (new-propagated
+ (match expr
+ ((path *** ('propagated-inputs
+ ('quasiquote ((label ('unquote sym)) ...)))) label)
+ (_ '())))
+ (current-regular
+ (map input->name (package-inputs package)))
+ (current-native
+ (map input->name (package-native-inputs package)))
+ (current-propagated
+ (map input->name (package-propagated-inputs package))))
+ (append-map
+ (match-lambda
+ ((action type names)
+ (map (lambda (name)
+ (upstream-input-change
+ (name name)
+ (type type)
+ (action action)))
+ names)))
+ `((add regular
+ ,(lset-difference equal?
+ new-regular current-regular))
+ (remove regular
+ ,(lset-difference equal?
+ current-regular new-regular))
+ (add native
+ ,(lset-difference equal?
+ new-native current-native))
+ (remove native
+ ,(lset-difference equal?
+ current-native new-native))
+ (add propagated
+ ,(lset-difference equal?
+ new-propagated current-propagated))
+ (remove propagated
+ ,(lset-difference equal?
+ current-propagated new-propagated))))))
+ (_ '())))
(define (url-prefix-predicate prefix)
"Return a predicate that returns true when passed a package where one of its
@@ -268,12 +342,12 @@ values: the item from LST1 and the item from LST2 that match PRED."
(define* (package-update store package updaters
#:key (key-download 'interactive))
- "Return the new version and the file name of the new version tarball for
-PACKAGE, or #f and #f when PACKAGE is up-to-date. KEY-DOWNLOAD specifies a
-download policy for missing OpenPGP keys; allowed values: 'always', 'never',
-and 'interactive' (default)."
+ "Return the new version, the file name of the new version tarball and input
+changes for PACKAGE, or #f and #f when PACKAGE is up-to-date. KEY-DOWNLOAD
+specifies a download policy for missing OpenPGP keys; allowed values:
+'always', 'never', and 'interactive' (default)."
(match (package-latest-release* package updaters)
- (($ <upstream-source> _ version urls signature-urls)
+ (($ <upstream-source> _ version urls signature-urls changes)
(let*-values (((name)
(package-name package))
((archive-type)
@@ -299,9 +373,9 @@ and 'interactive' (default)."
(or signature-urls (circular-list #f)))))
(let ((tarball (download-tarball store url signature-url
#:key-download key-download)))
- (values version tarball))))
+ (values version tarball changes))))
(#f
- (values #f #f))))
+ (values #f #f #f))))
(define (update-package-source package version hash)
"Modify the source file that defines PACKAGE to refer to VERSION,
--
2.20.1
L
L
Ludovic Courtès wrote on 12 Jan 2019 14:40
(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 34040@debbugs.gnu.org)
8736py6lk0.fsf@gnu.org
Hello,

Ricardo Wurmus <rekado@elephly.net> skribis:

Toggle quote (8 lines)
> * guix/upstream.scm (<upstream-source>)[input-changes]: New field.
> (<upstream-input-change>): New record.
> (upstream-input-change?, upstream-input-change-name,
> upstream-input-change-type, upstream-input-change-action, changed-inputs): New
> procedures.
> (package-update): Pass along input changes.
> * guix/script/refresh.scm (update-package): Process input changes.

Really cool!

Toggle quote (8 lines)
> +;; Representation of an upstream input change.
> +(define-record-type* <upstream-input-change>
> + upstream-input-change make-upstream-input-change
> + upstream-input-change?
> + (name upstream-input-change-name) ;string
> + (type upstream-input-change-type) ;symbol: regular | native | propagated
> + (action upstream-input-change-action)) ;symbol: add | remove

Perhaps in some cases in action could be, say, (upgrade "1.2"). Though
that’s of course something we can add later.

Toggle quote (7 lines)
> (define* (package-update store package updaters
> #:key (key-download 'interactive))
> - "Return the new version and the file name of the new version tarball for
> -PACKAGE, or #f and #f when PACKAGE is up-to-date. KEY-DOWNLOAD specifies a
> -download policy for missing OpenPGP keys; allowed values: 'always', 'never',
> -and 'interactive' (default)."
> + "Return the new version, the file name of the new version tarball and input
^
Missing comma. :-)

Toggle quote (1 lines)
> +changes for PACKAGE, or #f and #f when PACKAGE is up-to-date. KEY-DOWNLOAD
^
Instead of “, or …”:

; return #f (three values) when PACKAGE is up-to-date.

Otherwise LGTM, thanks!

Ludo’.
L
L
Ludovic Courtès wrote on 12 Jan 2019 14:42
Re: [bug#34040] [PATCH 2/2] import: cran: Suggest input changes.
(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 34040@debbugs.gnu.org)
87y37q56wy.fsf@gnu.org
Ricardo Wurmus <rekado@elephly.net> skribis:

Toggle quote (3 lines)
> * guix/import/cran.scm (latest-cran-release, latest-bioconductor-release):
> Return input-changes.

LGTM!

Toggle quote (4 lines)
> -(define (latest-cran-release package)
> - "Return an <upstream-source> for the latest release of PACKAGE."
> +(define (latest-cran-release pkg)

Unless there’s a name clash I think it’s OK to keep ‘package’.

Thanks!

Ludo’.
R
R
Ricardo Wurmus wrote on 12 Jan 2019 22:11
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 34040@debbugs.gnu.org)
87ef9hoa2q.fsf@mdc-berlin.de
Hi,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (7 lines)
> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> * guix/import/cran.scm (latest-cran-release, latest-bioconductor-release):
>> Return input-changes.
>
> LGTM!

Thank you for taking the time to review this!

Toggle quote (6 lines)
>> -(define (latest-cran-release package)
>> - "Return an <upstream-source> for the latest release of PACKAGE."
>> +(define (latest-cran-release pkg)
>
> Unless there’s a name clash I think it’s OK to keep ‘package’.

There is a name clash, which I found very surprising. Take this procedure:

Toggle snippet (23 lines)
(define (latest-cran-release package)
"Return an <upstream-source> for the latest release of the package PKG."

(define upstream-name
(package->upstream-name package))

(define meta
(fetch-description 'cran upstream-name))

(and meta
(let ((version (assoc-ref meta "Version")))
;; CRAN does not provide signatures.
(upstream-source
(package (package-name package))
(version version)
(urls (cran-uri upstream-name version))
(input-changes
(changed-inputs
package ; <– this is the value of the “package” field,
; not the value of the procedure argument.
(description->package 'cran meta)))))))

That’s why I renamed the argument to “pkg”.

--
Ricardo
L
L
Ludovic Courtès wrote on 15 Jan 2019 10:30
control message for bug #34040
(address . control@debbugs.gnu.org)
871s5e469f.fsf@gnu.org
tags 34040 fixed
close 34040
L
L
Ludovic Courtès wrote on 21 Jan 2019 22:34
Re: [bug#34040] [PATCH 1/2] refresh: Suggest input changes when updating.
(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 34040@debbugs.gnu.org)
87bm49itkj.fsf@gnu.org
Hello,

Other comments/questions came to mind… :-)

Ricardo Wurmus <rekado@elephly.net> skribis:

Toggle quote (8 lines)
> (version upstream-source-version) ;string
> (urls upstream-source-urls) ;list of strings
> (signature-urls upstream-source-signature-urls ;#f | list of strings
> - (default #f)))
> + (default #f))
> + (input-changes upstream-source-input-changes
> + (default '()) (thunked)))

Any particular reason for making ‘input-changes’ thunked?

This causes a failure in tests/upstream.scm (because two evaluator
procedures are unlikely to be eq?). I would fix it by removing the
‘thunked’ property but I’m not sure if it’d make sense.

Another thing: “upstream source” designates something
absolute/stateless, but “input changes” designates something
relative/stateful. So on second thought, I wonder whether
<upstream-source> is the right place for it.

I was thinking that updaters could maybe return two values
(<upstream-source> + list of changed inputs), which would be equivalent
but somewhat clearer. The downside is that we’d have to change all
updaters to return multiple values.

Alternately, we could change ‘input-changes’ to ‘inputs’, which would be
absolute, not relative, and thus ‘package-update’ would take care of
calling ‘changed-inputs’ etc.

WDYT?

Apologies for not asking these questions earlier!

Thanks,
Ludo’.
R
R
Ricardo Wurmus wrote on 25 Jan 2019 17:21
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 34040@debbugs.gnu.org)
87k1isvhbe.fsf@elephly.net
Hi,

sorry for not repyling earlier. I did not see this message.

Toggle quote (16 lines)
> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> (version upstream-source-version) ;string
>> (urls upstream-source-urls) ;list of strings
>> (signature-urls upstream-source-signature-urls ;#f | list of strings
>> - (default #f)))
>> + (default #f))
>> + (input-changes upstream-source-input-changes
>> + (default '()) (thunked)))
>
> Any particular reason for making ‘input-changes’ thunked?
>
> This causes a failure in tests/upstream.scm (because two evaluator
> procedures are unlikely to be eq?). I would fix it by removing the
> ‘thunked’ property but I’m not sure if it’d make sense.

Oh, I’m sorry for breaking this.

My thinking was that this field should be lazily evaluated, because it
is somewhat expensive. “input-changes” gets its value from calling
“changed-inputs” on a newly imported package. I wanted to avoid the
import when a user is not interested in the “input-changes” field.

Toggle quote (10 lines)
> Another thing: “upstream source” designates something
> absolute/stateless, but “input changes” designates something
> relative/stateful. So on second thought, I wonder whether
> <upstream-source> is the right place for it.
>
> I was thinking that updaters could maybe return two values
> (<upstream-source> + list of changed inputs), which would be equivalent
> but somewhat clearer. The downside is that we’d have to change all
> updaters to return multiple values.

This sounds like a good idea and I’m fine with changing all of the
importers.

Toggle quote (4 lines)
> Alternately, we could change ‘input-changes’ to ‘inputs’, which would be
> absolute, not relative, and thus ‘package-update’ would take care of
> calling ‘changed-inputs’ etc.

That would also work, but I think I prefer an updater to report changes
rather than a new list of inputs.

--
Ricardo
L
L
Ludovic Courtès wrote on 25 Jan 2019 22:15
(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 34040@debbugs.gnu.org)
87y378moax.fsf@gnu.org
Hello,

Ricardo Wurmus <rekado@elephly.net> skribis:

Toggle quote (23 lines)
>> Ricardo Wurmus <rekado@elephly.net> skribis:
>>
>>> (version upstream-source-version) ;string
>>> (urls upstream-source-urls) ;list of strings
>>> (signature-urls upstream-source-signature-urls ;#f | list of strings
>>> - (default #f)))
>>> + (default #f))
>>> + (input-changes upstream-source-input-changes
>>> + (default '()) (thunked)))
>>
>> Any particular reason for making ‘input-changes’ thunked?
>>
>> This causes a failure in tests/upstream.scm (because two evaluator
>> procedures are unlikely to be eq?). I would fix it by removing the
>> ‘thunked’ property but I’m not sure if it’d make sense.
>
> Oh, I’m sorry for breaking this.
>
> My thinking was that this field should be lazily evaluated, because it
> is somewhat expensive. “input-changes” gets its value from calling
> “changed-inputs” on a newly imported package. I wanted to avoid the
> import when a user is not interested in the “input-changes” field.

I see, that makes sense.

Toggle quote (8 lines)
>> I was thinking that updaters could maybe return two values
>> (<upstream-source> + list of changed inputs), which would be equivalent
>> but somewhat clearer. The downside is that we’d have to change all
>> updaters to return multiple values.
>
> This sounds like a good idea and I’m fine with changing all of the
> importers.

Cool!

Toggle quote (7 lines)
>> Alternately, we could change ‘input-changes’ to ‘inputs’, which would be
>> absolute, not relative, and thus ‘package-update’ would take care of
>> calling ‘changed-inputs’ etc.
>
> That would also work, but I think I prefer an updater to report changes
> rather than a new list of inputs.

OTOH if we return a complete list of inputs, then we don’t have to worry
about the cost of ‘changed-inputs’ since that would only be called on
demand; also there’d be a single call site for ‘changed-inputs’.

Well, you tell us! :-)

Thanks,
Ludo’.
R
R
Ricardo Wurmus wrote on 25 Jan 2019 22:48
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 34040@debbugs.gnu.org)
87fttgv25x.fsf@elephly.net
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (11 lines)
>>> Alternately, we could change ‘input-changes’ to ‘inputs’, which would be
>>> absolute, not relative, and thus ‘package-update’ would take care of
>>> calling ‘changed-inputs’ etc.
>>
>> That would also work, but I think I prefer an updater to report changes
>> rather than a new list of inputs.
>
> OTOH if we return a complete list of inputs, then we don’t have to worry
> about the cost of ‘changed-inputs’ since that would only be called on
> demand; also there’d be a single call site for ‘changed-inputs’.

The potentially expensive part is to get the inputs, though. Getting a
list of new inputs means that the importer needs to be run. For CRAN
packages this requires downloading the tarball and unpacking parts of
it.

--
Ricardo
L
L
Ludovic Courtès wrote on 26 Jan 2019 14:53
(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 34040@debbugs.gnu.org)
87ef8zle44.fsf@gnu.org
Halo!

Ricardo Wurmus <rekado@elephly.net> skribis:

Toggle quote (18 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>>>> Alternately, we could change ‘input-changes’ to ‘inputs’, which would be
>>>> absolute, not relative, and thus ‘package-update’ would take care of
>>>> calling ‘changed-inputs’ etc.
>>>
>>> That would also work, but I think I prefer an updater to report changes
>>> rather than a new list of inputs.
>>
>> OTOH if we return a complete list of inputs, then we don’t have to worry
>> about the cost of ‘changed-inputs’ since that would only be called on
>> demand; also there’d be a single call site for ‘changed-inputs’.
>
> The potentially expensive part is to get the inputs, though. Getting a
> list of new inputs means that the importer needs to be run. For CRAN
> packages this requires downloading the tarball and unpacking parts of
> it.

Oh right. Then perhaps <upstream-updater> could have a separate
‘inputs’ field, which would be either #f or that procedure to fetch the
list of inputs for the given package. Does that make sense?

(I’m thinking out loud…)

Ludo’.
Z
Z
zimoun wrote on 12 Apr 2022 11:58
Re: bug#34040: Suggest input changes when updating packages
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 34040-done@debbugs.gnu.org)
86y20a39np.fsf@gmail.com
Hi,


On Tue, 15 Jan 2019 at 10:30, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (3 lines)
> tags 34040 fixed
> close 34040

Still marked open although it is done and merged. Closing!

Cheers,
simon
Closed
?