Re: branch master updated: gnu: Add passff.

[John Kehayias]

Hi Clément,

On Sat, Oct 28, 2023 at 05:05 PM, Clément Lassieur wrote:

> On Sat, Oct 28 2023, Christopher Baines wrote:
>
>> This passff-host package looks a bit odd to me, one thing to mention is
>> that guix show says it has no dependencies, but I don't think that's
>> correct:
>>
>>   ./pre-inst-env guix show passff-host
>>   name: passff-host
>>   version: 1.2.3
>>   outputs:
>>   + out: everything
>>   systems: x86_64-linux mips64el-linux aarch64-linux powerpc64le-linux 
>> riscv64-linux
>>   + i686-linux armhf-linux i586-gnu powerpc-linux
>>   dependencies:
>
> I imagine it's a bug in `guix show`?  As doc says:
>
>    • Gexps carry information about the packages or derivations they
>      refer to, and these dependencies are automatically added as inputs
>      to the build processes that use them.
>

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.

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.

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.

Of course, thanks for the contribution!

John