unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#56441: guix time-machine broken by profiles speed-up
@ 2022-07-07 16:42 zimoun
  2022-07-07 17:29 ` Ricardo Wurmus
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: zimoun @ 2022-07-07 16:42 UTC (permalink / raw)
  To: 56441; +Cc: Ricardo Wurmus, Ludovic Courtès

Hi,

Bug#55499 [1] is fixed by 4ff12d1de7cd617b791996ee7ca1240660b4c20e.
However, because the manifest version is going from 3 to 4 with new
fields, the new Guix cannot builds the old Guix.

Commit 4ff12d1de7cd617b791996ee7ca1240660b4c20e is not able to go to its
parent 9b8c442b254b82196fe2492142b3c3bbbd891a1b.

--8<---------------cut here---------------start------------->8---
$ git rev-parse 4ff12d1de7^
9b8c442b254b82196fe2492142b3c3bbbd891a1b

$ guix time-machine --commit=4ff12d1de7 -- time-machine --commit=9b8c442b25 -- help
Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
Computing Guix derivation for 'x86_64-linux'... \
The following derivation will be built:
  /gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv

building package cache...
|builder for `/gnu/store/19nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv' failed to produce output path `/gnu/store/axqgrls563slnp76x60dqlv7sdwcm2ly-guix-package-cache'
build of /gnu/store/19nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv failed
View build log at '/var/log/guix/drvs/19/nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv.gz'.
cannot build derivation `/gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv': 1 dependencies couldn't be built
guix time-machine: error: build of `/gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv' failed
--8<---------------cut here---------------end--------------->8---


1: <https://issues.guix.gnu.org/55499>



Cheers,
simon




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#56441: guix time-machine broken by profiles speed-up
  2022-07-07 16:42 bug#56441: guix time-machine broken by profiles speed-up zimoun
@ 2022-07-07 17:29 ` Ricardo Wurmus
  2022-07-07 17:52   ` Ricardo Wurmus
  2022-07-08 10:46 ` bug#56441: Time travel doesn't resist profile format changes Ricardo Wurmus
       [not found] ` <handler.56441.D56441.165731768325904.notifdone@debbugs.gnu.org>
  2 siblings, 1 reply; 11+ messages in thread
From: Ricardo Wurmus @ 2022-07-07 17:29 UTC (permalink / raw)
  To: zimoun; +Cc: Ludovic Courtès, 56441


zimoun <zimon.toutoune@gmail.com> writes:

> Bug#55499 [1] is fixed by 4ff12d1de7cd617b791996ee7ca1240660b4c20e.
> However, because the manifest version is going from 3 to 4 with new
> fields, the new Guix cannot builds the old Guix.
>
> Commit 4ff12d1de7cd617b791996ee7ca1240660b4c20e is not able to go to its
> parent 9b8c442b254b82196fe2492142b3c3bbbd891a1b.
>
> $ git rev-parse 4ff12d1de7^
> 9b8c442b254b82196fe2492142b3c3bbbd891a1b
>
> $ guix time-machine --commit=4ff12d1de7 -- time-machine --commit=9b8c442b25 -- help
> Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
> Computing Guix derivation for 'x86_64-linux'... \
> The following derivation will be built:
>   /gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv
>
> building package cache...
> |builder for `/gnu/store/19nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv' failed to produce output path `/gnu/store/axqgrls563slnp76x60dqlv7sdwcm2ly-guix-package-cache'
> build of /gnu/store/19nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv failed
> View build log at '/var/log/guix/drvs/19/nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv.gz'.
> cannot build derivation `/gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv': 1 dependencies couldn't be built
> guix time-machine: error: build of `/gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv' failed

IIUC the problem here is that PACKAGE-CACHE-FILE, a profile hook running
inside an inferior (= with an older Guix), is operating on a manifest
that uses the new version 4 format.  That manifest was built with the
*current* version of Guix that understands the version 4 format.

PACKAGE-CACHE-FILE uses GEXP->DERIVATION-IN-INFERIOR and the inferior is
a PROFILE that’s made from a given MANIFEST value.  That PROFILE is
*not* built inside the inferior, so it doesn’t use the old manifest
format.

The cache generation is happening in
/gnu/store/17v5781w8kl1snp826jl6z40z5lbbw1y-inferior-script.scm.drv, which — as the name indicates — is
run inside the inferior, i.e. the older Guix.

