From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp10.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id uPg6D7XPQmPv4QAAbAwnHQ (envelope-from ) for ; Sun, 09 Oct 2022 15:42:13 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp10.migadu.com with LMTPS id 2It0DrXPQmPnNwEAG6o9tA (envelope-from ) for ; Sun, 09 Oct 2022 15:42:13 +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 AE89E3147B for ; Sun, 9 Oct 2022 15:42:12 +0200 (CEST) Received: from localhost ([::1]:38190 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ohWZH-0002xx-QK for larch@yhetil.org; Sun, 09 Oct 2022 09:42:11 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:39178) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ohWZ9-0002xo-0u for guix-patches@gnu.org; Sun, 09 Oct 2022 09:42:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:43254) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ohWZ8-0005Sv-P5 for guix-patches@gnu.org; Sun, 09 Oct 2022 09:42:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ohWZ8-0004sA-CQ for guix-patches@gnu.org; Sun, 09 Oct 2022 09:42:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#48314] [PATCH v5] Install guix system on Raspberry Pi Resent-From: Stefan Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 09 Oct 2022 13:42:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 48314 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Vagrant Cascadian Cc: Danny Milosavljevic , Ludovic =?UTF-8?Q?Court=C3=A8s?= , phodina , 48314@debbugs.gnu.org Received: via spool by 48314-submit@debbugs.gnu.org id=B48314.166532290818709 (code B ref 48314); Sun, 09 Oct 2022 13:42:02 +0000 Received: (at 48314) by debbugs.gnu.org; 9 Oct 2022 13:41:48 +0000 Received: from localhost ([127.0.0.1]:42332 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ohWYt-0004rg-Ny for submit@debbugs.gnu.org; Sun, 09 Oct 2022 09:41:48 -0400 Received: from mr5.vodafonemail.de ([145.253.228.165]:37200) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ohWYq-0004rR-9a for 48314@debbugs.gnu.org; Sun, 09 Oct 2022 09:41:45 -0400 Received: from smtp.vodafone.de (unknown [10.0.0.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by mr5.vodafonemail.de (Postfix) with ESMTPS id 4MljrL5Dcwz1y6R; Sun, 9 Oct 2022 13:41:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vodafonemail.de; s=vfde-mb-mr2-21dec; t=1665322898; bh=KT56GhuvAg5wRbZqjbEErxm3G8Oey6At33o4jnplOuM=; h=Content-Type:Subject:From:In-Reply-To:Date:Message-Id:References: To:X-Mailer:From; b=ObSIdlZPBKS7ygFBSelO9bWdpafSPDTsWl5GmgyCdjyKWqe/ho4gQ7IhF5/kByt9J tHmqfXCsTORXmo0i6buFvupx0Aj35wQf0ohTYs23Daw15GZmRhkMNqD8bUpVYHZFRI 3nuuGTnwm2fjp0BVnyJ63hkEQUKNIfe+UK/zkic4= Received: from macbook-pro.kuh-wiese.my-router.de (aftr-62-216-210-253.dynamic.mnet-online.de [62.216.210.253]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.vodafone.de (Postfix) with ESMTPSA id 4Mljr42Z4fz9t3n; Sun, 9 Oct 2022 13:41:21 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) In-Reply-To: <87y1tq8evp.fsf@contorta> Date: Sun, 9 Oct 2022 15:41:08 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: <4B0EE79B-F91F-4963-8E69-7328583BF790@vodafonemail.de> References: <2IN6BsQe0_wSC9iwf7LHT5LUk7wXLVXytkDtcg7RIYByyYFTsuC9BZPR_wdv4eDMncsZfy17h7z9jIRRSC6kfV2odXkt0hp4Lilq5sGYdVo=@protonmail.com> <204332DD-AA02-4A31-9B48-FB3FAB9BD8F3@vodafonemail.de> <87y1tq8evp.fsf@contorta> X-Mailer: Apple Mail (2.3124) X-purgate-type: clean X-purgate: clean X-purgate-size: 8604 X-purgate-ID: 155817::1665322897-167FB4DE-5C51EA64/0/0 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" Reply-to: Stefan X-ACL-Warn: , Stefan via Guix-patches From: Stefan via Guix-patches via X-Migadu-Flow: FLOW_IN X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1665322933; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: 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=KT56GhuvAg5wRbZqjbEErxm3G8Oey6At33o4jnplOuM=; b=QI/uIhEuo4HmP2ivUrLoCSpdi24xEvrHURt1qjvPk22Kuu5QWYK2EuMCL2o4aduheaCyTE jgBDcbdFqay4FwJ9epJzOD2O0nmUdvfQp19QjlV8UXxJ0YUNluA68uhFyrLUh946UMs1ZP PbqTqcukTo/eX89JVf7SAVmW+ATeP7LXLysQAgTf8ECTmUgGPrnKKGG+tzcTvcjJucdM1w 3d1BPjxx2ety48IS/27hyyj0DwJo6gpgBiL31v/NX8VTu6gXpM4iTy0O2k2HGXm9UN+7pw +8EXEvJy7TfIC3lMvcW0XVbzdtp1smBbxEJf5gl0GKmpbs4Xwbvw3Vfrsd/y9Q== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1665322933; a=rsa-sha256; cv=none; b=GYuUSvOrhvkTSuLsIsh7FYGePkITEsRrjzZbfcit2AYqCQxP7m+i/cYeCBoIVubDRWdmrU 0cQSgoWPwqtmQtaO0z0b9hFEmec41/y5Ye31nmYPg7yWxcYLs8ldsyE/lbFJYiSftZkCRJ eM87wQeeqn4ofwb7Nh756K2B4WY3yuap7BXh0czSuy5j/cwgg7yfuAgWYH2JD/0iS0ELTQ dDO3XFmp48aVBugoWObTOIwsQVq9cPiElbL5fpNmomcxjYxEKJ6ewl1AOJYbKKF4ltBhj2 i8HSAtAh58WXBbhNd4K4gIKyfHtscVmEHajQH4vvonec8UevYwEUM4mZ4GN7Mw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=vodafonemail.de header.s=vfde-mb-mr2-21dec header.b=ObSIdlZP; dmarc=pass (policy=none) header.from=gnu.org; 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: -2.68 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=vodafonemail.de header.s=vfde-mb-mr2-21dec header.b=ObSIdlZP; dmarc=pass (policy=none) header.from=gnu.org; 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: AE89E3147B X-Spam-Score: -2.68 X-Migadu-Scanner: scn0.migadu.com X-TUID: UqjYFs6AorO/ Hi Vagrant! >> The function modify-defconfig in guix/build/kconfig.scm no longer >> interprets "CONFIG_XY=3D" like "# CONFIG_XY is not set". >=20 > Nervous about how that actually works, but hopefully it plays out = correctly. This is described in the doc-string of config-string->pair in the module = (guix build kconfig), which contains a regular expression for the = parsing. Basically any =E2=80=9C# CONFIG_X is not set=E2=80=9D or =E2=80=9CCONFIG_X= =E2=80=9D is treated as unset and produces a ("CONFIG_X" . #f). The = latter is just for convenience, as the first one is hard to remember and = easy to get wrong. Any =E2=80=9CCONFIG=3D=E2=80=A6=E2=80=9D is treated = as set and produces a ("CONFIG_X" . "=E2=80=A6"). Anything else except comments with =E2=80=9C#=E2=80=A6=E2=80=9D or empty = lines will throw an error. In a previous patch =E2=80=9CCONFIG_X=3D=E2=80=9C was also treated as = unset, which was confusing, as this is actually a valid makefile = assignment. The function pair->config-string produces a =E2=80=9C# CONFIG_X is not = set=E2=80=9D for any #f value, or a proper assignment otherwise. > This whole patch series is large and overwhelming and at least a bit > beyond my abilities to wrap my head around (which has certainly caused > me to wait a bit to review)... so I cannot possibly comment on weather > the series as a whole is "good", through no fault of the patch > author(s)! I will try and comment where I can, but really need help to > review it in any meaningful way. I=E2=80=99ll try to help. > Also from what I recall on earlier iterations of this patch series, > different reviewers seemed to have differing style recommendations > around weather to split patches into smaller commits or merge patches > into combined commits, which can surely be frustrating. I don't *want* > to be frustrating, but I lean towards splitting patches into as small = a > commit as possible to wrap my head around the distinct ideas. >=20 > I also like to refactor out anything that can be applied directly to > master as soon as possible (e.g. the description-appending patches = look > promising for that) with the hope to get the remaining patch series > smaller and smaller with each iteration. Some people may want to do an > all-or-nothing merge. I don't know what's "right" for the guix > community=E2=80=A6 The single patch files can be applied separately, they don=E2=80=99t = break anything. Some reordering is also possible. In particular, = 02-gnu-bootloader-rework-chaining.patch (the monster) can be applied = later but before 09-gnu-raspberry-pi-add-a.patch. > With all that out of the way... here goes my attempt to review! >=20 >=20 >> gnu: linux: Fix the extra-version parameter in make-linux-libre*. > Overall, this looks good, to me, though have one question=E2=80=A6 Great! :-) > Why is native-inputs removed from the 'install phase? Is it no longer > needed? Was it not actually needed before? I see no mention of this > change in the comment. Exactly, it was even not needed before, an obsolete argument, which I = removed. I figured this out by chance when selecting the needed = arguments for the 'set-environment phase. >> gnu: bootloader: Rework chaining, add = grub-efi-netboot-removable-bootloader. > There is too much going on here for me to follow, but it is perhaps = just > doing a lot of big changes=E2=80=A6 help? :) As noted, this patch can be delayed before = 09-gnu-raspberry-pi-add-a.patch is to be applied. Before this patch, the bootloader-installer of efi-bootloader-chain = called grub-mknetdir (actually the installer of = grub-efi-netboot-bootloader) and copied the content of a special = collection folder of the bootloader-profile. That collection folder was = very special and did not fit well to the bootloader-profile idea. Well, = actually the grub-efi package with all its tools being part of the = profile was the problem, making the collection folder necessary.=20 With this patch the packages of grub-efi-netboot-bootloader and = grub-efi-netboot-removable-bootloader are already pre-installed =E2=80=93 = grub-mknetdir is already called during package creation. Their installer = just copies the whole package/profile into the target directory. The = efi-bootloader-chain only creates a profile with the bootloader (e.g. = grub-efi-netboot-bootloader) and additional packages or files. The = collection folder is not needed any more. Most complexity got moved from = the bootloader installation time to the package build time. Other patches generate more =E2=80=9Cpre-installed=E2=80=9C files like = u-boot.bin, config.txt, device-tree files, etc., which now all fit much = bettor to the bootloader-profile idea, to just be copied to a target = directory. This patch is also inspired by older comments. Maybe take a look at the = comments below and = especially at I don=E2=80=99t think splitting this into smaller parts is possible = without a breakage. =20 >> build: kconfig: Add new module to modify defconfig files. > I like how this simplifies the various u-boot-* package definitions! Great! :-) >> gnu: bootloader: Add U-Boot packages for Raspberry Pi models. > This seems good to me. Great! :-) > It would be nice to first switch the existing u-boot-* packages to use > the new append-description feature one commit (I think this could even > be applied directly to master?), and then add the new functionality > (e.g. make-u-boot-bin-package, *u-boot-rpi-*, etc.) in another commit = or > a couple commits. At least, that would make it a little easier for me = to > read. Splitting is possible. >> gnu: linux: New function to modify the configuration of a Linux = kernel. > Looks ok to me, though to say I fully understand it would be a = stretch. :) The idea here is very similar to the use for u-boot: Take some Linux, = pretend some defconfig being used, do simple modifications to it, and = verify the result. The defconfig can be provided via #:defconfig (any file-like-object or a = file-name) or will otherwise be generated with =E2=80=9Cmake = savedefconfig=E2=80=9D. The function (make-defconfig) is a helper to use a defconfig file from = some url.=20 The function system->linux-srcarch is needed to locate existing = defconfig files in the Linux sources like = arch/arm/configs/bcm2835_defconfig, so that you can just pass the = filename =E2=80=9Cbcm2835_defconfig=E2=80=9D to #:defconfig. This = enables the use of defconfig files existing in the Linux sources. As = Kbuild expects defconfig files at a certain location, this function is = also needed to copy an own defconfig file there.=20 There was once a blog post about a custom Linux kernel stating = =E2=80=9CSuggestions and contributions toward working toward a = satisfactory custom initrd and kernel are welcome!=E2=80=9D, see = . This is my take including a verification for the = kernel. :-) >> gnu: raspberry-pi: Add defconfig objects to build customized Linux = kernels. > Seems good. I think I even understand this one! Great! :-) >> gnu: raspberry-pi: Add helpers for config.txt file generation. > Seems good. Great! :-) >> gnu: raspberry-pi: Add a bootloader-chain for the Raspberry Pi and os = examples. > I'd split this into two commits, one adding > grub-efi-bootloader-chain-raspi-64, and one adding examples using it, > but that is really a judgement call. True. Combining the example with the new function was my choice. If you = don't mind, I'd keep it this way. Thanks a lot for the review, Vagrant! How to proceed from here?=20 I'd suggest to postpone 02-gnu-bootloader-rework-chaining.patch and = 09-gnu-raspberry-pi-add-a.patch, until all others got merged. To me it seems possible to commit these patches as they are in this = order: 01-gnu-linux-fix-extra-version.patch 03-build-kconfig-add-new-module.patch 05-gnu-linux-new-function-to.patch 06-gnu-raspberry-pi-add-defconfig.patch 07-gnu-raspberry-pi-add-helpers.patch 08-gnu-raspberry-pi-new-function.patch Then I could send a reduced patch-series with: splitted 04-gnu-bootloader-add-u-boot.patch 02-gnu-bootloader-rework-chaining.patch 09-gnu-raspberry-pi-add-a.patch What do you think? Bye Stefan