[PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[45mg]

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

Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[Arnaud Daby-Seesaram]

[-- Attachment #1: Type: text/plain, Size: 4146 bytes --]

Hi,

Thank you for your patch.

45mg <45mg.writes@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

[bug#75145] [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[45mg]

Hi Arnaud!

Arnaud Daby-Seesaram <ds-ac@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.

Re: [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[Felix Lechner via Development of GNU Guix and the GNU System distribution.]

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

Re: [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[Attila Lendvai]

> > 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)

Re: [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[Ricardo Wurmus]

Attila Lendvai <attila@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

[bug#75145] [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[Attila Lendvai]

> 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.

Re: [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[Ricardo Wurmus]

Attila Lendvai <attila@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

[bug#75145] [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[Attila Lendvai]

> 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'

[PATCH v2 1/1] services: network-manager: Add extra-configuration-files field.

[45mg]

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

Re: [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[45mg]

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@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 :/

Re: [bug#75145] [PATCH v2 0/1] services: network-manager: Add extra-configuration-files field.

[Maxim Cournoyer]

Hi 45mg,

45mg <45mg.writes@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