GNU bug report logs - #64358
“guix refresh” chokes on cran.scm

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 64358 in the body.
You can then email your comments to 64358 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: bug-guix <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: “guix refresh” chokes on cran.scm
Date: Thu, 29 Jun 2023 23:26:48 +0200

The new refresh behavior to update inputs automatically seems to
sometimes cause the updater to lose track of the position in the file,
leading to errors like this:

--8<---------------cut here---------------start------------->8---
Backtrace:
In ice-9/boot-9.scm:
  1752:10 18 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
In unknown file:
          17 (apply-smob/0 #<thunk 7f4fe56ba2a0>)
In ice-9/boot-9.scm:
    724:2 16 (call-with-prompt _ _ #<procedure default-prompt-handler (k proc)>)
In ice-9/eval.scm:
    619:8 15 (_ #(#(#<directory (guile-user) 7f4fe56bfc80>)))
In guix/ui.scm:
   2309:7 14 (run-guix . _)
  2272:10 13 (run-guix-command _ . _)
In ice-9/boot-9.scm:
  1752:10 12 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
  1752:10 11 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
In guix/store.scm:
   659:37 10 (thunk)
  2168:25  9 (run-with-store #<store-connection 256.99 7f4fc7ad02d0> _ #:guile-for-build _ #:system _ #:target _)
In guix/scripts/refresh.scm:
   592:16  8 (_ _)
In srfi/srfi-1.scm:
    634:9  7 (for-each #<procedure 7f4fc6860080 at guix/scripts/refresh.scm:593:17 (update)> _)
In guix/scripts/refresh.scm:
   363:21  6 (update-package _ #<package r-dygraphs <at> 1.1.1.6 /home/rekado/dev/gx/branches/master/gnu/packages/cran.scm:30341 7f4fc8e040b0> _ …)
In ice-9/boot-9.scm:
  1747:15  5 (with-exception-handler #<procedure 7f4fc0fe54e0 at ice-9/boot-9.scm:1831:7 (exn)> _ #:unwind? _ #:unwind-for-type _)
In ice-9/ports.scm:
   433:17  4 (call-with-input-file _ _ #:binary _ #:encoding _ #:guess-encoding _)
In guix/packages.scm:
   762:17  3 (_ #<input: /home/rekado/dev/gx/branches/master/gnu/packages/cran.scm 16>)
In guix/utils.scm:
   423:23  2 (go-to-location #<input: /home/rekado/dev/gx/branches/master/gnu/packages/cran.scm 16> 30341 2)
In ice-9/boot-9.scm:
  1685:16  1 (raise-exception _ #:continuable? _)
  1685:16  0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
unexpected end of line #<input: /home/rekado/dev/gx/branches/master/gnu/packages/cran.scm 16>
--8<---------------cut here---------------end--------------->8---

Prior to that we see warnings like this:

--8<---------------cut here---------------start------------->8---
/home/rekado/dev/gx/branches/master/gnu/packages/cran.scm:36350:2: r-readtext: updating from version 0.82 to version 0.90...
/home/rekado/dev/gx/branches/master/gnu/packages/cran.scm:36350:2: warning: r-readtext: no `version' field in source; skipping
--8<---------------cut here---------------end--------------->8---

It’s as if the position in the file has been lost and it tries to update
the definition of r-readtext that is no longer where the current port
position is.

I use “./pre-inst-env guix refresh -t cran -u”, which updates dozens of
packages.  Perhaps the changes result in untracked position shifts in
the file?

-- 
Ricardo

Message #8 received at 64358 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 64358 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org
Subject: “guix refresh” chokes on cran.scm
Date: Mon, 03 Jul 2023 21:29:09 +0200

> Prior to that we see warnings like this:
>
> /home/rekado/dev/gx/branches/master/gnu/packages/cran.scm:36350:2: r-readtext: updating from version 0.82 to version 0.90...
> /home/rekado/dev/gx/branches/master/gnu/packages/cran.scm:36350:2: warning: r-readtext: no `version' field in source; skipping
>
> It’s as if the position in the file has been lost and it tries to update
> the definition of r-readtext that is no longer where the current port
> position is.

It seems that this is indeed the problem.  The value for a <package>’s
“location” field is known at compile/eval time and this value will not
be correct after the first substantial edit has taken place.

I see these options:

1 - pass a value to “update-package” that corresponds to line changes so
  far and let it return an updated value, making “update-package” aware
  of file changes.  This would be rather ugly.

2 - compute the location of the target package anew if the file it is
  located in has since been edited.

3 - never rely on the line number of the package location value; just open
  the specified file and always search it for a package definition.
  Optionally take the line number into account as a starting point for a
  search in both directions.

4 - integrate changes with git, so that all edits are commits that are
  applied to the very same base commit.

The third option seems the most reasonable and lightweight to me.

-- 
Ricardo

Message #11 received at 64358 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 64358 <at> debbugs.gnu.org
Subject: Re: bug#64358: “guix refresh” chokes on
 cran.scm
Date: Fri, 07 Jul 2023 15:42:59 +0200

Hi,

Ricardo Wurmus <rekado <at> elephly.net> skribis:

>> Prior to that we see warnings like this:
>>
>> /home/rekado/dev/gx/branches/master/gnu/packages/cran.scm:36350:2: r-readtext: updating from version 0.82 to version 0.90...
>> /home/rekado/dev/gx/branches/master/gnu/packages/cran.scm:36350:2: warning: r-readtext: no `version' field in source; skipping
>>
>> It’s as if the position in the file has been lost and it tries to update
>> the definition of r-readtext that is no longer where the current port
>> position is.
>
> It seems that this is indeed the problem.  The value for a <package>’s
> “location” field is known at compile/eval time and this value will not
> be correct after the first substantial edit has taken place.

The way ‘guix style -S inputs’ handles it is by starting editing
packages from the bottom of the file and upwards (see the bottom of
(guix scripts style)).  That way, source location is valid as it edits
things.

Perhaps we can do that here?

Ludo’.

Message #14 received at 64358 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 64358 <at> debbugs.gnu.org
Subject: Re: bug#64358: “guix refresh” chokes on
 cran.scm
Date: Fri, 07 Jul 2023 22:13:52 +0200

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

> The way ‘guix style -S inputs’ handles it is by starting editing
> packages from the bottom of the file and upwards (see the bottom of
> (guix scripts style)).  That way, source location is valid as it edits
> things.
>
> Perhaps we can do that here?

Oh, that’s a very good idea.  Worth a try!

-- 
Ricardo

Message #17 received at 64358 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 64358 <at> debbugs.gnu.org
Cc: Ricardo Wurmus <rekado <at> elephly.net>
Subject: [PATCH] refresh: Sort update specs by package location.
Date: Mon, 10 Jul 2023 20:41:41 +0200

Fixes <https://issues.guix.gnu.org/64358>.

* guix/scripts/refresh.scm (guix-refresh): Sort update specs by location from
bottom to top before updating packages.
---
 guix/scripts/refresh.scm | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/guix/scripts/refresh.scm b/guix/scripts/refresh.scm
index 9676271542..7a0c312f9d 100644
--- a/guix/scripts/refresh.scm
+++ b/guix/scripts/refresh.scm
@@ -6,7 +6,7 @@
 ;;; Copyright © 2016 Ben Woodcroft <donttrustben <at> gmail.com>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe <at> gmail.com>
 ;;; Copyright © 2018 Efraim Flashner <efraim <at> flashner.co.il>
-;;; Copyright © 2019 Ricardo Wurmus <rekado <at> elephly.net>
+;;; Copyright © 2019, 2023 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2020 Simon Tournier <zimon.toutoune <at> gmail.com>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
 ;;; Copyright © 2022 Hartmut Goebel <h.goebel <at> crazy-compilers.com>
@@ -589,16 +589,27 @@ (define-command (guix-refresh . args)
                               (or (assoc-ref opts 'keyring)
                                   (string-append (config-directory)
                                                  "/upstream/trustedkeys.kbx"))))
-                (for-each
-                 (lambda (update)
-                   (update-package store
-                                   (update-spec-package update)
-                                   (update-spec-version update)
-                                   updaters
-                                   #:key-server (%openpgp-key-server)
-                                   #:key-download key-download
-                                   #:warn? warn?))
-                 update-specs)
+                (let* ((get-line
+                        (compose location-line
+                                 package-location
+                                 update-spec-package))
+                       ;; Sort the specs so that we update packages from the
+                       ;; bottom of the file to the top.  This way we can be
+                       ;; sure that the package locations are always correct
+                       ;; and never shifted due to previous edits.
+                       (sorted-update-specs
+                        (sort update-specs
+                              (lambda (a b) (> (get-line a) (get-line b))))))
+                  (for-each
+                   (lambda (update)
+                     (update-package store
+                                     (update-spec-package update)
+                                     (update-spec-version update)
+                                     updaters
+                                     #:key-server (%openpgp-key-server)
+                                     #:key-download key-download
+                                     #:warn? warn?))
+                   sorted-update-specs))
                 (return #t)))
              (else
               (for-each (cut check-for-package-update <> updaters

base-commit: d0296970fb8ed97ac17bd4c580351af961a8c0fb
-- 
2.40.1

Message #20 received at 64358 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Christopher Baines <mail <at> cbaines.net>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, 64358 <at> debbugs.gnu.org
Subject: Re: bug#64358: [PATCH] refresh: Sort update specs by package location.
Date: Mon, 10 Jul 2023 23:49:55 +0200

Hi,

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> Fixes <https://issues.guix.gnu.org/64358>.
>
> * guix/scripts/refresh.scm (guix-refresh): Sort update specs by location from
> bottom to top before updating packages.

[…]

> +                (let* ((get-line
> +                        (compose location-line
> +                                 package-location
> +                                 update-spec-package))

Maybe s/get-line/spec-line/ ?

Otherwise LGTM, thanks!

Ludo’.

Message #23 received at 64358 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Christopher Baines <mail <at> cbaines.net>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, 64358 <at> debbugs.gnu.org
Subject: Re: bug#64358: [PATCH] refresh: Sort update specs by package location.
Date: Tue, 11 Jul 2023 00:17:43 +0200

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

> Ricardo Wurmus <rekado <at> elephly.net> skribis:
>
>> Fixes <https://issues.guix.gnu.org/64358>.
>>
>> * guix/scripts/refresh.scm (guix-refresh): Sort update specs by location from
>> bottom to top before updating packages.
>
> […]
>
>> +                (let* ((get-line
>> +                        (compose location-line
>> +                                 package-location
>> +                                 update-spec-package))
>
> Maybe s/get-line/spec-line/ ?

Okay, will change it.

I found that it’s probably not a good idea to sort by location-line
without also taking into account the file name.  My latest version uses
“location->string” and “string>” instead of “location-line” and “>”,
ensuring that changes to the same file remain grouped.

I’ll push this together with the bulk update of CRAN packages that I’m
working on.

Thanks for the review!

-- 
Ricardo

Message #28 received at 64358-done <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 64358-done <at> debbugs.gnu.org
Subject: “guix refresh” chokes on cran.scm
Date: Wed, 12 Jul 2023 14:35:13 +0200

Fixed in commit b43841c124d15eaecc41b3928f08a26dbd5c653a
-- 
Ricardo

Message #31 received at 64358 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Christopher Baines <mail <at> cbaines.net>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, 64358 <at> debbugs.gnu.org
Subject: Re: bug#64358: [PATCH] refresh: Sort update specs by package location.
Date: Fri, 14 Jul 2023 15:19:28 +0200

Hallo,

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> I found that it’s probably not a good idea to sort by location-line
> without also taking into account the file name.  My latest version uses
> “location->string” and “string>” instead of “location-line” and “>”,
> ensuring that changes to the same file remain grouped.

Oops, good point.

> I’ll push this together with the bulk update of CRAN packages that I’m
> working on.

Awesome, thanks!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.