unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Mikhail Tsykalov <tsymsh@gmail.com>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: 41143@debbugs.gnu.org
Subject: [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
Date: Thu, 24 Sep 2020 19:09:57 +0300	[thread overview]
Message-ID: <9ac560cc-8660-f034-c0d8-536b68d20f68@gmail.com> (raw)
In-Reply-To: <874ko6n0s5.fsf@gnu.org>

Hi Ludovic,

On 09.09.2020 23:38, Ludovic Courtès wrote:
>> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
>> index 0c154d1c4e..3d09e8220c 100644
>> --- a/gnu/services/base.scm
>> +++ b/gnu/services/base.scm
>> @@ -408,7 +408,10 @@ FILE-SYSTEM."
>>   (define (mapped-device->shepherd-service-name md)
>>     "Return the symbol that denotes the shepherd service of MD, a
>> <mapped-device>."
>>     (symbol-append 'device-mapping-
>> -                 (string->symbol (mapped-device-target md))))
>> +                 (string->symbol (string-join
>> +                                  (let ((t (mapped-device-target md)))
>> +                                    (if (list? t) t (list t)))
>> +                                  "-"))))
> To avoid duplicating the (if (list? t) …) everywhere, I propose instead
> the following approach:
>
>    1. Rename ‘target’ to ‘targets’ (plural) and likewise for the
>       accessor, and agree that it always contains a list;
>
>    2. Rename ‘mapped-device’ to ‘%mapped-device’ and add a
>       ‘mapped-device’ backward-compatibility macro that allows for a
>       ‘target’ (singular) field and automatically turns its value into a
>       list.  See the ‘origin’ macro in (guix packages) for an example of
>       how to do that (that macro allows users to specify ‘sha256’ instead
>       of ‘hash’).
>
>    3. Add a deprecated ‘mapped-device-target’ (singular) that returns the
>       first element returned by ‘mapped-device-targets’.

While this looks like a good idea, doesn't this break code that 
implements mapped-device and assumes that target is a string. Suddenly 
passing a string to a mapped-device constructor results in a list passed 
to open/close. Also, what functions should do if they expect a string 
but get a list of them? Ignore everything but the first item? Implement 
mandatory check function? Doesn't this change push complexity out of 
mapped-device to implementations of it.

Basically, this change will break all current implementations of 
mapped-devices, so I wanted to hear your thoughts on it before continuing.

Mikhail





  reply	other threads:[~2020-09-24 16:46 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-09  1:12 [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings tsmish
2020-05-09  1:22 ` [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping' tsmish
2020-09-09 20:48   ` Ludovic Courtès
2020-05-14 22:53 ` [bug#41143] Some clarification Mikhail Tsykalov
2020-05-15  1:17 ` [bug#41143] [PATCH] mapped-devices: Document lvm-mapping-device Mikhail Tsykalov
2020-06-06 13:40 ` [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings Lars-Dominik Braun
2020-06-06 20:16   ` Mikhail Tsykalov
2020-06-07  6:48     ` Lars-Dominik Braun
2020-09-09 20:38 ` Ludovic Courtès
2020-09-24 16:09   ` Mikhail Tsykalov [this message]
2020-09-25  9:34     ` Ludovic Courtès
2020-09-25 13:36       ` Mikhail Tsykalov
2020-09-25 16:20         ` Ludovic Courtès
2020-10-01 22:48           ` [bug#41143] [PATCH v2 " Mikhail Tsykalov
2020-10-01 22:49             ` [bug#41143] [PATCH v2 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
2020-10-04 10:34               ` Ludovic Courtès
2020-10-04 10:28             ` [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
2020-11-05  9:48               ` Ludovic Courtès
2020-11-06  9:47                 ` [bug#41143] [PATCH v3 " Mikhail Tsykalov
2020-11-06  9:47                   ` [bug#41143] [PATCH v3 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
2020-11-25 23:09                   ` bug#41143: [PATCH v3 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
2020-10-01 23:15           ` [bug#41143] [PATCH " Mikhail Tsykalov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9ac560cc-8660-f034-c0d8-536b68d20f68@gmail.com \
    --to=tsymsh@gmail.com \
    --cc=41143@debbugs.gnu.org \
    --cc=ludo@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).