guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] refresh: Suggest changes to inputs when updating.


From: Ludovic Courtès
Subject: Re: [PATCH] refresh: Suggest changes to inputs when updating.
Date: Fri, 28 Oct 2016 15:04:21 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)

Hello!

Thanks for tackling this, it’s more and more needed!

Ricardo Wurmus <address@hidden> skribis:

> * guix/scripts/refresh.scm (updater->importer-info): New procedure.
> (mock): New syntax rule.
> (update-package): Run matching importer to suggest changes to inputs.

Tests and a bit of documentation would be welcome for the final
revision.

> +(define (updater->importer-info updater-name)
> +  "Return a list containing an update procedure, a package name converter,
> +and, optionally, an archive symbol for the given UPDATER-NAME.  Return #F for
> +an unknown updater."
> +  (case updater-name
> +    ((gnu)
> +     (list gnu->guix-package
> +           package-name))
> +    ((elpa)
> +     (list elpa->guix-package
> +           package-name))
> +    ((cran)
> +     (list cran->guix-package
> +           (@@ (guix import cran) package->upstream-name)))
> +    ((bioconductor)
> +     (list cran->guix-package
> +           (@@ (guix import cran) package->upstream-name)
> +           'bioconductor))
> +    ((hackage)
> +     (list hackage->guix-package
> +           (@@ (guix import gem) guix-package->hackage-name)))
> +    ((pypi)
> +     (list pypi->guix-package
> +           guix-package->pypi-name))
> +    ((gem)
> +     (list gem->guix-package
> +           (@@ (guix import gem) guix-package->gem-name)))
> +    (else #f)))

This is not OK because it breaks the <upstream-updater> abstraction.

> +;; FIXME: copied from (guix tests)
> +(define-syntax-rule (mock (module proc replacement) body ...)
> +  "Within BODY, replace the definition of PROC from MODULE with the 
> definition
> +given by REPLACEMENT."
> +  (let* ((m (resolve-module 'module))
> +         (original (module-ref m 'proc)))
> +    (dynamic-wind
> +      (lambda () (module-set! m 'proc replacement))
> +      (lambda () body ...)
> +      (lambda () (module-set! m 'proc original)))))

This is OK for tests but not for Real Code.  :-)

>  (define* (update-package store package updaters
>                           #:key (key-download 'interactive))
>    "Update the source file that defines PACKAGE with the new version.
> @@ -246,7 +287,62 @@ values: 'interactive' (default), 'always', and 'never'."
>                      (package-version package) version)
>              (let ((hash (call-with-input-file tarball
>                            port-sha256)))
> -              (update-package-source package version hash)))
> +              (update-package-source package version hash))
> +
> +            ;; Run importer to compare inputs and suggest changes.
> +            (let* ((updater (find (lambda (updater)
> +                                    ((upstream-updater-predicate updater) 
> package))
> +                                  updaters))
> +                   (updater-name (upstream-updater-name updater)))
> +              (match (updater->importer-info updater-name)
> +                (#f #t) ; do nothing if there's no matching importer
> +                ((importer convert-name . archive)
> +                 ;; Replace "download-to-store" to avoid downloading the
> +                 ;; tarball again.
> +                 (match (mock ((guix download) download-to-store
> +                               (lambda _ tarball))
> +                         (apply importer (convert-name package) archive))
> +                   ((and expr ('package fields ...))
> +                    ;; FIXME: Is there a nicer way to match names in the
> +                    ;; package expression?  Could we compare actual packages
> +                    ;; instead of only their labels?
> +                    (let* ((imported-inputs
> +                            (append
> +                             (match expr
> +                               ((path *** ('inputs
> +                                           ('quasiquote ((label ('unquote 
> sym)) ...)))) label)
> +                               (_ '()))

What if we first changed importers to return a <package> instead of an
sexp?

That way we could (1) factorize the ‘package->sexp’ machinery in (guix
upstream), and (2) simplify this specific use case.

A downside is that syntactic sugar, like when the importer returns an
sexp representing a call to ‘pypi-uri’, would be lost.  Hmm.

Anyway, I think the functionality (determining the set of inputs that
needs to be changed) should go to (guix upstream).  The user messages
(“consider removing these inputs”, etc.) should go to (guix refresh).

Concretely, ‘package-update-path’ could be changed to return not just an
<upstream-source> but also dependency information.  That could be
achieved by adding native/propagated/regular inputs to
<upstream-source>, or maybe preferably by creating a new
<upstream-dependency> record to carry that information.

WDYT?

That said, I can imagine that this patch has already been extremely
helpful for mass updates…

Thank you!

Ludo’.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]