From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: Patches to implement system roll-back and switch-generation Date: Tue, 11 Oct 2016 23:05:10 +0200 Message-ID: <87pon6sik9.fsf@gnu.org> References: <87a8ep6h6g.fsf@gmail.com> <8737kcbfhc.fsf@gnu.org> <87lgxy11py.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:54415) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bu4Ep-0003RC-UA for guix-devel@gnu.org; Tue, 11 Oct 2016 17:05:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bu4El-0004pt-OF for guix-devel@gnu.org; Tue, 11 Oct 2016 17:05:26 -0400 In-Reply-To: <87lgxy11py.fsf@gmail.com> (Chris Marusich's message of "Sat, 08 Oct 2016 23:22:01 -0700") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Chris Marusich Cc: guix-devel@gnu.org Hello, Chris Marusich skribis: > ludo@gnu.org (Ludovic Court=C3=A8s) writes: [...] >> Sorry about that! Hopefully we can work around the conflicts. > > I think we can. But I think it will require backwards incompatible > changes to the boot parameters file. Here's why: > > Many of the existing procedures in (gnu system grub) take a "file > system" object as input (e.g. the 'grub-configuration-file' procedure). > However, the boot parameters file does not currently contain all the > information that a "file system" object contains. Good point. This =E2=80=98store-fs=E2=80=99 argument was added in response= to . > Here's an example of what it contains today: > > (boot-parameters > (version 0) > (label "GNU with Linux-Libre 4.1.20 (beta)") > (root-device "root") > (kernel > "/gnu/store/zygby8db0adcyj3m6rjflr80jarfy9b5-linux-libre-4.1.20") > (kernel-arguments ()) > (initrd > (string-append > "/gnu/store/hlra3a0g3a14bjvdn3vbagwfvy4nmhn8-base-initrd" > "/initrd"))) > > To avoid backwards-incompatible changes to the structure of the boot > parameters file, I had originally planned to refactor the procedures in > (gnu system grub) so that I could use them with the limited information > that is contained in the version 0 boot parameters file. However, > commit 0f65f54e has modified these procedures in a way that makes it > very awkward to refactor the "file system" object out of them. Now, to > re-use the existing procedures, I believe I will need to add this > missing information (i.e., the information contained in a file system > object) to the boot parameters file, so that I can construct a "file > system" object to pass to those procedures. Does that sound right to > you? Yes, I think so. More precisely, I think we need to add a =E2=80=98device=E2=80=99 field to = , which could be the UUID or label of the device where the kernel and initrd are to be found, or #f, in which case grub.cfg would contain a =E2=80=9Csearch --file=E2=80=9D command (instead of =E2=80=9Csearch --label= =E2=80=9D or =E2=80=9Csearch --fs-uuid=E2=80=9D). Correspondingly, we=E2=80=99d add a =E2=80=98device=E2=80=99 (or =E2=80=98b= oot-device=E2=80=99?) field to . The key is that =E2=80=98device=E2=80=99 can be differe= nt from =E2=80=98root-device=E2=80=99 because the store and root devices can be dif= ferent from one another. That way we could remove the =E2=80=98store-fs=E2=80=99 parameter of =E2=80=98grub-configuration-file=E2=80=99 since that information would now = be contained in each . > If I do that, then it will probably be a backwards-incompatible change, > so I will do it in the following way. I will simply store an entire > "file system" object in the boot parameters file. I will bump the > version of the boot parameters file from 0 to 1. To ensure that all new > system generations use version 1, I will update commands like > "reconfigure" to always create a version 1 boot parameters file. I will > make the new commands (roll-back and switch-generation) refuse to switch > to any system generation which uses version 0 (because it isn't possible > to construct a complete "file system" object from a version 0 boot > parameters file). I will also update existing commands like > 'list-generations' so that they will gracefully handle both versions. > > Does this sound like the right approach to you? I think we don=E2=80=99t need to bump the version number: =E2=80=98read-boo= t-parameters=E2=80=99 can simply do what it currently does for =E2=80=98initrd=E2=80=99 and =E2=80=98kernel-arguments=E2=80=99, which is to provide a default value whe= n they=E2=80=99re missing. Here the default value could be =E2=80=98root-device=E2=80=99. > I've tried using 'git send-email' on GuixSD before, and it didn't work > for me (because a mail transfer agent is not running on my GuixSD > system). When the new patches are ready, I'll try once more to get it > working. AFAICT an MTA is not needed. >>> - "Return the GRUB configuration file corresponding to CONFIG, a >>> - object, and where the store is available at STORE= -FS, a >>> - object. OLD-ENTRIES is taken to be a list of menu entri= es >>> -corresponding to old generations of the system." >>> + "Return a derivation which builds the GRUB configuration file corres= ponding >>> +to CONFIG, a object, and where the store is avail= able at >>> +STORE-FS, a object. OLD-ENTRIES is taken to be a list o= f menu >>> +entries corresponding to old generations of the system." >> >> OK, although I often write =E2=80=9CReturn something=E2=80=9D when that = really means >> =E2=80=9CReturn a derivation that builds something=E2=80=9D. > > Upon closer inspection, it looks like this procedure, > 'grub-configuration-file', actually returns a monadic value (in the > store monad), which "contains" a derivation, which in turn builds the > grub configuration file. Even in a case like this, where there is so > much indirection, is it appropriate to elide all those details? > > If this is the style we should consistently use in our documentation, > then that's fine, and I will happily follow suit in the name of > consistency. However, as a newcomer to this code base, to gexps, to > derivations, and to monads, in the beginning I was very confused about > how to use this procedure's return value. > > If I can think of a good way to make stuff like this more obvious for > newcomers, I'll let you know. For now, though, I think the best thing > to do is to change my patches to conform to the existing style. I think so. :-) That said, I can understand that the indirections can be confusing, esp. since these parts are not properly documented. That =E2=80=9Creturn a file=E2=80=9D really means =E2=80=9Creturn a derivation as a monadic value= =E2=80=9D is non obvious. We can now avoid monadic procedures by using the declarative counterpart of the monadic API. That is, we could write: (define (grub-configuration-file =E2=80=A6) ;normal proc (computed-file "grub.cfg" builder)) instead of: (define (grub-configuration-file =E2=80=A6) ;monadic proc (gexp->derivation "grub.cfg" builder)) I would welcome such changes. > Thank you for your patience. I'll send the updated patches when they're > ready. Awesome, thanks! Ludo=E2=80=99.