-- 
Ricardo




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#56441: guix time-machine broken by profiles speed-up
  2022-07-07 17:29 ` Ricardo Wurmus
@ 2022-07-07 17:52   ` Ricardo Wurmus
  2022-07-08  8:52     ` Ludovic Courtès
  0 siblings, 1 reply; 11+ messages in thread
From: Ricardo Wurmus @ 2022-07-07 17:52 UTC (permalink / raw)
  To: zimoun; +Cc: Ludovic Courtès, 56441


Ricardo Wurmus <rekado@elephly.net> writes:

> zimoun <zimon.toutoune@gmail.com> writes:
>
>> Bug#55499 [1] is fixed by 4ff12d1de7cd617b791996ee7ca1240660b4c20e.
>> However, because the manifest version is going from 3 to 4 with new
>> fields, the new Guix cannot builds the old Guix.
>>
>> Commit 4ff12d1de7cd617b791996ee7ca1240660b4c20e is not able to go to its
>> parent 9b8c442b254b82196fe2492142b3c3bbbd891a1b.
>>
>> $ git rev-parse 4ff12d1de7^
>> 9b8c442b254b82196fe2492142b3c3bbbd891a1b
>>
>> $ guix time-machine --commit=4ff12d1de7 -- time-machine --commit=9b8c442b25 -- help
>> Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
>> Computing Guix derivation for 'x86_64-linux'... \
>> The following derivation will be built:
>>   /gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv
>>
>> building package cache...
>> |builder for
>> `/gnu/store/19nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv'
>> failed to produce output path
>> `/gnu/store/axqgrls563slnp76x60dqlv7sdwcm2ly-guix-package-cache'
>> build of /gnu/store/19nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv failed
>> View build log at '/var/log/guix/drvs/19/nk2x26s0dp68r7d36ifbg0ck0q3xps-guix-package-cache.drv.gz'.
>> cannot build derivation `/gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv': 1 dependencies couldn't be built
>> guix time-machine: error: build of `/gnu/store/r5qk23fibxn5ryd2k7b8qkbryqv4m3ds-profile.drv' failed
>
> IIUC the problem here is that PACKAGE-CACHE-FILE, a profile hook running
> inside an inferior (= with an older Guix), is operating on a manifest
> that uses the new version 4 format.  That manifest was built with the
> *current* version of Guix that understands the version 4 format.
>
> PACKAGE-CACHE-FILE uses GEXP->DERIVATION-IN-INFERIOR and the inferior is
> a PROFILE that’s made from a given MANIFEST value.  That PROFILE is
> *not* built inside the inferior, so it doesn’t use the old manifest
> format.
>
> The cache generation is happening in
> /gnu/store/17v5781w8kl1snp826jl6z40z5lbbw1y-inferior-script.scm.drv, which — as the name indicates — is
> run inside the inferior, i.e. the older Guix.

It’s worse than that: an older Guix living in a new profile (i.e a
profile with a version 4 manifest) cannot describe itself, because it
cannot read the manifest.

I took the same profile that was generated by the time machine and
contains the old Guix.  Here’s the manifest:

--8<---------------cut here---------------start------------->8---
$ head /gnu/store/mwbgfyl0zzipyac1lbgss2gcji67fp4s-profile/manifest
;; This file was automatically generated and is for internal use only.
;; It cannot be passed to the '--manifest' option.
;; Run 'guix package --export-manifest' if you want to export a file
;; suitable for '--manifest'.

(manifest
  (version 4)
  (packages
    (("guix"
      "9b8c442"
--8<---------------cut here---------------end--------------->8---

And then:

--8<---------------cut here---------------start------------->8---
$ /gnu/store/mwbgfyl0zzipyac1lbgss2gcji67fp4s-profile/bin/guix describe
guix describe: error: unsupported manifest format
--8<---------------cut here---------------end--------------->8---

This old Guix lives in a new profile.  We must prevent this.  But how?

How about this:

Build Guix with only the 'guix channel (no any other custom channels
allowed), and then build the channel’s profile (with all requested
channels) inside an inferior of that plain Guix.  Since the inferior
Guix can only build profiles that it supports the resulting *profile*
will be compatible with the same Guix (and any additional channels).

-- 
Ricardo




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#56441: guix time-machine broken by profiles speed-up
  2022-07-07 17:52   ` Ricardo Wurmus
@ 2022-07-08  8:52     ` Ludovic Courtès
  2022-07-08 10:36       ` Ludovic Courtès
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2022-07-08  8:52 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: 56441, zimoun

Hi,

Ricardo Wurmus <rekado@elephly.net> skribis:

> It’s worse than that: an older Guix living in a new profile (i.e a
> profile with a version 4 manifest) cannot describe itself, because it
> cannot read the manifest.

Argh.

[...]

> This old Guix lives in a new profile.  We must prevent this.  But how?
>
> How about this:
>
> Build Guix with only the 'guix channel (no any other custom channels
> allowed), and then build the channel’s profile (with all requested
> channels) inside an inferior of that plain Guix.  Since the inferior
> Guix can only build profiles that it supports the resulting *profile*
> will be compatible with the same Guix (and any additional channels).

