GNU bug report logs - #30572
[PATCH 0/7] Add "guix system docker-image" command

Previous Next

Full log

View this message in rfc822 format

From: ludo <at> gnu.org (Ludovic Courtès)
To: Chris Marusich <cmmarusich <at> gmail.com>
Cc: Danny Milosavljevic <dannym <at> scratchpost.org>, 30572 <at> debbugs.gnu.org
Subject: [bug#30572] [PATCH 6/7] system: Add "guix system docker-image" command.
Date: Wed, 21 Mar 2018 21:50:00 +0100

Hi Chris,

Chris Marusich <cmmarusich <at> gmail.com> skribis:

> ludo <at> gnu.org (Ludovic Courtès) writes:
>
>>> +  (define json
>>> +    ;; Pick the guile-json package that corresponds to the Guile used to build
>>> +    ;; derivations.
>>> +    (if (string-prefix? "2.0" (package-version (default-guile)))
>>> +        guile2.0-json
>>> +        guile-json))
>>
>> I think we can use ‘guile-json’ unconditionally here.
>
> Good point.  Nobody using this new code will be using Guile 2.0,

Specifically, we know that derivations are built with ‘guile-final’,
which is now 2.2.

> so we can just use guile-json unconditionally.  Does that mean we can
> also clean up the same conditional statement from other places in Guix
> code now?

Yes, indeed!

>>> +              (mkdir root-directory)
>>> +              (initialize root-directory)
>>> +              (build-docker-image
>>> +               (string-append "/xchg/" #$name) ;; The output file.
>>> +               (cons* root-directory
>>> +                      (call-with-input-file (string-append "/xchg/" #$graph)
>>> +                        read-reference-graph))
>>> +               #$os-drv
>>> +               #:compressor '(#+(file-append gzip "/bin/gzip") "-9n")
>>> +               #:creation-time (make-time time-utc 0 1)
>>> +               #:transformations `((,root-directory -> "")))))))
>>
>> Am I right that the whole point of passing several file names to
>> ‘build-docker-image’ is that here we don’t need to copy the whole store
>> to ‘root-directory’, right?
>
> The primary reason why I made this change was because it was the
> simplest way I could find to re-use the existing code.  The fact that we
> copy less is a nice secondary effect, but it is not the primary reason
> why I structured it this way.  There might be a simpler way to
> accomplish this, but this way works, which I think is a good start.  I
> would like to commit this as-is for now.  If we can figure out a simpler
> way to implement the same logic, I'd be all for it, but it seems tricky.
>
> The original role of the "PATH" argument was surprising (for example, it
> was not actually used for adding any paths to the final Docker image!).
> In addition, the original code assumed that all the paths to add (loaded
> from the "CLOSURE" graph file argument - not from PATH!) are store
> paths, which is not true when creating a GuixSD Docker image (unless we
> try to copy everything created by root-partition-initializer back into
> the store).  So, some of the paths I need to add are store paths, and
> some of them are paths to special files outside the store, like device
> files.  Maybe we can copy all of this (including device files and socket
> files) into a single directory in the store (or outside of the store).
> I don't know.  If it's possible, I agree it would be a nice improvement.
> But I tried various methods, and my latest patch was the simplest method
> I found that worked.  I would definitely be happy if we could simplify
> it even more, but I also want to move forward with this patch series.

I see, thanks for taking the time to explain!

I’m always wary about adding too many options in an interface, but like
you wrote, it may be unavoidable here.

> Is it OK to commit this as-is (with just the guile-json change you
> suggested above)?

Yes, sure; sorry for the extra delay.  We can always adjust later if we
have a better idea.

Thanks a lot for working on this!

Ludo’.

Previous Next

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