Generic updaters should run last

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Maxim Cournoyer
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 27 Sep 2024 08:49
(name . bug-guix)(address . bug-guix@gnu.org)
878qvdegl1.fsf@gmail.com
Hi,

Currently the updaters are run in the order of their alphabetical
names. For example, with the following instrumentation to see the
updater names tried in guix/upstream.scm:

Toggle snippet (10 lines)
@@ -259,6 +271,7 @@ (define* (package-latest-release package
one."
(any (match-lambda
(($ <upstream-updater> name description pred import)
+ (pk 'trying-updater: name)
(and (pred package)
(import package #:version version))))
updaters))

attempting to update gnome-desktop produces:

Toggle snippet (24 lines)
./pre-inst-env guix refresh gnome-desktop

;;; (trying-updater: bioconductor)

;;; (trying-updater: composer)

;;; (trying-updater: cpan)

;;; (trying-updater: cran)

;;; (trying-updater: crate)

;;; (trying-updater: egg)

;;; (trying-updater: elpa)

;;; (trying-updater: gem)

;;; (trying-updater: generic-git)

;;; (trying-updater: generic-html)
gnu/packages/gnome.scm:2265:13: gnome-desktop would be upgraded from 44.0 to 44.1

It seems to me the generic updaters, being generic thus less
likely to be precise/correct, should be tried last by 'guix refresh'.

This is ensured with the following patch:

Toggle snippet (48 lines)
modified guix/upstream.scm
@@ -48,6 +48,7 @@ (define-module (guix upstream)
#:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-35)
+ #:use-module (srfi srfi-71)
#:use-module (rnrs bytevectors)
#:use-module (ice-9 match)
#:use-module (ice-9 regex)
@@ -226,15 +227,26 @@ (define (importer-modules)
(define %updaters
;; The list of publically-known updaters, alphabetically sorted.
(delay
- (sort (fold-module-public-variables (lambda (obj result)
- (if (upstream-updater? obj)
- (cons obj result)
- result))
- '()
- (importer-modules))
- (lambda (updater1 updater2)
- (string<? (symbol->string (upstream-updater-name updater1))
- (symbol->string (upstream-updater-name updater2)))))))
+ (let* ((updaters
+ (sort (fold-module-public-variables
+ (lambda (obj result)
+ (if (upstream-updater? obj)
+ (cons obj result)
+ result))
+ '()
+ (importer-modules))
+ (lambda (updater1 updater2)
+ (string<?
+ (symbol->string (upstream-updater-name updater1))
+ (symbol->string (upstream-updater-name updater2))))))
+ (generic-updaters rest (partition
+ (compose (cut string-prefix? "generic" <>)
+ symbol->string
+ upstream-updater-name)
+ updaters)))
+ ;; Ensure the generic updaters are tried last, as otherwise they could
+ ;; return less accurate results.
+ (append rest generic-updaters))))
;; Tests need to mock this variable so mark it as "non-declarative".
(set! %updaters %updaters)
@@ -259,6 +271,7 @@ (define* (package-latest-release package

Now, the 'gnome-updater' at least ends up being tried before the
'generic-html':

Toggle snippet (23 lines)
;;; (trying-updater: bioconductor)

;;; (trying-updater: composer)

;;; (trying-updater: cpan)

;;; (trying-updater: cran)

;;; (trying-updater: crate)

;;; (trying-updater: egg)

;;; (trying-updater: elpa)

;;; (trying-updater: gem)

;;; (trying-updater: github)

;;; (trying-updater: gnome)

gnu/packages/gnome.scm:2265:13: gnome-desktop would be upgraded from 44.0 to 44.1

I'll send a 'git format-patch' version in a follow-up.

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 27 Sep 2024 09:02
[PATCH] upstream: Try the generic importers last.
(address . 73508@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
6f3b073a68e5dc8f93c54b4427cfd555aee0e9f0.1727420518.git.maxim.cournoyer@gmail.com
* guix/upstream.scm (%updaters): Ensure the updaters with a name starting by
'generic' appear last in the list.

Change-Id: I98977f6c925c14303273755b5b4dc36035f78bda
---
guix/upstream.scm | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)

Toggle diff (52 lines)
diff --git a/guix/upstream.scm b/guix/upstream.scm
index 753916be64..0593c363aa 100644
--- a/guix/upstream.scm
+++ b/guix/upstream.scm
@@ -48,6 +48,7 @@ (define-module (guix upstream)
#:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-35)
+ #:use-module (srfi srfi-71)
#:use-module (rnrs bytevectors)
#:use-module (ice-9 match)
#:use-module (ice-9 regex)
@@ -226,15 +227,26 @@ (define (importer-modules)
(define %updaters
;; The list of publically-known updaters, alphabetically sorted.
(delay
- (sort (fold-module-public-variables (lambda (obj result)
- (if (upstream-updater? obj)
- (cons obj result)
- result))
- '()
- (importer-modules))
- (lambda (updater1 updater2)
- (string<? (symbol->string (upstream-updater-name updater1))
- (symbol->string (upstream-updater-name updater2)))))))
+ (let* ((updaters
+ (sort (fold-module-public-variables
+ (lambda (obj result)
+ (if (upstream-updater? obj)
+ (cons obj result)
+ result))
+ '()
+ (importer-modules))
+ (lambda (updater1 updater2)
+ (string<?
+ (symbol->string (upstream-updater-name updater1))
+ (symbol->string (upstream-updater-name updater2))))))
+ (generic-updaters rest (partition
+ (compose (cut string-prefix? "generic" <>)
+ symbol->string
+ upstream-updater-name)
+ updaters)))
+ ;; Ensure the generic updaters are tried last, as otherwise they could
+ ;; return less accurate results.
+ (append rest generic-updaters))))
;; Tests need to mock this variable so mark it as "non-declarative".
(set! %updaters %updaters)

base-commit: a4ea332bc219e14560d3a5daaa658425d898ec37
--
2.46.0
L
L
Ludovic Courtès wrote on 14 Oct 2024 13:01
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87msj73q46.fsf@gnu.org
Hello,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (5 lines)
> * guix/upstream.scm (%updaters): Ensure the updaters with a name starting by
> 'generic' appear last in the list.
>
> Fixes: https://issues.guix.gnu.org/

Toggle quote (2 lines)
> Change-Id: I98977f6c925c14303273755b5b4dc36035f78bda

Apart from that, LGTM, thanks!

Ludo’.
M
M
Maxim Cournoyer wrote on 19 Oct 2024 14:52
Re: bug#73508: Generic updaters should run last
(name . Ludovic Courtès)(address . ludo@gnu.org)
87v7xocl1d.fsf_-_@gmail.com
Hi,

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

Toggle quote (15 lines)
> Hello,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * guix/upstream.scm (%updaters): Ensure the updaters with a name starting by
>> 'generic' appear last in the list.
>>
>> Fixes: https://issues.guix.gnu.org/
>
> This should be <https://issues.guix.gnu.org/73508>.
>
>> Change-Id: I98977f6c925c14303273755b5b4dc36035f78bda
>
> Apart from that, LGTM, thanks!

Fixed the above, and pushed!

--
Thanks,
Maxim
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 73508@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 73508
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch