Hi John,
Thanks for taking the time to write this message.
On Tue, Oct 31 2023, John Kehayias wrote:
> Right, it uses gexps but I think here the better and more explicit
> style would be to use inputs/native-inputs. Then instead of
> referencing directly like #$<package-variable-name> use
> #$(this-package-input "package-name") to get the store path. This I
> think is clearer and I believe better allows for inheritance,
> input-rewriting, and so on.
You are right, I'll amend my patch, taking this into account.
I believe `guix lint` should say something about it though, maybe warn
when using an input that is not explicitly added in the `input` field.
Those errors are hard to catch and are in our code base at several
places already.
I've also noticed there is some subtle in-between style in our code
base: people adding x as input but still using #$x in the package
definition instead of using #$(this-package-input "x"). It's wrong
because in this case input rewriting does not work.
G-Expressions allow for weird things. Here's another weird pattern from
our code base: going from (inputs `("foo", ,(origin ...))) to #$(origin
...) without inputs. This removes input rewriting too.
> Feel free for anyone else to chime in on this point, I'm always
> looking to learn to improve my own packaging and review, but this is
> what I understand is preferred when possible.
>
>>> Was this change sent as a patch to guix-patches?
>>
>> No it wasn't.
>
> The mantra I've heard, and agree with, is that the
> trivial-build-system is anything but trivial. Not saying it wasn't the
> best choice here, or has anything to do with the above points, but
> thought it worth mentioning for anyone else.
Agreed.
> But this is also why I think it would have been better to have it go
> through review. I see there's been several followup commits to improve
> the style and fix things which also could have been avoided. Not a
> huge deal perhaps, but I would err on the side of review for something
> like this.
I thought my patch was trivial, but I was obviously wrong since I
(indeed) amended it twice, and now a third time.
> Of course, thanks for the contribution!
Thanks for the kind words :)
Clément
