From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp10.migadu.com ([2001:41d0:403:4789::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms9.migadu.com with LMTPS id QFH7NpJEIGV+YgAAG6o9tA:P1 (envelope-from ) for ; Fri, 06 Oct 2023 19:32:03 +0200 Received: from aspmx1.migadu.com ([2001:41d0:403:4789::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp10.migadu.com with LMTPS id QFH7NpJEIGV+YgAAG6o9tA (envelope-from ) for ; Fri, 06 Oct 2023 19:32:02 +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 1458E5F60C for ; Fri, 6 Oct 2023 19:32:01 +0200 (CEST) Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=planete-kraus.eu header.s=albinoniB header.b=IulFV3Nc; 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"; dmarc=pass (policy=none) header.from=gnu.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1696613522; 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=Nj0E/YbLHYX7zd0h9BZOoImOKLRvsLB9fDG1vrqLwd4=; b=WpEN78KFE5u9ojNRNDbzyfmHCLhf6RGHHTd0ieRUW/HAwTTcx8Fs27csLYndRdhToW3F1s +paMAJvnX9BnYdROJgDMnU7g2Xbc+ACKe5fXVfv+5kEvHPWToi848P65DWovR5hoHUCQi6 L/qhDnbwvkaUVAVUSD+T1Q7pbbpSvV6u9rvwo9DJhP07UtD/a8eq7WmWev/x6eO3vPX4CV N7TgMHZcddi/57VyZ6hT00v2SMTnFbrq4J1QQGpvPQAi9HtMODd/ZB1fyUhJ5bDgUUZ9GU 1FvTz/go9bwBJJDVNUMRqhWjcdMMe5kNtWmA2WgDv4mk38Y+n6c0e+/bzANdcQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=planete-kraus.eu header.s=albinoniB header.b=IulFV3Nc; 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"; dmarc=pass (policy=none) header.from=gnu.org ARC-Seal: i=1; s=key1; d=yhetil.org; t=1696613522; a=rsa-sha256; cv=none; b=D74GUtpN1wkbtXmyIa9Bk/WulLW1i3Htaf/YXQnqxBABYuF6J0Ky0pDwJZ9sF7wMmikJcp ka3o0a9l65sux4R3YffhsXMyhdZ6EhjsEMzCyDWph7Qo6WIpUo1iR4kYtMICUISgjn9tQt oBUXnXPTvbO0qpVxxN3P+005VJxaVMOVHfUgI2GVe2qZYki4mVTWKAzByZWWrgB91GPdkB rGIyZwlmLxyebHvWSpqDtwzrc7Ab7rPjM8oQh5xdBujvygNo0m9v43ZRDlqOx/jJWmkbqy 0bSkltSyefIgom4CgrynBx+qhW6qR3mhOP+/EWtCBojFyUE00jKYMDmfc3c6QA== Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qoofy-0006lj-LP; Fri, 06 Oct 2023 13:31:53 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qoofw-0006kx-9O for guix-patches@gnu.org; Fri, 06 Oct 2023 13:31:45 -0400 Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qoofw-0006YB-1a for guix-patches@gnu.org; Fri, 06 Oct 2023 13:31:44 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qoogE-0000WV-Hn for guix-patches@gnu.org; Fri, 06 Oct 2023 13:32:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#66099] [PATCH gnome-team v8 0/6] Update eudev, udev-service-type, upower Resent-From: Vivien Kraus Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 06 Oct 2023 17:32:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 66099 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Liliana Marie Prikler , Maxim Cournoyer , 66099@debbugs.gnu.org Cc: rg@raghavgururajan.name Received: via spool by 66099-submit@debbugs.gnu.org id=B66099.16966134871905 (code B ref 66099); Fri, 06 Oct 2023 17:32:02 +0000 Received: (at 66099) by debbugs.gnu.org; 6 Oct 2023 17:31:27 +0000 Received: from localhost ([127.0.0.1]:52562 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qoofe-0000Ue-AM for submit@debbugs.gnu.org; Fri, 06 Oct 2023 13:31:27 -0400 Received: from planete-kraus.eu ([89.234.140.182]:43846) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qoofb-0000UU-Km for 66099@debbugs.gnu.org; Fri, 06 Oct 2023 13:31:25 -0400 Received: from planete-kraus.eu (localhost.lan [127.0.0.1]) by planete-kraus.eu (OpenSMTPD) with ESMTP id 6965ecc4; Fri, 6 Oct 2023 17:31:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=planete-kraus.eu; h= message-id:in-reply-to:references:from:date:subject:mime-version :content-type:content-transfer-encoding:to:cc; s=albinoniB; bh=b 32EIDFfT6SULt6oR8ApdhdMSQA=; b=IulFV3NcJsXhy+5i4v3V56ug9ps/m2wPU b2gmGAtM7gqKFFvaCeni1so3nQJgyzKZBuO02qaNVvY0vMqFvwtGWJdogGgR+G5N Yft6hkLwIR4wcxgbtmp9GDGNEMDIs7KMcSybRPBoY0jHfGi9indoBeKDbqc8Pa2V HJoBG3arQBaS/A7GeyTrN793DG/I+k/qlYkI4SsSUvuRZSkTltODqQAdw7uIK2ug aczGkEfxCuFeX0WnoOw+hgtb/THeO4JPtUyLsuEU+k5eC7RXJQqeRHeQWrXjscDy haIiX5blW1Lz7pFJrUCrHG1ShiRs3oO4RFgR24CZdx0yIsFu5kKFA== Received: by planete-kraus.eu (OpenSMTPD) with ESMTPSA id 61cb4693 (TLSv1.3:TLS_CHACHA20_POLY1305_SHA256:256:NO); Fri, 6 Oct 2023 17:31:02 +0000 (UTC) Message-ID: In-Reply-To: <0825bf42daa34ee464c5237b04ad6bf90942e489.camel@gmail.com>, , <1a1089275d13e02b4485c5de6e908ece88f5d2b6.camel@gmail.com><87y1ghryel.fsf@gmail.com>, <87pm1try6q.fsf@gmail.com>, <87lechrvhl.fsf@gmail.com> References: <0825bf42daa34ee464c5237b04ad6bf90942e489.camel@gmail.com> , ,<1a1089275d13e02b4485c5de6e908ece88f5d2b6.camel@gmail.com> <87y1ghryel.fsf@gmail.com>,<87pm1try6q.fsf@gmail.com> ,<87lechrvhl.fsf@gmail.com> Date: Fri, 6 Oct 2023 18:45:46 +0200 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User-Agent: Evolution 3.46.4 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: , Reply-to: Vivien Kraus X-ACL-Warn: , Vivien Kraus via Guix-patches From: Vivien Kraus via Guix-patches via Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: guix-patches-bounces+larch=yhetil.org@gnu.org X-Migadu-Flow: FLOW_IN X-Migadu-Country: US X-Migadu-Scanner: mx1.migadu.com X-Migadu-Spam-Score: -2.24 X-Spam-Score: -2.24 X-Migadu-Queue-Id: 1458E5F60C X-TUID: pDRWlDQFPl+A Dear guix, Thank you for your reviews! Since eudev released 3.2.14 in the mean time, I udpated it. I also reused more code in the udev-service-type helper functions. Le jeudi 05 octobre 2023 à 07:42 +0200, Liliana Marie Prikler a écrit : > Am Montag, dem 02.10.2023 um 21:08 +0200 schrieb Vivien Kraus: > > The "rules" field in the udev-configuration record can now hold > > both > > rules and hwdb files. > Yeah, how about we instead have separate fields – one named "rules" and one > named "hardware". We're already breaking ABI stability anyway, so we might > as well. Okay. I fixed the udev-service-type function so that both fields are populated correctly. > Speaking of which, we might want to rename "hwdb.d" to "hardware" in udev > itself to make that name readable, but keep /etc/udev/hwdb.bin as-is. I renamed to "hardware" in Guix documentation, but individual packages (if they behave like upower) expect to install their hardware files in "hwdb.d", so I think we should keep "hwdb.d" as the directory name. Le jeudi 05 octobre 2023 à 08:53 +0200, Liliana Marie Prikler a écrit : > Am Donnerstag, dem 05.10.2023 um 07:55 +0200 schrieb Vivien Kraus: > > Le jeudi 05 octobre 2023 à 07:42 +0200, Liliana Marie Prikler a > > écrit : > > > Am Montag, dem 02.10.2023 um 21:08 +0200 schrieb Vivien Kraus: > > > > The "rules" field in the udev-configuration record can now hold > > > > both rules and hwdb files. > > > Yeah, how about we instead have separate fields – one named > > > "rules" > > > and one named "hardware". > > > > Since the extensions are untyped anyway, I thought we would not > > care > > in the configuration record itself. Should we also introduce types > > for the extension values? > This is about field semantics rather than type theory, but if you > prefer, I think we can use list-of file-like? or similar for both. I addressed the "field semantics" view of the problem. Le jeudi 05 octobre 2023 à 09:28 -0400, Maxim Cournoyer a écrit : > > Eudev has a hardware database that installs descriptions of hardware > > in /etc, > > but they should go to /lib prior to being used. The build > > system can’t > > install to /etc, and should not, because this directory is owned by > > the > > udev-service-type. So they are installed directly in /lib. > > Another > > file, an empty /etc/udev.conf, is simply ignored. > > This sounds more like a limitation/bug in our udev-service-type than in > eudev? If eudev wants its files installed to /etc, they should be left > there I think. Going against this is more maintenance down the road, and > surprise from users. As discussed in IRC, we set sysconfdir to /etc so that it can have hardware files from many packages. So the hardware files are searched from either "/etc/udev/hwdb.d" or "$individual_package_prefix/lib/udev/hwdb.d". So, the only standard directory to install hwdb files during a build of a package is $package_prefix/lib/udev/hwdb.d. Le jeudi 05 octobre 2023 à 09:28 -0400, Maxim Cournoyer a écrit : > > The hwdb.bin file used to be generated in > > /etc/udev/hwdb.bin, but > > since the sysconf dir is now directly /etc, the hwdb.bin index will > > not be > > found under /etc/udev/hwdb.bin. > > > > * gnu/packages/linux.scm (eudev): Update to a post-v3.2.12 commit. > > [snippet]: New snippet to override the version number. > > [modules]: Import (guix build utils). > > [#:phases] : New phase. > > : New phase. > > : Remove phase. > > [#:configure-flags]: Set sysconfdir to avoid a prefix. > > The above should be commented in the code. What prefix? sysconfdir > typically defaults to /etc, if I recall correctly. I added comments in the code. As mentioned in IRC, sysconfdir defaults to $prefix/etc. If we use $eudev_prefix/etc/udev/hwdb.d as the directory where the highest priority hardware files are found, then we won’t be able to add files to it at all, and it will be disconnected from /etc/udev/hwdb.d (prepared by the udev-service-type). Le jeudi 05 octobre 2023 à 09:28 -0400, Maxim Cournoyer a écrit : > > + (add-before 'bootstrap 'install-in-lib > > + (lambda _ [...] > > That's rather complex. Could we instead modify udev-service-type to only > union what it needs under /etc/udev and not the undesired files from this > package? Considering the steps in this phase other than changing the installation directory of hardware files, the build system wants to install things to /etc, which is unaccessible when building the package. So we either have to change the installation directory of the empty template configuration, or disable the installation. Same for the mkdir -p, we either have to make the empty directory in the installation prefix of the package, or disable it. I think that the empty configuration file and the empty directory are not worth keeping. > > + "--sysconfdir=/etc"))) > > See comment above about justifying this flag. As discussed, we need it. The added comments should help justify it. > > @@ -20,11 +20,12 @@ this hack. > > - "/lib/udev/rules.d", > > - "/usr/lib/udev/rules.d", > > -#endif > > +- "/usr/local/lib/udev/rules.d", > > + NULL, /* placeholder for $EUDEV_RULES_DIRECTORY */ > > NULL}; > > > > struct udev_rules { > > -@@ -1704,6 +1700,9 @@ > > +@@ -1718,6 +1713,9 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names) { > > > > udev_rules_check_timestamp(rules); > > It'd be nicer if eudev's build system allowed to configure the above without > patches hard coding things. Indeed. For now, I think the patch is fine (I know very little about udev rules though). Le jeudi 05 octobre 2023 à 09:33 -0400, Maxim Cournoyer a écrit : > I'm not sure using Texinfo in plain docstrings provides much; there's no > tooling to render them and it's a convention to use FULL CAPS for variable > names in Guile doc strings, at least in the Guix code base. (discussing the docstrings use of texinfo) Le jeudi 05 octobre 2023 à 10:31 -0400, Maxim Cournoyer a écrit : > > Some udev-related auxiliary functions in (gnu services base) had > > non-texinfo > > variable references in their docstrings ("FILE-NAME" instead of > > "@var{file-name}"). > > That's fairly conventional. Texinfo is used in the manual documentation, > package synopsis/descriptions and services documentation, not for every doc > string in the code base. Since the ,describe top-level guile command does not try to render texinfo, I think you are right. I reverted the changes. I changed the documentation as indicated ("matter", double spaces, "@file", the udev-rule introduction, "@code{operating-system}", the android-udev-rules comment). > As discussed above, I'm not convinced about changing dostrings to use > Texinfo, but I see that was already the cases for some around. Hm. I guess that’s from guile people (in guile, the docstrings are extracted to be included in the manual). About the texinfocation of the udev-rules-service docstring: > I'd drop this change. Done. > > +@item @code{native-udev} (default: @code{eudev}) (type: file-like) > > +Native udev package to compile @code{hwdb.bin}. The trie format used for > > +@code{hwdb.bin} must be compatible with the @code{udev} and > > +@code{native-udev} fields of the udev configuration. To avoid generating > > +@code{hwdb.bin}, pass @code{#f} as the @code{native-udev} field. > > + > > +In any case, if the package version string differs between @code{udev} > > +and @code{native-udev}, @code{hwdb.bin} is @strong{not} created. > > Shouldn't that raise an error with a useful error message? Then it doesn't > need to be documented here. Thinking again, why is it necessary to have an > explicite native-udev field? The gexps mechanism of the service could > perhaps use #+udev instead of #$udev where needed, sharing the same 'udev' > field for both purposes? Further down, about the native-udev file in the configuration record definition: > As discussed earlier, I don't think that new field is needed. And further down: > > + (invoke #+(file-append native-udev "/bin/udevadm") > > [...] > > As discussed above, this can probably be simplified by dropping native-udev. > If it's truly needed, the pathological case where different versions are > needed should be handled earliest and an error raised with a useful message. I dropped the native-udev field, with a warning that it will be used both in the current system and the target system. > Could you please send a v8 with changes along those suggested? Here! Best regards, Vivien Vivien Kraus (6): gnu: eudev: Update to 3.2.14. services: udev: Rewrite udev-rule to use file->udev-rule. services: udev: Make udev-rule helper functions generic. gnu: udev-service-type: accept hardware description file extensions. gnu: libgudev: Update to 238. gnu: upower: Update to 1.90.2. doc/guix.texi | 57 ++++++-- gnu/packages/gnome.scm | 43 +++--- gnu/packages/linux.scm | 57 +++++--- .../patches/eudev-rules-directory.patch | 9 +- gnu/services/base.scm | 124 +++++++++++++----- 5 files changed, 205 insertions(+), 85 deletions(-) base-commit: b18b2d13488f2a92331ccad2dc8cbb54ee15582f -- 2.41.0