From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Marusich Subject: Re: [PATCH 01/10] * gnu/system.scm (): Add 'store-device' and 'store-fs-mount-point'. Date: Sun, 30 Oct 2016 02:41:39 -0700 Message-ID: <87lgx6fa24.fsf@gmail.com> References: <20161028100727.1182-1-cmmarusich@gmail.com> <20161028100727.1182-2-cmmarusich@gmail.com> <87h97uu23b.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:57223) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c0mce-0002sk-SK for guix-devel@gnu.org; Sun, 30 Oct 2016 05:41:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c0mcd-0001f7-Lv for guix-devel@gnu.org; Sun, 30 Oct 2016 05:41:48 -0400 In-Reply-To: <87h97uu23b.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Sun, 30 Oct 2016 02:12:24 +0200") 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: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: guix-devel@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable ludo@gnu.org (Ludovic Court=C3=A8s) writes: > General remark on commit logs: I can fix details there, but it would > help me if you could more closely follow conventions. Understood. I read the manual ((standards) Change Logs) and tried to make it look like existing commits, but I see I missed some stuff, so I'll do better next time. Thank you for pointing out the spots that need improvement. >> (define-record-type* >> boot-parameters make-boot-parameters boot-parameters? >> (label boot-parameters-label) >> + ;; Because we will use the 'store-device' to create the GRUB search c= ommand, >> + ;; the 'store-device' has slightly different semantics than 'root-dev= ice'. >> + ;; The 'store-device' can be a file system uuid, a file system label,= or #f, >> + ;; but it cannot be a device path such as "/dev/sda3", since GRUB wou= ld not >> + ;; understand that. The 'root-device', on the other hand, corresponds >> + ;; exactly to the device field of the object representi= ng the >> + ;; OS's root file system, so it might be a device path like "/dev/sda= 3". >> (root-device boot-parameters-root-device) >> + (store-device boot-parameters-store-device) >> + (store-fs-mount-point boot-parameters-store-fs-mount-point) > > In your patch series =E2=80=98store-fs-mount-point=E2=80=99 ends up being= unused. Is this true? I think we're using it. For example, in commit 18813be0053b4797071e20f373a8a9e8d669e32a (Implement switch-generation and roll-back), in procedure 'reinstall-grub', don't we use it to get the mount point for each ? > However, after rereading the whole series, I think what we need is a > =E2=80=98device-mount-point=E2=80=99 in , in a symmetrical fa= shion. That would work, and I'm open to it. However, before we decide to do that, what do you think about the alternative in which we simply require that all the paths in the and objects must be relative paths? By relative, I mean relative to the GRUB root. If all the paths were relative, then we would not need a 'device-mount-point' field in the first place. However, I suspect it might be tricky to get all the information we need if we do it that way. For example, during switch-generation, how would we know the correct paths to use for the files in the GRUB configuration's 'eye-candy' section? In light of that, I suspect it would be better to do what you've suggested: include the mount point in both and . > Attached is an updated patch. I have grouped together the patches of > your series that touch this topic, including the documentation part > (this is all one logical change so it=E2=80=99s best to commit it as such= ), and > made the above change. Thank you for doing this. I've looked it over. It looks good to me. Unless you think strongly that we should use relative paths in the and objects, I think it's good as-is! > I=E2=80=99ve also followed my suggestion at > to > not change the version of the sexp. OK. FYI, I bumped the version number in my patch series because I thought it would simplify the matching logic (it was still backwards compatible). However, looking at your version, I see now that it wasn't too hard to keep the version number the same. Your version looks good to me! > I=E2=80=99ve tested that the generated grub.cfg is what we expect in seve= ral > different scenario. Thank you for taking the extra time to test it. > If that=E2=80=99s fine with you, I=E2=80=99d like to commit this version.= With that > done, the rest of the patch series will be rather easy. > > WDYT? > Sounds good to me. Unless you think strongly that we should use relative paths in the and objects, I will re-submit my patches once you have committed this change to the official repo. > Thank you! Thank you for all of your help and tips! I really appreciate it, and it's all been very helpful. =2D-=20 Chris --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYFcBVAAoJEN1AmhXYIkadpP8QAInMevmqVOGMZkwD06nLRdhq G2uDF7jGVZc8+XyUfaRl46rSUgevfnscyKTcxKa5RQQuFmrYVHiGuGPdd3x9ECsY zJcBByBzgusVZm0EiBjnTX9+9sHiQWCIy/LoYn7GbNATmqOR3F8oWjdDeRV4rX+L zg2whihRN19Iqxuxw0wtF01HtUqRYIqpPxCJwRCxb6XPeHOjCnbWghd+ckolhKNF V27jF0H6OdBWyXHlOqXfRSoUBi+ftaK4HDHhW4SsVSxKFyLaNqrDwGcIwRrAtS8z R+X1VTxUMvnOMbc1f1sdUg639YmjbGIDg2idA+W/FHg1gCvVSa3W9vVwgj99KMJr MonLiNse0sS79hXAGq6CeYv9eo0iMoT2G42BfvqlbIFmr2pNDAUmq7sir9PKF4/k G6ywiQXaxZQ4Rl9TjQ+i0Gn864RsAsqcQnP5+zQga4RXhC6C2OXcjOf/OfEJZMmg Lc9sBbzM5GbPIYkDwdkY99mWor4n4g5XWttfxtsNeAlB/S9TWnCKY7EA5L9MKb7K TrasQbAtX85banG+8uwTRQKJBadSet8cZ8gV5Zw8sCWhixCVxp3eJOxy/WILncpy xfACmgFXvYe4lYcarJl2J3tvsbkhQaLNolN/l8TW3YR+PfrAZlExqQcQfNUDzCIV CTlKbBlsTgzfHvJ9hELT =yI/6 -----END PGP SIGNATURE----- --=-=-=--