‘guix pack --bootstrap’ is ineffective

Open

Details

2 participants

Chris Marusich
Ludovic Courtès

Owner: unassigned

Submitted by: Ludovic Courtès

Severity: normal

Debbugs page

Ludovic Courtès wrote 7 years ago

Recipients:(address . bug-guix@gnu.org)(name . Chris Marusich)(address . cmmarusich@gmail.com)

Message-ID:87k1pu9pv3.fsf@gnu.org

Hello,

While preparing the 0.15 release I realized that ‘guix pack --bootstrap’

had become ineffective. Concretely, ‘tests/guix-pack.sh’ would attempt

to build the world.

This is a consequence of c45477d2a1a651485feede20fe0f3d15aec48b39, which

introduced a dependency on guile-sqlite3 in derivations that build

packs, even if they don’t actually produce a ‘db.sqlite’ file in there.

I started looking for solutions, which led me to the patch below.

That’s quite intrusive and it doesn’t work because then we have a

similar issue with (guix hash) trying to dlopen libgcrypt.

I’m not sure how to best address it. Another approach would be to do

away with ‘--bootstrap’ and instead write those tests as “system tests”

in a VM, though that’s maybe less satisfactory.

Thoughts?

Thanks,

Ludo’.

Toggle diff (60 lines)diff --git a/guix/scripts/pack.scm b/guix/scripts/pack.scm
index 6d5d745bc..45eeb2e7b 100644
--- a/guix/scripts/pack.scm
+++ b/guix/scripts/pack.scm
@@ -98,8 +98,25 @@ found."
 
 (define guile-sqlite3&co
   ;; Guile-SQLite3 and its propagated inputs.
-  (cons guile-sqlite3
-        (package-transitive-propagated-inputs guile-sqlite3)))
+  (make-parameter
+   (cons guile-sqlite3
+         (package-transitive-propagated-inputs guile-sqlite3))))
+
+(define guile-sqlite3/mock
+  ;; Mock of Guile-SQLite3 used by '--bootstrap', for testing purposes.
+  (computed-file "guile-sqlite3-mock"
+                 (with-imported-modules '((guix build utils))
+                   #~(begin
+                       (use-modules (guix build utils))
+
+                       (let ((modules (string-append
+                                       #$output "/share/guile/site/2.0")))
+                         (mkdir-p modules)
+                         (call-with-output-file (string-append modules
+                                                               "/sqlite3.scm")
+                           (lambda (port)
+                             (display "(define-module (sqlite3))\n" port))))))
+                 #:guile %bootstrap-guile))
 
 (define* (self-contained-tarball name profile
                                  #:key target
@@ -134,7 +151,7 @@ added to the pack."
                                   (guix build store-copy)
                                   (gnu build install))
                                 #:select? not-config?))
-      (with-extensions guile-sqlite3&co
+      (with-extensions (guile-sqlite3&co)
         #~(begin
             (use-modules (guix build utils)
                          ((guix build union) #:select (relative-file-name))
@@ -267,7 +284,7 @@ added to the pack."
                                   (guix build store-copy)
                                   (gnu build install))
                                 #:select? not-config?))
-      (with-extensions guile-sqlite3&co
+      (with-extensions (guile-sqlite3&co)
         #~(begin
             (use-modules (guix build utils)
                          (gnu build install)
@@ -717,6 +734,9 @@ Create a bundle of PACKAGE.\n"))
       (set-build-options-from-command-line store opts)
 
       (parameterize ((%graft? (assoc-ref opts 'graft?))
+                     (guile-sqlite3&co (if (assoc-ref opts 'bootstrap?)
+                                           (list guile-sqlite3/mock)
+                                           (guile-sqlite3&co)))
                      (%guile-for-build (package-derivation
                                         store
                                         (if (assoc-ref opts 'bootstrap?)

Chris Marusich wrote 7 years ago

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . bug-guix@gnu.org)

Message-ID:87r2k1ul77.fsf@gmail.com

ludo@gnu.org (Ludovic Courtès) writes:

Toggle quote (32 lines)> While preparing the 0.15 release I realized that ‘guix pack --bootstrap’
> had become ineffective.  Concretely, ‘tests/guix-pack.sh’ would attempt
> to build the world.
>
> This is a consequence of c45477d2a1a651485feede20fe0f3d15aec48b39w, which
> introduced a dependency on guile-sqlite3 in derivations that build
> packs, even if they don’t actually produce a ‘db.sqlite’ file in there.
>
> I started looking for solutions, which led me to the patch below.
> That’s quite intrusive and it doesn’t work because then we have a
> similar issue with (guix hash) trying to dlopen libgcrypt.
>
> [...]
>
> diff --git a/guix/scripts/pack.scm b/guix/scripts/pack.scm
> index 6d5d745bc..45eeb2e7b 100644
> --- a/guix/scripts/pack.scm
> +++ b/guix/scripts/pack.scm
>
> [...]
> 
> @@ -717,6 +734,9 @@ Create a bundle of PACKAGE.\n"))
>        (set-build-options-from-command-line store opts)
>  
>        (parameterize ((%graft? (assoc-ref opts 'graft?))
> +                     (guile-sqlite3&co (if (assoc-ref opts 'bootstrap?)
> +                                           (list guile-sqlite3/mock)
> +                                           (guile-sqlite3&co)))
>                       (%guile-for-build (package-derivation
>                                          store
>                                          (if (assoc-ref opts 'bootstrap?)

You mentioned that you felt this change is "quite intrusive." How is

this more intrusive than using parameters for %graft? and

%guile-for-build to control how things get built? At first blush, this

seems no worse than what we already doing here.

You also mentioned that the proposed patch doesn't actually work, since

we run into a similar problem with (guix hash) trying to dlopen

libgcrypt. I applied your patch and ran the test, and I think I see

what you mean. Somehow (I have to admit, I'm not sure how), the normal

libgcrypt gets pulled in by the derivation, so we have to build it...

Although I suppose it isn't ideal, maybe we can get away with working

around it via similar parameterize tricks?

If the intent of --bootstrap is to enable the tool to run quickly in

tests, then it seems we MUST either find a way to substitute every heavy

dependency (including libgcrypt) with a light replacement (e.g. the

guile-sqlite3/mock you made), or find another way to short-circuit the

build.

Basically, we're manually doing dependency injection here depending on

whether or not --bootstrap was given, right? Instead of parameterizing

the dependencies, what if we used a dependency injector (or "oracle", or

"container", or whatever you want to call it) that, when invoked, would

give us the dependency that is appropriate for the current context?

Perhaps we could control the context via a single parameter. For

example, something like this:

(parameterize ((test-environment? #t))

(injector-get-dependency guile))

would return the bootstrap guile, but something like this:

(parameterize ((test-environment? #f))

(injector-get-dependency guile))

would return the usual guile. This isn't much different from using

parameters like we're already doing, except that it might save us from

having to remember multiple parameters, and it might make the code

cleaner by hiding the dependency selection/construction logic behind the

injector abstraction.

What do you think of that idea?

Toggle quote (4 lines)

> I’m not sure how to best address it. Another approach would be to do

> away with ‘--bootstrap’ and instead write those tests as “system tests”

> in a VM, though that’s maybe less satisfactory.

If we ran these tests in a VM as "system tests" (instead of turning
guile-sqlite3&co into a parameter), would we still need to build the
world?

-- 
Chris

Ludovic Courtès wrote 7 years ago

Recipients:(name . Chris Marusich)(address . cmmarusich@gmail.com)(address . bug-guix@gnu.org)

Message-ID:87r2jqyvol.fsf@gnu.org

Hi Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

Toggle quote (24 lines)> Basically, we're manually doing dependency injection here depending on
> whether or not --bootstrap was given, right?  Instead of parameterizing
> the dependencies, what if we used a dependency injector (or "oracle", or
> "container", or whatever you want to call it) that, when invoked, would
> give us the dependency that is appropriate for the current context?
> Perhaps we could control the context via a single parameter.  For
> example, something like this:
>
>     (parameterize ((test-environment? #t))
>       (injector-get-dependency guile))
>
> would return the bootstrap guile, but something like this:
>
>     (parameterize ((test-environment? #f))
>       (injector-get-dependency guile))
>
> would return the usual guile.  This isn't much different from using
> parameters like we're already doing, except that it might save us from
> having to remember multiple parameters, and it might make the code
> cleaner by hiding the dependency selection/construction logic behind the
> injector abstraction.
>
> What do you think of that idea?

Yes, that makes sense.  Or we could simply have something like:

  (define (lookup-package name)
    (if (test-environment?)
        (match name
          ("guile" …)
          …)
        (match name
          …)))

Another option that comes to mind is using the little-known
‘map-derivation’ procedure: we would compute the ‘guix pack’ derivation
as usual, but at the end we’d call ‘map-derivation’ to replace
libgcrypt.drv, guile-sqlite3.drv, etc. with dummy variants.  That might
turn out to be more complicated that the above solution, though.

Ludo’.

Ludovic Courtès wrote 6 years ago

Re: bug#32184: ‘guix pack --bootstr ap’ is ineffective

Recipients:(name . Chris Marusich)(address . cmmarusich@gmail.com)(address . 32184@debbugs.gnu.org)

Message-ID:87va5xevkt.fsf@gnu.org

Hi Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

Toggle quote (10 lines)> ludo@gnu.org (Ludovic Courtès) writes:
>
>> While preparing the 0.15 release I realized that ‘guix pack --bootstrap’
>> had become ineffective.  Concretely, ‘tests/guix-pack.sh’ would attempt
>> to build the world.
>>
>> This is a consequence of c45477d2a1a651485feede20fe0f3d15aec48b39w, which
>> introduced a dependency on guile-sqlite3 in derivations that build
>> packs, even if they don’t actually produce a ‘db.sqlite’ file in there.

[...]

Toggle quote (24 lines)> Basically, we're manually doing dependency injection here depending on
> whether or not --bootstrap was given, right?  Instead of parameterizing
> the dependencies, what if we used a dependency injector (or "oracle", or
> "container", or whatever you want to call it) that, when invoked, would
> give us the dependency that is appropriate for the current context?
> Perhaps we could control the context via a single parameter.  For
> example, something like this:
>
>     (parameterize ((test-environment? #t))
>       (injector-get-dependency guile))
>
> would return the bootstrap guile, but something like this:
>
>     (parameterize ((test-environment? #f))
>       (injector-get-dependency guile))
>
> would return the usual guile.  This isn't much different from using
> parameters like we're already doing, except that it might save us from
> having to remember multiple parameters, and it might make the code
> cleaner by hiding the dependency selection/construction logic behind the
> injector abstraction.
>
> What do you think of that idea?

It was sad to see these tests were not running so I decided to bite the

bullet. :-)

In commit 19c924af4f3726688ca155a905ebf1cb9acdfca2, I went for a

different solution: now these ‘guix pack’ tests run using the user’s

store (normally /gnu/store) rather than the test store

($builddir/test-tmp), when possible.

This allows us to run these tests and possibly write other more complex

tests.

WDYT?

At this point I think we can remove ‘--bootstrap’ from ‘guix pack’

altogether.

Thoughts?

Thanks,

Ludo’.

Your comment

Commenting via the web interface is currently disabled.

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

To respond to this issue using the mumi CLI, first switch to it

mumi current 32184

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

You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags

mumi command -t +confirmed -t +easy

Or, remove the moreinfo tag and set the help tag

mumi command -t -moreinfo -t +help

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date