[bug#48849] [PATCH core-updates]: Add #:sh argument towrap-qt-program

[M <maximedevos@telenet.be>]

[-- Attachment #1: Type: text/plain, Size: 3167 bytes --]

>> Subject: [PATCH 1/8] qt-utils: Allow overriding the shell interpreter in
>>  'wrap-qt-program'.
>>
>> * guix/build/qt-utils.scm (wrap-qt-program): Introduce a #:sh keyword
>>   argument and pass it to 'wrap-program'.
>
> If bash-minimal is added to inputs as we do for other packages making
> use of wrap-program, we don't need to do more, no?  Why do we need to
> explicit the argument here?

Post-hoc reason (for the first patch): wrap-program has #:sh argument, wrap-qt-program doesn’t, which is inconsistent.

For the rest (to be clear I think the remaining patches can be removed):

Right, technically we don’t. The reason is to make sure that it’s the bash from inputs instead of the bash native-inputs. Currently, at first it gets the (wrong) native bash, and later on this is fixed up by the patch-shebangs phase, IIRC.

However, (IIRC) that behaviour is a bug – patch-shebangs is for /usr/bin/… -> /gnu/store/… stuff – if the code “make install” or the like already set a proper /gnu/store/… shebang, why automatically change it to something else? Presumably it set it to the right interpreter, and now patch-shebangs might autocorrupt it.

Another problem: there might not even be a patch-shebangs phase, uses of wrap-program, wrap-qt-program and the phase of the qt-build-system that uses wrap-qt-program (IIRC there exists such a phase) should be usable in isolation. Also, there is a hidden assumption that the uses of wrap-program are _before_ the shebang patching, whereas it might be run afterwards as wll.

Instead, I think it’s better for the uses of ‘wrap-program’ to directly set it to the _right_ bash.
That’s what the #:sh argument is for, but #:sh is set to by default (which “bash”), which is incorrect. Hence, #:sh needs to be set explicitly, and hence wrap-qt-program needs a #:sh argument or the like to pass on to wrap-program.

That said, I don’t think all this explicit #:sh is appropriate either – it would need to be repeated for every single package definition refering to wrap-program, etc.. Instead, for the future, I’d propose to eliminate the argument list of phases, turning phase procedures in phase thunks and stuffing the old arguments in parameter objects instead.

Then, the #:sh of ‘wrap-program’ could default to (search-input-file (inputs) “bin/inputs”) – automatically correct (without needing patch-shebangs) both for native and cross-compilation, and when cross-compiling without “bash” in (implicit) inputs, it automatically errors out (instead of doing the wrong thing as done currently).

The phases would also be a bit less verbose to write – (lambda* (#:key this that #:allow-other-keys) (proc this) stuff …) could become (lambda () (proc) stuff …).

(The ‘procedure’ syntax (inputs) for parameter objects might not be the best here, but that’s nothing some bikeshedding over the precise syntax can’t fix.)

Bst regards,
Maxime Devos

(p.s. I received the mails for the other patches but I’m not responding at the time – not active with Guix currently, and borrowing another computer because of repairs.)

[-- Attachment #2: Type: text/html, Size: 7478 bytes --]