GNU bug report logs - #60735
[PATCH 0/2] Implement etc-hosts-service-type

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 60735 <at> debbugs.gnu.org
Subject: [bug#60735] [PATCH 0/2] Implement etc-hosts-service-type
Date: Sat, 14 Jan 2023 18:30:08 +0100

Hello Bruno,

Bruno Victal <mirai <at> makinata.eu> skribis:

> * gnu/services.scm (etc-hosts-service-type): New variable.
> * gnu/system.scm (operating-system-hosts-file): Deprecate procedure.
> (warn-hosts-file-field-deprecation): New procedure, helper for
> deprecated variable).
> (operating-system)[hosts-file]: Use helper to warn deprecated field.
> (operating-system-default-essential-services)
> (hurd-default-essential-services): Use etc-hosts-service-type.
> (local-host-aliases): Return a list of strings representing hosts file entries.
> (default-/etc/hosts): Remove procedure.
> (operating-system-etc-service): Remove hosts file.
> * doc/guix.texi: Document it.

Neat!  Some comments:

> +@defvar etc-hosts-service-type
> +Type of the service that populates the entries for (@file{/etc/hosts}).
> +This service can be extended by passing it lists of strings such as:
> +
> +@c TRANSLATORS: The domain names below SHOULD NOT be translated.
> +@c They're domains reserved for use in documentation. (RFC6761 Section 6.5)
> +@lisp
> +(list "127.0.0.1    example.com example.net"
> +      "::1          example.com example.net"
> +@end lisp
> +@end defvar

[...]

> +(define etc-hosts-service-type
> +  ;; Extend etc-service-type with a entry for @file{/etc/hosts}.
> +  (service-type
> +   (name 'etc-hosts)
> +   (extensions
> +    (list
> +     (service-extension etc-service-type
> +                        (lambda (lst)
> +                          `(("hosts"
> +                             ,(plain-file "hosts"
> +                                          (string-join lst "\n"
> +                                                       'suffix))))))))
> +   (compose concatenate)
> +   (extend append)
> +   (description "Populate the @file{/etc/hosts} file.")))

Two suggestions:

  1. Calling it ‘hosts-service-type’.

  2. Instead of plain strings, take records along the lines of:

       (define-record-type* <host> host make-host
         host?
         (address         host-address)  ;string
         (canonical-name  host-canonical-name)  ;string
         (aliases         host-aliases (default '()))) ;list of strings

WDYT?

If “host” is too likely to clash, we can call it <host-name-binding> or
something, but I think it should be fine.

> +(define-with-syntax-properties (warn-hosts-file-field-deprecation
> +                                (value properties))
> +  (when value
> +    (warning (source-properties->location properties)
> +             (G_ "the 'hosts-file' field is deprecated, please use \
> +'etc-hosts-service-type' instead~%")))
> +  value)

Could you move deprecation to a separate patch?

Apart from that it LGTM, thank you!

Ludo’.

Previous Next

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