GNU bug report logs - #40955
[PATCH 0/5] Add new image API.

Previous Next

Full log

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 40955 <at> debbugs.gnu.org
Subject: Re: [bug#40955] [PATCH 4/5] image: Add a new API.
Date: Tue, 05 May 2020 15:52:30 +0200

> Would it make sense to separate #:owner-uid and #:owner-gid?

Yes, fixed.

>
> It does mean that we can only create images where all the files have the
> same UID/GID.
>
> Looking at (gnu build install), there’s one case where it might be
> problematic: the store’s GID is supposed to match the ‘guixbuilder’
> group.  But the good news is that the daemon does this:
>
>     if (chown(chrootStoreDir.c_str(), 0, buildUser.getGID()) == -1)
>         throw SysError(format("cannot change ownership of ‘%1%’") % chrootStoreDir);
>
> So we can just remove the UID/GID from the directives that are in (gnu
> build install).

Done in a separate commit.

>
>> +(define* (genimage config target)
>> +  "Use genimage to generate in TARGET directory, the image described in the
>> +given CONFIG file."
>> +  ;; genimage needs a 'root' directory.
>> +  (mkdir "root")
>> +  (invoke "genimage" "--config" config
>> +          "--outputpath" target))
>
> I had missed that bit, so we still need genimage in the end?

genimage is used to assemble the disk-image together. It's a matter of
doing some 'dd' with the right offset. Once this part is implemented in
(gnu build image), we can get rid of genimage.

>> +(define (register-bootcfg-root target bootcfg)
>> +  "On file system TARGET, register BOOTCFG as a GC root."
>> +  (let ((directory (string-append target "/var/guix/gcroots")))
>> +    (mkdir-p directory)
>> +    (symlink bootcfg (string-append directory "/bootcfg"))))
>
> Maybe just ‘register-gc-root’?

Turns out, I don't think this is useful anymore, so I removed it.

>
>> +(define* (register-closure prefix closure
>> +                           #:key
>> +                           (deduplicate? #t) (reset-timestamps? #t)
>> +                           (schema (sql-schema)))
>> +  "Register CLOSURE in PREFIX, where PREFIX is the directory name of the
>> +target store and CLOSURE is the name of a file containing a reference graph as
>> +produced by #:references-graphs..  As a side effect, if RESET-TIMESTAMPS? is
>> +true, reset timestamps on store files and, if DEDUPLICATE? is true,
>> +deduplicates files common to CLOSURE and the rest of PREFIX."
>> +  (let ((items (call-with-input-file closure read-reference-graph)))
>> +    (register-items items
>> +                    #:prefix prefix
>> +                    #:deduplicate? deduplicate?
>> +                    #:reset-timestamps? reset-timestamps?
>> +                    #:registration-time %epoch
>> +                    #:schema schema)))
>
> This is duplicated from (guix build vm).  Should we instead factorize it
> in (guix build store-copy)?

I tried it. The problem is that it introduces a dependency to
guile-sqlite3, so a bunch of with-extensions needs to be added in
various places. I kept the copy, as (gnu build vm) will be
deprecated/removed anyways.

> I’d suggest either ‘srfi-1:’ as the prefix or, better, hide whichever
> binding is causing a name clash.

I need partition from srfi-1 and partition from (gnu image). That's not
great :( I opted for the srfi-1 prefix as you suggested.

> Can we use ‘computed-file’ as well instead of ‘gexp->derivation’?  That
> way, the API is entirely non-monadic and hopefully easier to use.

Ok, so I used computed-file as suggested. However, the "system-image"
procedure is calling the monadic system-disk-image-in-vm. So I had to
keep this one monadic.

The good news is once, system-disk-image-in-vm is no longer needed, this
whole file will be entirely non-monadic.

> That’s all!

Thanks a lot for going though all of this.

Mathieu

Previous Next

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