all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: Mikhail Tsykalov <tsymsh@gmail.com>
Cc: 41143@debbugs.gnu.org
Subject: [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings
Date: Fri, 25 Sep 2020 11:34:49 +0200	[thread overview]
Message-ID: <87r1qq2o92.fsf@gnu.org> (raw)
In-Reply-To: <9ac560cc-8660-f034-c0d8-536b68d20f68@gmail.com> (Mikhail Tsykalov's message of "Thu, 24 Sep 2020 19:09:57 +0300")

Hi Mikhail,

Mikhail Tsykalov <tsymsh@gmail.com> skribis:

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

The intent of what I propose above is (1) to not break existing code,
and (2) to avoid duplicating checks and conversions at every call site.

#1 is achieved by providing a deprecated ‘mapped-device-target’
(singular) procedure, for example.

Does that make sense?

Thanks,
Ludo’.




  reply	other threads:[~2020-09-25  9:35 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
2020-09-25  9:34     ` Ludovic Courtès [this message]
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

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

  git send-email \
    --in-reply-to=87r1qq2o92.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=41143@debbugs.gnu.org \
    --cc=tsymsh@gmail.com \
    /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 external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.