Another option (thinking out loud):

  • in ‘package-cache-file’, unconditionally generate a v3 profile (we
    could add a ‘version’ field to <profile> etc.);

  • likewise in ‘channel-instances->derivation’.

Problem: it doesn’t address the case where you install the ‘guix’
package in a regular profile (I think?).

Other option: run the whole ‘profile-derivation’ call of
‘channel-instances->derivation’ in the inferior, instead of just
‘package-cache-file’.

Needs more thought…

Ludo’.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#56441: guix time-machine broken by profiles speed-up
  2022-07-08  8:52     ` Ludovic Courtès
@ 2022-07-08 10:36       ` Ludovic Courtès
  2022-07-08 14:34         ` bug#56441: Time travel doesn't resist profile format changes zimoun
  2022-07-08 22:01         ` bug#56441: guix time-machine broken by profiles speed-up Ludovic Courtès
  0 siblings, 2 replies; 11+ messages in thread
From: Ludovic Courtès @ 2022-07-08 10:36 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: 56441, zimoun

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

Hi,

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

> Another option (thinking out loud):
>
>   • in ‘package-cache-file’, unconditionally generate a v3 profile (we
>     could add a ‘version’ field to <profile> etc.);
>
>   • likewise in ‘channel-instances->derivation’.

The patches below do that.  As discussed on IRC, it’s not pretty but
it’s pragmatic.

Tested with:

  ./pre-inst-env guix time-machine \
    --commit=85a5110de79f4fe9fd822ede3915654ee699d6c5 -- describe

How does that sound?

> Problem: it doesn’t address the case where you install the ‘guix’
> package in a regular profile (I think?).

I haven’t yet checked whether this is the case.

Ludo’.


