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

Previous Next

Full log

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

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

[Message part 1 (text/plain, inline)]

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, 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?

>> +              (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.

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

-- 
Chris

[signature.asc (application/pgp-signature, inline)]

Previous Next

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