From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp10.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id 4DbDAkiuFGNmNgEAbAwnHQ (envelope-from ) for ; Sun, 04 Sep 2022 15:55:20 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp10.migadu.com with LMTPS id 2NffAUiuFGOOgwAAG6o9tA (envelope-from ) for ; Sun, 04 Sep 2022 15:55:20 +0200 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id D6A6B2CFD7 for ; Sun, 4 Sep 2022 15:55:18 +0200 (CEST) Received: from localhost ([::1]:46020 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oUq5l-0004Q2-VP for larch@yhetil.org; Sun, 04 Sep 2022 09:55:17 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:47498) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oUq5X-0004Lh-0L for guix-patches@gnu.org; Sun, 04 Sep 2022 09:55:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:55003) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oUq5W-0000YR-NH for guix-patches@gnu.org; Sun, 04 Sep 2022 09:55:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oUq5W-0001Jx-C9 for guix-patches@gnu.org; Sun, 04 Sep 2022 09:55:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#57496] [PATCH 1/2] gnu: bootloader: Extend `' for chain-loader. Resent-From: tiantian Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 04 Sep 2022 13:55:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57496 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Julien Lepiller Cc: 57496@debbugs.gnu.org Received: via spool by 57496-submit@debbugs.gnu.org id=B57496.16622996955055 (code B ref 57496); Sun, 04 Sep 2022 13:55:02 +0000 Received: (at 57496) by debbugs.gnu.org; 4 Sep 2022 13:54:55 +0000 Received: from localhost ([127.0.0.1]:43702 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oUq5O-0001JS-54 for submit@debbugs.gnu.org; Sun, 04 Sep 2022 09:54:54 -0400 Received: from out162-62-57-252.mail.qq.com ([162.62.57.252]:50225) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oUq5I-0001JB-Rd for 57496@debbugs.gnu.org; Sun, 04 Sep 2022 09:54:53 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foxmail.com; s=s201512; t=1662299678; bh=88/yggXcZz+RNFqFNJMyX1P8baGaLu31VwuE1RK9k4Q=; h=References:From:To:Cc:Subject:Date:In-reply-to; b=Y+RgVOwHlUGzaWzG70W51PsBnsZ79CypxDn9Ad2Rd49M58+H57rlQF8ufUBnIoCTx 5x+7o6dz7TO5IQc01VYnxP6pnkjgXlwZfjkbt1fUM8Jwhga3N2Pr4iqRUANYXCEIJY tWCvG7BIA4pSdSLNxw/OQ2lSL5L88BoZyJa1mZDo= Received: from guix-pc ([240e:398:28a4:3121:5ae1:85dd:ab7b:103f]) by newxmesmtplogicsvrszc13.qq.com (NewEsmtp) with SMTP id DA4B486C; Sun, 04 Sep 2022 21:54:36 +0800 X-QQ-mid: xmsmtpt1662299676tpufax31z Message-ID: X-QQ-XMAILINFO: OLnGMPzD2sDVLbiq8ukbNSjpy4lgCFRKS+4Axa6u/SjS6mS6U4P/ED++eRxkXg zp8OBlrCh7r0e6j2wZXUqmpGhFnh/zbpv/md99eStKeY+4qLYZMACEV0zolZGSI1M0/89iK9tX6p kXHl51S9FPyAM7g932jHZIfPma8wdXdQXEpieVSbWj4GjtGaE1XyCtMo4QDTk02thTBCVARMklwL 19SKjzJezfnHJvDqDI1FbpAGCtcCoMyx8KiiNmGonShmk5jx/rc1fe3dQbjZT6pAdQ0B3zY7iVH2 uxN4VphilkaYR0HwvqDDI3uC84YETwPQM3+sDEHjgIquq1ncTO2cTIKjRxhT0Rp7PfAEnbeFfHMk eAdl4FGO0OLYFoDjhnBtcbK5TshWgWEnsKPog7vaevXSnC1nNLrYIdx2G7s8iRsm+zdJ5o1/pnqq KA2Igfh6EiYMEK2qKKzt3M9vu5OMkJX/jAmiSyAmdXOFx3V+R7r9K3bFII2hfeq6Z9AbcQOF9/3b 62x+QPMSuuGKRjZQhClWUXeOatqByHO2Q0t0mpc6sYXXDj+T+YvndmEmPCiGnPN1WmbhWNBvyhI8 Idbc7d/nWB2QCq2KgTQVIybwhDF3U8+I8F89OWPe+IzplQl4H9383p9YgQumFVFppc/2o/h8PXgv 9liMiR1Ob88rVUUk+2tYIgc7/M6bSuQxSfiTcrN1UOePMcfDD7wO1u4n1bIRczQ9Gl++IhuGUXci eXvkdDZPBrURNx8gLOu4SAyRGzOVK4cB5KIbzM0wvSbQKsHCQLLbxHQC5R/t7IXju2KH9r/dOTSD qCnIfBmHDIEY300N+Pv8UQPI9jO4NsHRezaR0N/exjBSWPouFrScV6J4dTFg3wPZ6Gn7sqzJ6NAR CCsG9trH9m+NMn1f0gYWV/draFIeCvwsUYg4LZ9PQjHE/yeb/Nv4pd3lTJs52GmZyZLgLEEvm8sb kAcKviXYHs/Afz6pULRugiEL+edP4BLSR0VqUHkSA= References: <55016216-84d9-e2e6-8bf5-0efdfa0e6ac1@foxmail.com> <20220831213406.3ec92474@sybil.lepiller.eu> <7xy1v37ny8.fsf@foxmail.com> <20220903220835.52d60d1d@sybil.lepiller.eu> User-agent: mu4e 1.8.9; emacs 28.1 From: tiantian Date: Sun, 04 Sep 2022 21:09:35 +0800 In-reply-to: <20220903220835.52d60d1d@sybil.lepiller.eu> Message-ID: <7xbkrv9rdv.fsf@foxmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN X-Migadu-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1662299719; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=88/yggXcZz+RNFqFNJMyX1P8baGaLu31VwuE1RK9k4Q=; b=GZuLJ13INuettKcLdnl2oI/ua6JjyFQQdYZk2vAyoWDIrDGkqJOXpvGOSDD2huI6ebRXkV YO5J/w9tzLxyErUPrcntOzCzhDzLvQQYKeJU0aLiZCAs6PwkTTVrIwTlromEa+J8yCNjC6 j0ZeiKr+Yh4JkIcy9GB0QeWrpEJ1qafhQYA3mhjRXTosrQrSvhGpkpLrndz9k1kOkSnDC9 f6MTnScMwvBWTUeVgCVboYFI4dL/9vZ5tooqBawmSjR++pHFhjOBxrMEm/PLSp4ioDo+i4 Ms9tlNBgmzgq5t8z9Ahb1AoeigTi5Oe/E3MBCcV9aU+F1IxTfmuwH3PnqjCQbA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1662299719; a=rsa-sha256; cv=none; b=PAvTcZenuUFhj8a29zWEvVYzzS6BXN6NWtmjS5O9DfnQaHnUj+i+31I5WVuDUgEfUGV//L rTBGFXxKJb8jBZuNqF/FVOWkLP9oTwKKrCaTtiP5WST8KBH4lTWfLhhu79AGSRYQIC723d dU6l4jj0txrNiLlRxeyOIbeDS5zbX6XtUJ+GxVBLXSFLbex7fRDbMa83NHTr2c5oYDXmYS 9A69qPbes1t4Vpfbwy9eBOOL88fmp6NKrqOFOYI1xcIeoDzGTEAi2daT4T/kjMaYSs2nla W/WP0JFsVTYaj4EvtNxtoer1fbtYCIUNKG4vzRgO9HQuXh2btnjtPlYCGsGt9g== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=foxmail.com header.s=s201512 header.b=Y+RgVOwH; dmarc=fail reason="SPF not aligned (relaxed)" header.from=foxmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: 9.72 X-Spam: Yes Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=foxmail.com header.s=s201512 header.b=Y+RgVOwH; dmarc=fail reason="SPF not aligned (relaxed)" header.from=foxmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: D6A6B2CFD7 X-Spam-Score: 9.72 X-Migadu-Spam: Yes X-Migadu-Scanner: scn0.migadu.com X-TUID: YzBxiA7e/hEw Hi Julien, Julien Lepiller writes: > Hi tiantian, > > Le Fri, 02 Sep 2022 09:04:15 +0800, > tiantian a =C3=A9crit : > >> Hi Lepiller, > > Please call me Julien, I'm more used to being called by my first name. > Ok. If I have offended you before, please forgive me. > > You're right, I made a mistake here, I meant the multiboot-kernel > field, like that second example. > OK, I'll write the second one in the document. > > I have no problem pushing the patches you have so far without any error > message, although I can still see one issue. In the grub manual I see > this example of chain-loading: > > menuentry "Windows" { > insmod chain > insmod ntfs > set root=3D(hd0,1) > chainloader +1 > } > > which I cannot reproduce. I've used "chainloader +1" (by writing my own > grub.cfg manually) with Haiku which is another free OS (though I think > it uses nonfree blobs for hardware), so I think we should support this > use case too, not just booting another EFI entry. When I=C2=A0set > chain-loader "+1", I=C2=A0get the following grub config: > > menuentry "haiku" {=20 > > chainloader +1 > } > > With no root. I think this is because of our grub-root-search > procedure, which doesn't work in that case. > > Do you have any ideas how to support that use-case? If it's too complex > I'm fine leaving it behind for now. It should not slow us down. > For the present `grub-root-search', I should not try to modify it for the time being. Because the device field may be associated with the boot-parameters, and modifying it may require more work. I have never used 'chainloader +1'. But if you just want to specify root device by the form like '(hd0,1)', you can try like this: (chain-loader "(hd0,1)+1") You may get some help from this article. (https://blog.cykerway.com/posts/2016/12/27/grub-chainloader-1.html) You may also get some help from "16.3.10 chainloader" and "13.3 How to specify block lists" in grub manual. I can't test these on my computer because I don't know how to use them. I hope these will help you. >>=20 >> There are many situations that I need to check. I can't find a case in >> menu-entry->sexp to solve it. So, I wrote a function alone. After I >> have preliminarily completed the function that checks menu-entry, I >> found that it seems that the explicit pattern is more readable than my >> individual function. >>=20 >> The procedure that check menu-entry will check that there is no boot, >> different fields of boot are mixed, and there are multiple boot >> modes. I haven't tested it yet. The expected effect is as follows. >>=20 >> --- start examples ---- >>=20 > > Those examples are nice :) > Thank you for your praise. >>=20 >> --- end examples --- >>=20 >> But the procedure lead more difficult to read and understand the >> code. Maybe it's because my code level is not enough. For the current >> code, I'm not sure if the error message is worth the performance it >> consumes and the code that becomes difficult to understand. The check >> of the procedure is not strong. It only checks whether some fields >> are set, but not whether the contents of the fields are correct. >>=20 >> And I think the most important point is that the procedure just check >> menu-entry. menu-entry->sexp still need to check linux, multiboot and >> chain-loader. if not check, An incorrect match will occur in >> menu-entry->sexp, and an error message that is not helpful may be >> reported by 'guix system'. >>=20 >> I think it might be good to use the menu-entry->sexp in v2 patch. >> Should I keep menu-entry->sexp of v2 in v3 patch, in addition to >> adding some necessary comments? > > Let's keep the code from v2. > I thought so before, but your code has brought me new inspiration. Perhaps, it will have some fine-tuning. > > Maybe it would be easier to have something like this: > > (define (check-menu-entry menu-entry) > (define all-linux-fields > (and (menu-entry-linux menu-entry) > (menu-entry-linux-arguments menu-entry) > (menu-entry-linux-modules menu-entry))) > (define all-multiboot-fields > ...) > (define all-chainload-fields > ...) > (define count-methods > (+ (if all-linux-fields 1 0) > (if all-multiboot-fields 1 0) > (if all-chainload-fields 1 0))) > > (define (report err) > (raise > (condition > (&message > (message > (format #f (G_ "invalid menu-entry: ~a" err)))) > (&fix-hint > (hint > (G_ "Please chose only one of: > @enumerate > @item direct boot by specifying fields @code{linux}, > @code{linux-arguments} and @code{linux-modules}, > @item multiboot by specifying fields @code{multiboot-kernel}, > @code{multiboot-arguments} and @code{multiboot-modules}, > @item chain-loader by specifying field @code{chain-loader}. > @end enumerate")))))) > > (cond > ((and (not all-linux-fields) (not all-multiboot-fields) > (not all-chainload-fields)) > (report (G_ "No boot method was chosen for this menu entry."))) > ((> count-methods 1) > (report (G_ "More than two boot methods were selected for this > menu entry."))) > (else #t))) > > This is untested, so there might be bugs :) > > Note that we need to have all strings translatable (with G_). > > In any case, that new code for error messages should go in a third, > separate patch. > Thanks your help. When I first add and test your check-menu-entry, I was amazed by its good-looking error reporting information. I like the error message. But I still have to work out the situation that linux,linux-argumet and initrd are set, and multiboot-kernel are also set, which can pass the check, but it will fail to match in menu-entry->sexp. Yes, multiboot-kernel shouldn't be ignored silently. But error message of no match is simple and crude. And It is also inconsistent with the previous exquisite error reporting information. When thinking about how to solve this problem, I get inspiration from it. It correct my previous wrong ideas. Mistakes are always various, and it is difficult for us to consider all of them. But the correctness is certain, and we can consider it more easily. I should not try to cover all the error cases, because it is very difficult. Even if it succeeds, the code is too complex and difficult to read. However, as long as I rule out the right situations, the rest is the wrong situation. "There are many situations that I need to check. I can't find a case in menu-entry->sexp to solve it." It is caused by my wrong thinking direction. So I split `check-menu-entry', merge the checked part intoe the pattern of `menu-entry->sexp', and separate the function of reporting error information into a procedure `report menu entry error'. Then add 'else' to the pattern in `menu-entry->sexp' to report error message. About the error message,I give up to point out what might be wrong, but display `menu-entry' and hint message. There are two main reasons: 1. As mentioned above, errors are unpredictable, and wrong error reporting information may mislead users. 2. The error is not caused by `menu-entry->sexp', but by an external procedure. This needs to display the `menu-entry' accepted by `menu-entry->sexp' to let the user know. Thank you for your code's inspiration. And your hint information is really great. I use your code about error message directly. >>=20 >> Without any exceptions, v3 patch may be the last version. So how can I >> modify the submission information to record your help to me? > > You don't have to because I didn't contribute much to that patch, and I > will sign it off when commiting. If you take the code above as is and > don't modify it too much, then you can add something like this at the > end of the commit message (only for the patch that contains my code): > > Co-Authored-By: Julien Lepiller > No, you give me great help. you correct the document, give me advice of using the pattern, and take time to find bug of my code and give me some advice how to work out it. Thanks your help. I will add "Co-Authored-By: Julien Lepiller " to commit messages of my first and third patch. Thanks, tiantian