[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#55912] [PATCH] home: Add OpenSSH service.
From: |
Maxime Devos |
Subject: |
[bug#55912] [PATCH] home: Add OpenSSH service. |
Date: |
Sat, 11 Jun 2022 21:51:20 +0200 |
User-agent: |
Evolution 3.38.3-1 |
Hi,
Some comments on the code.
Ludovic Courtès schreef op za 11-06-2022 om 18:49 [+0200]:
> + (port
> + (maybe-integer 'disabled)
> + "TCP port number to connect to.")
TCP only allows natural numbers up to some bound, and in practice
implementations only support non-zero natural numbers, so maybe the
predicate can be refined a bit?
> + (formatted-message
> + (G_ "~s: unsupported address family")
Maybe a hint:
hint: AF_INET and AF_INET6 are supported.
> + (define (serialize-string field value)
> + (string-append " " (serialize-field-name field)
> + " " value "\n"))
> + (name
> + (string)
> + "Name of this host declaration.")
> [...]
> + (proxy-command
> + (maybe-string 'disabled)
Attila Lendvai has a patch series at 54674 that changes 'disabled' ->
*unspecified* -- I think it would be better to apply that patch series
first.
Wouldn't the value need to be escaped? Or at least a check that it
doesn't contain special characters like \n or whatever special
charaters an OpenSSH configuration has.
>+ (define* (file-join name files #:optional (delimiter " "))
>+ "Return a file in the store called @var{name} that is the
>+ concatenation
>+ of all the file-like objects listed in @var{files}, with
@var{delimited}
>+ inserted after each of them."
Does this work for files with non-ASCII characters and for file names
that contain non-ASCII characters?
>+ (service-extension home-profile-service-type
>+ (compose
>+ list
>+ home-openssh-configuration-openssh))
>+ (service-extension home-activation-service-type
>+ (const openssh-activation))))
>+ (description "Configure the OpenSSH @acronym{SSH, secure shell}
>+client and _add it to the user profile_.")
(emphasis added). Why is it automagically added to the user profile?
This is considered bad practice for system services. Maybe the user
keeps all their remote communication things in a single profile, maybe
the user only uses openssh things via other tools like 'guix deploy' or
'gnome-shell-extension-gsconnect' and hence has no need for 'openssh'
in their home profile. Maybe the user never ssh's _from_ the computer
that has the openssh home configuration and only connects _to_ the
computer and hence the 'openssh' in the profile isn't necessary.
Now there are two ways to add 'openssh' to the environment: the Guix
Home equivalent of a 'packages' field and the openssh home service,
with AFAICT no mechanism for deciding which one ‘wins’ and no mechanism
for a proper error message like ‘only add the openssh package to the
profile or use the openssh home service, not both!’, which doesn't seem
ideal to me.
reetings,
Maxime.
signature.asc
Description: This is a digitally signed message part
[bug#55912] [PATCH v2] home: Add OpenSSH service., Ludovic Courtès, 2022/06/15