GNU bug report logs - #75145
[PATCH] services: NetworkManager: configuration-directory

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 75145 in the body.
You can then email your comments to 75145 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: 45mg <45mg.writes <at> gmail.com>
Subject: [PATCH] services: NetworkManager: configuration-directory
Date: Fri, 27 Dec 2024 13:18:06 -0500

Give users a way to configure NetworkManager, by specifying a directory
containing configuration files. This directory will then be symlinked to
`/etc/NetworkManager/conf.d` (NetworkManager's default configuration
directory location).

* gnu/services/networking.scm (<network-manager-configuration>)
[configuration-directory]: new option.
* doc/guix.texi: document it.

Change-Id: I243a71593b9235cc11ebf9fea1926a1840d993d2
---
 doc/guix.texi               | 30 ++++++++++++++++++++++++++++++
 gnu/services/networking.scm | 10 ++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index da4d2f5ebc..ee0fbf59f9 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21441,6 +21441,36 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
+@item @code{configuration-directory} (default: @code{#f})
+A directory (file-like object) that will be symlinked as
+@file{/etc/NetworkManager/conf.d}. NetworkManager will read
+configuration files from this directory. For the configuration file
+format, see the @command{NetworkManager.conf(5)} man page.
+
+For example, you could supply an existing directory:
+
+@lisp
+(service network-manager-service-type
+         (network-manager-configuration
+          (configuration-directory
+           (local-file "files/NetworkManager/conf.d" #:recursive? #t))))
+@end lisp
+
+Or create a directory using @code{file-union}:
+
+@lisp
+(service network-manager-service-type
+         (network-manager-configuration
+          (configuration-directory
+           (file-union "my-configuration-directory"
+                       `(("existing-file" ,(local-file "001-basic.conf"))
+                         ("constructed-file" ,(plain-file "002-unmanaged.conf"
+                                                      "[keyfile]
+unmanaged-devices=interface-name:wlo1_ap
+")))))))
+@end lisp
+
+
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 48a86b3694..7152c5a95f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -23,6 +23,7 @@
 ;;; Copyright © 2023 Bruno Victal <mirai <at> makinata.eu>
 ;;; Copyright © 2023 muradm <mail <at> muradm.net>
 ;;; Copyright © 2024 Nigko Yerden <nigko.yerden <at> gmail.com>
+;;; Copyright © 2024 45mg <45mg.writes <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1262,18 +1263,23 @@ (define-record-type* <network-manager-configuration>
                (default '()))
   (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
         (default #f)
-        (sanitize warn-iwd?-field-deprecation)))
+        (sanitize warn-iwd?-field-deprecation))
+  (configuration-directory network-manager-configuration-configuration-directory ;file-like
+                           (default #f)))
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins)
+    (network-manager dns vpn-plugins configuration-directory)
     #~(begin
         (use-modules (guix build utils))
         (mkdir-p "/etc/NetworkManager/system-connections")
         #$@(if (equal? dns "dnsmasq")
                ;; create directory to store dnsmasq lease file
                '((mkdir-p "/var/lib/misc"))
+               '())
+        #$@(if configuration-directory
+               `((symlink ,configuration-directory "/etc/NetworkManager/conf.d"))
                '()))))
 
 (define (vpn-plugin-directory plugins)

base-commit: 831b94a1efcea8f793afc949b5123a6235c9bb1a
-- 
2.47.1

Message #8 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: 75145 <at> debbugs.gnu.org
Cc: guix-devel <at> gnu.org, 45mg <45mg.writes <at> gmail.com>
Subject: [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Thu,  9 Jan 2025 12:24:30 +0000

Hi Guix,

As the first revision of this patch failed to get any attention in two weeks,
I'm CC'ing guix-devel on this one, hoping it'll get reviewed this time. Let me
know if I should have done anything differently; I'm still relatively new to
contributing.

This patch allows users to specify configuration files for NetworkManager.
While perhaps it would be more Guix-y to instead have a field in
`network-manager-configuration` for every configuration option, this would be
a monumental undertaking (look at the number of options listed in
NetworkManager.conf(5)!). At any rate, I think any means of configuring
NetworkManager is better than none.

The difference from the first revision is that instead of specifying a single
directory (file-like object) containing the configuration files (which was
then symlinked to /etc/NetworkManager/conf.d), you now specify an alist
mapping file names to file-like objects, like with `etc-service-type`; and
those are then added to /etc/NetworkManager/conf.d. The rationale behind this
change is that it doesn't rule out putting our own stuff in
/etc/NetworkManager/conf.d. For example, if we wanted a default set of files
in there, we could modify the procedure
`network-manager-configuration-directory` to add the files supplied via the
field to our default set; in the first revision, this wouldn't be possible as
the user specifies the entire directory. (I don't know whether we'd ever
actually want to do this, but I thought it best to leave our options open.)

45mg (1):
  services: network-manager: Add extra-configuration-files field.

 doc/guix.texi               | 21 +++++++++++++++++++++
 gnu/services/networking.scm | 26 ++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)


base-commit: 7f27dc47c52886b785359799b6dc67b61f638544
-- 
2.47.1

Message #11 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: 75145 <at> debbugs.gnu.org
Cc: guix-devel <at> gnu.org, 45mg <45mg.writes <at> gmail.com>
Subject: [PATCH v2 1/1] services: network-manager: Add
 extra-configuration-files field.
Date: Thu,  9 Jan 2025 12:24:31 +0000

Allow users to specify additional configuration files for
NetworkManager. These files will be added to
`/etc/NetworkManager/conf.d` (NetworkManager's default configuration
directory location).

* gnu/services/networking.scm (<network-manager-configuration>)
[extra-configuration-files]: New field.
(network-manager-configuration-directory): New procedure.
(network-manager-activation): Honor the new field.
* doc/guix.texi (Networking Setup): Document the new field.

Change-Id: I07479958e4d0aa318328c666a9630b779230b300
---
 doc/guix.texi               | 21 +++++++++++++++++++++
 gnu/services/networking.scm | 26 ++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index caebe3b03c..279fdb838b 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21455,6 +21455,27 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
+@item @code{extra-configuration-files} (default: @code{'()})
+An alist of file names to file-like objects, representing configuration
+files which will be added to @file{/etc/NetworkManager/conf.d}.
+NetworkManager will read additional configuration from this directory.
+For details on configuration file precedence and the configuration file
+format, see the @command{NetworkManager.conf(5)} man page.
+
+For example, to add two files @file{001-basic.conf} and
+@file{002-unmanaged.conf}:
+
+@lisp
+(service network-manager-service-type
+         (network-manager-configuration
+          (extra-configuration-files
+           `(("existing-file" ,(local-file "001-basic.conf"))
+             ("constructed-file" ,(plain-file "002-unmanaged.conf"
+                                              "[keyfile]
+unmanaged-devices=interface-name:wlo1_ap
+"))))))
+@end lisp
+
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 48a86b3694..4355158225 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -1262,18 +1262,40 @@ (define-record-type* <network-manager-configuration>
                (default '()))
   (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
         (default #f)
-        (sanitize warn-iwd?-field-deprecation)))
+        (sanitize warn-iwd?-field-deprecation))
+  (extra-configuration-files network-manager-configuration-extra-configuration-files
+                             (default '())))  ;alist of file names to file-like objects
+
+(define (network-manager-configuration-directory extra-configuration-files)
+  "Return a directory containing EXTRA-CONFIGURATION-FILES."
+  (with-imported-modules (source-module-closure '((guix build utils)))
+    (computed-file
+     "network-manager-configuration-directory"
+     #~(begin
+         (use-modules (guix build utils))
+         (mkdir-p #$output)
+         (for-each (lambda (pair)
+                     (let* ((filename (list-ref pair 0))
+                            (file (list-ref pair 1))
+                            (dest (string-append #$output "/" filename)))
+                       (copy-file file dest)))
+                   '#$extra-configuration-files)))))
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins)
+                (network-manager dns vpn-plugins extra-configuration-files)
     #~(begin
         (use-modules (guix build utils))
         (mkdir-p "/etc/NetworkManager/system-connections")
         #$@(if (equal? dns "dnsmasq")
                ;; create directory to store dnsmasq lease file
                '((mkdir-p "/var/lib/misc"))
+               '())
+        #$@(if extra-configuration-files
+               `((symlink
+                  ,(network-manager-configuration-directory extra-configuration-files)
+                  "/etc/NetworkManager/conf.d"))
                '()))))
 
 (define (vpn-plugin-directory plugins)
-- 
2.47.1

Message #14 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
To: 45mg <45mg.writes <at> gmail.com>
Cc: guix-devel <at> gnu.org,
 Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Thu, 09 Jan 2025 15:44:27 +0100

[Message part 1 (text/plain, inline)]

Hi,

Thank you for your patch.

45mg <45mg.writes <at> gmail.com> writes:
> As the first revision of this patch failed to get any attention in two weeks,
> I'm CC'ing guix-devel on this one, hoping it'll get reviewed this time. Let me
> know if I should have done anything differently; I'm still relatively new to
> contributing.

I am also new to contributing, so the following may not be fully
accurate.

I think that you did everything right.  Unfortunately, some patches
"fall through the cracks".  I have seen some people replying to
issues [0], so that emails would show up in people' mailboxes (similar
to what you did) and restart the discussion.

I think that it is also possible to suggest simple patches for patch
review events [1].  I do not know if proposing one's own patches is
frowned upon or not, but I would not expect it to be as long as there is
no abuse.

[0] Random example: https://issues.guix.gnu.org/64620#21
[1] Wiki page for last year:
    https://libreplanet.org/wiki/Group:Guix/PatchReviewSessions2024

> This patch allows users to specify configuration files for NetworkManager.
> While perhaps it would be more Guix-y to instead have a field in
> `network-manager-configuration` for every configuration option, this would be
> a monumental undertaking (look at the number of options listed in
> NetworkManager.conf(5)!). At any rate, I think any means of configuring
> NetworkManager is better than none.

I agree that an escape hatch enabling to configure NetworkManager would
be nice indeed.

> The difference from the first revision is that instead of specifying a single
> directory (file-like object) containing the configuration files (which was
> then symlinked to /etc/NetworkManager/conf.d), you now specify an alist
> mapping file names to file-like objects, like with `etc-service-type`; and
> those are then added to /etc/NetworkManager/conf.d. The rationale behind this
> change is that it doesn't rule out putting our own stuff in
> /etc/NetworkManager/conf.d. For example, if we wanted a default set of files
> in there, we could modify the procedure
> `network-manager-configuration-directory` to add the files supplied via the
> field to our default set; in the first revision, this wouldn't be possible as
> the user specifies the entire directory. (I don't know whether we'd ever
> actually want to do this, but I thought it best to leave our options open.)

On adding default files in .../conf.d/:
```````````````````````````````````````

Adding a default configuration could one day be interesting.  However,
I would rather (personal opinion) see the default value in an exported
variable %default-networkmanager-files (non-contractual name) if needed.
This variable could become the default value of the field.

This way, it would be more transparent to users, and would enable them
to easily opt-out.  Adding their files could be done with the following
configuration:

--8<---------------cut here---------------start------------->8---
(extra-configuration-files
  (cons* `("file1" ,(plain-file ...))
         ...
         %default-networkmanager-files))
--8<---------------cut here---------------end--------------->8---


On doing a similar job than `etc-service-type':
```````````````````````````````````````````````

As you said, you are doing something similar to `etc-service-type'.  Is
there a reason not to extend [2] it directly (e.g. like
`greetd-service-type' does in `(gnu services base)')?

You could, for example, prepend "NetworkManager/conf.d/" to file names
and pass the value to `etc-service-type'.  WDYT?

[2] more on service extension can be found in (guix)Service Reference:
    https://guix.gnu.org/manual/devel/en/html_node/Service-Reference.html


Final thought:
``````````````

Would it make sense to allow the NetworkManager service to be extended?
(E.g to allow another service to add a configuration file.)

Note: this is not a blocker for your patch and could be done in a later
      patch.  It is simply a thought that I wanted to share.


Best regards,

-- 
Arnaud

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>, 45mg <45mg.writes <at> gmail.com>
Cc: guix-devel <at> gnu.org,
 Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Thu, 09 Jan 2025 12:03:13 -0500

Hi Arnaud!

Arnaud Daby-Seesaram <ds-ac <at> nanein.fr> writes:

> Adding a default configuration could one day be interesting.  However,
> I would rather (personal opinion) see the default value in an exported
> variable %default-networkmanager-files (non-contractual name) if needed.
> This variable could become the default value of the field.
>
> This way, it would be more transparent to users, and would enable them
> to easily opt-out.  Adding their files could be done with the following
> configuration:
>
> --8<---------------cut here---------------start------------->8---
> (extra-configuration-files
>   (cons* `("file1" ,(plain-file ...))
>          ...
>          %default-networkmanager-files))
> --8<---------------cut here---------------end--------------->8---

Yes, I think I agree. Of course, it's a moot point for now since we
don't have any use for such a variable yet, but it seems like a good
approach.

> As you said, you are doing something similar to `etc-service-type'.  Is
> there a reason not to extend [2] it directly (e.g. like
> `greetd-service-type' does in `(gnu services base)')?
>
> You could, for example, prepend "NetworkManager/conf.d/" to file names
> and pass the value to `etc-service-type'.  WDYT?

That's a good suggestion! I don't know why it didn't occur to me.

So I tried it; the problem is that if I pass something like this:
--8<---------------cut here---------------start------------->8---
(service-extension etc-service-type
                   (lambda (config)
                     `(("NetworkManager/conf.d"
                        ,(network-manager-configuration-directory config)))))
--8<---------------cut here---------------end--------------->8---

...then I get this error when creating a container:
--8<---------------cut here---------------start------------->8---
ERROR: In procedure primitive-load:
In procedure mkdir: Read-only file system
--8<---------------cut here---------------end--------------->8---

It looks like this is because of this line in `network-manager-activation`:
--8<---------------cut here---------------start------------->8---
        (mkdir-p "/etc/NetworkManager/system-connections")
--8<---------------cut here---------------end--------------->8---

When we use `etc-service-type`, "/etc/NetworkManager" becomes a symlink
to "/etc/static/NetworkManager"; and "/etc/static" is a symlink to the
result of building the derivation returned by `etc-entry` (see (gnu
services)). And derivations are in the store, which is read-only. So we
can't create "/etc/NetworkManager/system-connections/". And this won't
do, since NetworkManager itself needs to be able to write to that
directory to manage saved connections.

So it looks like that won't work.

> Would it make sense to allow the NetworkManager service to be extended?
> (E.g to allow another service to add a configuration file.)
>
> Note: this is not a blocker for your patch and could be done in a later
>       patch.  It is simply a thought that I wanted to share.

Yeah, this was another reason for this revision. Again, it's not
actually something we need right now; it's just worth leaving open as a
possibility.

Message #20 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Felix Lechner <felix.lechner <at> lease-up.com>
To: 45mg <45mg.writes <at> gmail.com>
Cc: guix-devel <at> gnu.org, 75145 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Thu, 09 Jan 2025 11:34:12 -0800

Hi 45mg,

On Thu, Jan 09 2025, 45mg wrote:

> this patch failed to get any attention in two weeks

Two weeks isn't a long time in free software projects.  A more common
time frame would be between one year and eighteen months.

> I'm CC'ing guix-devel on this one

Pings are generally frowned upon, but I also just did it yesterday. [1]
Pings are usually not worth it anywhere, not just in GNU Guix.  Pings
are likely to be ineffective in hastening the acceptance of your patch
but will almost certainly affect your social relationships negatively.

> This patch allows users to specify configuration files for NetworkManager.

If you require that change personally, as I suspect you do, my
recommendation would be to clone the Guix repo and pull your 'guix'
executable from your custom branch.  For example, I have in my
channels.scm a branch that I rebase periodically. [2]

I pull with

    guix pull --allow-downgrades --disable-authentication

That kind of setup totally removes the timing pressure from your
contribution.  It also allows you to test your patch in real life and
then argue with committers solely about its merits.  It's better for
everybody, I think.

It may leave everyone, most importantly yourself, in a happier place.

Kind regards
Felix

[1] https://lists.gnu.org/archive/html/guix-devel/2025-01/msg00056.html
[2] https://codeberg.org/lechner/guix-config/src/branch/history/channels.scm

Message #23 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: Felix Lechner <felix.lechner <at> lease-up.com>, 45mg <45mg.writes <at> gmail.com>
Cc: guix-devel <at> gnu.org, 75145 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Sun, 12 Jan 2025 17:43:59 +0000

Hi Guix,

I'd just like to apologise for CC'ing Guix-Devel at this point; this
patch isn't nearly critical enough to warrant the noise after two weeks'
wait - which, as Felix points out, really isn't that long in the grand
scheme of things. I got impatient; I'm sorry. I'll try to remember to
remove Guix-Devel from further replies to this Debbug.

(...but it is a good patch i promise, pls do review it so it doesnt feel
sad and neglected 🥹)

Felix Lechner <felix.lechner <at> lease-up.com> writes:

> If you require that change personally, as I suspect you do, my
> recommendation would be to clone the Guix repo and pull your 'guix'
> executable from your custom branch.  For example, I have in my
> channels.scm a branch that I rebase periodically. [2]

I guess it's time for me to bite the bullet and finally do this, then :|
I had hoped to avoid the annoyance, but maybe it was inevitable. And
you're right - I do need this change personally.

I'm working on this. For anyone here who's interested, I'm currently
trying to figure out a way to have an authenticated local fork. The
current design of `guix git authenticate` prevents merging upstream into
such a fork, so this is a bit of a challenge. I've outlined an idea in
the threads linked below, which will require modifying the authorization
invariant a bit; I'll probably share the full proposal to Devel once I
have a working patch:

https://lists.gnu.org/archive/html/help-guix/2023-09/msg00010.html
https://lists.gnu.org/archive/html/help-guix/2025-01/msg00093.html

> Pings are generally frowned upon, but I also just did it yesterday. [1]
> Pings are usually not worth it anywhere, not just in GNU Guix.  Pings
> are likely to be ineffective in hastening the acceptance of your patch
> but will almost certainly affect your social relationships negatively.

Thanks, good to know. Will try to keep that in mind. I do wish there was
something I could do to get things moving faster, but I guess this is a
limitation of projects that run on (primarily) unpaid volunteer work :/

Message #26 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Attila Lendvai <attila <at> lendvai.name>
To: Felix Lechner <felix.lechner <at> lease-up.com>
Cc: guix-devel <at> gnu.org, 45mg <45mg.writes <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Mon, 13 Jan 2025 13:47:40 +0000

> > this patch failed to get any attention in two weeks
>
>
> Two weeks isn't a long time in free software projects. A more common
> time frame would be between one year and eighteen months.


...and with such a delay most patches bitrot beyond recognition, then after one too many burdensome rebase the contributor gives up, and then the whole thing gets forgotten.

or an alternative reading of this is that this process selects for contributors who are willful enough about that specific patch to push it through the finish line of such a marathon.

in my opensource experience 1+ year is nowhere near the average, especially not for patches with a localized effect.

unfortunately, i only have a vague/vapourware proposal to resolve this, which is to develop better support for channels, and then allow a thousand flowers to bloom (i.e. allow/facilitate the division of labor). it's not necessarily a safe/simple/straightforward path forward, but it would solve the bottleneck of the central control.

but either way, i wanted to speak up against normalizing 1+ years of delays.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Action has meaning only in relationship; without understanding relationship, action on any level will only breed conflict. The understanding of relationship is infinitely more important than the search for any plan of action.”
	— Jiddu Krishnamurti (1895–1986), 'Relationship', Colombo Ceylon 2nd radio talk (1950)

Message #29 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Attila Lendvai <attila <at> lendvai.name>
Cc: guix-devel <at> gnu.org, 45mg <45mg.writes <at> gmail.com>,
 Felix Lechner <felix.lechner <at> lease-up.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Mon, 13 Jan 2025 16:47:49 +0100

Attila Lendvai <attila <at> lendvai.name> writes:

>> > this patch failed to get any attention in two weeks
>>
>>
>> Two weeks isn't a long time in free software projects. A more common
>> time frame would be between one year and eighteen months.
>
>
> ...and with such a delay most patches bitrot beyond recognition, then
> after one too many burdensome rebase the contributor gives up, and
> then the whole thing gets forgotten.

I agree.  Let's not normalize long delays.
Pinging is, in fact, encouraged.

We're still in the slow process of forming self-organized teams that
cover a manageable amount of packages/files and have enough dedicated
committers to review patches.  This works fine for some of our
sub-communities, but for others it still doesn't.

Once the community grew beyond a size where I recognized each and every
contributers the problems of structurelessness have become painfully
obvious.  It is not something we want people to get used to, but it's
also not something a handful of people can fix by decree.

Speaking for myself: I burned out a few years ago and haven't recovered
even a fraction of my capacity today.  This is something we really want
to avoid, and ideally people would self-organize around committers they
know, who can champion their contributions --- instead of calling for
the proliferation of private channels, a different kind of unmanageable
structurelessness.

-- 
Ricardo

Message #32 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Attila Lendvai <attila <at> lendvai.name>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: guix-devel <guix-devel <at> gnu.org>, 45mg <45mg.writes <at> gmail.com>,
 Felix Lechner <felix.lechner <at> lease-up.com>,
 "75145 <at> debbugs.gnu.org" <75145 <at> debbugs.gnu.org>
Subject: Re: [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Fri, 17 Jan 2025 13:43:36 +0000

> instead of calling for
>  the proliferation of private channels, a different kind of unmanageable
>  structurelessness.

not private channels, simply channels that are not owned/controlled by the exact same set of committers as guix proper, and not demanding the exact same requirements from their contributors.

this may seem like nitpicking, but i think it's an important detail in this context.

Message #35 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Attila Lendvai <attila <at> lendvai.name>
Cc: guix-devel <guix-devel <at> gnu.org>, 45mg <45mg.writes <at> gmail.com>,
 Felix Lechner <felix.lechner <at> lease-up.com>,
 "75145 <at> debbugs.gnu.org" <75145 <at> debbugs.gnu.org>
Subject: Re: [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Fri, 17 Jan 2025 21:25:11 +0100

Attila Lendvai <attila <at> lendvai.name> writes:

>> instead of calling for
>>  the proliferation of private channels, a different kind of unmanageable
>>  structurelessness.
>
> not private channels, simply channels that are not owned/controlled by
> the exact same set of committers as guix proper, and not demanding the
> exact same requirements from their contributors.
>
> this may seem like nitpicking, but i think it's an important detail in this context.

Thanks for the correction.  I did mean personal channels, i.e. operated
(or abandoned) by a single person.

The guix-science collective demonstrates an alternative model, where
people of similar interest band together to provide a channel (or set of
channels) that is governed by different rules than Guix itself.  This
started out as a set of independently run channels (guix-bimsb,
guix-bimsb-nonfree, guix-science, guix-science-nonfree, guix-hpc, etc)
and is now a much more streamlined and cohesive shared effort.

I would advise against running a multitude of personal channels in favor
of collaborating with others on a much smaller number of channels -- and
ideally as a team in Guix itself.

-- 
Ricardo

Message #38 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Sat, 18 Jan 2025 22:13:58 +0100

[Message part 1 (text/plain, inline)]

Hi,

I apologise for the late answer.

45mg <45mg.writes <at> gmail.com> writes:
> Arnaud Daby-Seesaram <ds-ac <at> nanein.fr> writes:
>> As you said, you are doing something similar to `etc-service-type'.  Is
>> there a reason not to extend [2] it directly (e.g. like
>> `greetd-service-type' does in `(gnu services base)')?
>>
>> You could, for example, prepend "NetworkManager/conf.d/" to file names
>> and pass the value to `etc-service-type'.  WDYT?
>
> [...]
>
> When we use `etc-service-type`, "/etc/NetworkManager" becomes a symlink
> to "/etc/static/NetworkManager"; and "/etc/static" is a symlink to the
> result of building the derivation returned by `etc-entry` (see (gnu
> services)). And derivations are in the store, which is read-only. So we
> can't create "/etc/NetworkManager/system-connections/". And this won't
> do, since NetworkManager itself needs to be able to write to that
> directory to manage saved connections.
>
> So it looks like that won't work.

That is unfortunate; thank you for trying it out!

Re-reading your patch and the implementation of `etc-service-type',
maybe `network-manager-configuration-directory' could be replaced by
`file-union' defined in `(guix gexp)', WDYT?

Otherwise, your patch seems good to me :).  I will let others comment
and give their opinion.


Best regards,

-- 
Arnaud

[signature.asc (application/pgp-signature, inline)]

Message #41 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 45mg <45mg.writes <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Tue, 21 Jan 2025 13:43:51 +0900

Hi,

Arnaud Daby-Seesaram <ds-ac <at> nanein.fr> writes:

[...]

>> When we use `etc-service-type`, "/etc/NetworkManager" becomes a symlink
>> to "/etc/static/NetworkManager"; and "/etc/static" is a symlink to the
>> result of building the derivation returned by `etc-entry` (see (gnu
>> services)). And derivations are in the store, which is read-only. So we
>> can't create "/etc/NetworkManager/system-connections/". And this won't
>> do, since NetworkManager itself needs to be able to write to that
>> directory to manage saved connections.
>>
>> So it looks like that won't work.
>
> That is unfortunate; thank you for trying it out!
>
> Re-reading your patch and the implementation of `etc-service-type',
> maybe `network-manager-configuration-directory' could be replaced by
> `file-union' defined in `(guix gexp)', WDYT?
>
> Otherwise, your patch seems good to me :).  I will let others comment
> and give their opinion.

A file-union also result in a read-only directory in the store, so if
NetworkManager needs write access to that location, that also wouldn't
work.

-- 
Thanks,
Maxim

Message #44 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45mg <45mg.writes <at> gmail.com>
Cc: guix-devel <at> gnu.org, Felix Lechner <felix.lechner <at> lease-up.com>,
 75145 <at> debbugs.gnu.org
Subject: Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Tue, 21 Jan 2025 13:52:11 +0900

Hi 45mg,

45mg <45mg.writes <at> gmail.com> writes:

[...]

> Thanks, good to know. Will try to keep that in mind. I do wish there was
> something I could do to get things moving faster, but I guess this is a
> limitation of projects that run on (primarily) unpaid volunteer work :/

As mentioned by Ricardo, a good way to try to tackle the load would be
to get involved with a Guix team and conduct reviews.  There's no need
to be an expert; even just spotting typos and small details help.  If
you have time, applying it locally and giving it a try and sharing your
experience is also very valuable.

-- 
Thanks,
Maxim

Message #47 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 45mg <45mg.writes <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Tue, 21 Jan 2025 09:36:41 +0100

[Message part 1 (text/plain, inline)]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:
> Hi,
>
> Arnaud Daby-Seesaram <ds-ac <at> nanein.fr> writes:
>
> [...]
>
>>> When we use `etc-service-type`, "/etc/NetworkManager" becomes a symlink
>>> to "/etc/static/NetworkManager"; and "/etc/static" is a symlink to the
>>> result of building the derivation returned by `etc-entry` (see (gnu
>>> services)). And derivations are in the store, which is read-only. So we
>>> can't create "/etc/NetworkManager/system-connections/". And this won't
>>> do, since NetworkManager itself needs to be able to write to that
>>> directory to manage saved connections.
>>>
>>> So it looks like that won't work.
>>
>> That is unfortunate; thank you for trying it out!
>>
>> Re-reading your patch and the implementation of `etc-service-type',
>> maybe `network-manager-configuration-directory' could be replaced by
>> `file-union' defined in `(guix gexp)', WDYT?
>>
>> Otherwise, your patch seems good to me :).  I will let others comment
>> and give their opinion.
>
> A file-union also result in a read-only directory in the store, so if
> NetworkManager needs write access to that location, that also wouldn't
> work.

Sorry, I was imprecise.

NetworkManager needs to write to /etc/NetworkManager/system-connections
The issue with extending etc-service-type was that "/etc/NetworkManager"
became a link to the store, and thus system-connections could no longer
be created/written to.

However, the configuration directory is /etc/NetworkManager/conf.d .

What I was proposing was to use file-union to create the configuration
directory (in the store) and symlink it to /etc/NetworkManager/conf.d .

This is close to what the patch already does.  I merely suggested to
replace a homemade function by a function of (guix gexp):
network-manager-configuration-directory and file-union do a similar job
if I read them correctly.


Best regards,

-- 
Arnaud

[signature.asc (application/pgp-signature, inline)]

Message #50 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 45mg <45mg.writes <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Tue, 21 Jan 2025 22:02:03 +0900

Hi Arnaud,

Arnaud Daby-Seesaram <ds-ac <at> nanein.fr> writes:

[...]

> Sorry, I was imprecise.
>
> NetworkManager needs to write to /etc/NetworkManager/system-connections
> The issue with extending etc-service-type was that "/etc/NetworkManager"
> became a link to the store, and thus system-connections could no longer
> be created/written to.
>
> However, the configuration directory is /etc/NetworkManager/conf.d .
>
> What I was proposing was to use file-union to create the configuration
> directory (in the store) and symlink it to /etc/NetworkManager/conf.d .
>
> This is close to what the patch already does.  I merely suggested to
> replace a homemade function by a function of (guix gexp):
> network-manager-configuration-directory and file-union do a similar job
> if I read them correctly.

Oh, I understand now, thank you for explaining.  It seems to make sense
to reuse file-union then.  45mg, do you also agree it makes sense?
If so, could you rework your change to use it and send a v3?

-- 
Thanks,
Maxim

Message #53 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Attila Lendvai <attila <at> lendvai.name>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: guix-devel <guix-devel <at> gnu.org>, 45mg <45mg.writes <at> gmail.com>,
 Felix Lechner <felix.lechner <at> lease-up.com>,
 "75145 <at> debbugs.gnu.org" <75145 <at> debbugs.gnu.org>
Subject: Re: [PATCH v2 0/1] services: network-manager: Add
 extra-configuration-files field.
Date: Wed, 22 Jan 2025 00:16:50 +0000

> I would advise against running a multitude of personal channels in favor
> of collaborating with others on a much smaller number of channels -- and


i agree. i meant to use the expression 'let a thousand flowers bloom' to be more about freedom and (friendly) competition, not the actual number.


> ideally as a team in Guix itself.


i don't think that will ever be satisfying because it's not facilitating the division of labor, which requires delegating responsibility (aka freedom), which in turn requires isolated parts of the system with isolated consequences.

there's no reason that the same set of people, or the same level of "security clearance" should be needed for the core guix tools, and e.g. updating the packages of the python ecosystem, or updating gnome.

i'm not saying it's easy to draw such demarcation lines. what i'm trying to say here is that i don't see any discussion or at least some wishful thinking that it could/should be like that.

and meanwhile there's a clear bottleneck at the center, no matter how much we talk about consensus and equality... at the end of the day a commit bit is a commit bit (which is ok, because it couldn't work any other way).

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Since olden times conscience has been understood by many people less as a psychic function than as a divine intervention; indeed, its dictates were regarded as […] the voice of God. This view shows what value and significance were, and still are, attached to the phenomenon of conscience […] Conscience […] commands the individual to obey his inner voice even at the risk of going astray.”
	— Carl Jung (1875–1961), 'Civilization in Transition'

Message #56 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: 75145 <at> debbugs.gnu.org
Cc: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>, 45mg <45mg.writes <at> gmail.com>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3] services: network-manager: Add extra-configuration-files
 field.
Date: Sun,  2 Feb 2025 01:19:26 +0530

Allow users to specify additional configuration files for
NetworkManager. These files will be added to
`/etc/NetworkManager/conf.d` (NetworkManager's default configuration
directory location).

* gnu/services/networking.scm (<network-manager-configuration>)
[extra-configuration-files]: New field.
(network-manager-activation): Honor the new field.
* doc/guix.texi (Networking Setup): Document the new field.

Change-Id: I07479958e4d0aa318328c666a9630b779230b300
---
 doc/guix.texi               | 21 +++++++++++++++++++++
 gnu/services/networking.scm | 13 +++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index b1b6d98e74..58ce4663c3 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21541,6 +21541,27 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
+@item @code{extra-configuration-files} (default: @code{'()})
+An alist of file names to file-like objects, representing configuration
+files which will be added to @file{/etc/NetworkManager/conf.d}.
+NetworkManager will read additional configuration from this directory.
+For details on configuration file precedence and the configuration file
+format, see the @command{NetworkManager.conf(5)} man page.
+
+For example, to add two files @file{001-basic.conf} and
+@file{002-unmanaged.conf}:
+
+@lisp
+(service network-manager-service-type
+         (network-manager-configuration
+          (extra-configuration-files
+           `(("existing-file" ,(local-file "001-basic.conf"))
+             ("constructed-file" ,(plain-file "002-unmanaged.conf"
+                                              "[keyfile]
+unmanaged-devices=interface-name:wlo1_ap
+"))))))
+@end lisp
+
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index af28bd0626..4e455cc114 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -23,6 +23,7 @@
 ;;; Copyright © 2023 Bruno Victal <mirai <at> makinata.eu>
 ;;; Copyright © 2023 muradm <mail <at> muradm.net>
 ;;; Copyright © 2024 Nigko Yerden <nigko.yerden <at> gmail.com>
+;;; Copyright © 2025 45mg <45mg.writes <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1253,18 +1254,26 @@ (define-record-type* <network-manager-configuration>
                (default '()))
   (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
         (default #f)
-        (sanitize warn-iwd?-field-deprecation)))
+        (sanitize warn-iwd?-field-deprecation))
+  (extra-configuration-files network-manager-configuration-extra-configuration-files
+                             (default '())))  ;alist of file names to file-like objects
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins)
+                (network-manager dns vpn-plugins extra-configuration-files)
     #~(begin
         (use-modules (guix build utils))
         (mkdir-p "/etc/NetworkManager/system-connections")
         #$@(if (equal? dns "dnsmasq")
                ;; create directory to store dnsmasq lease file
                '((mkdir-p "/var/lib/misc"))
+               '())
+        #$@(if extra-configuration-files
+               `((symlink
+                  ,(file-union "network-manager-configuration-directory"
+                               extra-configuration-files)
+                  "/etc/NetworkManager/conf.d"))
                '()))))
 
 (define (vpn-plugin-directory plugins)

base-commit: b85d20e853192a92093cd8d6a5756ec80e94c658
-- 
2.48.1

Message #59 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] services: network-manager: Add
 extra-configuration-files field.
Date: Sun, 02 Feb 2025 00:31:16 +0100

[Message part 1 (text/plain, inline)]

Hi,

Thank you for the v3.  I just have two minor comments.


45mg <45mg.writes <at> gmail.com> writes:

> @@ -1253,18 +1254,26 @@ (define-record-type* <network-manager-configuration>
>                 (default '()))
>    (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
>          (default #f)
> -        (sanitize warn-iwd?-field-deprecation)))
> +        (sanitize warn-iwd?-field-deprecation))
> +  (extra-configuration-files network-manager-configuration-extra-configuration-files
> +                             (default '())))  ;alist of file names to file-like objects
>  
>  (define (network-manager-activation config)
>    ;; Activation gexp for NetworkManager
>    (match-record config <network-manager-configuration>
> -    (network-manager dns vpn-plugins)
> +                (network-manager dns vpn-plugins extra-configuration-files)
>      #~(begin
>          (use-modules (guix build utils))
>          (mkdir-p "/etc/NetworkManager/system-connections")
>          #$@(if (equal? dns "dnsmasq")
>                 ;; create directory to store dnsmasq lease file
>                 '((mkdir-p "/var/lib/misc"))
> +               '())
> +        #$@(if extra-configuration-files
> +               `((symlink
> +                  ,(file-union "network-manager-configuration-directory"
> +                               extra-configuration-files)
> +                  "/etc/NetworkManager/conf.d"))
>                 '()))))

If I understand, the `if' is here to avoid creating a symbolic link to
an empty directory: if no extra configuration files are provided, do not
symlink.  However, in Guile, it seems that (if '() 'yes 'no) evaluates
to 'yes.  Therefore, the symlink is always created.  A solution is to
use `pair?':
--8<---------------cut here---------------start------------->8---
        #$@(if (pair? extra-configuration-files)
               `((symlink
                  ,(file-union "network-manager-configuration-directory"
                               extra-configuration-files)
                  "/etc/NetworkManager/conf.d"))
               '()))))
--8<---------------cut here---------------end--------------->8---

Apologies, I should have realised that before my first review.


> +@item @code{extra-configuration-files} (default: @code{'()})
> +An alist of file names to file-like objects, representing configuration
> +files which will be added to @file{/etc/NetworkManager/conf.d}.
> +NetworkManager will read additional configuration from this directory.
> +For details on configuration file precedence and the configuration file
> +format, see the @command{NetworkManager.conf(5)} man page.

I am not sure that "alist" is the correct term.  When I hear alist, I
think of something of the form
((key-1 . val-1)
 ...
 (key-n . val-n))

The documentation of `file-union' uses the term "two-element list" for
its argument.  Maybe it would be more precise to do the same (I am not
very opinionated on this).


Best regards,

-- 
Arnaud

[signature.asc (application/pgp-signature, inline)]

Message #62 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 45mg <45mg.writes <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] services: network-manager: Add
 extra-configuration-files field.
Date: Sun, 02 Feb 2025 15:46:57 +0900

Hi Arnaud,

Arnaud Daby-Seesaram <ds-ac <at> nanein.fr> writes:

> Hi,
>
> Thank you for the v3.  I just have two minor comments.
>
>
> 45mg <45mg.writes <at> gmail.com> writes:
>
>> @@ -1253,18 +1254,26 @@ (define-record-type* <network-manager-configuration>
>>                 (default '()))
>>    (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
>>          (default #f)
>> -        (sanitize warn-iwd?-field-deprecation)))
>> +        (sanitize warn-iwd?-field-deprecation))
>> +  (extra-configuration-files network-manager-configuration-extra-configuration-files
>> +                             (default '())))  ;alist of file names to file-like objects
>>  
>>  (define (network-manager-activation config)
>>    ;; Activation gexp for NetworkManager
>>    (match-record config <network-manager-configuration>
>> -    (network-manager dns vpn-plugins)
>> +                (network-manager dns vpn-plugins extra-configuration-files)
>>      #~(begin
>>          (use-modules (guix build utils))
>>          (mkdir-p "/etc/NetworkManager/system-connections")
>>          #$@(if (equal? dns "dnsmasq")
>>                 ;; create directory to store dnsmasq lease file
>>                 '((mkdir-p "/var/lib/misc"))
>> +               '())
>> +        #$@(if extra-configuration-files
>> +               `((symlink
>> +                  ,(file-union "network-manager-configuration-directory"
>> +                               extra-configuration-files)
>> +                  "/etc/NetworkManager/conf.d"))
>>                 '()))))
>
> If I understand, the `if' is here to avoid creating a symbolic link to
> an empty directory: if no extra configuration files are provided, do not
> symlink.  However, in Guile, it seems that (if '() 'yes 'no) evaluates
> to 'yes.  Therefore, the symlink is always created.  A solution is to
> use `pair?':
>
>         #$@(if (pair? extra-configuration-files)
>                `((symlink
>                   ,(file-union "network-manager-configuration-directory"
>                                extra-configuration-files)
>                   "/etc/NetworkManager/conf.d"))
>                '()))))
>
> Apologies, I should have realised that before my first review.

That's fine, and good catch!

>> +@item @code{extra-configuration-files} (default: @code{'()})
>> +An alist of file names to file-like objects, representing configuration
>> +files which will be added to @file{/etc/NetworkManager/conf.d}.
>> +NetworkManager will read additional configuration from this directory.
>> +For details on configuration file precedence and the configuration file
>> +format, see the @command{NetworkManager.conf(5)} man page.
>
> I am not sure that "alist" is the correct term.  When I hear alist, I
> think of something of the form
> ((key-1 . val-1)
>  ...
>  (key-n . val-n))
>
> The documentation of `file-union' uses the term "two-element list" for
> its argument.  Maybe it would be more precise to do the same (I am not
> very opinionated on this).

Indeed, I think a list of pure lists (rather than pair) cannot be called
an association list (alist), as that shoud be made up of pairs (using
cons or the dot notation).

See info '(guile) Association Lists'.

Thank you for the review!

45mg, would you mind sending a v4?

-- 
Maxim

Message #65 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: 75145 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>,
 Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 45mg <45mg.writes <at> gmail.com>
Subject: [PATCH v4] services: network-manager: Add extra-configuration-files
 field.
Date: Wed, 12 Feb 2025 09:56:42 +0000

Allow users to specify additional configuration files for
NetworkManager. These files will be added to
`/etc/NetworkManager/conf.d` (NetworkManager's default configuration
directory location).

* gnu/services/networking.scm (<network-manager-configuration>)
[extra-configuration-files]: New field.
(network-manager-activation): Honor the new field.
* doc/guix.texi (Networking Setup): Document the new field.

Change-Id: I07479958e4d0aa318328c666a9630b779230b300
---
 gnu/services/networking.scm | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index a5aa86fd43..c93ed58cf5 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -23,6 +23,7 @@
 ;;; Copyright © 2023 Bruno Victal <mirai <at> makinata.eu>
 ;;; Copyright © 2023 muradm <mail <at> muradm.net>
 ;;; Copyright © 2024 Nigko Yerden <nigko.yerden <at> gmail.com>
+;;; Copyright © 2025 45mg <45mg.writes <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1257,21 +1258,6 @@ (define-record-type* <network-manager-configuration>
   (extra-configuration-files network-manager-configuration-extra-configuration-files
                              (default '())))  ;'((file-name-string file-like-object) ...)
 
-(define (network-manager-configuration-directory extra-configuration-files)
-  "Return a directory containing EXTRA-CONFIGURATION-FILES."
-  (with-imported-modules (source-module-closure '((guix build utils)))
-    (computed-file
-     "network-manager-configuration-directory"
-     #~(begin
-         (use-modules (guix build utils))
-         (mkdir-p #$output)
-         (for-each (lambda (pair)
-                     (let* ((filename (list-ref pair 0))
-                            (file (list-ref pair 1))
-                            (dest (string-append #$output "/" filename)))
-                       (copy-file file dest)))
-                   '#$extra-configuration-files)))))
-
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
   (match-record config <network-manager-configuration>
@@ -1285,7 +1271,8 @@ (define (network-manager-activation config)
                '())
         #$@(if (pair? extra-configuration-files)  ;if non-empty
                `((symlink
-                  ,(network-manager-configuration-directory extra-configuration-files)
+                  ,(file-union "network-manager-configuration-directory"
+                               extra-configuration-files)
                   "/etc/NetworkManager/conf.d"))
                '()))))
 

base-commit: d59a13b6401e7494d8d5c9c9f66df3318451be79
prerequisite-patch-id: 8f30976d00586f571762f6333bf29dd355a64dbb
-- 
2.48.1

Message #68 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: 45mg <45mg.writes <at> gmail.com>, 75145 <at> debbugs.gnu.org, Maxim Cournoyer
 <maxim.cournoyer <at> gmail.com>, Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 45mg <45mg.writes <at> gmail.com>
Subject: Re: [PATCH v4] services: network-manager: Add
 extra-configuration-files field.
Date: Wed, 12 Feb 2025 10:00:03 +0000

Wait - I sent the wrong commit here. Please ignore this; I'll send a v5.

Message #71 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: 75145 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>,
 Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 45mg <45mg.writes <at> gmail.com>
Subject: [PATCH v5] services: network-manager: Add extra-configuration-files
 field.
Date: Wed, 12 Feb 2025 15:39:22 +0530

Allow users to specify additional configuration files for
NetworkManager. These files will be added to
`/etc/NetworkManager/conf.d` (NetworkManager's default configuration
directory location).

* gnu/services/networking.scm (<network-manager-configuration>)
[extra-configuration-files]: New field.
(network-manager-activation): Honor the new field.
* doc/guix.texi (Networking Setup): Document the new field.

Change-Id: I07479958e4d0aa318328c666a9630b779230b300
---
 doc/guix.texi               | 23 +++++++++++++++++++++++
 gnu/services/networking.scm | 13 +++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ce780682ed..648398ed63 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21635,6 +21635,29 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
+@item @code{extra-configuration-files} (default: @code{'()})
+A list of two-element lists; the first element of each list is a file
+name (as a string), and the second is a file-like object.  Used to
+specify configuration files which will be added to
+@file{/etc/NetworkManager/conf.d}.  NetworkManager will read additional
+configuration from this directory.  For details on configuration file
+precedence and the configuration file format, see the
+@command{NetworkManager.conf(5)} man page.
+
+For example, to add two files @file{001-basic.conf} and
+@file{002-unmanaged.conf}:
+
+@lisp
+(service network-manager-service-type
+         (network-manager-configuration
+          (extra-configuration-files
+           `(("existing-file" ,(local-file "001-basic.conf"))
+             ("constructed-file" ,(plain-file "002-unmanaged.conf"
+                                              "[keyfile]
+unmanaged-devices=interface-name:wlo1_ap
+"))))))
+@end lisp
+
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index af28bd0626..c93ed58cf5 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -23,6 +23,7 @@
 ;;; Copyright © 2023 Bruno Victal <mirai <at> makinata.eu>
 ;;; Copyright © 2023 muradm <mail <at> muradm.net>
 ;;; Copyright © 2024 Nigko Yerden <nigko.yerden <at> gmail.com>
+;;; Copyright © 2025 45mg <45mg.writes <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1253,18 +1254,26 @@ (define-record-type* <network-manager-configuration>
                (default '()))
   (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
         (default #f)
-        (sanitize warn-iwd?-field-deprecation)))
+        (sanitize warn-iwd?-field-deprecation))
+  (extra-configuration-files network-manager-configuration-extra-configuration-files
+                             (default '())))  ;'((file-name-string file-like-object) ...)
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins)
+                (network-manager dns vpn-plugins extra-configuration-files)
     #~(begin
         (use-modules (guix build utils))
         (mkdir-p "/etc/NetworkManager/system-connections")
         #$@(if (equal? dns "dnsmasq")
                ;; create directory to store dnsmasq lease file
                '((mkdir-p "/var/lib/misc"))
+               '())
+        #$@(if (pair? extra-configuration-files)  ;if non-empty
+               `((symlink
+                  ,(file-union "network-manager-configuration-directory"
+                               extra-configuration-files)
+                  "/etc/NetworkManager/conf.d"))
                '()))))
 
 (define (vpn-plugin-directory plugins)

base-commit: d59a13b6401e7494d8d5c9c9f66df3318451be79
-- 
2.48.1

Message #74 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
To: 45mg <45mg.writes <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [PATCH v5] services: network-manager: Add
 extra-configuration-files field.
Date: Wed, 12 Feb 2025 15:39:59 +0100

[Message part 1 (text/plain, inline)]

Hi,

Thank you for the v5.  The patch applies cleanly on master, and works as
expected.  I have one remark left (it's the last one, I promise :/).

45mg <45mg.writes <at> gmail.com> writes:

> * gnu/services/networking.scm (<network-manager-configuration>)
> [extra-configuration-files]: New field.
> (network-manager-activation): Honor the new field.
> * doc/guix.texi (Networking Setup): Document the new field.
>
> Change-Id: I07479958e4d0aa318328c666a9630b779230b300
> ---
>  doc/guix.texi               | 23 +++++++++++++++++++++++
>  gnu/services/networking.scm | 13 +++++++++++--
>  2 files changed, 34 insertions(+), 2 deletions(-)

- the configuration snippet of the documentation indeed adds files with
  the right content and names in /etc/NetworkManager/conf.d,

- the directory conf.d is no longer created when empty,

- the man page mentioned in the documentation is correct,

- last remark (non-blocking):

--8<---------------cut here---------------start------------->8---
     (service network-manager-service-type
              (network-manager-configuration
               (extra-configuration-files
                `(("existing-file" ,(local-file "001-basic.conf"))
                  ("constructed-file" ,(plain-file "002-unmanaged.conf"
                                                   "[keyfile]
     unmanaged-devices=interface-name:wlo1_ap
     "))))))
--8<---------------cut here---------------end--------------->8---

adds files named "existing-file" and "constructed-file".  This is the
expected behaviour (explained in the doc).  However, the example is
introduced by:

--8<---------------cut here---------------start------------->8---
For example, to add two files ‘001-basic.conf’ and
‘002-unmanaged.conf’:
--8<---------------cut here---------------end--------------->8---

This may seem counter-intuitive to some readers.


Best regards,

-- 
Arnaud

[signature.asc (application/pgp-signature, inline)]

Message #77 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>,
	45mg <45mg.writes <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: [PATCH v6] services: network-manager: Add extra-configuration-files
 field.
Date: Wed, 12 Feb 2025 20:52:52 +0530

Allow users to specify additional configuration files for
NetworkManager. These files will be added to
`/etc/NetworkManager/conf.d` (NetworkManager's default configuration
directory location).

* gnu/services/networking.scm (<network-manager-configuration>)
[extra-configuration-files]: New field.
(network-manager-activation): Honor the new field.
* doc/guix.texi (Networking Setup): Document the new field.

Change-Id: I07479958e4d0aa318328c666a9630b779230b300
---
 doc/guix.texi               | 24 ++++++++++++++++++++++++
 gnu/services/networking.scm | 13 +++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ce780682ed..d71bed7838 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -21635,6 +21635,30 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
+@item @code{extra-configuration-files} (default: @code{'()})
+A list of two-element lists; the first element of each list is a file
+name (as a string), and the second is a file-like object.  Used to
+specify configuration files which will be added to
+@file{/etc/NetworkManager/conf.d}.  NetworkManager will read additional
+configuration from this directory.  For details on configuration file
+precedence and the configuration file format, see the
+@command{NetworkManager.conf(5)} man page.
+
+For example, to add two files @file{001-basic.conf} and
+@file{002-unmanaged.conf}:
+
+@lisp
+(service network-manager-service-type
+         (network-manager-configuration
+          (extra-configuration-files
+           `(("001-basic.conf" ,(local-file "basic.conf"))
+             ("002-unmanaged.conf" ,(plain-file "constructed-unmanaged.conf"
+                                                "\
+[keyfile]
+unmanaged-devices=interface-name:wlo1_ap
+"))))))
+@end lisp
+
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index af28bd0626..c93ed58cf5 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -23,6 +23,7 @@
 ;;; Copyright © 2023 Bruno Victal <mirai <at> makinata.eu>
 ;;; Copyright © 2023 muradm <mail <at> muradm.net>
 ;;; Copyright © 2024 Nigko Yerden <nigko.yerden <at> gmail.com>
+;;; Copyright © 2025 45mg <45mg.writes <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1253,18 +1254,26 @@ (define-record-type* <network-manager-configuration>
                (default '()))
   (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
         (default #f)
-        (sanitize warn-iwd?-field-deprecation)))
+        (sanitize warn-iwd?-field-deprecation))
+  (extra-configuration-files network-manager-configuration-extra-configuration-files
+                             (default '())))  ;'((file-name-string file-like-object) ...)
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins)
+                (network-manager dns vpn-plugins extra-configuration-files)
     #~(begin
         (use-modules (guix build utils))
         (mkdir-p "/etc/NetworkManager/system-connections")
         #$@(if (equal? dns "dnsmasq")
                ;; create directory to store dnsmasq lease file
                '((mkdir-p "/var/lib/misc"))
+               '())
+        #$@(if (pair? extra-configuration-files)  ;if non-empty
+               `((symlink
+                  ,(file-union "network-manager-configuration-directory"
+                               extra-configuration-files)
+                  "/etc/NetworkManager/conf.d"))
                '()))))
 
 (define (vpn-plugin-directory plugins)

base-commit: d59a13b6401e7494d8d5c9c9f66df3318451be79
-- 
2.48.1

Message #80 received at 75145 <at> debbugs.gnu.org (full text, mbox):

From: 45mg <45mg.writes <at> gmail.com>
To: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>, 45mg <45mg.writes <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 75145 <at> debbugs.gnu.org
Subject: Re: [PATCH v5] services: network-manager: Add
 extra-configuration-files field.
Date: Wed, 12 Feb 2025 15:31:11 +0000

Hi Arnaud,

Arnaud Daby-Seesaram <ds-ac <at> nanein.fr> writes:


> --8<---------------cut here---------------start------------->8---
>      (service network-manager-service-type
>               (network-manager-configuration
>                (extra-configuration-files
>                 `(("existing-file" ,(local-file "001-basic.conf"))
>                   ("constructed-file" ,(plain-file "002-unmanaged.conf"
>                                                    "[keyfile]
>      unmanaged-devices=interface-name:wlo1_ap
>      "))))))
> --8<---------------cut here---------------end--------------->8---
>
> adds files named "existing-file" and "constructed-file".  This is the
> expected behaviour (explained in the doc).  However, the example is
> introduced by:
>
> --8<---------------cut here---------------start------------->8---
> For example, to add two files ‘001-basic.conf’ and
> ‘002-unmanaged.conf’:
> --8<---------------cut here---------------end--------------->8---
>
> This may seem counter-intuitive to some readers.

You're right; fixed in the v6 I just sent. I tried to say so via a patch
annotation, but for some reason that didn't work. So I'll say this here
instead: thank you for the very thorough review, and for sticking around
for six revisions :)

Message #85 received at 75145-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45mg <45mg.writes <at> gmail.com>
Cc: 75145-done <at> debbugs.gnu.org, Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>,
 Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [PATCH v5] services: network-manager: Add
 extra-configuration-files field.
Date: Thu, 13 Feb 2025 01:05:09 +0900

Hello!

45mg <45mg.writes <at> gmail.com> writes:

> Allow users to specify additional configuration files for
> NetworkManager. These files will be added to
> `/etc/NetworkManager/conf.d` (NetworkManager's default configuration
> directory location).
>
> * gnu/services/networking.scm (<network-manager-configuration>)
> [extra-configuration-files]: New field.
> (network-manager-activation): Honor the new field.
> * doc/guix.texi (Networking Setup): Document the new field.
>
> Change-Id: I07479958e4d0aa318328c666a9630b779230b300

LGTM.  I've taken the liberty to apply the following nitpick edits:

--8<---------------cut here---------------start------------->8---
modified   doc/guix.texi
@@ -21639,13 +21639,13 @@ Networking Setup
 @item @code{extra-configuration-files} (default: @code{'()})
 A list of two-element lists; the first element of each list is a file
 name (as a string), and the second is a file-like object.  Used to
-specify configuration files which will be added to
+specify configuration files which will be added to the
 @file{/etc/NetworkManager/conf.d}.  NetworkManager will read additional
 configuration from this directory.  For details on configuration file
-precedence and the configuration file format, see the
-@command{NetworkManager.conf(5)} man page.
+precedence and the configuration file format, see @samp{man 5
+NetworkManager.conf}.
 
-For example, to add two files @file{001-basic.conf} and
+For example, to add two files named @file{001-basic.conf} and
 @file{002-unmanaged.conf}:
 
 @lisp
@@ -21655,8 +21655,7 @@ Networking Setup
            `(("existing-file" ,(local-file "001-basic.conf"))
              ("constructed-file" ,(plain-file "002-unmanaged.conf"
                                               "[keyfile]
-unmanaged-devices=interface-name:wlo1_ap
-"))))))
+unmanaged-devices=interface-name:wlo1_ap\n"))))))
 @end lisp
 
 @end table
modified   gnu/services/networking.scm
@@ -1255,8 +1255,9 @@ (define-record-type* <network-manager-configuration>
   (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
         (default #f)
         (sanitize warn-iwd?-field-deprecation))
-  (extra-configuration-files network-manager-configuration-extra-configuration-files
-                             (default '())))  ;'((file-name-string file-like-object) ...)
+  (extra-configuration-files
+   network-manager-configuration-extra-configuration-files
+   (default '())))                 ;'((file-name-string file-like-object) ...)
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
--8<---------------cut here---------------end--------------->8---

I hope that's fine.

Finally pushed as commit 0caba8f5db!  Thank you for contributing to Guix!

--
Maxim

Message #88 received at 75145-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>
Cc: 75145-done <at> debbugs.gnu.org, 45mg <45mg.writes <at> gmail.com>
Subject: Re: [PATCH v5] services: network-manager: Add
 extra-configuration-files field.
Date: Thu, 13 Feb 2025 01:06:21 +0900

Hi again,

Apologies to Arnaud for failing to add a Reviewed-by: git trailer for
their review here.  Thanks to you as well!

-- 
Thanks,
Maxim

Message #91 received at 75145-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45mg <45mg.writes <at> gmail.com>
Cc: 75145-done <at> debbugs.gnu.org, Arnaud Daby-Seesaram <ds-ac <at> nanein.fr>,
 Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [PATCH v6] services: network-manager: Add
 extra-configuration-files field.
Date: Thu, 13 Feb 2025 14:33:14 +0900

Hi,

45mg <45mg.writes <at> gmail.com> writes:

> Allow users to specify additional configuration files for
> NetworkManager. These files will be added to
> `/etc/NetworkManager/conf.d` (NetworkManager's default configuration
> directory location).

Not sure if you noticed, but that's already been applied :-).  So I've
cherry pick you v6 doc fix and pushed to it to master.

Seeing it, I also remember something similar we have for adding
pulseaudio extra script files.  It has a extra-script-files->file-union
procedure that takes the file-like objects and extract the file n ames
from themvia the nested file-like->name definition, which could have
been used here to avoid duplicating the information.

I'll let you decide if you'd like to use that or leave it the way it is,
which is also fine.

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.