GNU bug report logs - #60014
[PATCH v2] doc: Clarify special-files-service-type expected value.

Previous Next

Full log

View this message in rfc822 format

From: mirai <mirai <at> makinata.eu>
To: Josselin Poiret <dev <at> jpoiret.xyz>, 60014 <at> debbugs.gnu.org
Subject: [bug#60014] [PATCH] activation: make install-special-file match against pairs as well.
Date: Tue, 13 Dec 2022 13:04:07 +0000

On 2022-12-13 10:15, Josselin Poiret wrote:
> Hi Bruno,
> 
> mirai <mirai <at> makinata.eu> writes:
> 
>> The documentation for it says:
>> --8<---------------cut here---------------start------------->8---
>> The value associated with special-files-service-type services must be a list of tuples where the first element is the “special file” and the second element is its target. 
>> --8<---------------cut here---------------end--------------->8---
>>
>> Which I think is the natural way of doing it. (and communicates the intent, a pair with a path and a file-like object.)
> 
> Right, that's unfortunate, although that could be changed to “list of
> lists” to make it clearer.
> 
>> Of course, (list "path" file-like-obj) works as well but imo the pair is clearer in purpose.
>> (what meaning would the third element and so on have, if ever present?)
>> This I found out the hard way by getting strange errors until I looked into what happens behind
>> `special-files-service-type' and realizing that only lists were accepted.
>>
>> The mixing of cases is unfortunate (it should have been pairs from the start) but preserves
>> compatibility with existing syntax. 
> 

> I agree with you here, but then I think to avoid having to maintain both
> cases at the same time, all existing uses of special-files-service-type
> should also be modified, and only one kind should remain, with the other
> triggering some deprecation warning.  You could match to `(path
> . file-like)`, and if (list? file-like), throw an exception.

The `(= car target) (= cdr file)' match pattern is lifted from
https://git.savannah.gnu.org/cgit/guile.git/tree/module/ice-9/match.upstream.scm?id=b54263dc98b2700fa777745405ad7651601bcdc6#n139
as Guile's Pattern Matching page doesn't specify how to match against pairs when I was looking into it.

> As a sidenote, the main problem is that Guile is not a statically typed
> language, but that's a whole other debate to have.
> 
> In any case, I don't think this patch will be accepted as-is.  I would
> only be partially in favor of the second solution (because it breaks
> existing code), while the first solution is low-effort and should work
> well enough.  Up to you (and maintainers) to decide.

A breaking change here (or a non-breaking "deprecated" warning similar to how
bootloader target field was renamed to targets can be done too, but before
any further changes its best to discuss if such a change will be received.


On 2022-12-12 20:34, Josselin Poiret wrote:
> Otherwise, you're missing the ChangeLog entry format for the commit
> message, which you can find described at [1].  You can take some
> inspiration from other commits in the repository.

I'm missing the link at [1], could you resend it?

Cheers,
Bruno

Previous Next

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