[-- Attachment #2: 0001-profiles-Support-the-creation-of-profiles-with-versi.patch --]
[-- Type: text/x-patch, Size: 9113 bytes --]

From e1d7117cb4f81f4f590ff9c9a8fe14797cc210f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Fri, 8 Jul 2022 12:26:50 +0200
Subject: [PATCH 1/2] profiles: Support the creation of profiles with version 3
 manifests.

* guix/profiles.scm (%manifest-format-version): New variable.
(manifest->gexp): Add optional 'format-version' parameter.
[optional, entry->gexp]: Honor it.
(profile-derivation): Add #:format-version parameter and honor it.
(<profile>)[format-version]: New field.
(profile-compiler): Honor it.
* guix/build/profiles.scm (manifest-sexp->inputs+search-paths): Support
both versions 3 and 4.  Remove unused 'properties' variable.
* tests/profiles.scm ("profile-derivation format version 3"): New test.
---
 guix/build/profiles.scm |  6 +++---
 guix/profiles.scm       | 48 ++++++++++++++++++++++++++++++-----------
 tests/profiles.scm      | 28 ++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 16 deletions(-)

diff --git a/guix/build/profiles.scm b/guix/build/profiles.scm
index 2ab76bde74..0c92f222b4 100644
--- a/guix/build/profiles.scm
+++ b/guix/build/profiles.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2015, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2015, 2017-2022 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -162,7 +162,7 @@ (define-syntax let-fields
        (begin body ...))))
 
   (match manifest                            ;this must match 'manifest->gexp'
-    (('manifest ('version 4)
+    (('manifest ('version (or 3 4))
                 ('packages (entries ...)))
      (let loop ((entries entries)
                 (inputs '())
@@ -170,7 +170,7 @@ (define-syntax let-fields
        (match entries
          (((name version output item fields ...) . rest)
           (let ((paths search-paths))
-            (let-fields fields (propagated-inputs search-paths properties)
+            (let-fields fields (propagated-inputs search-paths)
               (loop (append rest propagated-inputs) ;breadth-first traversal
                     (cons item inputs)
                     (append search-paths paths)))))
diff --git a/guix/profiles.scm b/guix/profiles.scm
index a21cc432dc..d1dfa13e98 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -452,12 +452,23 @@ (define (inferior->entry)
          packages)
     manifest-entry=?)))
 
-(define (manifest->gexp manifest)
-  "Return a representation of MANIFEST as a gexp."
+(define %manifest-format-version
+  ;; The current manifest format version.
+  4)
+
+(define* (manifest->gexp manifest #:optional
+                         (format-version %manifest-format-version))
+  "Return a representation in FORMAT-VERSION of MANIFEST as a gexp."
   (define (optional name value)
-    (if (null? value)
-        #~()
-        #~((#$name #$value))))
+    (match format-version
+      (4
+       (if (null? value)
+           #~()
+           #~((#$name #$value))))
+      (3
+       (match name
+         ('properties #~((#$name #$@value)))
+         (_           #~((#$name #$value)))))))
 
   (define (entry->gexp entry)
     ;; Maintain in state monad a vhash of visited entries, indexed by their
@@ -467,10 +478,11 @@ (define (entry->gexp entry)
     ;; the presence of propagated inputs, where we could otherwise end up
     ;; repeating large trees.
     (mlet %state-monad ((visited (current-state)))
-      (if (match (vhash-assq (manifest-entry-item entry) visited)
-            ((_ . previous-entry)
-             (manifest-entry=? previous-entry entry))
-            (#f #f))
+      (if (and (= format-version 4)
+               (match (vhash-assq (manifest-entry-item entry) visited)
+                 ((_ . previous-entry)
+                  (manifest-entry=? previous-entry entry))
+                 (#f #f)))
           (return #~(repeated #$(manifest-entry-name entry)
                               #$(manifest-entry-version entry)
                               (ungexp (manifest-entry-item entry)
@@ -500,9 +512,14 @@ (define (entry->gexp entry)
                                               search-paths))
                             #$@(optional 'properties properties))))))))))
 
+  (unless (memq format-version '(3 4))
+    (raise (formatted-message
+            (G_ "cannot emit manifests formatted as version ~a")
+            format-version)))
+
   (match manifest
     (($ <manifest> (entries ...))
-     #~(manifest (version 4)
+     #~(manifest (version #$format-version)
                  (packages #$(run-with-state
                                  (mapm %state-monad entry->gexp entries)
                                vlist-null))))))
@@ -1883,6 +1900,7 @@ (define* (profile-derivation manifest
                              (allow-unsupported-packages? #f)
                              (allow-collisions? #f)
                              (relative-symlinks? #f)
+                             (format-version %manifest-format-version)
                              system target)
   "Return a derivation that builds a profile (aka. 'user environment') with
 the given MANIFEST.  The profile includes additional derivations returned by
@@ -1968,7 +1986,7 @@ (define builder
 
             #+(if locales? set-utf8-locale #t)
 
-            (build-profile #$output '#$(manifest->gexp manifest)
+            (build-profile #$output '#$(manifest->gexp manifest format-version)
                            #:extra-inputs '#$extra-inputs
                            #:symlink #$(if relative-symlinks?
                                            #~symlink-relative
@@ -2007,19 +2025,23 @@ (define-record-type* <profile> profile make-profile
   (allow-collisions?  profile-allow-collisions?   ;Boolean
                       (default #f))
   (relative-symlinks? profile-relative-symlinks?  ;Boolean
-                      (default #f)))
+                      (default #f))
+  (format-version     profile-format-version      ;integer
+                      (default %manifest-format-version)))
 
 (define-gexp-compiler (profile-compiler (profile <profile>) system target)
   "Compile PROFILE to a derivation."
   (match profile
     (($ <profile> name manifest hooks
-                  locales? allow-collisions? relative-symlinks?)
+                  locales? allow-collisions? relative-symlinks?
+                  format-version)
      (profile-derivation manifest
                          #:name name
                          #:hooks hooks
                          #:locales? locales?
                          #:allow-collisions? allow-collisions?
                          #:relative-symlinks? relative-symlinks?
+                         #:format-version format-version
                          #:system system #:target target))))
 
 (define* (profile-search-paths profile
diff --git a/tests/profiles.scm b/tests/profiles.scm
index f002dfc5e4..7bed946bf3 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -286,6 +286,34 @@ (define transform1
                  (string=? (dirname (readlink bindir))
                            (derivation->output-path guile))))))
 
+(test-assertm "profile-derivation format version 3"
+  ;; Make sure we can create and read a version 3 manifest.
+  (mlet* %store-monad
+      ((entry ->    (package->manifest-entry %bootstrap-guile
+                                             #:properties '((answer . 42))))
+       (manifest -> (manifest (list entry)))
+       (drv1        (profile-derivation manifest
+                                        #:format-version 3 ;old version
+                                        #:hooks '()
+                                        #:locales? #f))
+       (drv2        (profile-derivation manifest
+                                        #:hooks '()
+                                        #:locales? #f))
+       (profile1 -> (derivation->output-path drv1))
+       (profile2 -> (derivation->output-path drv2))
+       (_          (built-derivations (list drv1 drv2))))
+    (return (let ((manifest1 (profile-manifest profile1))
+                  (manifest2 (profile-manifest profile2)))
+              (match (manifest-entries manifest1)
+                ((entry1)
+                 (match (manifest-entries manifest2)
+                   ((entry2)
+                    (and (manifest-entry=? entry1 entry2)
+                         (equal? (manifest-entry-properties entry1)
+                                 '((answer . 42)))
+                         (equal? (manifest-entry-properties entry2)
+                                 '((answer . 42))))))))))))
+
 (test-assertm "profile-derivation, ordering & collisions"
   ;; ENTRY1 and ENTRY2 both provide 'bin/guile'--a collision.  Make sure
   ;; ENTRY1 "wins" over ENTRY2.  See <https://bugs.gnu.org/49102>.

base-commit: 33179b180a5b02b730b4b37369ccf48204597940
-- 
2.36.1


[-- Attachment #3: 0002-channels-Emit-version-3-profiles.patch --]
[-- Type: text/x-patch, Size: 2609 bytes --]

From d404ef913482c6a9c692bad2142cf7202ebc1cc4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Fri, 8 Jul 2022 12:31:25 +0200
Subject: [PATCH 2/2] channels: Emit version 3 profiles.

Fixes <https://issues.guix.gnu.org/56441>.
Reported by zimoun <zimon.toutoune@gmail.com>.

* guix/channels.scm (package-cache-file): Add 'format-version' field to
PROFILE.
(channel-instances->derivation): Pass #:format-version to
'profile-derivation'.
---
 guix/channels.scm | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/guix/channels.scm b/guix/channels.scm
index ce1a60436f..689b30e0eb 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018-2022 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2019 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
@@ -896,7 +896,12 @@ (define (instance->entry instance drv)
 (define (package-cache-file manifest)
   "Build a package cache file for the instance in MANIFEST.  This is meant to
 be used as a profile hook."
-  (let ((profile (profile (content manifest) (hooks '()))))
+  ;; Note: Emit a profile in format version 3, which was introduced in 2017
+  ;; and is readable by Guix since before version 1.0.  This ensures that the
+  ;; Guix in MANIFEST is able to read the manifest file created for its own
+  ;; profile below.  See <https://issues.guix.gnu.org/56441>.
+  (let ((profile (profile (content manifest) (hooks '())
+                          (format-version 3))))
     (define build
       #~(begin
           (use-modules (gnu packages))
@@ -937,8 +942,12 @@ (define (channel-instances->derivation instances)
   "Return the derivation of the profile containing INSTANCES, a list of
 channel instances."
   (mlet %store-monad ((manifest (channel-instances->manifest instances)))
+    ;; Emit a profile in format version so that, if INSTANCES denotes an old
+    ;; Guix, it can still read that profile, for instance for the purposes of
+    ;; 'guix describe'.
     (profile-derivation manifest
-                        #:hooks %channel-profile-hooks)))
+                        #:hooks %channel-profile-hooks
+                        #:format-version 3)))
 
 (define latest-channel-instances*
   (store-lift latest-channel-instances))
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#56441: Time travel doesn't resist profile format changes
  2022-07-07 16:42 bug#56441: guix time-machine broken by profiles speed-up zimoun
  2022-07-07 17:29 ` Ricardo Wurmus
@ 2022-07-08 10:46 ` Ricardo Wurmus
  2022-07-08 20:42   ` bug#56441: guix time-machine broken by profiles speed-up Ludovic Courtès
       [not found] ` <handler.56441.D56441.165731768325904.notifdone@debbugs.gnu.org>
  2 siblings, 1 reply; 11+ messages in thread
From: Ricardo Wurmus @ 2022-07-08 10:46 UTC (permalink / raw)
  To: 56441

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

Attached is a patch that attempts to build the manifest in an inferior.
This fails because manifest->gexp returns a gexp that we can’t get out
of the inferior to pass to build-profile.

So even more of the surrounding code would have to be evaluated in the
inferior.

@Ludo: your patch looks good to me.  It’s a pragmatic, minimally
invasive fix, so thumbs up emoji from me!  Thank you!

-- 
Ricardo



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-channel-stuff.patch --]
[-- Type: text/x-patch, Size: 7845 bytes --]

From da3eaeea0d0082138284720dce60f6bdfc796f2b Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Fri, 8 Jul 2022 08:40:27 +0200
Subject: [PATCH] channel stuff

---
 guix/channels.scm | 17 ++++++++--
 guix/profiles.scm | 81 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 75 insertions(+), 23 deletions(-)

diff --git a/guix/channels.scm b/guix/channels.scm
index ce1a60436f..45fba89685 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -896,6 +896,7 @@ (define (instance->entry instance drv)
 (define (package-cache-file manifest)
   "Build a package cache file for the instance in MANIFEST.  This is meant to
 be used as a profile hook."
+  (pk 'package-cache-file manifest)
   (let ((profile (profile (content manifest) (hooks '()))))
     (define build
       #~(begin
@@ -918,7 +919,12 @@ (define build
               (mkdir #$output))))
 
     (gexp->derivation-in-inferior "guix-package-cache" build
-                                  profile
+                                  (pk 'guixx (manifest-entry-item
+                                              (manifest-lookup
+                                               manifest
+                                               (manifest-pattern
+                                                 (name "guix")))))
+                                  ;profile
 
                                   ;; If the Guix in PROFILE is too old and
                                   ;; lacks 'guix repl', don't build the cache
@@ -936,8 +942,15 @@ (define %channel-profile-hooks
 (define (channel-instances->derivation instances)
   "Return the derivation of the profile containing INSTANCES, a list of
 channel instances."
-  (mlet %store-monad ((manifest (channel-instances->manifest instances)))
+  (mlet* %store-monad ((manifest (channel-instances->manifest instances))
+                       (drv -> (manifest-entry-item
+                                (manifest-lookup
+                                 manifest
+                                 (manifest-pattern
+                                   (name "guix")))))
+                       (built (built-derivations (list drv))))
     (profile-derivation manifest
+                        #:inferior-guix (derivation->output-path drv)
                         #:hooks %channel-profile-hooks)))
 
 (define latest-channel-instances*
diff --git a/guix/profiles.scm b/guix/profiles.scm
index 701852ae98..2d482cf91a 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -38,6 +38,10 @@ (define-module (guix profiles)
   #:use-module (guix records)
   #:use-module (guix packages)
   #:use-module (guix derivations)
+  #:autoload   (guix inferior) (gexp->derivation-in-inferior
+                                open-inferior
+                                close-inferior
+                                inferior-eval)
   #:use-module (guix search-paths)
   #:use-module (guix gexp)
   #:use-module (guix modules)
@@ -1946,6 +1950,7 @@ (define* (profile-derivation manifest
                              (allow-unsupported-packages? #f)
                              (allow-collisions? #f)
                              (relative-symlinks? #f)
+                             (inferior-guix #f)
                              system target)
   "Return a derivation that builds a profile (aka. 'user environment') with
 the given MANIFEST.  The profile includes additional derivations returned by
@@ -2013,6 +2018,33 @@ (define set-utf8-locale
                                   (package-version glibc-utf8-locales))))
           (setlocale LC_ALL "en_US.utf8")))
 
+    (define (manifest-gexp)
+      (pk 'man-gexp
+          (if inferior-guix
+              (let* ((inferior (open-inferior inferior-guix))
+                     (result
+                      ;; TODO: this doesn't work because we can't lift the
+                      ;; gexp out of the inferior.
+                      (inferior-eval `(begin
+                                        (use-modules (guix profiles)
+                                                     (guix derivations))
+                                        ((@@ (guix profiles) manifest->gexp)
+                                         (manifest
+                                          (list
+                                           ,@(map (lambda (entry)
+                                                    `(manifest-entry
+                                                       (name ,(manifest-entry-name entry))
+                                                       (version ,(manifest-entry-version entry))
+                                                       (output ,(manifest-entry-output entry))
+                                                       (item (read-derivation-from-file
+                                                              ,(derivation-file-name
+                                                                (manifest-entry-item entry))))))
+                                                  (manifest-entries manifest))))))
+                                     inferior)))
+                (close-inferior inferior)
+                result)
+              (manifest->gexp manifest))))
+
     (define builder
       (with-imported-modules '((guix build profiles)
                                (guix build union)
@@ -2031,32 +2063,39 @@ (define builder
 
             #+(if locales? set-utf8-locale #t)
 
-            (build-profile #$output '#$(manifest->gexp manifest)
+            (build-profile #$output '#$(manifest-gexp)
                            #:extra-inputs '#$extra-inputs
                            #:symlink #$(if relative-symlinks?
                                            #~symlink-relative
                                            #~symlink)))))
 
-    (gexp->derivation name builder
-                      #:system system
-                      #:target target
-
-                      ;; Don't complain about _IO* on Guile 2.2.
-                      #:env-vars '(("GUILE_WARN_DEPRECATED" . "no"))
-
-                      ;; Not worth offloading.
-                      #:local-build? #t
-
-                      ;; Disable substitution because it would trigger a
-                      ;; connection to the substitute server, which is likely
-                      ;; to have no substitute to offer.
-                      #:substitutable? #f
-
-                      #:properties `((type . profile)
-                                     (profile
-                                      (count
-                                       . ,(length
-                                           (manifest-entries manifest))))))))
+    (let ((proc (if inferior-guix
+                    (lambda args
+                      (apply gexp->derivation-in-inferior
+                             name builder (pk 'inf inferior-guix)
+                             args))
+                    (lambda args
+                      (apply gexp->derivation
+                             name builder args)))))
+      (proc #:system system
+            #:target target
+
+            ;; Don't complain about _IO* on Guile 2.2.
+            #:env-vars '(("GUILE_WARN_DEPRECATED" . "no"))
+
+            ;; Not worth offloading.
+            #:local-build? #t
+
+            ;; Disable substitution because it would trigger a
+            ;; connection to the substitute server, which is likely
+            ;; to have no substitute to offer.
+            #:substitutable? #f
+
+            #:properties `((type . profile)
+                           (profile
+                            (count
+                             . ,(length
+                                 (manifest-entries manifest)))))))))
 
 ;; Declarative profile.
 (define-record-type* <profile> profile make-profile
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#56441: Time travel doesn't resist profile format changes
  2022-07-08 10:36       ` Ludovic Courtès
@ 2022-07-08 14:34         ` zimoun
  2022-07-08 16:13           ` Ludovic Courtès
  2022-07-08 22:01         ` bug#56441: guix time-machine broken by profiles speed-up Ludovic Courtès
  1 sibling, 1 reply; 11+ messages in thread
From: zimoun @ 2022-07-08 14:34 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Ricardo Wurmus, 56441

Hi,

On ven., 08 juil. 2022 at 12:36, Ludovic Courtès <ludo@gnu.org> wrote:
> Hi,
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> Another option (thinking out loud):
>>
>>   • in ‘package-cache-file’, unconditionally generate a v3 profile (we
>>     could add a ‘version’ field to <profile> etc.);
>>
>>   • likewise in ‘channel-instances->derivation’.
>
> The patches below do that.  As discussed on IRC, it’s not pretty but
> it’s pragmatic.
>
> Tested with:
>
>   ./pre-inst-env guix time-machine \
>     --commit=85a5110de79f4fe9fd822ede3915654ee699d6c5 -- describe
>
> How does that sound?

It sounds good!  Here a check from v1.3 to v0.16.

--8<---------------cut here---------------start------------->8---
$ for ci in a0178d34f582b50e9bdbb0403943129ae5b560ff   \
              a099685659b4bfa6b3218f84953cbb7ff9e88063 \
              d62c9b2671be55ae0305bebfda17b595f33797f2 \
              d68de958b60426798ed62797ff7c96c327a672ac \
              6298c3ffd9654d3231a6f25390b056483e8f407c \
              4a0b87f0ec5b6c2dcf82b372dd20ca7ea6acdd9c;
do
    ./pre-inst-env guix time-machine --commit=$ci -- describe
done

  guix a0178d3
    repository URL: https://git.savannah.gnu.org/git/guix.git
    commit: a0178d34f582b50e9bdbb0403943129ae5b560ff
  guix a099685
    repository URL: https://git.savannah.gnu.org/git/guix.git
    commit: a099685659b4bfa6b3218f84953cbb7ff9e88063
guile: warning: failed to install locale
  guix d62c9b2
    repository URL: https://git.savannah.gnu.org/git/guix.git
    commit: d62c9b2671be55ae0305bebfda17b595f33797f2
guile: warning: failed to install locale
  guix d68de95
    repository URL: https://git.savannah.gnu.org/git/guix.git
    commit: d68de958b60426798ed62797ff7c96c327a672ac
guile: warning: failed to install locale
  guix 6298c3f
    repository URL: https://git.savannah.gnu.org/git/guix.git
    commit: 6298c3ffd9654d3231a6f25390b056483e8f407c
guile: warning: failed to install locale
hint: Consider installing the `glibc-utf8-locales' or `glibc-locales' package and defining `GUIX_LOCPATH', along these
lines:

     guix package -i glibc-utf8-locales
     export GUIX_LOCPATH="$HOME/.guix-profile/lib/locale"

See the "Application Setup" section in the manual, for more info.


  guix 4a0b87f
    repository URL: https://git.savannah.gnu.org/git/guix.git
    commit: 4a0b87f0ec5b6c2dcf82b372dd20ca7ea6acdd9c
--8<---------------cut here---------------end--------------->8---



Cheers,
simon




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#56441: Time travel doesn't resist profile format changes
  2022-07-08 14:34         ` bug#56441: Time travel doesn't resist profile format changes zimoun
@ 2022-07-08 16:13           ` Ludovic Courtès
  0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2022-07-08 16:13 UTC (permalink / raw)
  To: zimoun; +Cc: Ricardo Wurmus, 56441

Hello,

zimoun <zimon.toutoune@gmail.com> skribis:

> It sounds good!  Here a check from v1.3 to v0.16.
>
> $ for ci in a0178d34f582b50e9bdbb0403943129ae5b560ff   \
>               a099685659b4bfa6b3218f84953cbb7ff9e88063 \
>               d62c9b2671be55ae0305bebfda17b595f33797f2 \
>               d68de958b60426798ed62797ff7c96c327a672ac \
>               6298c3ffd9654d3231a6f25390b056483e8f407c \
>               4a0b87f0ec5b6c2dcf82b372dd20ca7ea6acdd9c;
> do
>     ./pre-inst-env guix time-machine --commit=$ci -- describe
> done

That’s a great test, thanks for checking!

I would really like us to run that kind of test automatically.  It’s
expensive, requires network access, Git repo access, etc., so I’m not
sure it’s suitable for “make check”.  But we need to think about it.

I’ll commit either tonight or on Monday.

Ludo’.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#56441: guix time-machine broken by profiles speed-up
  2022-07-08 10:46 ` bug#56441: Time travel doesn't resist profile format changes Ricardo Wurmus
@ 2022-07-08 20:42   ` Ludovic Courtès
  0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2022-07-08 20:42 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: 56441

Ricardo Wurmus <rekado@elephly.net> skribis:

> Attached is a patch that attempts to build the manifest in an inferior.
> This fails because manifest->gexp returns a gexp that we can’t get out
> of the inferior to pass to build-profile.

Thanks for sharing!  This sounded like the “right” approach, but there’s
a part that made me feel uneasy about it:

> +    (define (manifest-gexp)
> +      (pk 'man-gexp
> +          (if inferior-guix
> +              (let* ((inferior (open-inferior inferior-guix))
> +                     (result
> +                      ;; TODO: this doesn't work because we can't lift the
> +                      ;; gexp out of the inferior.
> +                      (inferior-eval `(begin
> +                                        (use-modules (guix profiles)
> +                                                     (guix derivations))
> +                                        ((@@ (guix profiles) manifest->gexp)
> +                                         (manifest
> +                                          (list
> +                                           ,@(map (lambda (entry)
> +                                                    `(manifest-entry
> +                                                       (name ,(manifest-entry-name entry))
> +                                                       (version ,(manifest-entry-version entry))
> +                                                       (output ,(manifest-entry-output entry))
> +                                                       (item (read-derivation-from-file
> +                                                              ,(derivation-file-name
> +                                                                (manifest-entry-item entry))))))
> +                                                  (manifest-entries manifest))))))

Here we have to rely on a larger part of the API: a subset of (guix
profiles) and (guix derivations).

Conversely, in (guix channels), the only assumption made about the API
implemented by the other Guix (which might be older or might be newer)
is the ‘generate-package-cache’ procedure.  It’s a small requirement, so
potentially easier to satisfy in future versions for a long time.


When dealing with these time travel issues, I feel we have difficult
choices to make.  It’s a bit of an unusual requirement that we have.

Ludo’.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#56441: guix time-machine broken by profiles speed-up
  2022-07-08 10:36       ` Ludovic Courtès
  2022-07-08 14:34         ` bug#56441: Time travel doesn't resist profile format changes zimoun
@ 2022-07-08 22:01         ` Ludovic Courtès
  1 sibling, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2022-07-08 22:01 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: zimoun, 56441-done

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

> The patches below do that.  As discussed on IRC, it’s not pretty but
> it’s pragmatic.

Pushed!

  e80f0cda96 etc: Add 'time-travel-manifest.scm'.
  c9fbd40785 channels: Emit version 3 profiles.
  89e2288751 profiles: Support the creation of profiles with version 3 manifests.

The last commit was inspired by zimoun’s test.  I plan to add a
‘time-travel’ jobset on ci.guix.

Thank you zimoun & Ricardo!

Ludo’.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#56441: guix time-machine broken by profiles speed-up
       [not found] ` <handler.56441.D56441.165731768325904.notifdone@debbugs.gnu.org>
@ 2022-07-11  8:27   ` Ludovic Courtès
  0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2022-07-11  8:27 UTC (permalink / raw)
  To: zimoun, 56441

Hi!

>   e80f0cda96 etc: Add 'time-travel-manifest.scm'.
>   c9fbd40785 channels: Emit version 3 profiles.
>   89e2288751 profiles: Support the creation of profiles with version 3 manifests.

And here’s the CI job:

  https://ci.guix.gnu.org/jobset/time-travel

OK, it’s currently broken, but it should be unbroken by commit
5d0437ea8ce0147b47b9df866fa2413b48f71a1a (I’ve set it to run every 12
hours at most, so we’ll have to check.)

Ludo’.




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-07-11  8:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 16:42 bug#56441: guix time-machine broken by profiles speed-up zimoun
2022-07-07 17:29 ` Ricardo Wurmus
2022-07-07 17:52   ` Ricardo Wurmus
2022-07-08  8:52     ` Ludovic Courtès
2022-07-08 10:36       ` Ludovic Courtès
2022-07-08 14:34         ` bug#56441: Time travel doesn't resist profile format changes zimoun
2022-07-08 16:13           ` Ludovic Courtès
2022-07-08 22:01         ` bug#56441: guix time-machine broken by profiles speed-up Ludovic Courtès
2022-07-08 10:46 ` bug#56441: Time travel doesn't resist profile format changes Ricardo Wurmus
2022-07-08 20:42   ` bug#56441: guix time-machine broken by profiles speed-up Ludovic Courtès
     [not found] ` <handler.56441.D56441.165731768325904.notifdone@debbugs.gnu.org>
2022-07-11  8:27   ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).