all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Chris Marusich <cmmarusich@gmail.com>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: 30572@debbugs.gnu.org
Subject: [bug#30572] [PATCH 6/7] system: Add "guix system docker-image" command.
Date: Wed, 21 Mar 2018 04:58:35 +0100	[thread overview]
Message-ID: <87370u6pw4.fsf@gmail.com> (raw)
In-Reply-To: <877eqal62w.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Sat, 17 Mar 2018 22:56:07 +0100")

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

ludo@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2018-03-21  3:59 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-22 10:29 [bug#30572] [PATCH 0/7] Add "guix system docker-image" command Chris Marusich
2018-03-15  4:09 ` [bug#30572] [PATCH 0/7] Add "guix system docker-image" command (v2) Chris Marusich
2018-03-15  4:09   ` [bug#30572] [PATCH 1/7] gnu: bootstrap: Add trivial packages for bash, mkdir, tar, and xz Chris Marusich
2018-03-16 22:16     ` Danny Milosavljevic
2018-03-20  3:13       ` Chris Marusich
2018-03-20 10:09         ` Danny Milosavljevic
2018-03-21  4:19           ` Chris Marusich
2018-03-21  9:17             ` Danny Milosavljevic
2018-03-17 21:58     ` Ludovic Courtès
2018-03-21  4:22       ` Chris Marusich
2018-03-21 20:54         ` Ludovic Courtès
2018-03-22  4:37           ` Chris Marusich
2018-03-15  4:09   ` [bug#30572] [PATCH 2/7] tests: Add tests for "guix pack" Chris Marusich
2018-03-16 21:07     ` Danny Milosavljevic
2018-03-17 18:23       ` Ludovic Courtès
2018-03-21  4:00         ` Chris Marusich
2018-03-21  4:28           ` Chris Marusich
2018-03-22  4:41             ` Chris Marusich
2018-03-22  9:22               ` Ludovic Courtès
2018-03-24  2:05                 ` bug#30572: " Chris Marusich
2018-03-24 17:15                   ` [bug#30572] " Ludovic Courtès
2018-03-15  4:09   ` [bug#30572] [PATCH 3/7] vm: Allow control of deduplication in root-partition-initializer Chris Marusich
2018-03-16 20:47     ` Danny Milosavljevic
2018-03-17 18:21     ` Ludovic Courtès
2018-03-15  4:09   ` [bug#30572] [PATCH 4/7] gnu: When building in a VM, share a temporary directory Chris Marusich
2018-03-16 22:00     ` Danny Milosavljevic
2018-03-20  3:20       ` Chris Marusich
2018-03-15  4:09   ` [bug#30572] [PATCH 5/7] guix: Rewrite build-docker-image to allow more paths Chris Marusich
2018-03-16 22:29     ` Danny Milosavljevic
2018-03-20  3:26       ` Chris Marusich
2018-03-15  4:09   ` [bug#30572] [PATCH 6/7] system: Add "guix system docker-image" command Chris Marusich
2018-03-16 22:11     ` Danny Milosavljevic
2018-03-17 21:56     ` Ludovic Courtès
2018-03-21  3:58       ` Chris Marusich [this message]
2018-03-21  4:25         ` Chris Marusich
2018-03-21 20:50         ` Ludovic Courtès
2018-03-15  4:09   ` [bug#30572] [PATCH 7/7] tests: Add tests for "guix system disk-image" et al Chris Marusich
2018-03-16 22:04     ` Danny Milosavljevic
     [not found] <handler.30572.B.151929540925748.ack@debbugs.gnu.org>
2018-02-22 10:35 ` [bug#30572] [PATCH 1/7] tests: Add tests for "guix pack" Chris Marusich
2018-02-22 10:35   ` [bug#30572] [PATCH 6/7] system: Add "guix system docker-image" command Chris Marusich
2018-02-26 16:30     ` Chris Marusich
2018-02-27 17:17       ` Ludovic Courtès
2018-03-03  7:31         ` Chris Marusich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87370u6pw4.fsf@gmail.com \
    --to=cmmarusich@gmail.com \
    --cc=30572@debbugs.gnu.org \
    --cc=ludo@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.